chore: handle Browser.close over CDP (#23436)

BUILD.gn

@@ -364,6 +364,7 @@ source_set("electron_lib") {
     "//third_party/blink/public:blink",
     "//third_party/boringssl",
     "//third_party/electron_node:node_lib",
+    "//third_party/inspector_protocol:crdtp",
     "//third_party/leveldatabase",
     "//third_party/libyuv",
     "//third_party/webrtc_overrides:webrtc_component",

shell/browser/ui/devtools_manager_delegate.cc

@@ -27,7 +27,9 @@
 #include "net/base/net_errors.h"
 #include "net/socket/stream_socket.h"
 #include "net/socket/tcp_server_socket.h"
+#include "shell/browser/browser.h"
 #include "shell/common/electron_paths.h"
+#include "third_party/inspector_protocol/crdtp/dispatch.h"
 #include "ui/base/resource/resource_bundle.h"
 
 namespace electron {
@@ -80,6 +82,8 @@ std::unique_ptr<content::DevToolsSocketFactory> CreateSocketFactory() {
       new TCPServerSocketFactory("127.0.0.1", port));
 }
 
+const char kBrowserCloseMethod[] = "Browser.close";
+
 }  // namespace
 
 // DevToolsManagerDelegate ---------------------------------------------------
@@ -102,6 +106,20 @@ void DevToolsManagerDelegate::HandleCommand(
     content::DevToolsAgentHostClientChannel* channel,
     base::span<const uint8_t> message,
     NotHandledCallback callback) {
+  crdtp::Dispatchable dispatchable(crdtp::SpanFrom(message));
+  DCHECK(dispatchable.ok());
+  if (crdtp::SpanEquals(crdtp::SpanFrom(kBrowserCloseMethod),
+                        dispatchable.Method())) {
+    // In theory, we should respond over the protocol saying that the
+    // Browser.close was handled. But doing so requires instantiating the
+    // protocol UberDispatcher and generating proper protocol handlers.
+    // Since we only have one method and it is supposed to close Electron,
+    // we don't need to add this complexity. Should we decide to support
+    // metohds like Browser.setWindowBounds, we'll need to do it though.
+    base::PostTask(FROM_HERE, {content::BrowserThread::UI},
+                   base::BindOnce([]() { Browser::Get()->Quit(); }));
+    return;
+  }
   std::move(callback).Run(message);
 }
 

spec-main/chromium-spec.ts

@@ -248,13 +248,20 @@ describe('web security', () => {
 });
 
 describe('command line switches', () => {
+  let appProcess: ChildProcess.ChildProcessWithoutNullStreams | undefined;
+  afterEach(() => {
+    if (appProcess && !appProcess.killed) {
+      appProcess.kill();
+      appProcess = undefined;
+    }
+  });
   describe('--lang switch', () => {
     const currentLocale = app.getLocale();
     const testLocale = (locale: string, result: string, done: () => void) => {
       const appPath = path.join(fixturesPath, 'api', 'locale-check');
       const electronPath = process.execPath;
       let output = '';
-      const appProcess = ChildProcess.spawn(electronPath, [appPath, `--lang=${locale}`]);
+      appProcess = ChildProcess.spawn(electronPath, [appPath, `--lang=${locale}`]);
 
       appProcess.stdout.on('data', (data) => { output += data; });
       appProcess.stdout.on('end', () => {
@@ -271,7 +278,7 @@ describe('command line switches', () => {
   describe('--remote-debugging-pipe switch', () => {
     it('should expose CDP via pipe', async () => {
       const electronPath = process.execPath;
-      const appProcess = ChildProcess.spawn(electronPath, ['--remote-debugging-pipe'], {
+      appProcess = ChildProcess.spawn(electronPath, ['--remote-debugging-pipe'], {
         stdio: ['pipe', 'pipe', 'pipe', 'pipe', 'pipe']
       });
       const stdio = appProcess.stdio as unknown as [NodeJS.ReadableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.ReadableStream];
@@ -282,15 +289,14 @@ describe('command line switches', () => {
       expect(message.id).to.equal(1);
       expect(message.result.product).to.contain('Chrome');
       expect(message.result.userAgent).to.contain('Electron');
-      appProcess.kill();
     });
     it('should override --remote-debugging-port switch', async () => {
       const electronPath = process.execPath;
-      const appProcess = ChildProcess.spawn(electronPath, ['--remote-debugging-pipe', '--remote-debugging-port=0'], {
+      appProcess = ChildProcess.spawn(electronPath, ['--remote-debugging-pipe', '--remote-debugging-port=0'], {
         stdio: ['pipe', 'pipe', 'pipe', 'pipe', 'pipe']
       });
       let stderr = '';
-      appProcess.stderr.on('data', (data) => { stderr += data; });
+      appProcess.stderr.on('data', (data: string) => { stderr += data; });
       const stdio = appProcess.stdio as unknown as [NodeJS.ReadableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.ReadableStream];
       const pipe = new PipeTransport(stdio[3], stdio[4]);
       const versionPromise = new Promise(resolve => { pipe.onmessage = resolve; });
@@ -298,7 +304,16 @@ describe('command line switches', () => {
       const message = (await versionPromise) as any;
       expect(message.id).to.equal(1);
       expect(stderr).to.not.include('DevTools listening on');
-      appProcess.kill();
+    });
+    it('should shut down Electron upon Browser.close CDP command', async () => {
+      const electronPath = process.execPath;
+      appProcess = ChildProcess.spawn(electronPath, ['--remote-debugging-pipe'], {
+        stdio: ['pipe', 'pipe', 'pipe', 'pipe', 'pipe']
+      });
+      const stdio = appProcess.stdio as unknown as [NodeJS.ReadableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.WritableStream, NodeJS.ReadableStream];
+      const pipe = new PipeTransport(stdio[3], stdio[4]);
+      pipe.send({ id: 1, method: 'Browser.close', params: {} });
+      await new Promise(resolve => { appProcess!.on('exit', resolve); });
     });
   });
 
@@ -306,17 +321,16 @@ describe('command line switches', () => {
     it('should display the discovery page', (done) => {
       const electronPath = process.execPath;
       let output = '';
-      const appProcess = ChildProcess.spawn(electronPath, ['--remote-debugging-port=']);
+      appProcess = ChildProcess.spawn(electronPath, ['--remote-debugging-port=']);
 
       appProcess.stderr.on('data', (data) => {
         output += data;
         const m = /DevTools listening on ws:\/\/127.0.0.1:(\d+)\//.exec(output);
         if (m) {
-          appProcess.stderr.removeAllListeners('data');
+          appProcess!.stderr.removeAllListeners('data');
           const port = m[1];
           http.get(`http://127.0.0.1:${port}`, (res) => {
             res.destroy();
-            appProcess.kill();
             expect(res.statusCode).to.eql(200);
             expect(parseInt(res.headers['content-length']!)).to.be.greaterThan(0);
             done();