Browse Source

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
Raymond Zhao 3 years ago
parent
commit
746927c972

+ 37 - 1
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...

+ 1 - 1
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

+ 292 - 40
patches/chromium/feat_add_data_parameter_to_processsingleton.patch → 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 <[email protected]>
 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<void(const base::span<const uint8_t>* 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<bool(const base::CommandLine& command_line,
 -                                   const base::FilePath& current_directory)>;
 +                                   const base::FilePath& current_directory,
-+                                   const std::vector<const uint8_t> additional_data)>;
++                                   const std::vector<const uint8_t> 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<const uint8_t> 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<const uint8_t> 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<const uint8_t> 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<const uint8_t>* response);
++
+   std::set<std::unique_ptr<SocketReader>, 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<const uint8_t>* 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<const char*>(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<const uint8_t*>(remaining_args.c_str());
-+    additional_data = std::vector<const uint8_t>(
-+        additional_data_bits, additional_data_bits + additional_data_size);
++    additional_data = std::vector<const uint8_t>(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<const uint8_t> 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<std::string>& 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<const uint8_t*>(buf + base::size(kACKToken) - 1);
++      base::span<const uint8_t> 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<std::vector<const uint8_t>> 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<const uint8_t>* ack_data) {
++  if (ack_data) {
++    g_ack_data = std::make_unique<std::vector<const uint8_t>>(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<COPYDATASTRUCT*>(lparam);
@@ -240,39 +415,116 @@ index 679350dd08ca0211653ea669405e3f4f86c2fc0f..daab2f170d857082df56966c7bd8abc5
    base::FilePath current_directory;
 -  if (!ParseCommandLine(cds, &parsed_command_line, &current_directory)) {
 +  std::vector<const uint8_t> additional_data;
-+  if (!ParseCommandLine(cds, &parsed_command_line, &current_directory, &additional_data)) {
++  if (!ParseCommandLine(cds, &parsed_command_line, &current_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<const uint8_t> 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<const uint8_t> 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

+ 57 - 13
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<const uint8_t> additional_data)>& callback,
+             const std::vector<const uint8_t> additional_data,
+             const ProcessSingleton::NotificationAckCallback& ack_callback)>&
+        callback,
     const base::CommandLine& cmd,
     const base::FilePath& cwd,
-    const std::vector<const uint8_t> additional_data) {
+    const std::vector<const uint8_t> 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<base::SingleThreadTaskRunner> 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<const uint8_t> additional_data) {
+void App::OnFirstInstanceAck(
+    const base::span<const uint8_t>* 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<v8::Value> 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<const uint8_t> additional_data,
+    const ProcessSingleton::NotificationAckCallback& ack_callback) {
   v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
   v8::Locker locker(isolate);
   v8::HandleScope handle_scope(isolate);
   v8::Local<v8::Value> 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<ProcessSingleton>(
       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<ProcessSingleton>(
-      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()) {

+ 6 - 3
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<const uint8_t> additional_data);
+  void OnFirstInstanceAck(const base::span<const uint8_t>* first_instance_data);
+  void OnSecondInstance(
+      const base::CommandLine& cmd,
+      const base::FilePath& cwd,
+      const std::vector<const uint8_t> additional_data,
+      const ProcessSingleton::NotificationAckCallback& ack_callback);
   bool HasSingleInstanceLock() const;
   bool RequestSingleInstanceLock(gin::Arguments* args);
   void ReleaseSingleInstanceLock();

+ 85 - 29
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) {

+ 36 - 5
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);
+    });
   });
 });