From 746927c972869863c2a0a3a72f9a3f2c0ff3a731 Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Mon, 10 Jan 2022 08:54:46 -0800 Subject: [PATCH] feat: Add `first-instance-ack` event to the `app.requestSingleInstanceLock()` flow (#31460) * feat: Add onFirstInstanceAck event for requestSingleInstanceLock * Add tests * Apply patch fix * Add back missing docs * Rebase * Listen for exit earlier in test * Rebase --- docs/api/app.md | 38 +- patches/chromium/.patches | 2 +- ...ansfer_to_requestsingleinstancelock.patch} | 332 +++++++++++++++--- shell/browser/api/electron_api_app.cc | 70 +++- shell/browser/api/electron_api_app.h | 9 +- spec-main/api-app-spec.ts | 114 ++++-- spec/fixtures/api/singleton-data/main.js | 41 ++- 7 files changed, 514 insertions(+), 92 deletions(-) rename patches/chromium/{feat_add_data_parameter_to_processsingleton.patch => feat_add_data_transfer_to_requestsingleinstancelock.patch} (53%) diff --git a/docs/api/app.md b/docs/api/app.md index 99567cda3cd6..03420de5a4ee 100755 --- a/docs/api/app.md +++ b/docs/api/app.md @@ -484,6 +484,7 @@ Returns: * `argv` string[] - An array of the second instance's command line arguments * `workingDirectory` string - The second instance's working directory * `additionalData` unknown - A JSON object of additional data passed from the second instance +* `ackCallback` unknown - A function that can be used to send data back to the second instance This event will be emitted inside the primary instance of your application when a second instance has been executed and calls `app.requestSingleInstanceLock()`. @@ -495,12 +496,35 @@ non-minimized. **Note:** If the second instance is started by a different user than the first, the `argv` array will not include the arguments. +**Note:** `ackCallback` allows the user to send data back to the +second instance during the `app.requestSingleInstanceLock()` flow. +This callback can be used for cases where the second instance +needs to obtain additional information from the first instance +before quitting. + +Currently, the limit on the message size is kMaxMessageLength, +or around 32kB. To be safe, keep the amount of data passed to 31kB at most. + +In order to call the callback, `event.preventDefault()` must be called, first. +If the callback is not called in either case, `null` will be sent back. +If `event.preventDefault()` is not called, but `ackCallback` is called +by the user in the event, then the behaviour is undefined. + This event is guaranteed to be emitted after the `ready` event of `app` gets emitted. **Note:** Extra command line arguments might be added by Chromium, such as `--original-process-start-time`. +### Event: 'first-instance-ack' + +Returns: + +* `event` Event +* `additionalData` unknown - A JSON object of additional data passed from the first instance, in response to the first instance's `second-instance` event. + +This event will be emitted within the second instance during the call to `app.requestSingleInstanceLock()`, when the first instance calls the `ackCallback` provided by the `second-instance` event handler. + ## Methods The `app` object has the following methods: @@ -959,6 +983,13 @@ starts: const { app } = require('electron') let myWindow = null +app.on('first-instance-ack', (event, additionalData) => { + // Print out the ack received from the first instance. + // Note this event handler must come before the requestSingleInstanceLock call. + // Expected output: '{"myAckKey":"myAckValue"}' + console.log(JSON.stringify(additionalData)) +}) + const additionalData = { myKey: 'myValue' } const gotTheLock = app.requestSingleInstanceLock(additionalData) @@ -966,14 +997,19 @@ if (!gotTheLock) { app.quit() } else { app.on('second-instance', (event, commandLine, workingDirectory, additionalData) => { + // We must call preventDefault if we're sending back data. + event.preventDefault() // Print out data received from the second instance. - console.log(additionalData) + // Expected output: '{"myKey":"myValue"}' + console.log(JSON.stringify(additionalData)) // Someone tried to run a second instance, we should focus our window. if (myWindow) { if (myWindow.isMinimized()) myWindow.restore() myWindow.focus() } + const ackData = { myAckKey: 'myAckValue' } + ackCallback(ackData) }) // Create myWindow, load the rest of the app, etc... diff --git a/patches/chromium/.patches b/patches/chromium/.patches index df4766e95c9b..dc46a1bafdf3 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -106,8 +106,8 @@ chore_do_not_use_chrome_windows_in_cryptotoken_webrequestsender.patch process_singleton.patch fix_expose_decrementcapturercount_in_web_contents_impl.patch add_ui_scopedcliboardwriter_writeunsaferawdata.patch -feat_add_data_parameter_to_processsingleton.patch mas_gate_private_enterprise_APIs.patch load_v8_snapshot_in_browser_process.patch fix_patch_out_permissions_checks_in_exclusive_access.patch fix_aspect_ratio_with_max_size.patch +feat_add_data_transfer_to_requestsingleinstancelock.patch diff --git a/patches/chromium/feat_add_data_parameter_to_processsingleton.patch b/patches/chromium/feat_add_data_transfer_to_requestsingleinstancelock.patch similarity index 53% rename from patches/chromium/feat_add_data_parameter_to_processsingleton.patch rename to patches/chromium/feat_add_data_transfer_to_requestsingleinstancelock.patch index 92002b6cdf78..99d6a9e4cc00 100644 --- a/patches/chromium/feat_add_data_parameter_to_processsingleton.patch +++ b/patches/chromium/feat_add_data_transfer_to_requestsingleinstancelock.patch @@ -1,19 +1,25 @@ From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Tue, 7 Sep 2021 14:54:25 -0700 -Subject: feat: Add data parameter to ProcessSingleton +Subject: feat: Add data transfer mechanism to requestSingleInstanceLock flow -This patch adds an additional_data parameter to the constructor of -ProcessSingleton, so that the second instance can send additional -data over to the first instance while requesting the ProcessSingleton -lock. +This patch adds code that allows for the second instance to send +additional data to the first instance, and for the first instance +to send additional data back to the second instance, during the +app.requestSingleInstanceLock call. -On the Electron side, we then expose an extra parameter to the -app.requestSingleInstanceLock API so that users can pass in a JSON -object for the second instance to send to the first instance. +Firstly, this patch adds an additional_data parameter +to the constructor of ProcessSingleton, so that the second instance +can send additional data over to the first instance +while requesting the ProcessSingleton lock. + +Then, we add additional processing to the second-instance event, both +so the first instance can receive additional data from the second +instance, but also so the second instance can send back additional +data to the first instance if needed. diff --git a/chrome/browser/process_singleton.h b/chrome/browser/process_singleton.h -index 13b325ecad9ba48398173e89680287c63efd4fa6..2061374805de9f9e646b896c4212bb3dcf2b8ed7 100644 +index 13b325ecad9ba48398173e89680287c63efd4fa6..e8188e4640b28d41559822e6c1bdd27dcccae93c 100644 --- a/chrome/browser/process_singleton.h +++ b/chrome/browser/process_singleton.h @@ -18,6 +18,7 @@ @@ -24,24 +30,39 @@ index 13b325ecad9ba48398173e89680287c63efd4fa6..2061374805de9f9e646b896c4212bb3d #include "ui/gfx/native_widget_types.h" #if defined(OS_POSIX) && !defined(OS_ANDROID) -@@ -100,21 +101,24 @@ class ProcessSingleton { +@@ -93,6 +94,9 @@ class ProcessSingleton { + + static constexpr int kNumNotifyResults = LAST_VALUE + 1; + ++ using NotificationAckCallback = ++ base::RepeatingCallback* ack_data)>; ++ + // Implement this callback to handle notifications from other processes. The + // callback will receive the command line and directory with which the other + // Chrome process was launched. Return true if the command line will be +@@ -100,21 +104,27 @@ class ProcessSingleton { // should handle it (i.e., because the current process is shutting down). using NotificationCallback = base::RepeatingCallback; + const base::FilePath& current_directory, -+ const std::vector additional_data)>; ++ const std::vector additional_data, ++ const NotificationAckCallback& ack_callback)>; #if defined(OS_WIN) ProcessSingleton(const std::string& program_name, const base::FilePath& user_data_dir, + const base::span additional_data, bool is_sandboxed, - const NotificationCallback& notification_callback); +- const NotificationCallback& notification_callback); ++ const NotificationCallback& notification_callback, ++ const NotificationAckCallback& ack_notification_callback); #else ProcessSingleton(const base::FilePath& user_data_dir, +- const NotificationCallback& notification_callback); + const base::span additional_data, - const NotificationCallback& notification_callback); ++ const NotificationCallback& notification_callback, ++ const NotificationAckCallback& ack_notification_callback); +#endif ProcessSingleton(const ProcessSingleton&) = delete; @@ -51,19 +72,42 @@ index 13b325ecad9ba48398173e89680287c63efd4fa6..2061374805de9f9e646b896c4212bb3d ~ProcessSingleton(); // Notify another process, if available. Otherwise sets ourselves as the -@@ -178,6 +182,8 @@ class ProcessSingleton { +@@ -177,7 +187,13 @@ class ProcessSingleton { + #endif private: - NotificationCallback notification_callback_; // Handler for notifications. +- NotificationCallback notification_callback_; // Handler for notifications. ++ // A callback to run when the first instance receives data from the second. ++ NotificationCallback notification_callback_; ++ // A callback to run when the second instance ++ // receives an acknowledgement from the first. ++ NotificationAckCallback notification_ack_callback_; + // Custom data to pass to the other instance during notify. + base::span additional_data_; #if defined(OS_WIN) bool EscapeVirtualization(const base::FilePath& user_data_dir); +@@ -190,6 +206,7 @@ class ProcessSingleton { + HANDLE lock_file_; + base::FilePath user_data_dir_; + ShouldKillRemoteProcessCallback should_kill_remote_process_callback_; ++ HANDLE ack_pipe_; + #elif defined(OS_POSIX) && !defined(OS_ANDROID) + // Return true if the given pid is one of our child processes. + // Assumes that the current pid is the root of all pids of the current diff --git a/chrome/browser/process_singleton_posix.cc b/chrome/browser/process_singleton_posix.cc -index 0d74cd93a21b937f65f3d417b4734ae5b87acdf2..66a4369747377fe64c91b68f57ee4178c426ea61 100644 +index 0d74cd93a21b937f65f3d417b4734ae5b87acdf2..9bf6e633b5adc68a67d23e4f17460dce24a02cfa 100644 --- a/chrome/browser/process_singleton_posix.cc +++ b/chrome/browser/process_singleton_posix.cc +@@ -147,7 +147,7 @@ const char kACKToken[] = "ACK"; + const char kShutdownToken[] = "SHUTDOWN"; + const char kTokenDelimiter = '\0'; + const int kMaxMessageLength = 32 * 1024; +-const int kMaxACKMessageLength = base::size(kShutdownToken) - 1; ++const int kMaxACKMessageLength = kMaxMessageLength; + + bool g_disable_prompt = false; + bool g_skip_is_chrome_process_check = false; @@ -627,6 +627,7 @@ class ProcessSingleton::LinuxWatcher // |reader| is for sending back ACK message. void HandleMessage(const std::string& current_dir, @@ -72,7 +116,17 @@ index 0d74cd93a21b937f65f3d417b4734ae5b87acdf2..66a4369747377fe64c91b68f57ee4178 SocketReader* reader); private: -@@ -681,13 +682,16 @@ void ProcessSingleton::LinuxWatcher::StartListening(int socket) { +@@ -651,6 +652,9 @@ class ProcessSingleton::LinuxWatcher + // The ProcessSingleton that owns us. + ProcessSingleton* const parent_; + ++ bool ack_callback_called_ = false; ++ void AckCallback(SocketReader* reader, const base::span* response); ++ + std::set, base::UniquePtrComparator> readers_; + }; + +@@ -681,16 +685,21 @@ void ProcessSingleton::LinuxWatcher::StartListening(int socket) { } void ProcessSingleton::LinuxWatcher::HandleMessage( @@ -84,14 +138,46 @@ index 0d74cd93a21b937f65f3d417b4734ae5b87acdf2..66a4369747377fe64c91b68f57ee4178 DCHECK(ui_task_runner_->BelongsToCurrentThread()); DCHECK(reader); - if (parent_->notification_callback_.Run(base::CommandLine(argv), +- if (parent_->notification_callback_.Run(base::CommandLine(argv), - base::FilePath(current_dir))) { +- // Send back "ACK" message to prevent the client process from starting up. +- reader->FinishWithACK(kACKToken, base::size(kACKToken) - 1); +- } else { ++ auto wrapped_ack_callback = ++ base::BindRepeating(&ProcessSingleton::LinuxWatcher::AckCallback, ++ base::Unretained(this), reader); ++ ack_callback_called_ = false; ++ if (!parent_->notification_callback_.Run(base::CommandLine(argv), + base::FilePath(current_dir), -+ std::move(additional_data))) { - // Send back "ACK" message to prevent the client process from starting up. - reader->FinishWithACK(kACKToken, base::size(kACKToken) - 1); - } else { -@@ -735,7 +739,8 @@ void ProcessSingleton::LinuxWatcher::SocketReader:: ++ std::move(additional_data), ++ wrapped_ack_callback)) { + LOG(WARNING) << "Not handling interprocess notification as browser" + " is shutting down"; + // Send back "SHUTDOWN" message, so that the client process can start up +@@ -700,6 +709,22 @@ void ProcessSingleton::LinuxWatcher::HandleMessage( + } + } + ++void ProcessSingleton::LinuxWatcher::AckCallback( ++ SocketReader* reader, ++ const base::span* response) { ++ // Send back "ACK" message to prevent the client process from starting up. ++ if (!ack_callback_called_) { ++ ack_callback_called_ = true; ++ std::string ack_message; ++ ack_message.append(kACKToken, base::size(kACKToken) - 1); ++ if (response && response->size_bytes()) { ++ ack_message.append(reinterpret_cast(response->data()), ++ response->size_bytes()); ++ } ++ reader->FinishWithACK(ack_message.c_str(), ack_message.size()); ++ } ++} ++ + void ProcessSingleton::LinuxWatcher::RemoveSocketReader(SocketReader* reader) { + DCHECK_CURRENTLY_ON(BrowserThread::IO); + DCHECK(reader); +@@ -735,7 +760,8 @@ void ProcessSingleton::LinuxWatcher::SocketReader:: } } @@ -101,7 +187,7 @@ index 0d74cd93a21b937f65f3d417b4734ae5b87acdf2..66a4369747377fe64c91b68f57ee4178 const size_t kMinMessageLength = base::size(kStartToken) + 4; if (bytes_read_ < kMinMessageLength) { buf_[bytes_read_] = 0; -@@ -765,10 +770,28 @@ void ProcessSingleton::LinuxWatcher::SocketReader:: +@@ -765,10 +791,28 @@ void ProcessSingleton::LinuxWatcher::SocketReader:: tokens.erase(tokens.begin()); tokens.erase(tokens.begin()); @@ -118,8 +204,8 @@ index 0d74cd93a21b937f65f3d417b4734ae5b87acdf2..66a4369747377fe64c91b68f57ee4178 + std::string(1, kTokenDelimiter)); + const uint8_t* additional_data_bits = + reinterpret_cast(remaining_args.c_str()); -+ additional_data = std::vector( -+ additional_data_bits, additional_data_bits + additional_data_size); ++ additional_data = std::vector(additional_data_bits, ++ additional_data_bits + additional_data_size); + } + // Return to the UI thread to handle opening a new browser tab. @@ -131,18 +217,21 @@ index 0d74cd93a21b937f65f3d417b4734ae5b87acdf2..66a4369747377fe64c91b68f57ee4178 fd_watch_controller_.reset(); // LinuxWatcher::HandleMessage() is in charge of destroying this SocketReader -@@ -797,8 +820,10 @@ void ProcessSingleton::LinuxWatcher::SocketReader::FinishWithACK( +@@ -797,8 +841,12 @@ void ProcessSingleton::LinuxWatcher::SocketReader::FinishWithACK( // ProcessSingleton::ProcessSingleton( const base::FilePath& user_data_dir, +- const NotificationCallback& notification_callback) + const base::span additional_data, - const NotificationCallback& notification_callback) ++ const NotificationCallback& notification_callback, ++ const NotificationAckCallback& notification_ack_callback) : notification_callback_(notification_callback), ++ notification_ack_callback_(notification_ack_callback), + additional_data_(additional_data), current_pid_(base::GetCurrentProcId()), watcher_(new LinuxWatcher(this)) { socket_path_ = user_data_dir.Append(chrome::kSingletonSocketFilename); -@@ -915,7 +940,8 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessWithTimeout( +@@ -915,7 +963,8 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessWithTimeout( sizeof(socket_timeout)); // Found another process, prepare our command line @@ -152,7 +241,7 @@ index 0d74cd93a21b937f65f3d417b4734ae5b87acdf2..66a4369747377fe64c91b68f57ee4178 std::string to_send(kStartToken); to_send.push_back(kTokenDelimiter); -@@ -925,11 +951,21 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessWithTimeout( +@@ -925,11 +974,21 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessWithTimeout( to_send.append(current_dir.value()); const std::vector& argv = cmd_line.argv(); @@ -174,11 +263,52 @@ index 0d74cd93a21b937f65f3d417b4734ae5b87acdf2..66a4369747377fe64c91b68f57ee4178 // Send the message if (!WriteToSocket(socket.fd(), to_send.data(), to_send.length())) { // Try to kill the other process, because it might have been dead. +@@ -969,6 +1028,17 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessWithTimeout( + linux_ui->NotifyWindowManagerStartupComplete(); + #endif + ++ size_t ack_data_len = len - (base::size(kACKToken) - 1); ++ if (ack_data_len) { ++ const uint8_t* raw_ack_data = ++ reinterpret_cast(buf + base::size(kACKToken) - 1); ++ base::span ack_data = ++ base::make_span(raw_ack_data, raw_ack_data + ack_data_len); ++ notification_ack_callback_.Run(&ack_data); ++ } else { ++ notification_ack_callback_.Run(nullptr); ++ } ++ + // Assume the other process is handling the request. + return PROCESS_NOTIFIED; + } diff --git a/chrome/browser/process_singleton_win.cc b/chrome/browser/process_singleton_win.cc -index 679350dd08ca0211653ea669405e3f4f86c2fc0f..daab2f170d857082df56966c7bd8abc53924631d 100644 +index 679350dd08ca0211653ea669405e3f4f86c2fc0f..16ad742721e9c5af13224f74e864e648c27a2a34 100644 --- a/chrome/browser/process_singleton_win.cc +++ b/chrome/browser/process_singleton_win.cc -@@ -98,10 +98,12 @@ BOOL CALLBACK BrowserWindowEnumeration(HWND window, LPARAM param) { +@@ -22,6 +22,7 @@ + #include "base/strings/utf_string_conversions.h" + #include "base/time/time.h" + #include "base/trace_event/trace_event.h" ++#include "base/timer/timer.h" + #include "base/win/registry.h" + #include "base/win/scoped_handle.h" + #include "base/win/windows_version.h" +@@ -44,6 +45,14 @@ + namespace { + + const char kLockfile[] = "lockfile"; ++const LPCWSTR kPipeName = L"\\\\.\\pipe\\electronAckPipe"; ++const DWORD kPipeTimeout = 10000; ++const DWORD kMaxMessageLength = 32 * 1024; ++ ++std::unique_ptr> g_ack_data; ++base::OneShotTimer g_ack_timer; ++HANDLE g_write_ack_pipe; ++bool g_write_ack_callback_called = false; + + // A helper class that acquires the given |mutex| while the AutoLockMutex is in + // scope. +@@ -98,10 +107,12 @@ BOOL CALLBACK BrowserWindowEnumeration(HWND window, LPARAM param) { bool ParseCommandLine(const COPYDATASTRUCT* cds, base::CommandLine* parsed_command_line, @@ -193,7 +323,7 @@ index 679350dd08ca0211653ea669405e3f4f86c2fc0f..daab2f170d857082df56966c7bd8abc5 static const int min_message_size = 7; if (cds->cbData < min_message_size * sizeof(wchar_t) || cds->cbData % sizeof(wchar_t) != 0) { -@@ -151,6 +153,37 @@ bool ParseCommandLine(const COPYDATASTRUCT* cds, +@@ -151,11 +162,82 @@ bool ParseCommandLine(const COPYDATASTRUCT* cds, const std::wstring cmd_line = msg.substr(second_null + 1, third_null - second_null); *parsed_command_line = base::CommandLine::FromString(cmd_line); @@ -231,7 +361,52 @@ index 679350dd08ca0211653ea669405e3f4f86c2fc0f..daab2f170d857082df56966c7bd8abc5 return true; } return false; -@@ -167,16 +200,16 @@ bool ProcessLaunchNotification( + } + ++void StoreAck(const base::span* ack_data) { ++ if (ack_data) { ++ g_ack_data = std::make_unique>(ack_data->begin(), ++ ack_data->end()); ++ } else { ++ g_ack_data = nullptr; ++ } ++} ++ ++void SendBackAck() { ++ // This is the first instance sending the ack back to the second instance. ++ if (!g_write_ack_callback_called) { ++ g_write_ack_callback_called = true; ++ const uint8_t* data_buffer = nullptr; ++ DWORD data_to_send_size = 0; ++ if (g_ack_data) { ++ data_buffer = g_ack_data->data(); ++ DWORD ack_data_size = g_ack_data->size() * sizeof(uint8_t); ++ data_to_send_size = (ack_data_size < kMaxMessageLength) ? ack_data_size : kMaxMessageLength; ++ } ++ ++ ::ConnectNamedPipe(g_write_ack_pipe, NULL); ++ ++ DWORD bytes_written = 0; ++ ::WriteFile(g_write_ack_pipe, ++ (LPCVOID)data_buffer, ++ data_to_send_size, ++ &bytes_written, ++ NULL); ++ DCHECK(bytes_written == data_to_send_size); ++ ++ ::FlushFileBuffers(g_write_ack_pipe); ++ ::DisconnectNamedPipe(g_write_ack_pipe); ++ ++ if (g_ack_data) { ++ g_ack_data.reset(); ++ } ++ } ++} ++ + bool ProcessLaunchNotification( + const ProcessSingleton::NotificationCallback& notification_callback, + UINT message, +@@ -167,16 +249,23 @@ bool ProcessLaunchNotification( // Handle the WM_COPYDATA message from another process. const COPYDATASTRUCT* cds = reinterpret_cast(lparam); @@ -240,39 +415,116 @@ index 679350dd08ca0211653ea669405e3f4f86c2fc0f..daab2f170d857082df56966c7bd8abc5 base::FilePath current_directory; - if (!ParseCommandLine(cds, &parsed_command_line, ¤t_directory)) { + std::vector additional_data; -+ if (!ParseCommandLine(cds, &parsed_command_line, ¤t_directory, &additional_data)) { ++ if (!ParseCommandLine(cds, &parsed_command_line, ¤t_directory, ++ &additional_data)) { *result = TRUE; return true; } - *result = notification_callback.Run(parsed_command_line, current_directory) ? - TRUE : FALSE; -+ *result = notification_callback.Run(parsed_command_line, -+ current_directory, std::move(additional_data)) ? TRUE : FALSE; ++ g_write_ack_callback_called = false; ++ *result = notification_callback.Run(parsed_command_line, current_directory, ++ std::move(additional_data), ++ base::BindRepeating(&StoreAck)) ++ ? TRUE ++ : FALSE; ++ g_ack_timer.Start(FROM_HERE, base::Seconds(0), ++ base::BindOnce(&SendBackAck)); return true; } -@@ -273,9 +306,11 @@ bool ProcessSingleton::EscapeVirtualization( +@@ -273,9 +362,13 @@ bool ProcessSingleton::EscapeVirtualization( ProcessSingleton::ProcessSingleton( const std::string& program_name, const base::FilePath& user_data_dir, + const base::span additional_data, bool is_app_sandboxed, - const NotificationCallback& notification_callback) +- const NotificationCallback& notification_callback) ++ const NotificationCallback& notification_callback, ++ const NotificationAckCallback& notification_ack_callback) : notification_callback_(notification_callback), ++ notification_ack_callback_(notification_ack_callback), + additional_data_(additional_data), program_name_(program_name), is_app_sandboxed_(is_app_sandboxed), is_virtualized_(false), -@@ -300,7 +335,7 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcess() { +@@ -290,6 +383,37 @@ ProcessSingleton::~ProcessSingleton() { + ::CloseHandle(lock_file_); + } + ++void ReadAck(const ProcessSingleton::NotificationAckCallback& ack_callback) { ++ // We are reading the ack from the first instance. ++ // First, wait for the pipe. ++ ::WaitNamedPipe(kPipeName, NMPWAIT_USE_DEFAULT_WAIT); ++ ++ HANDLE read_ack_pipe = ::CreateFile(kPipeName, ++ GENERIC_READ, ++ FILE_SHARE_READ, ++ NULL, ++ OPEN_EXISTING, ++ FILE_ATTRIBUTE_NORMAL, ++ NULL); ++ CHECK(read_ack_pipe != INVALID_HANDLE_VALUE); ++ ++ DWORD bytes_read; ++ uint8_t read_ack_buffer[kMaxMessageLength]; ++ ::ReadFile(read_ack_pipe, ++ (LPVOID)read_ack_buffer, ++ kMaxMessageLength, ++ &bytes_read, ++ NULL); ++ ++ if (!bytes_read) { ++ ack_callback.Run(nullptr); ++ } else { ++ base::span out_span(read_ack_buffer, read_ack_buffer + bytes_read); ++ ack_callback.Run(&out_span); ++ } ++ ::CloseHandle(read_ack_pipe); ++} ++ + // Code roughly based on Mozilla. + ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcess() { + if (is_virtualized_) +@@ -300,8 +424,9 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcess() { return PROCESS_NONE; } - switch (chrome::AttemptToNotifyRunningChrome(remote_window_)) { + switch (chrome::AttemptToNotifyRunningChrome(remote_window_, additional_data_)) { case chrome::NOTIFY_SUCCESS: ++ ReadAck(notification_ack_callback_); return PROCESS_NOTIFIED; case chrome::NOTIFY_FAILED: + remote_window_ = NULL; +@@ -431,6 +556,18 @@ bool ProcessSingleton::Create() { + << "Lock file can not be created! Error code: " << error; + + if (lock_file_ != INVALID_HANDLE_VALUE) { ++ // We are the first instance. Create a pipe to send out ack data. ++ ack_pipe_ = ::CreateNamedPipe(kPipeName, ++ PIPE_ACCESS_OUTBOUND, ++ PIPE_TYPE_BYTE | PIPE_REJECT_REMOTE_CLIENTS, ++ PIPE_UNLIMITED_INSTANCES, ++ kMaxMessageLength, ++ 0, ++ kPipeTimeout, ++ NULL); ++ CHECK(ack_pipe_ != INVALID_HANDLE_VALUE); ++ g_write_ack_pipe = ack_pipe_; ++ + // Set the window's title to the path of our user data directory so + // other Chrome instances can decide if they should forward to us. + bool result = +@@ -457,6 +594,7 @@ bool ProcessSingleton::Create() { + } + + void ProcessSingleton::Cleanup() { ++ ::CloseHandle(ack_pipe_); + } + + void ProcessSingleton::OverrideShouldKillRemoteProcessCallbackForTesting( diff --git a/chrome/browser/win/chrome_process_finder.cc b/chrome/browser/win/chrome_process_finder.cc index b4fec8878c37b9d157ea768e3b6d99399a988c75..e1cb0f21f752aaeee2c360ce9c5fd08bfede26e3 100644 --- a/chrome/browser/win/chrome_process_finder.cc diff --git a/shell/browser/api/electron_api_app.cc b/shell/browser/api/electron_api_app.cc index a9262afd261f..a2748d2ab00a 100644 --- a/shell/browser/api/electron_api_app.cc +++ b/shell/browser/api/electron_api_app.cc @@ -517,21 +517,24 @@ bool NotificationCallbackWrapper( const base::RepeatingCallback< void(const base::CommandLine& command_line, const base::FilePath& current_directory, - const std::vector additional_data)>& callback, + const std::vector additional_data, + const ProcessSingleton::NotificationAckCallback& ack_callback)>& + callback, const base::CommandLine& cmd, const base::FilePath& cwd, - const std::vector additional_data) { + const std::vector additional_data, + const ProcessSingleton::NotificationAckCallback& ack_callback) { // Make sure the callback is called after app gets ready. if (Browser::Get()->is_ready()) { - callback.Run(cmd, cwd, std::move(additional_data)); + callback.Run(cmd, cwd, std::move(additional_data), ack_callback); } else { scoped_refptr task_runner( base::ThreadTaskRunnerHandle::Get()); // Make a copy of the span so that the data isn't lost. - task_runner->PostTask(FROM_HERE, - base::BindOnce(base::IgnoreResult(callback), cmd, cwd, - std::move(additional_data))); + task_runner->PostTask( + FROM_HERE, base::BindOnce(base::IgnoreResult(callback), cmd, cwd, + std::move(additional_data), ack_callback)); } // ProcessSingleton needs to know whether current process is quiting. return !Browser::Get()->is_shutting_down(); @@ -1079,15 +1082,54 @@ std::string App::GetLocaleCountryCode() { return region.size() == 2 ? region : std::string(); } -void App::OnSecondInstance(const base::CommandLine& cmd, - const base::FilePath& cwd, - const std::vector additional_data) { +void App::OnFirstInstanceAck( + const base::span* first_instance_data) { + v8::Isolate* isolate = JavascriptEnvironment::GetIsolate(); + v8::Locker locker(isolate); + v8::HandleScope handle_scope(isolate); + base::Value data_to_send; + if (first_instance_data) { + // Don't send back the local directly, because it might be empty. + v8::Local data_local; + data_local = DeserializeV8Value(isolate, *first_instance_data); + if (!data_local.IsEmpty()) { + gin::ConvertFromV8(isolate, data_local, &data_to_send); + } + } + Emit("first-instance-ack", data_to_send); +} + +// This function handles the user calling +// the callback parameter sent out by the second-instance event. +static void AckCallbackWrapper( + const ProcessSingleton::NotificationAckCallback& ack_callback, + gin::Arguments* args) { + blink::CloneableMessage ack_message; + args->GetNext(&ack_message); + if (!ack_message.encoded_message.empty()) { + ack_callback.Run(&ack_message.encoded_message); + } else { + ack_callback.Run(nullptr); + } +} + +void App::OnSecondInstance( + const base::CommandLine& cmd, + const base::FilePath& cwd, + const std::vector additional_data, + const ProcessSingleton::NotificationAckCallback& ack_callback) { v8::Isolate* isolate = JavascriptEnvironment::GetIsolate(); v8::Locker locker(isolate); v8::HandleScope handle_scope(isolate); v8::Local data_value = DeserializeV8Value(isolate, std::move(additional_data)); - Emit("second-instance", cmd.argv(), cwd, data_value); + auto cb = base::BindRepeating(&AckCallbackWrapper, ack_callback); + bool prevent_default = + Emit("second-instance", cmd.argv(), cwd, data_value, cb); + if (!prevent_default) { + // Call the callback ourselves, and send back nothing. + ack_callback.Run(nullptr); + } } bool App::HasSingleInstanceLock() const { @@ -1106,6 +1148,9 @@ bool App::RequestSingleInstanceLock(gin::Arguments* args) { base::PathService::Get(chrome::DIR_USER_DATA, &user_dir); auto cb = base::BindRepeating(&App::OnSecondInstance, base::Unretained(this)); + auto wrapped_cb = base::BindRepeating(NotificationCallbackWrapper, cb); + auto ack_cb = + base::BindRepeating(&App::OnFirstInstanceAck, base::Unretained(this)); blink::CloneableMessage additional_data_message; args->GetNext(&additional_data_message); @@ -1114,11 +1159,10 @@ bool App::RequestSingleInstanceLock(gin::Arguments* args) { IsSandboxEnabled(base::CommandLine::ForCurrentProcess()); process_singleton_ = std::make_unique( program_name, user_dir, additional_data_message.encoded_message, - app_is_sandboxed, base::BindRepeating(NotificationCallbackWrapper, cb)); + app_is_sandboxed, wrapped_cb, ack_cb); #else process_singleton_ = std::make_unique( - user_dir, additional_data_message.encoded_message, - base::BindRepeating(NotificationCallbackWrapper, cb)); + user_dir, additional_data_message.encoded_message, wrapped_cb, ack_cb); #endif switch (process_singleton_->NotifyOtherProcessOrCreate()) { diff --git a/shell/browser/api/electron_api_app.h b/shell/browser/api/electron_api_app.h index 4f48584e996d..c0de967f140f 100644 --- a/shell/browser/api/electron_api_app.h +++ b/shell/browser/api/electron_api_app.h @@ -192,9 +192,12 @@ class App : public ElectronBrowserClient::Delegate, void SetDesktopName(const std::string& desktop_name); std::string GetLocale(); std::string GetLocaleCountryCode(); - void OnSecondInstance(const base::CommandLine& cmd, - const base::FilePath& cwd, - const std::vector additional_data); + void OnFirstInstanceAck(const base::span* first_instance_data); + void OnSecondInstance( + const base::CommandLine& cmd, + const base::FilePath& cwd, + const std::vector additional_data, + const ProcessSingleton::NotificationAckCallback& ack_callback); bool HasSingleInstanceLock() const; bool RequestSingleInstanceLock(gin::Arguments* args); void ReleaseSingleInstanceLock(); diff --git a/spec-main/api-app-spec.ts b/spec-main/api-app-spec.ts index 79632682f6e4..9406eddc749b 100644 --- a/spec-main/api-app-spec.ts +++ b/spec-main/api-app-spec.ts @@ -208,18 +208,23 @@ describe('app module', () => { interface SingleInstanceLockTestArgs { args: string[]; expectedAdditionalData: unknown; + expectedAck: unknown; } it('prevents the second launch of app', async function () { - this.timeout(120000); - const appPath = path.join(fixturesPath, 'api', 'singleton-data'); + this.timeout(60000); + const appPath = path.join(fixturesPath, 'api', 'singleton'); const first = cp.spawn(process.execPath, [appPath]); + const firstExited = emittedOnce(first, 'exit'); await emittedOnce(first.stdout, 'data'); + // Start second app when received output. const second = cp.spawn(process.execPath, [appPath]); - const [code2] = await emittedOnce(second, 'exit'); + const secondExited = emittedOnce(second, 'exit'); + + const [code2] = await secondExited; expect(code2).to.equal(1); - const [code1] = await emittedOnce(first, 'exit'); + const [code1] = await firstExited; expect(code1).to.equal(0); }); @@ -238,6 +243,11 @@ describe('app module', () => { const secondInstanceArgs = [process.execPath, appPath, ...testArgs.args, '--some-switch', 'some-arg']; const second = cp.spawn(secondInstanceArgs[0], secondInstanceArgs.slice(1)); const secondExited = emittedOnce(second, 'exit'); + const secondStdoutLines = second.stdout.pipe(split()); + let ackData; + while ((ackData = await emittedOnce(secondStdoutLines, 'data'))[0].toString().length === 0) { + // This isn't valid data. + } const [code2] = await secondExited; expect(code2).to.equal(1); @@ -247,6 +257,7 @@ describe('app module', () => { const [args, additionalData] = dataFromSecondInstance[0].toString('ascii').split('||'); const secondInstanceArgsReceived: string[] = JSON.parse(args.toString('ascii')); const secondInstanceDataReceived = JSON.parse(additionalData.toString('ascii')); + const dataAckReceived = JSON.parse(ackData[0].toString('ascii')); // Ensure secondInstanceArgs is a subset of secondInstanceArgsReceived for (const arg of secondInstanceArgs) { @@ -255,69 +266,113 @@ describe('app module', () => { } expect(secondInstanceDataReceived).to.be.deep.equal(testArgs.expectedAdditionalData, `received data ${JSON.stringify(secondInstanceDataReceived)} is not equal to expected data ${JSON.stringify(testArgs.expectedAdditionalData)}.`); + expect(dataAckReceived).to.be.deep.equal(testArgs.expectedAck, + `received data ${JSON.stringify(dataAckReceived)} is not equal to expected data ${JSON.stringify(testArgs.expectedAck)}.`); } - it('passes arguments to the second-instance event no additional data', async () => { + const expectedAdditionalData = { + level: 1, + testkey: 'testvalue1', + inner: { + level: 2, + testkey: 'testvalue2' + } + }; + + const expectedAck = { + level: 1, + testkey: 'acktestvalue1', + inner: { + level: 2, + testkey: 'acktestvalue2' + } + }; + + it('passes arguments to the second-instance event with no additional data', async () => { await testArgumentPassing({ args: [], - expectedAdditionalData: null + expectedAdditionalData: null, + expectedAck: null }); }); - it('sends and receives JSON object data', async () => { - const expectedAdditionalData = { - level: 1, - testkey: 'testvalue1', - inner: { - level: 2, - testkey: 'testvalue2' - } - }; + it('passes arguments to the second-instance event', async () => { await testArgumentPassing({ args: ['--send-data'], - expectedAdditionalData + expectedAdditionalData, + expectedAck: null + }); + }); + + it('gets back an ack after preventing default', async () => { + await testArgumentPassing({ + args: ['--send-ack', '--prevent-default'], + expectedAdditionalData: null, + expectedAck + }); + }); + + it('is able to send back empty ack after preventing default', async () => { + await testArgumentPassing({ + args: ['--prevent-default'], + expectedAdditionalData: null, + expectedAck: null + }); + }); + + it('sends and receives data', async () => { + await testArgumentPassing({ + args: ['--send-ack', '--prevent-default', '--send-data'], + expectedAdditionalData, + expectedAck }); }); it('sends and receives numerical data', async () => { await testArgumentPassing({ - args: ['--send-data', '--data-content=2'], - expectedAdditionalData: 2 + args: ['--send-ack', '--ack-content=1', '--prevent-default', '--send-data', '--data-content=2'], + expectedAdditionalData: 2, + expectedAck: 1 }); }); it('sends and receives string data', async () => { await testArgumentPassing({ - args: ['--send-data', '--data-content="data"'], - expectedAdditionalData: 'data' + args: ['--send-ack', '--ack-content="ack"', '--prevent-default', '--send-data', '--data-content="data"'], + expectedAdditionalData: 'data', + expectedAck: 'ack' }); }); it('sends and receives boolean data', async () => { await testArgumentPassing({ - args: ['--send-data', '--data-content=false'], - expectedAdditionalData: false + args: ['--send-ack', '--ack-content=true', '--prevent-default', '--send-data', '--data-content=false'], + expectedAdditionalData: false, + expectedAck: true }); }); it('sends and receives array data', async () => { await testArgumentPassing({ - args: ['--send-data', '--data-content=[2, 3, 4]'], - expectedAdditionalData: [2, 3, 4] + args: ['--send-ack', '--ack-content=[1, 2, 3]', '--prevent-default', '--send-data', '--data-content=[2, 3, 4]'], + expectedAdditionalData: [2, 3, 4], + expectedAck: [1, 2, 3] }); }); it('sends and receives mixed array data', async () => { await testArgumentPassing({ - args: ['--send-data', '--data-content=["2", true, 4]'], - expectedAdditionalData: ['2', true, 4] + args: ['--send-ack', '--ack-content=["1", true, 3]', '--prevent-default', '--send-data', '--data-content=["2", false, 4]'], + expectedAdditionalData: ['2', false, 4], + expectedAck: ['1', true, 3] }); }); it('sends and receives null data', async () => { await testArgumentPassing({ - args: ['--send-data', '--data-content=null'], - expectedAdditionalData: null + args: ['--send-ack', '--ack-content=null', '--prevent-default', '--send-data', '--data-content=null'], + expectedAdditionalData: null, + expectedAck: null }); }); @@ -325,7 +380,8 @@ describe('app module', () => { try { await testArgumentPassing({ args: ['--send-ack', '--ack-content="undefined"', '--prevent-default', '--send-data', '--data-content="undefined"'], - expectedAdditionalData: undefined + expectedAdditionalData: undefined, + expectedAck: undefined }); assert(false); } catch (e) { diff --git a/spec/fixtures/api/singleton-data/main.js b/spec/fixtures/api/singleton-data/main.js index 50a623410ea8..f79ed9ccc9bc 100644 --- a/spec/fixtures/api/singleton-data/main.js +++ b/spec/fixtures/api/singleton-data/main.js @@ -1,12 +1,18 @@ const { app } = require('electron'); -// Send data from the second instance to the first instance. -const sendAdditionalData = app.commandLine.hasSwitch('send-data'); - app.whenReady().then(() => { console.log('started'); // ping parent }); +// Send data from the second instance to the first instance. +const sendAdditionalData = app.commandLine.hasSwitch('send-data'); + +// Prevent the default behaviour of second-instance, which sends back an empty ack. +const preventDefault = app.commandLine.hasSwitch('prevent-default'); + +// Send an object back for the ack rather than undefined. +const sendAck = app.commandLine.hasSwitch('send-ack'); + let obj = { level: 1, testkey: 'testvalue1', @@ -15,20 +21,45 @@ let obj = { testkey: 'testvalue2' } }; +let ackObj = { + level: 1, + testkey: 'acktestvalue1', + inner: { + level: 2, + testkey: 'acktestvalue2' + } +}; + if (app.commandLine.hasSwitch('data-content')) { obj = JSON.parse(app.commandLine.getSwitchValue('data-content')); if (obj === 'undefined') { obj = undefined; } } +if (app.commandLine.hasSwitch('ack-content')) { + ackObj = JSON.parse(app.commandLine.getSwitchValue('ack-content')); + if (ackObj === 'undefined') { + ackObj = undefined; + } +} + +app.on('first-instance-ack', (event, additionalData) => { + console.log(JSON.stringify(additionalData)); +}); const gotTheLock = sendAdditionalData ? app.requestSingleInstanceLock(obj) : app.requestSingleInstanceLock(); -app.on('second-instance', (event, args, workingDirectory, data) => { +app.on('second-instance', (event, args, workingDirectory, data, ackCallback) => { + if (preventDefault) { + event.preventDefault(); + } setImmediate(() => { console.log([JSON.stringify(args), JSON.stringify(data)].join('||')); - app.exit(0); + sendAck ? ackCallback(ackObj) : ackCallback(); + setImmediate(() => { + app.exit(0); + }); }); });