From 506f7ae5efba75d01142d767f9f73b019ce6f4a0 Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Thu, 4 Nov 2021 01:14:09 -0700 Subject: [PATCH] fix: second-instance additionalData parameter (#31661) * test: second-instance additionalData parameter * Fix posix implementation --- ...d_data_parameter_to_processsingleton.patch | 21 ++-- spec-main/api-app-spec.ts | 99 ++++++++++++++++--- spec/fixtures/api/singleton-data/main.js | 15 ++- spec/fixtures/api/singleton/main.js | 4 +- 4 files changed, 110 insertions(+), 29 deletions(-) diff --git a/patches/chromium/feat_add_data_parameter_to_processsingleton.patch b/patches/chromium/feat_add_data_parameter_to_processsingleton.patch index ce84714a39bb2..2bc279b5ef530 100644 --- a/patches/chromium/feat_add_data_parameter_to_processsingleton.patch +++ b/patches/chromium/feat_add_data_parameter_to_processsingleton.patch @@ -61,7 +61,7 @@ index eec994c4252f17d9c9c41e66d5dae6509ed98a18..e538c9b76da4d4435e10cd3848438446 #if defined(OS_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 a04d139f958a7aaef9b96e8c29317ccf7c97f009..29188668a69047b3ad3bebd1f0057565a330b509 100644 +index a04d139f958a7aaef9b96e8c29317ccf7c97f009..e77cebd31967d28f3cb0db78e736011510634c0e 100644 --- a/chrome/browser/process_singleton_posix.cc +++ b/chrome/browser/process_singleton_posix.cc @@ -567,6 +567,7 @@ class ProcessSingleton::LinuxWatcher @@ -101,7 +101,7 @@ index a04d139f958a7aaef9b96e8c29317ccf7c97f009..29188668a69047b3ad3bebd1f0057565 const size_t kMinMessageLength = base::size(kStartToken) + 4; if (bytes_read_ < kMinMessageLength) { buf_[bytes_read_] = 0; -@@ -705,10 +710,25 @@ void ProcessSingleton::LinuxWatcher::SocketReader:: +@@ -705,10 +710,28 @@ void ProcessSingleton::LinuxWatcher::SocketReader:: tokens.erase(tokens.begin()); tokens.erase(tokens.begin()); @@ -110,13 +110,16 @@ index a04d139f958a7aaef9b96e8c29317ccf7c97f009..29188668a69047b3ad3bebd1f0057565 + std::vector command_line(tokens.begin() + 1, tokens.begin() + 1 + num_args); + + std::vector additional_data; -+ if (tokens.size() == 3 + num_args) { ++ 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(tokens[2 + num_args].c_str()); -+ additional_data = std::vector(additional_data_bits, -+ additional_data_bits + additional_data_size); ++ 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. @@ -128,7 +131,7 @@ index a04d139f958a7aaef9b96e8c29317ccf7c97f009..29188668a69047b3ad3bebd1f0057565 fd_watch_controller_.reset(); // LinuxWatcher::HandleMessage() is in charge of destroying this SocketReader -@@ -737,8 +757,10 @@ void ProcessSingleton::LinuxWatcher::SocketReader::FinishWithACK( +@@ -737,8 +760,10 @@ void ProcessSingleton::LinuxWatcher::SocketReader::FinishWithACK( // ProcessSingleton::ProcessSingleton( const base::FilePath& user_data_dir, @@ -139,7 +142,7 @@ index a04d139f958a7aaef9b96e8c29317ccf7c97f009..29188668a69047b3ad3bebd1f0057565 current_pid_(base::GetCurrentProcId()), watcher_(new LinuxWatcher(this)) { socket_path_ = user_data_dir.Append(chrome::kSingletonSocketFilename); -@@ -855,7 +877,8 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessWithTimeout( +@@ -855,7 +880,8 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessWithTimeout( sizeof(socket_timeout)); // Found another process, prepare our command line @@ -149,7 +152,7 @@ index a04d139f958a7aaef9b96e8c29317ccf7c97f009..29188668a69047b3ad3bebd1f0057565 std::string to_send(kStartToken); to_send.push_back(kTokenDelimiter); -@@ -865,11 +888,21 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessWithTimeout( +@@ -865,11 +891,21 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessWithTimeout( to_send.append(current_dir.value()); const std::vector& argv = cmd_line.argv(); diff --git a/spec-main/api-app-spec.ts b/spec-main/api-app-spec.ts index bb79595cad86c..79632682f6e4f 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'; @@ -205,6 +205,11 @@ describe('app module', () => { }); describe('app.requestSingleInstanceLock', () => { + interface SingleInstanceLockTestArgs { + args: string[]; + expectedAdditionalData: unknown; + } + it('prevents the second launch of app', async function () { this.timeout(120000); const appPath = path.join(fixturesPath, 'api', 'singleton-data'); @@ -218,9 +223,9 @@ describe('app module', () => { expect(code1).to.equal(0); }); - async function testArgumentPassing (fixtureName: string, expectedSecondInstanceData: unknown) { - const appPath = path.join(fixturesPath, 'api', fixtureName); - const first = cp.spawn(process.execPath, [appPath]); + async function testArgumentPassing (testArgs: SingleInstanceLockTestArgs) { + const appPath = path.join(fixturesPath, 'api', 'singleton-data'); + const first = cp.spawn(process.execPath, [appPath, ...testArgs.args]); const firstExited = emittedOnce(first, 'exit'); // Wait for the first app to boot. @@ -228,16 +233,18 @@ describe('app module', () => { while ((await emittedOnce(firstStdoutLines, 'data')).toString() !== 'started') { // wait. } - const data2Promise = emittedOnce(firstStdoutLines, 'data'); + const additionalDataPromise = emittedOnce(firstStdoutLines, 'data'); - const secondInstanceArgs = [process.execPath, appPath, '--some-switch', 'some-arg']; + const secondInstanceArgs = [process.execPath, appPath, ...testArgs.args, '--some-switch', 'some-arg']; const second = cp.spawn(secondInstanceArgs[0], secondInstanceArgs.slice(1)); - const [code2] = await emittedOnce(second, 'exit'); + const secondExited = emittedOnce(second, 'exit'); + + const [code2] = await secondExited; expect(code2).to.equal(1); const [code1] = await firstExited; expect(code1).to.equal(0); - const received = await data2Promise; - const [args, additionalData] = received[0].toString('ascii').split('||'); + const dataFromSecondInstance = await additionalDataPromise; + const [args, additionalData] = dataFromSecondInstance[0].toString('ascii').split('||'); const secondInstanceArgsReceived: string[] = JSON.parse(args.toString('ascii')); const secondInstanceDataReceived = JSON.parse(additionalData.toString('ascii')); @@ -246,12 +253,19 @@ describe('app module', () => { expect(secondInstanceArgsReceived).to.include(arg, `argument ${arg} is missing from received second args`); } - expect(secondInstanceDataReceived).to.be.deep.equal(expectedSecondInstanceData, - `received data ${JSON.stringify(secondInstanceDataReceived)} is not equal to expected data ${JSON.stringify(expectedSecondInstanceData)}.`); + expect(secondInstanceDataReceived).to.be.deep.equal(testArgs.expectedAdditionalData, + `received data ${JSON.stringify(secondInstanceDataReceived)} is not equal to expected data ${JSON.stringify(testArgs.expectedAdditionalData)}.`); } - it('passes arguments to the second-instance event', async () => { - const expectedSecondInstanceData = { + it('passes arguments to the second-instance event no additional data', async () => { + await testArgumentPassing({ + args: [], + expectedAdditionalData: null + }); + }); + + it('sends and receives JSON object data', async () => { + const expectedAdditionalData = { level: 1, testkey: 'testvalue1', inner: { @@ -259,11 +273,64 @@ describe('app module', () => { testkey: 'testvalue2' } }; - await testArgumentPassing('singleton-data', expectedSecondInstanceData); + await testArgumentPassing({ + args: ['--send-data'], + expectedAdditionalData + }); }); - it('passes arguments to the second-instance event no additional data', async () => { - await testArgumentPassing('singleton', null); + it('sends and receives numerical data', async () => { + await testArgumentPassing({ + args: ['--send-data', '--data-content=2'], + expectedAdditionalData: 2 + }); + }); + + it('sends and receives string data', async () => { + await testArgumentPassing({ + args: ['--send-data', '--data-content="data"'], + expectedAdditionalData: 'data' + }); + }); + + it('sends and receives boolean data', async () => { + await testArgumentPassing({ + args: ['--send-data', '--data-content=false'], + expectedAdditionalData: false + }); + }); + + it('sends and receives array data', async () => { + await testArgumentPassing({ + args: ['--send-data', '--data-content=[2, 3, 4]'], + expectedAdditionalData: [2, 3, 4] + }); + }); + + it('sends and receives mixed array data', async () => { + await testArgumentPassing({ + args: ['--send-data', '--data-content=["2", true, 4]'], + expectedAdditionalData: ['2', true, 4] + }); + }); + + it('sends and receives null data', async () => { + await testArgumentPassing({ + args: ['--send-data', '--data-content=null'], + expectedAdditionalData: 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 + }); + assert(false); + } catch (e) { + // This is expected. + } }); }); diff --git a/spec/fixtures/api/singleton-data/main.js b/spec/fixtures/api/singleton-data/main.js index 02257199e3582..50a623410ea89 100644 --- a/spec/fixtures/api/singleton-data/main.js +++ b/spec/fixtures/api/singleton-data/main.js @@ -1,10 +1,13 @@ 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 }); -const obj = { +let obj = { level: 1, testkey: 'testvalue1', inner: { @@ -12,7 +15,15 @@ const obj = { testkey: 'testvalue2' } }; -const gotTheLock = app.requestSingleInstanceLock(obj); +if (app.commandLine.hasSwitch('data-content')) { + obj = JSON.parse(app.commandLine.getSwitchValue('data-content')); + if (obj === 'undefined') { + obj = undefined; + } +} + +const gotTheLock = sendAdditionalData + ? app.requestSingleInstanceLock(obj) : app.requestSingleInstanceLock(); app.on('second-instance', (event, args, workingDirectory, data) => { setImmediate(() => { diff --git a/spec/fixtures/api/singleton/main.js b/spec/fixtures/api/singleton/main.js index 67b822bcef9be..4bd6433eea215 100644 --- a/spec/fixtures/api/singleton/main.js +++ b/spec/fixtures/api/singleton/main.js @@ -6,9 +6,9 @@ app.whenReady().then(() => { const gotTheLock = app.requestSingleInstanceLock(); -app.on('second-instance', (event, args, workingDirectory, data) => { +app.on('second-instance', (event, args, workingDirectory) => { setImmediate(() => { - console.log([JSON.stringify(args), JSON.stringify(data)].join('||')); + console.log(JSON.stringify(args)); app.exit(0); }); });