diff --git a/docs/api/app.md b/docs/api/app.md index c93d383fa7444..aa277b0578487 100755 --- a/docs/api/app.md +++ b/docs/api/app.md @@ -987,7 +987,8 @@ 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. - console.log(additionalData) + // Expected output: '{"myAckKey":"myAckValue"}' + console.log(JSON.stringify(additionalData)) }) const additionalData = { myKey: 'myValue' } @@ -1000,7 +1001,8 @@ if (!gotTheLock) { // 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) { diff --git a/patches/chromium/feat_add_data_transfer_to_requestsingleinstancelock.patch b/patches/chromium/feat_add_data_transfer_to_requestsingleinstancelock.patch index 690c195cc4870..384d31138b728 100644 --- a/patches/chromium/feat_add_data_transfer_to_requestsingleinstancelock.patch +++ b/patches/chromium/feat_add_data_transfer_to_requestsingleinstancelock.patch @@ -28,12 +28,12 @@ index eec994c4252f17d9c9c41e66d5dae6509ed98a18..c16343259158493b914c794f5ec5ae28 #include "base/process/process.h" +#include "base/containers/span.h" #include "ui/gfx/native_widget_types.h" - + #if defined(OS_POSIX) && !defined(OS_ANDROID) @@ -94,6 +95,9 @@ class ProcessSingleton { - + static constexpr int kNumNotifyResults = LAST_VALUE + 1; - + + using NotificationAckCallback = + base::RepeatingCallback* ack_data)>; + @@ -48,7 +48,7 @@ index eec994c4252f17d9c9c41e66d5dae6509ed98a18..c16343259158493b914c794f5ec5ae28 + const base::FilePath& current_directory, + const std::vector additional_data, + const NotificationAckCallback& ack_callback)>; - + #if defined(OS_WIN) ProcessSingleton(const std::string& program_name, const base::FilePath& user_data_dir, @@ -64,17 +64,17 @@ index eec994c4252f17d9c9c41e66d5dae6509ed98a18..c16343259158493b914c794f5ec5ae28 + 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 @@ -178,7 +188,13 @@ class ProcessSingleton { #endif - + private: - NotificationCallback notification_callback_; // Handler for notifications. + // A callback to run when the first instance receives data from the second. @@ -84,7 +84,7 @@ index eec994c4252f17d9c9c41e66d5dae6509ed98a18..c16343259158493b914c794f5ec5ae28 + NotificationAckCallback notification_ack_callback_; + // Custom data to pass to the other instance during notify. + base::span additional_data_; - + #if defined(OS_WIN) bool EscapeVirtualization(const base::FilePath& user_data_dir); @@ -191,6 +207,7 @@ class ProcessSingleton { @@ -96,7 +96,7 @@ index eec994c4252f17d9c9c41e66d5dae6509ed98a18..c16343259158493b914c794f5ec5ae28 // 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 a04d139f958a7aaef9b96e8c29317ccf7c97f009..29188668a69047b3ad3bebd1f0057565a330b509 100644 +index a04d139f958a7aaef9b96e8c29317ccf7c97f009..f0551b476f4ecfa438f424375acd76bc20d5b325 100644 --- a/chrome/browser/process_singleton_posix.cc +++ b/chrome/browser/process_singleton_posix.cc @@ -122,7 +122,7 @@ const char kACKToken[] = "ACK"; @@ -105,7 +105,7 @@ index a04d139f958a7aaef9b96e8c29317ccf7c97f009..29188668a69047b3ad3bebd1f0057565 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; @@ -567,6 +567,7 @@ class ProcessSingleton::LinuxWatcher @@ -114,21 +114,21 @@ index a04d139f958a7aaef9b96e8c29317ccf7c97f009..29188668a69047b3ad3bebd1f0057565 const std::vector& argv, + const std::vector additional_data, SocketReader* reader); - + private: @@ -591,6 +592,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_; }; - + @@ -621,16 +625,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, @@ -137,7 +137,7 @@ index a04d139f958a7aaef9b96e8c29317ccf7c97f009..29188668a69047b3ad3bebd1f0057565 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. @@ -157,7 +157,7 @@ index a04d139f958a7aaef9b96e8c29317ccf7c97f009..29188668a69047b3ad3bebd1f0057565 @@ -640,6 +649,22 @@ void ProcessSingleton::LinuxWatcher::HandleMessage( } } - + +void ProcessSingleton::LinuxWatcher::AckCallback( + SocketReader* reader, + const base::span* response) { @@ -180,7 +180,7 @@ index a04d139f958a7aaef9b96e8c29317ccf7c97f009..29188668a69047b3ad3bebd1f0057565 @@ -675,7 +700,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. @@ -190,7 +190,7 @@ index a04d139f958a7aaef9b96e8c29317ccf7c97f009..29188668a69047b3ad3bebd1f0057565 @@ -705,10 +731,25 @@ 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); @@ -212,7 +212,7 @@ index a04d139f958a7aaef9b96e8c29317ccf7c97f009..29188668a69047b3ad3bebd1f0057565 + parent_, current_dir, command_line, + std::move(additional_data), this)); fd_watch_controller_.reset(); - + // LinuxWatcher::HandleMessage() is in charge of destroying this SocketReader @@ -737,8 +778,12 @@ void ProcessSingleton::LinuxWatcher::SocketReader::FinishWithACK( // @@ -230,17 +230,17 @@ index a04d139f958a7aaef9b96e8c29317ccf7c97f009..29188668a69047b3ad3bebd1f0057565 socket_path_ = user_data_dir.Append(chrome::kSingletonSocketFilename); @@ -855,7 +900,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); - + @@ -865,11 +911,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())); @@ -248,7 +248,7 @@ index a04d139f958a7aaef9b96e8c29317ccf7c97f009..29188668a69047b3ad3bebd1f0057565 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); @@ -263,7 +263,7 @@ index a04d139f958a7aaef9b96e8c29317ccf7c97f009..29188668a69047b3ad3bebd1f0057565 @@ -909,6 +965,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 = @@ -292,7 +292,7 @@ index 19d5659d665321da54e05cee01be7da02e0c283b..9a894fae1fbe62ee9bc37bf7c658b037 #include "base/win/windows_version.h" @@ -45,6 +46,14 @@ namespace { - + const char kLockfile[] = "lockfile"; +const LPCWSTR kPipeName = L"\\\\.\\pipe\\electronAckPipe"; +const DWORD kPipeTimeout = 10000; @@ -302,11 +302,11 @@ index 19d5659d665321da54e05cee01be7da02e0c283b..9a894fae1fbe62ee9bc37bf7c658b037 +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. @@ -99,10 +108,12 @@ BOOL CALLBACK BrowserWindowEnumeration(HWND window, LPARAM param) { - + bool ParseCommandLine(const COPYDATASTRUCT* cds, base::CommandLine* parsed_command_line, - base::FilePath* current_directory) { @@ -359,7 +359,7 @@ index 19d5659d665321da54e05cee01be7da02e0c283b..9a894fae1fbe62ee9bc37bf7c658b037 } return false; } - + +void StoreAck(const base::span* ack_data) { + if (ack_data) { + g_ack_data = std::make_unique>(ack_data->begin(), @@ -404,7 +404,7 @@ index 19d5659d665321da54e05cee01be7da02e0c283b..9a894fae1fbe62ee9bc37bf7c658b037 const ProcessSingleton::NotificationCallback& notification_callback, UINT message, @@ -168,16 +250,23 @@ bool ProcessLaunchNotification( - + // Handle the WM_COPYDATA message from another process. const COPYDATASTRUCT* cds = reinterpret_cast(lparam); - @@ -417,7 +417,7 @@ index 19d5659d665321da54e05cee01be7da02e0c283b..9a894fae1fbe62ee9bc37bf7c658b037 *result = TRUE; return true; } - + - *result = notification_callback.Run(parsed_command_line, current_directory) ? - TRUE : FALSE; + g_write_ack_callback_called = false; @@ -430,7 +430,7 @@ index 19d5659d665321da54e05cee01be7da02e0c283b..9a894fae1fbe62ee9bc37bf7c658b037 + base::BindOnce(&SendBackAck)); return true; } - + @@ -274,9 +363,13 @@ bool ProcessSingleton::EscapeVirtualization( ProcessSingleton::ProcessSingleton( const std::string& program_name, @@ -449,7 +449,7 @@ index 19d5659d665321da54e05cee01be7da02e0c283b..9a894fae1fbe62ee9bc37bf7c658b037 @@ -291,6 +384,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. @@ -487,7 +487,7 @@ index 19d5659d665321da54e05cee01be7da02e0c283b..9a894fae1fbe62ee9bc37bf7c658b037 @@ -301,8 +425,9 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcess() { return PROCESS_NONE; } - + - switch (chrome::AttemptToNotifyRunningChrome(remote_window_)) { + switch (chrome::AttemptToNotifyRunningChrome(remote_window_, additional_data_)) { case chrome::NOTIFY_SUCCESS: @@ -497,7 +497,7 @@ index 19d5659d665321da54e05cee01be7da02e0c283b..9a894fae1fbe62ee9bc37bf7c658b037 remote_window_ = NULL; @@ -432,6 +557,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, @@ -516,11 +516,11 @@ index 19d5659d665321da54e05cee01be7da02e0c283b..9a894fae1fbe62ee9bc37bf7c658b037 bool result = @@ -458,6 +595,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 @@ -529,7 +529,7 @@ index b4fec8878c37b9d157ea768e3b6d99399a988c75..e1cb0f21f752aaeee2c360ce9c5fd08b @@ -34,7 +34,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, @@ -539,7 +539,7 @@ index b4fec8878c37b9d157ea768e3b6d99399a988c75..e1cb0f21f752aaeee2c360ce9c5fd08b DWORD thread_id = GetWindowThreadProcessId(remote_window, &process_id); @@ -42,7 +44,8 @@ NotifyChromeResult AttemptToNotifyRunningChrome(HWND remote_window) { return NOTIFY_FAILED; - + // Send the command line to the remote chrome window. - // Format is "START\0<<>>\0<<>>". + // Format is @@ -550,7 +550,7 @@ index b4fec8878c37b9d157ea768e3b6d99399a988c75..e1cb0f21f752aaeee2c360ce9c5fd08b @@ -53,6 +56,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 @@ -575,12 +575,12 @@ index 5516673cee019f6060077091e59498bf9038cd6e..8edea5079b46c2cba67833114eb9c21d --- 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 @@ -590,6 +590,6 @@ index 5516673cee019f6060077091e59498bf9038cd6e..8edea5079b46c2cba67833114eb9c21d +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/spec-main/api-app-spec.ts b/spec-main/api-app-spec.ts index b67bac74ad002..07f996807cbd7 100644 --- a/spec-main/api-app-spec.ts +++ b/spec-main/api-app-spec.ts @@ -1,4 +1,4 @@ -import { expect } from 'chai'; +import { assert, expect } from 'chai'; import * as cp from 'child_process'; import * as https from 'https'; import * as http from 'http'; @@ -228,7 +228,7 @@ describe('app module', () => { async function testArgumentPassing (testArgs: SingleInstanceLockTestArgs) { const appPath = path.join(fixturesPath, 'api', 'singleton-data'); const first = cp.spawn(process.execPath, [appPath, ...testArgs.args]); - + const firstExit = emittedOnce(first, 'exit'); // Wait for the first app to boot. const firstStdoutLines = first.stdout.pipe(split()); while ((await emittedOnce(firstStdoutLines, 'data')).toString() !== 'started') { @@ -238,15 +238,16 @@ describe('app module', () => { const secondInstanceArgs = [process.execPath, appPath, ...testArgs.args, '--some-switch', 'some-arg']; const second = cp.spawn(secondInstanceArgs[0], secondInstanceArgs.slice(1)); + const secondExit = 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 emittedOnce(second, 'exit'); + const [code2] = await secondExit; expect(code2).to.equal(1); - const [code1] = await emittedOnce(first, 'exit'); + const [code1] = await firstExit; expect(code1).to.equal(0); const dataFromSecondInstance = await dataPromise; const [args, additionalData] = dataFromSecondInstance[0].toString('ascii').split('||'); @@ -315,13 +316,74 @@ describe('app module', () => { }); }); - it('sends and receives data', async () => { + it('sends and receives JSON object data', async () => { await testArgumentPassing({ args: ['--send-ack', '--prevent-default', '--send-data'], expectedAdditionalData, expectedAck }); }); + + 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 + }); + }); + + 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' + }); + }); + + 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 + }); + }); + + 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] + }); + }); + + 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] + }); + }); + + 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 + }); + }); + + it('cannot send or receive undefined data', async () => { + try { + await testArgumentPassing({ + args: ['--send-ack', '--ack-content="undefined"', '--prevent-default', '--send-data', '--data-content="undefined"'], + expectedAdditionalData: undefined, + expectedAck: undefined + }); + assert(false); + } catch (e) { + // This is expected. + } + }); }); describe('app.relaunch', () => { diff --git a/spec/fixtures/api/singleton-data/main.js b/spec/fixtures/api/singleton-data/main.js index e1ba175f3669a..f79ed9ccc9bce 100644 --- a/spec/fixtures/api/singleton-data/main.js +++ b/spec/fixtures/api/singleton-data/main.js @@ -13,7 +13,7 @@ const preventDefault = app.commandLine.hasSwitch('prevent-default'); // Send an object back for the ack rather than undefined. const sendAck = app.commandLine.hasSwitch('send-ack'); -const obj = { +let obj = { level: 1, testkey: 'testvalue1', inner: { @@ -21,7 +21,7 @@ const obj = { testkey: 'testvalue2' } }; -const ackObj = { +let ackObj = { level: 1, testkey: 'acktestvalue1', inner: { @@ -30,6 +30,19 @@ const ackObj = { } }; +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)); });