From 38c21b7acad9f7ee234a190addad5f25f59440de Mon Sep 17 00:00:00 2001 From: Keeley Hammond Date: Sun, 22 May 2022 22:20:54 -0700 Subject: [PATCH] revert: add first-instance-ack event to the `app.requestSingleInstanceLock()` flow (#34297) fix: revert "feat: add first-instance-ack event to the `app.requestSingleInstanceLock()` flow" --- docs/api/app.md | 40 +- patches/chromium/.patches | 2 +- ...d_data_parameter_to_processsingleton.patch | 347 ++++++++++ ...ransfer_to_requestsingleinstancelock.patch | 640 ------------------ shell/browser/api/electron_api_app.cc | 69 +- shell/browser/api/electron_api_app.h | 9 +- spec-main/api-app-spec.ts | 114 +--- spec/fixtures/api/singleton-data/main.js | 41 +- 8 files changed, 400 insertions(+), 862 deletions(-) create mode 100644 patches/chromium/feat_add_data_parameter_to_processsingleton.patch delete mode 100644 patches/chromium/feat_add_data_transfer_to_requestsingleinstancelock.patch diff --git a/docs/api/app.md b/docs/api/app.md index 16207e255c60..94b8c954fc44 100755 --- a/docs/api/app.md +++ b/docs/api/app.md @@ -484,7 +484,6 @@ 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()`. @@ -496,35 +495,12 @@ 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: @@ -998,33 +974,21 @@ 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) if (!gotTheLock) { app.quit() } else { - app.on('second-instance', (event, commandLine, workingDirectory, additionalData, ackCallback) => { - // We must call preventDefault if we're sending back data. - event.preventDefault() + app.on('second-instance', (event, commandLine, workingDirectory, additionalData) => { // Print out data received from the second instance. - // Expected output: '{"myKey":"myValue"}' - console.log(JSON.stringify(additionalData)) + console.log(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 f4512709cbcc..cc794e8aec17 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -96,11 +96,11 @@ 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 load_v8_snapshot_in_browser_process.patch fix_patch_out_permissions_checks_in_exclusive_access.patch fix_aspect_ratio_with_max_size.patch fix_dont_delete_SerialPortManager_on_main_thread.patch -feat_add_data_transfer_to_requestsingleinstancelock.patch fix_crash_when_saving_edited_pdf_files.patch port_autofill_colors_to_the_color_pipeline.patch build_disable_partition_alloc_on_mac.patch diff --git a/patches/chromium/feat_add_data_parameter_to_processsingleton.patch b/patches/chromium/feat_add_data_parameter_to_processsingleton.patch new file mode 100644 index 000000000000..8a8f91328112 --- /dev/null +++ b/patches/chromium/feat_add_data_parameter_to_processsingleton.patch @@ -0,0 +1,347 @@ +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 + +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. + +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. + +diff --git a/chrome/browser/process_singleton.h b/chrome/browser/process_singleton.h +index 5a64220aaf1309832dc0ad543e353de67fe0a779..55a2a78ce166a65cd11b26e0aa31968f6a10bec8 100644 +--- a/chrome/browser/process_singleton.h ++++ b/chrome/browser/process_singleton.h +@@ -18,6 +18,7 @@ + #include "base/files/file_path.h" + #include "base/memory/ref_counted.h" + #include "base/process/process.h" ++#include "base/containers/span.h" + #include "ui/gfx/native_widget_types.h" + + #if BUILDFLAG(IS_POSIX) && !BUILDFLAG(IS_ANDROID) +@@ -99,22 +100,25 @@ class ProcessSingleton { + // handled within the current browser instance or false if the remote process + // should handle it (i.e., because the current process is shutting down). + using NotificationCallback = +- base::RepeatingCallback; ++ base::RepeatingCallback additional_data)>; + + #if BUILDFLAG(IS_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); + #else + ProcessSingleton(const base::FilePath& user_data_dir, ++ const base::span additional_data, + const NotificationCallback& notification_callback); ++#endif + + ProcessSingleton(const ProcessSingleton&) = delete; + ProcessSingleton& operator=(const ProcessSingleton&) = delete; + +-#endif + ~ProcessSingleton(); + + // Notify another process, if available. Otherwise sets ourselves as the +@@ -177,7 +181,10 @@ class ProcessSingleton { + #endif + + private: ++ // A callback to run when the first instance receives data from the second. + NotificationCallback notification_callback_; // Handler for notifications. ++ // Custom data to pass to the other instance during notify. ++ base::span additional_data_; + + #if BUILDFLAG(IS_WIN) + bool EscapeVirtualization(const base::FilePath& user_data_dir); +diff --git a/chrome/browser/process_singleton_posix.cc b/chrome/browser/process_singleton_posix.cc +index 9bb12894da06fc7d281daced754b240afa9bedeb..5762d0778c2f368019b75364e81b66fc4e2f5751 100644 +--- a/chrome/browser/process_singleton_posix.cc ++++ b/chrome/browser/process_singleton_posix.cc +@@ -611,6 +611,7 @@ class ProcessSingleton::LinuxWatcher + // |reader| is for sending back ACK message. + void HandleMessage(const std::string& current_dir, + const std::vector& argv, ++ const std::vector additional_data, + SocketReader* reader); + + private: +@@ -665,13 +666,16 @@ void ProcessSingleton::LinuxWatcher::StartListening(int socket) { + } + + void ProcessSingleton::LinuxWatcher::HandleMessage( +- const std::string& current_dir, const std::vector& argv, ++ const std::string& current_dir, ++ const std::vector& argv, ++ const std::vector additional_data, + SocketReader* reader) { + DCHECK(ui_task_runner_->BelongsToCurrentThread()); + DCHECK(reader); + + if (parent_->notification_callback_.Run(base::CommandLine(argv), +- base::FilePath(current_dir))) { ++ base::FilePath(current_dir), ++ std::move(additional_data))) { + // Send back "ACK" message to prevent the client process from starting up. + reader->FinishWithACK(kACKToken, std::size(kACKToken) - 1); + } else { +@@ -719,7 +723,8 @@ void ProcessSingleton::LinuxWatcher::SocketReader:: + } + } + +- // Validate the message. The shortest message is kStartToken\0x\0x ++ // Validate the message. The shortest message kStartToken\0\00 ++ // The shortest message with additional data is kStartToken\0\00\00\0. + const size_t kMinMessageLength = std::size(kStartToken) + 4; + if (bytes_read_ < kMinMessageLength) { + buf_[bytes_read_] = 0; +@@ -749,10 +754,28 @@ void ProcessSingleton::LinuxWatcher::SocketReader:: + tokens.erase(tokens.begin()); + tokens.erase(tokens.begin()); + ++ size_t num_args; ++ base::StringToSizeT(tokens[0], &num_args); ++ std::vector command_line(tokens.begin() + 1, tokens.begin() + 1 + num_args); ++ ++ std::vector additional_data; ++ if (tokens.size() >= 3 + num_args) { ++ size_t additional_data_size; ++ base::StringToSizeT(tokens[1 + num_args], &additional_data_size); ++ std::string remaining_args = base::JoinString( ++ base::make_span(tokens.begin() + 2 + num_args, tokens.end()), ++ 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); ++ } ++ + // Return to the UI thread to handle opening a new browser tab. + ui_task_runner_->PostTask( + FROM_HERE, base::BindOnce(&ProcessSingleton::LinuxWatcher::HandleMessage, +- parent_, current_dir, tokens, this)); ++ parent_, current_dir, command_line, ++ std::move(additional_data), this)); + fd_watch_controller_.reset(); + + // LinuxWatcher::HandleMessage() is in charge of destroying this SocketReader +@@ -781,8 +804,10 @@ void ProcessSingleton::LinuxWatcher::SocketReader::FinishWithACK( + // + ProcessSingleton::ProcessSingleton( + const base::FilePath& user_data_dir, ++ const base::span additional_data, + const NotificationCallback& notification_callback) + : notification_callback_(notification_callback), ++ additional_data_(additional_data), + current_pid_(base::GetCurrentProcId()), + watcher_(new LinuxWatcher(this)) { + socket_path_ = user_data_dir.Append(chrome::kSingletonSocketFilename); +@@ -901,7 +926,8 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessWithTimeout( + sizeof(socket_timeout)); + + // Found another process, prepare our command line +- // format is "START\0\0\0...\0". ++ // format is "START\0\0\0\0...\0 ++ // \0\0". + std::string to_send(kStartToken); + to_send.push_back(kTokenDelimiter); + +@@ -911,11 +937,21 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessWithTimeout( + to_send.append(current_dir.value()); + + const std::vector& argv = cmd_line.argv(); ++ to_send.push_back(kTokenDelimiter); ++ to_send.append(base::NumberToString(argv.size())); + for (auto it = argv.begin(); it != argv.end(); ++it) { + to_send.push_back(kTokenDelimiter); + to_send.append(*it); + } + ++ size_t data_to_send_size = additional_data_.size_bytes(); ++ if (data_to_send_size) { ++ to_send.push_back(kTokenDelimiter); ++ to_send.append(base::NumberToString(data_to_send_size)); ++ to_send.push_back(kTokenDelimiter); ++ to_send.append(reinterpret_cast(additional_data_.data()), data_to_send_size); ++ } ++ + // 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. +diff --git a/chrome/browser/process_singleton_win.cc b/chrome/browser/process_singleton_win.cc +index 0c87fc8ccb4511904f19b76ae5e03a5df6664391..c34d4fe10781e6b9286a43176f7312da4e815caf 100644 +--- a/chrome/browser/process_singleton_win.cc ++++ b/chrome/browser/process_singleton_win.cc +@@ -80,10 +80,12 @@ BOOL CALLBACK BrowserWindowEnumeration(HWND window, LPARAM param) { + + bool ParseCommandLine(const COPYDATASTRUCT* cds, + base::CommandLine* parsed_command_line, +- base::FilePath* current_directory) { ++ base::FilePath* current_directory, ++ std::vector* parsed_additional_data) { + // We should have enough room for the shortest command (min_message_size) + // and also be a multiple of wchar_t bytes. The shortest command +- // possible is L"START\0\0" (empty current directory and command line). ++ // possible is L"START\0\0" (empty command line, current directory, ++ // and additional data). + static const int min_message_size = 7; + if (cds->cbData < min_message_size * sizeof(wchar_t) || + cds->cbData % sizeof(wchar_t) != 0) { +@@ -133,6 +135,37 @@ 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); ++ ++ const std::wstring::size_type fourth_null = ++ msg.find_first_of(L'\0', third_null + 1); ++ if (fourth_null == std::wstring::npos || ++ fourth_null == msg.length()) { ++ // No additional data was provided. ++ return true; ++ } ++ ++ // Get length of the additional data. ++ const std::wstring additional_data_length_string = ++ msg.substr(third_null + 1, fourth_null - third_null); ++ size_t additional_data_length; ++ base::StringToSizeT(additional_data_length_string, &additional_data_length); ++ ++ const std::wstring::size_type fifth_null = ++ msg.find_first_of(L'\0', fourth_null + 1); ++ if (fifth_null == std::wstring::npos || ++ fifth_null == msg.length()) { ++ LOG(WARNING) << "Invalid format for start command, we need a string in 6 " ++ "parts separated by NULLs"; ++ } ++ ++ // Get the actual additional data. ++ const std::wstring additional_data = ++ msg.substr(fourth_null + 1, fifth_null - fourth_null); ++ const uint8_t* additional_data_bytes = ++ reinterpret_cast(additional_data.c_str()); ++ *parsed_additional_data = std::vector(additional_data_bytes, ++ additional_data_bytes + additional_data_length); ++ + return true; + } + return false; +@@ -154,13 +187,14 @@ bool ProcessLaunchNotification( + + base::CommandLine parsed_command_line(base::CommandLine::NO_PROGRAM); + 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)) { + *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; + return true; + } + +@@ -261,9 +295,11 @@ 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) + : notification_callback_(notification_callback), ++ additional_data_(additional_data), + program_name_(program_name), + is_app_sandboxed_(is_app_sandboxed), + is_virtualized_(false), +@@ -290,7 +326,7 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcess() { + return PROCESS_NONE; + } + +- switch (chrome::AttemptToNotifyRunningChrome(remote_window_)) { ++ switch (chrome::AttemptToNotifyRunningChrome(remote_window_, additional_data_)) { + case chrome::NOTIFY_SUCCESS: + return PROCESS_NOTIFIED; + case chrome::NOTIFY_FAILED: +diff --git a/chrome/browser/win/chrome_process_finder.cc b/chrome/browser/win/chrome_process_finder.cc +index b64ed1d155a30582e48c9cdffcee9d0f25a53a6a..cfdb2d75532d270e3dd548eb7475a6cdbddf1016 100644 +--- a/chrome/browser/win/chrome_process_finder.cc ++++ b/chrome/browser/win/chrome_process_finder.cc +@@ -36,7 +36,9 @@ HWND FindRunningChromeWindow(const base::FilePath& user_data_dir) { + return base::win::MessageWindow::FindWindow(user_data_dir.value()); + } + +-NotifyChromeResult AttemptToNotifyRunningChrome(HWND remote_window) { ++NotifyChromeResult AttemptToNotifyRunningChrome( ++ HWND remote_window, ++ const base::span additional_data) { + TRACE_EVENT0("startup", "AttemptToNotifyRunningChrome"); + + DCHECK(remote_window); +@@ -50,7 +52,8 @@ NotifyChromeResult AttemptToNotifyRunningChrome(HWND remote_window) { + } + + // Send the command line to the remote chrome window. +- // Format is "START\0<<>>\0<<>>". ++ // Format is ++ // "START\0\0\0\0". + std::wstring to_send(L"START\0", 6); // want the NULL in the string. + base::FilePath cur_dir; + if (!base::GetCurrentDirectory(&cur_dir)) { +@@ -64,6 +67,22 @@ NotifyChromeResult AttemptToNotifyRunningChrome(HWND remote_window) { + base::CommandLine::ForCurrentProcess()->GetCommandLineString()); + to_send.append(L"\0", 1); // Null separator. + ++ size_t additional_data_size = additional_data.size_bytes(); ++ if (additional_data_size) { ++ // Send over the size, because the reinterpret cast to wchar_t could ++ // add padding. ++ to_send.append(base::UTF8ToWide(base::NumberToString(additional_data_size))); ++ to_send.append(L"\0", 1); // Null separator. ++ ++ size_t padded_size = additional_data_size / sizeof(wchar_t); ++ if (additional_data_size % sizeof(wchar_t) != 0) { ++ padded_size++; ++ } ++ to_send.append(reinterpret_cast(additional_data.data()), ++ padded_size); ++ to_send.append(L"\0", 1); // Null separator. ++ } ++ + // Allow the current running browser window to make itself the foreground + // window (otherwise it will just flash in the taskbar). + ::AllowSetForegroundWindow(process_id); +diff --git a/chrome/browser/win/chrome_process_finder.h b/chrome/browser/win/chrome_process_finder.h +index 5516673cee019f6060077091e59498bf9038cd6e..8edea5079b46c2cba67833114eb9c21d85cfc22d 100644 +--- a/chrome/browser/win/chrome_process_finder.h ++++ b/chrome/browser/win/chrome_process_finder.h +@@ -7,6 +7,7 @@ + + #include + ++#include "base/containers/span.h" + #include "base/time/time.h" + + namespace base { +@@ -27,7 +28,9 @@ HWND FindRunningChromeWindow(const base::FilePath& user_data_dir); + // Attempts to send the current command line to an already running instance of + // Chrome via a WM_COPYDATA message. + // Returns true if a running Chrome is found and successfully notified. +-NotifyChromeResult AttemptToNotifyRunningChrome(HWND remote_window); ++NotifyChromeResult AttemptToNotifyRunningChrome( ++ HWND remote_window, ++ const base::span additional_data); + + // Changes the notification timeout to |new_timeout|, returns the old timeout. + base::TimeDelta SetNotificationTimeoutForTesting(base::TimeDelta new_timeout); diff --git a/patches/chromium/feat_add_data_transfer_to_requestsingleinstancelock.patch b/patches/chromium/feat_add_data_transfer_to_requestsingleinstancelock.patch deleted file mode 100644 index cd4f45272657..000000000000 --- a/patches/chromium/feat_add_data_transfer_to_requestsingleinstancelock.patch +++ /dev/null @@ -1,640 +0,0 @@ -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 transfer mechanism to requestSingleInstanceLock flow - -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. - -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 5a64220aaf1309832dc0ad543e353de67fe0a779..5b701b1361707b610ed60c344e441e67ca701362 100644 ---- a/chrome/browser/process_singleton.h -+++ b/chrome/browser/process_singleton.h -@@ -18,6 +18,7 @@ - #include "base/files/file_path.h" - #include "base/memory/ref_counted.h" - #include "base/process/process.h" -+#include "base/containers/span.h" - #include "ui/gfx/native_widget_types.h" - - #if BUILDFLAG(IS_POSIX) && !BUILDFLAG(IS_ANDROID) -@@ -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 NotificationAckCallback& ack_callback)>; - - #if BUILDFLAG(IS_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 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 NotificationAckCallback& ack_notification_callback); -+#endif - - ProcessSingleton(const ProcessSingleton&) = delete; - ProcessSingleton& operator=(const ProcessSingleton&) = delete; - --#endif - ~ProcessSingleton(); - - // Notify another process, if available. Otherwise sets ourselves as the -@@ -177,7 +187,13 @@ class ProcessSingleton { - #endif - - private: -- 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 BUILDFLAG(IS_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 BUILDFLAG(IS_POSIX) && !BUILDFLAG(IS_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 9bb12894da06fc7d281daced754b240afa9bedeb..52717600e7346daeb0726c177162c718da53500a 100644 ---- a/chrome/browser/process_singleton_posix.cc -+++ b/chrome/browser/process_singleton_posix.cc -@@ -145,7 +145,7 @@ const char kACKToken[] = "ACK"; - const char kShutdownToken[] = "SHUTDOWN"; - const char kTokenDelimiter = '\0'; - const int kMaxMessageLength = 32 * 1024; --const int kMaxACKMessageLength = std::size(kShutdownToken) - 1; -+const int kMaxACKMessageLength = kMaxMessageLength; - - bool g_disable_prompt = false; - bool g_skip_is_chrome_process_check = false; -@@ -611,6 +611,7 @@ class ProcessSingleton::LinuxWatcher - // |reader| is for sending back ACK message. - void HandleMessage(const std::string& current_dir, - const std::vector& argv, -+ const std::vector additional_data, - SocketReader* reader); - - private: -@@ -635,6 +636,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_; - }; - -@@ -665,16 +669,21 @@ void ProcessSingleton::LinuxWatcher::StartListening(int socket) { - } - - void ProcessSingleton::LinuxWatcher::HandleMessage( -- const std::string& current_dir, const std::vector& argv, -+ const std::string& current_dir, -+ const std::vector& argv, -+ const std::vector additional_data, - SocketReader* reader) { - DCHECK(ui_task_runner_->BelongsToCurrentThread()); - DCHECK(reader); - -- 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, std::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), -+ 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 -@@ -684,6 +693,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, std::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); -@@ -719,7 +744,8 @@ void ProcessSingleton::LinuxWatcher::SocketReader:: - } - } - -- // Validate the message. The shortest message is kStartToken\0x\0x -+ // Validate the message. The shortest message kStartToken\0\00 -+ // The shortest message with additional data is kStartToken\0\00\00\0. - const size_t kMinMessageLength = std::size(kStartToken) + 4; - if (bytes_read_ < kMinMessageLength) { - buf_[bytes_read_] = 0; -@@ -749,10 +775,28 @@ void ProcessSingleton::LinuxWatcher::SocketReader:: - tokens.erase(tokens.begin()); - tokens.erase(tokens.begin()); - -+ size_t num_args; -+ base::StringToSizeT(tokens[0], &num_args); -+ std::vector command_line(tokens.begin() + 1, tokens.begin() + 1 + num_args); -+ -+ std::vector additional_data; -+ if (tokens.size() >= 3 + num_args) { -+ size_t additional_data_size; -+ base::StringToSizeT(tokens[1 + num_args], &additional_data_size); -+ std::string remaining_args = base::JoinString( -+ base::make_span(tokens.begin() + 2 + num_args, tokens.end()), -+ 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); -+ } -+ - // Return to the UI thread to handle opening a new browser tab. - ui_task_runner_->PostTask( - FROM_HERE, base::BindOnce(&ProcessSingleton::LinuxWatcher::HandleMessage, -- parent_, current_dir, tokens, this)); -+ parent_, current_dir, command_line, -+ std::move(additional_data), this)); - fd_watch_controller_.reset(); - - // LinuxWatcher::HandleMessage() is in charge of destroying this SocketReader -@@ -781,8 +825,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 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); -@@ -901,7 +949,8 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessWithTimeout( - sizeof(socket_timeout)); - - // Found another process, prepare our command line -- // format is "START\0\0\0...\0". -+ // format is "START\0\0\0\0...\0 -+ // \0\0". - std::string to_send(kStartToken); - to_send.push_back(kTokenDelimiter); - -@@ -911,11 +960,21 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessWithTimeout( - to_send.append(current_dir.value()); - - const std::vector& argv = cmd_line.argv(); -+ to_send.push_back(kTokenDelimiter); -+ to_send.append(base::NumberToString(argv.size())); - for (auto it = argv.begin(); it != argv.end(); ++it) { - to_send.push_back(kTokenDelimiter); - to_send.append(*it); - } - -+ size_t data_to_send_size = additional_data_.size_bytes(); -+ if (data_to_send_size) { -+ to_send.push_back(kTokenDelimiter); -+ to_send.append(base::NumberToString(data_to_send_size)); -+ to_send.push_back(kTokenDelimiter); -+ to_send.append(reinterpret_cast(additional_data_.data()), data_to_send_size); -+ } -+ - // 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. -@@ -957,6 +1016,17 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessWithTimeout( - linux_ui->NotifyWindowManagerStartupComplete(); - #endif - -+ size_t ack_data_len = len - (std::size(kACKToken) - 1); -+ if (ack_data_len) { -+ const uint8_t* raw_ack_data = -+ reinterpret_cast(buf + std::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 0c87fc8ccb4511904f19b76ae5e03a5df6664391..83ede77121c5dadbde8b0a4132de1dba6eeb5b52 100644 ---- a/chrome/browser/process_singleton_win.cc -+++ b/chrome/browser/process_singleton_win.cc -@@ -13,15 +13,18 @@ - #include "base/command_line.h" - #include "base/debug/activity_tracker.h" - #include "base/files/file_path.h" -+#include "base/files/file_util.h" - #include "base/logging.h" - #include "base/metrics/histogram_functions.h" - #include "base/metrics/histogram_macros.h" - #include "base/process/process.h" - #include "base/process/process_info.h" - #include "base/strings/escape.h" -+#include "base/rand_util.h" - #include "base/strings/string_number_conversions.h" - #include "base/strings/utf_string_conversions.h" - #include "base/time/time.h" -+#include "base/timer/timer.h" - #include "base/trace_event/base_tracing.h" - #include "base/win/registry.h" - #include "base/win/scoped_handle.h" -@@ -45,6 +48,13 @@ - namespace { - - const char kLockfile[] = "lockfile"; -+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. -@@ -80,10 +90,12 @@ BOOL CALLBACK BrowserWindowEnumeration(HWND window, LPARAM param) { - - bool ParseCommandLine(const COPYDATASTRUCT* cds, - base::CommandLine* parsed_command_line, -- base::FilePath* current_directory) { -+ base::FilePath* current_directory, -+ std::vector* parsed_additional_data) { - // We should have enough room for the shortest command (min_message_size) - // and also be a multiple of wchar_t bytes. The shortest command -- // possible is L"START\0\0" (empty current directory and command line). -+ // possible is L"START\0\0" (empty command line, current directory, -+ // and additional data). - static const int min_message_size = 7; - if (cds->cbData < min_message_size * sizeof(wchar_t) || - cds->cbData % sizeof(wchar_t) != 0) { -@@ -133,11 +145,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); -+ -+ const std::wstring::size_type fourth_null = -+ msg.find_first_of(L'\0', third_null + 1); -+ if (fourth_null == std::wstring::npos || -+ fourth_null == msg.length()) { -+ // No additional data was provided. -+ return true; -+ } -+ -+ // Get length of the additional data. -+ const std::wstring additional_data_length_string = -+ msg.substr(third_null + 1, fourth_null - third_null); -+ size_t additional_data_length; -+ base::StringToSizeT(additional_data_length_string, &additional_data_length); -+ -+ const std::wstring::size_type fifth_null = -+ msg.find_first_of(L'\0', fourth_null + 1); -+ if (fifth_null == std::wstring::npos || -+ fifth_null == msg.length()) { -+ LOG(WARNING) << "Invalid format for start command, we need a string in 6 " -+ "parts separated by NULLs"; -+ } -+ -+ // Get the actual additional data. -+ const std::wstring additional_data = -+ msg.substr(fourth_null + 1, fifth_null - fourth_null); -+ const uint8_t* additional_data_bytes = -+ reinterpret_cast(additional_data.c_str()); -+ *parsed_additional_data = std::vector(additional_data_bytes, -+ additional_data_bytes + additional_data_length); -+ - return true; - } - return false; - } - -+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, -@@ -151,16 +234,35 @@ bool ProcessLaunchNotification( - - // Handle the WM_COPYDATA message from another process. - const COPYDATASTRUCT* cds = reinterpret_cast(lparam); -- - base::CommandLine parsed_command_line(base::CommandLine::NO_PROGRAM); - 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)) { - *result = TRUE; - return true; - } - -- *result = notification_callback.Run(parsed_command_line, current_directory) ? -- TRUE : FALSE; -+ // notification_callback.Run waits for StoreAck to -+ // run to completion before moving onwards. -+ // Therefore, we cannot directly send the SendBackAck -+ // callback instead, as it would hang the program -+ // during the ConnectNamedPipe call. -+ g_write_ack_callback_called = false; -+ *result = notification_callback.Run(parsed_command_line, current_directory, -+ std::move(additional_data), -+ base::BindRepeating(&StoreAck)) -+ ? TRUE -+ : FALSE; -+ if (*result) { -+ // If *result is TRUE, we return NOTIFY_SUCCESS. -+ // Only for that case does the second process read -+ // the acknowledgement. Therefore, only send back -+ // the acknowledgement if *result is TRUE, -+ // otherwise the program hangs during the ConnectNamedPipe call. -+ g_ack_timer.Start(FROM_HERE, base::Seconds(0), -+ base::BindOnce(&SendBackAck)); -+ } - return true; - } - -@@ -261,9 +363,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 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), -@@ -278,6 +384,47 @@ ProcessSingleton::~ProcessSingleton() { - ::CloseHandle(lock_file_); - } - -+void ReadAck(const ProcessSingleton::NotificationAckCallback& ack_callback, -+ const std::string program_name, -+ base::FilePath& user_data_dir) { -+ // We are reading the ack from the first instance. -+ // First, wait for the pipe. -+ HWND remote_window = chrome::FindRunningChromeWindow(user_data_dir); -+ DWORD process_id; -+ DWORD thread_id = GetWindowThreadProcessId(remote_window, &process_id); -+ std::string identifier = base::NumberToString(process_id) + -+ base::NumberToString(thread_id); -+ std::wstring pipe_name = base::UTF8ToWide("\\\\.\\pipe\\" + identifier + -+ program_name); -+ const LPCWSTR w_pipe_name = pipe_name.c_str(); -+ ::WaitNamedPipe(w_pipe_name, NMPWAIT_USE_DEFAULT_WAIT); -+ -+ HANDLE read_ack_pipe = ::CreateFile(w_pipe_name, -+ 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() { - TRACE_EVENT0("startup", "ProcessSingleton::NotifyOtherProcess"); -@@ -290,8 +437,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_, program_name_, user_data_dir_); - return PROCESS_NOTIFIED; - case chrome::NOTIFY_FAILED: - remote_window_ = NULL; -@@ -429,6 +577,26 @@ 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. -+ // Create a per-process pipename using a combination of the -+ // username, process id, thread id, and program name. Pipe names max -+ // at 256 characters, can include any character other than a backslash -+ std::string identifier = base::NumberToString(::GetCurrentProcessId()) + -+ base::NumberToString(::GetCurrentThreadId()); -+ std::wstring pipe_name = base::UTF8ToWide("\\\\.\\pipe\\" + identifier + -+ program_name_); -+ const LPCWSTR w_pipe_name = pipe_name.c_str(); -+ ack_pipe_ = ::CreateNamedPipe(w_pipe_name, -+ 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. - TRACE_EVENT0("startup", "ProcessSingleton::Create:CreateWindow"); -@@ -456,6 +624,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 b64ed1d155a30582e48c9cdffcee9d0f25a53a6a..ce851d09d501ebcc6d6c4065e746e869d5275b2b 100644 ---- a/chrome/browser/win/chrome_process_finder.cc -+++ b/chrome/browser/win/chrome_process_finder.cc -@@ -36,9 +36,10 @@ HWND FindRunningChromeWindow(const base::FilePath& user_data_dir) { - return base::win::MessageWindow::FindWindow(user_data_dir.value()); - } - --NotifyChromeResult AttemptToNotifyRunningChrome(HWND remote_window) { -+NotifyChromeResult AttemptToNotifyRunningChrome( -+ HWND remote_window, -+ const base::span additional_data) { - TRACE_EVENT0("startup", "AttemptToNotifyRunningChrome"); -- - DCHECK(remote_window); - DWORD process_id = 0; - DWORD thread_id = GetWindowThreadProcessId(remote_window, &process_id); -@@ -50,7 +51,8 @@ NotifyChromeResult AttemptToNotifyRunningChrome(HWND remote_window) { - } - - // Send the command line to the remote chrome window. -- // Format is "START\0<<>>\0<<>>". -+ // Format is -+ // "START\0\0\0\0". - std::wstring to_send(L"START\0", 6); // want the NULL in the string. - base::FilePath cur_dir; - if (!base::GetCurrentDirectory(&cur_dir)) { -@@ -64,6 +66,22 @@ NotifyChromeResult AttemptToNotifyRunningChrome(HWND remote_window) { - base::CommandLine::ForCurrentProcess()->GetCommandLineString()); - to_send.append(L"\0", 1); // Null separator. - -+ size_t additional_data_size = additional_data.size_bytes(); -+ if (additional_data_size) { -+ // Send over the size, because the reinterpret cast to wchar_t could -+ // add padding. -+ to_send.append(base::UTF8ToWide(base::NumberToString(additional_data_size))); -+ to_send.append(L"\0", 1); // Null separator. -+ -+ size_t padded_size = additional_data_size / sizeof(wchar_t); -+ if (additional_data_size % sizeof(wchar_t) != 0) { -+ padded_size++; -+ } -+ to_send.append(reinterpret_cast(additional_data.data()), -+ padded_size); -+ to_send.append(L"\0", 1); // Null separator. -+ } -+ - // Allow the current running browser window to make itself the foreground - // window (otherwise it will just flash in the taskbar). - ::AllowSetForegroundWindow(process_id); -diff --git a/chrome/browser/win/chrome_process_finder.h b/chrome/browser/win/chrome_process_finder.h -index 5516673cee019f6060077091e59498bf9038cd6e..8edea5079b46c2cba67833114eb9c21d85cfc22d 100644 ---- a/chrome/browser/win/chrome_process_finder.h -+++ b/chrome/browser/win/chrome_process_finder.h -@@ -7,6 +7,7 @@ - - #include - -+#include "base/containers/span.h" - #include "base/time/time.h" - - namespace base { -@@ -27,7 +28,9 @@ HWND FindRunningChromeWindow(const base::FilePath& user_data_dir); - // Attempts to send the current command line to an already running instance of - // Chrome via a WM_COPYDATA message. - // Returns true if a running Chrome is found and successfully notified. --NotifyChromeResult AttemptToNotifyRunningChrome(HWND remote_window); -+NotifyChromeResult AttemptToNotifyRunningChrome( -+ HWND remote_window, -+ const base::span additional_data); - - // Changes the notification timeout to |new_timeout|, returns the old timeout. - base::TimeDelta SetNotificationTimeoutForTesting(base::TimeDelta new_timeout); diff --git a/shell/browser/api/electron_api_app.cc b/shell/browser/api/electron_api_app.cc index 1cff2713debe..975cb94a33d5 100644 --- a/shell/browser/api/electron_api_app.cc +++ b/shell/browser/api/electron_api_app.cc @@ -522,24 +522,21 @@ bool NotificationCallbackWrapper( const base::RepeatingCallback< void(const base::CommandLine& command_line, const base::FilePath& current_directory, - const std::vector additional_data, - const ProcessSingleton::NotificationAckCallback& ack_callback)>& - callback, + const std::vector additional_data)>& callback, const base::CommandLine& cmd, const base::FilePath& cwd, - const std::vector additional_data, - const ProcessSingleton::NotificationAckCallback& ack_callback) { + const std::vector additional_data) { // Make sure the callback is called after app gets ready. if (Browser::Get()->is_ready()) { - callback.Run(cmd, cwd, std::move(additional_data), ack_callback); + callback.Run(cmd, cwd, std::move(additional_data)); } 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), ack_callback)); + task_runner->PostTask(FROM_HERE, + base::BindOnce(base::IgnoreResult(callback), cmd, cwd, + std::move(additional_data))); } // ProcessSingleton needs to know whether current process is quiting. return !Browser::Get()->is_shutting_down(); @@ -1084,52 +1081,14 @@ std::string App::GetLocaleCountryCode() { return region.size() == 2 ? region : std::string(); } -void App::OnFirstInstanceAck( - const base::span* first_instance_data) { - v8::Isolate* isolate = JavascriptEnvironment::GetIsolate(); - 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) { +void App::OnSecondInstance(const base::CommandLine& cmd, + const base::FilePath& cwd, + const std::vector additional_data) { v8::Isolate* isolate = JavascriptEnvironment::GetIsolate(); v8::HandleScope handle_scope(isolate); v8::Local data_value = DeserializeV8Value(isolate, std::move(additional_data)); - 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); - } + Emit("second-instance", cmd.argv(), cwd, data_value); } bool App::HasSingleInstanceLock() const { @@ -1150,9 +1109,6 @@ bool App::RequestSingleInstanceLock(gin::Arguments* args) { base::CreateDirectoryAndGetError(user_dir, nullptr); 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); @@ -1161,10 +1117,11 @@ 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, wrapped_cb, ack_cb); + app_is_sandboxed, base::BindRepeating(NotificationCallbackWrapper, cb)); #else process_singleton_ = std::make_unique( - user_dir, additional_data_message.encoded_message, wrapped_cb, ack_cb); + user_dir, additional_data_message.encoded_message, + base::BindRepeating(NotificationCallbackWrapper, 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 ebf3cf247b67..d7ff219e0871 100644 --- a/shell/browser/api/electron_api_app.h +++ b/shell/browser/api/electron_api_app.h @@ -193,12 +193,9 @@ class App : public ElectronBrowserClient::Delegate, void SetDesktopName(const std::string& desktop_name); std::string GetLocale(); std::string GetLocaleCountryCode(); - 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); + void OnSecondInstance(const base::CommandLine& cmd, + const base::FilePath& cwd, + const std::vector additional_data); 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 f1af0bc51215..51c202366a51 100644 --- a/spec-main/api-app-spec.ts +++ b/spec-main/api-app-spec.ts @@ -208,23 +208,18 @@ describe('app module', () => { interface SingleInstanceLockTestArgs { args: string[]; expectedAdditionalData: unknown; - expectedAck: unknown; } it('prevents the second launch of app', async function () { - this.timeout(60000); - const appPath = path.join(fixturesPath, 'api', 'singleton'); + this.timeout(120000); + const appPath = path.join(fixturesPath, 'api', 'singleton-data'); 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 secondExited = emittedOnce(second, 'exit'); - - const [code2] = await secondExited; + const [code2] = await emittedOnce(second, 'exit'); expect(code2).to.equal(1); - const [code1] = await firstExited; + const [code1] = await emittedOnce(first, 'exit'); expect(code1).to.equal(0); }); @@ -250,11 +245,6 @@ 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); @@ -264,7 +254,6 @@ 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) { @@ -273,113 +262,69 @@ 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)}.`); } - 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 () => { + it('passes arguments to the second-instance event no additional data', async () => { await testArgumentPassing({ args: [], - expectedAdditionalData: null, - expectedAck: null + expectedAdditionalData: null }); }); - it('passes arguments to the second-instance event', async () => { + it('sends and receives JSON object data', async () => { + const expectedAdditionalData = { + level: 1, + testkey: 'testvalue1', + inner: { + level: 2, + testkey: 'testvalue2' + } + }; await testArgumentPassing({ args: ['--send-data'], - 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 + expectedAdditionalData }); }); it('sends and receives numerical data', async () => { await testArgumentPassing({ - args: ['--send-ack', '--ack-content=1', '--prevent-default', '--send-data', '--data-content=2'], - expectedAdditionalData: 2, - expectedAck: 1 + args: ['--send-data', '--data-content=2'], + expectedAdditionalData: 2 }); }); it('sends and receives string data', async () => { await testArgumentPassing({ - args: ['--send-ack', '--ack-content="ack"', '--prevent-default', '--send-data', '--data-content="data"'], - expectedAdditionalData: 'data', - expectedAck: 'ack' + args: ['--send-data', '--data-content="data"'], + expectedAdditionalData: 'data' }); }); it('sends and receives boolean data', async () => { await testArgumentPassing({ - args: ['--send-ack', '--ack-content=true', '--prevent-default', '--send-data', '--data-content=false'], - expectedAdditionalData: false, - expectedAck: true + args: ['--send-data', '--data-content=false'], + expectedAdditionalData: false }); }); it('sends and receives array data', async () => { await testArgumentPassing({ - 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] + args: ['--send-data', '--data-content=[2, 3, 4]'], + expectedAdditionalData: [2, 3, 4] }); }); it('sends and receives mixed array data', async () => { await testArgumentPassing({ - 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] + args: ['--send-data', '--data-content=["2", true, 4]'], + expectedAdditionalData: ['2', true, 4] }); }); it('sends and receives null data', async () => { await testArgumentPassing({ - args: ['--send-ack', '--ack-content=null', '--prevent-default', '--send-data', '--data-content=null'], - expectedAdditionalData: null, - expectedAck: null + args: ['--send-data', '--data-content=null'], + expectedAdditionalData: null }); }); @@ -387,8 +332,7 @@ describe('app module', () => { try { await testArgumentPassing({ args: ['--send-ack', '--ack-content="undefined"', '--prevent-default', '--send-data', '--data-content="undefined"'], - expectedAdditionalData: undefined, - expectedAck: undefined + expectedAdditionalData: undefined }); assert(false); } catch (e) { diff --git a/spec/fixtures/api/singleton-data/main.js b/spec/fixtures/api/singleton-data/main.js index f79ed9ccc9bc..50a623410ea8 100644 --- a/spec/fixtures/api/singleton-data/main.js +++ b/spec/fixtures/api/singleton-data/main.js @@ -1,17 +1,11 @@ const { app } = require('electron'); -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'); +app.whenReady().then(() => { + console.log('started'); // ping parent +}); let obj = { level: 1, @@ -21,45 +15,20 @@ 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, ackCallback) => { - if (preventDefault) { - event.preventDefault(); - } +app.on('second-instance', (event, args, workingDirectory, data) => { setImmediate(() => { console.log([JSON.stringify(args), JSON.stringify(data)].join('||')); - sendAck ? ackCallback(ackObj) : ackCallback(); - setImmediate(() => { - app.exit(0); - }); + app.exit(0); }); });