Simplify debugger launch flow to invoke Chrome directly, drop kill API (#44672)

Summary:
Pull Request resolved: #44672

Swaps out and simplifies the internals of the debugger launch flow.

We observed that we could achieve better launch/windowing behaviour by passing the `--app` argument directly to the detected Chrome path.

This shares the user's default Chrome profile:
- Fixes unwanted behaviour such as a separate dock icon on macOS (which, when clicked, would launch an unwanted empty window).
- Enables settings persistence.

This change also removes the `LaunchedBrowser.kill` API.

Changelog: [Internal]

Reviewed By: hoxyq

Differential Revision: D57726649

fbshipit-source-id: fc3a715dc852a50559048d1d1c378f64aeb2013f

Author: huntie

flow-typed/npm/temp-dir_2.x.x.js

@@ -33,7 +33,6 @@
     "open": "^7.0.3",
     "selfsigned": "^2.4.1",
     "serve-static": "^1.13.1",
-    "temp-dir": "^2.0.0",
     "ws": "^6.2.2"
   },
   "engines": {

packages/dev-middleware/package.json

@@ -11,7 +11,7 @@
 
 export {default as createDevMiddleware} from './createDevMiddleware';
 
-export type {BrowserLauncher, LaunchedBrowser} from './types/BrowserLauncher';
+export type {BrowserLauncher} from './types/BrowserLauncher';
 export type {EventReporter, ReportableEvent} from './types/EventReporter';
 export type {
   CustomMessageHandler,

packages/dev-middleware/src/index.flow.js

@@ -10,7 +10,7 @@
  */
 
 import type {InspectorProxyQueries} from '../inspector-proxy/InspectorProxy';
-import type {BrowserLauncher, LaunchedBrowser} from '../types/BrowserLauncher';
+import type {BrowserLauncher} from '../types/BrowserLauncher';
 import type {EventReporter} from '../types/EventReporter';
 import type {Experiments} from '../types/Experiments';
 import type {Logger} from '../types/Logger';
@@ -20,8 +20,6 @@ import type {IncomingMessage, ServerResponse} from 'http';
 import getDevToolsFrontendUrl from '../utils/getDevToolsFrontendUrl';
 import url from 'url';
 
-const debuggerInstances = new Map<string, ?LaunchedBrowser>();
-
 type Options = $ReadOnly<{
   serverBaseUrl: string,
   logger?: Logger,
@@ -117,20 +115,12 @@ export default function openDebuggerMiddleware({
       try {
         switch (launchType) {
           case 'launch':
-            const frontendInstanceId =
-              device != null
-                ? 'device:' + device
-                : 'app:' + (appId ?? '<null>');
-            await debuggerInstances.get(frontendInstanceId)?.kill();
-            debuggerInstances.set(
-              frontendInstanceId,
-              await browserLauncher.launchDebuggerAppWindow(
-                getDevToolsFrontendUrl(
-                  experiments,
-                  target.webSocketDebuggerUrl,
-                  serverBaseUrl,
-                  {launchId, useFuseboxEntryPoint},
-                ),
+            await browserLauncher.launchDebuggerAppWindow(
+              getDevToolsFrontendUrl(
+                experiments,
+                target.webSocketDebuggerUrl,
+                serverBaseUrl,
+                {launchId, useFuseboxEntryPoint},
               ),
             );
             res.end();

packages/dev-middleware/src/middleware/openDebuggerMiddleware.js

@@ -9,14 +9,6 @@
  * @oncall react_native
  */
 
-/**
- * Represents a launched web browser instance.
- */
-export type LaunchedBrowser = {
-  kill: () => void | Promise<void>,
-  ...
-};
-
 /**
  * An interface for integrators to provide a custom implementation for
  * opening URLs in a web browser.
@@ -27,5 +19,5 @@ export interface BrowserLauncher {
    * optionally returning an object to control the launched browser instance.
    * The browser used should be capable of running Chrome DevTools.
    */
-  launchDebuggerAppWindow: (url: string) => Promise<LaunchedBrowser | void>;
+  launchDebuggerAppWindow: (url: string) => Promise<void>;
 }

packages/dev-middleware/src/types/BrowserLauncher.js

@@ -9,12 +9,9 @@
  * @oncall react_native
  */
 
-import type {BrowserLauncher, LaunchedBrowser} from '../types/BrowserLauncher';
-
-import {promises as fs} from 'fs';
-import path from 'path';
-import osTempDir from 'temp-dir';
+import type {BrowserLauncher} from '../types/BrowserLauncher';
 
+const {spawn} = require('child_process');
 const ChromeLauncher = require('chrome-launcher');
 const {Launcher: EdgeLauncher} = require('chromium-edge-launcher');
 
@@ -27,15 +24,13 @@ const DefaultBrowserLauncher: BrowserLauncher = {
    * Attempt to open the debugger frontend in a Google Chrome or Microsoft Edge
    * app window.
    */
-  launchDebuggerAppWindow: async (url: string): Promise<LaunchedBrowser> => {
-    let browserType = 'chrome';
+  launchDebuggerAppWindow: async url => {
     let chromePath;
 
     try {
       // Locate Chrome installation path, will throw if not found
       chromePath = ChromeLauncher.getChromePath();
     } catch (e) {
-      browserType = 'edge';
       chromePath = EdgeLauncher.getFirstInstallation();
 
       if (chromePath == null) {
@@ -47,40 +42,28 @@ const DefaultBrowserLauncher: BrowserLauncher = {
       }
     }
 
-    const userDataDir = await createTempDir(
-      `react-native-debugger-frontend-${browserType}`,
-    );
-    const launchedChrome = await ChromeLauncher.launch({
-      ignoreDefaultFlags: true,
-      chromeFlags: [
-        ...ChromeLauncher.Launcher.defaultFlags().filter(
-          /**
-           * This flag controls whether Chrome treats a visually covered (occluded) tab
-           * as "backgrounded". We launch CDT as a single tab/window via `--app`, so we
-           * do want Chrome to treat our tab as "backgrounded" when the UI is covered.
-           * Omitting this flag allows "visibilitychange" events to fire properly.
-           */
-          flag => flag !== '--disable-backgrounding-occluded-windows',
-        ),
-        `--app=${url}`,
-        `--user-data-dir=${userDataDir}`,
-        '--window-size=1200,600',
-      ],
-      chromePath,
-    });
+    const chromeFlags = [`--app=${url}`, '--window-size=1200,600'];
 
-    return {
-      kill: async () => launchedChrome.kill(),
-    };
+    return new Promise((resolve, reject) => {
+      const childProcess = spawn(chromePath, chromeFlags, {
+        detached: true,
+        stdio: 'ignore',
+      });
+
+      childProcess.on('data', () => {
+        resolve();
+      });
+      childProcess.on('close', (code: number) => {
+        if (code !== 0) {
+          reject(
+            new Error(
+              `Failed to launch debugger app window: ${chromePath} exited with code ${code}`,
+            ),
+          );
+        }
+      });
+    });
   },
 };
 
-async function createTempDir(dirName: string): Promise<string> {
-  const tempDir = path.join(osTempDir, dirName);
-
-  await fs.mkdir(tempDir, {recursive: true});
-
-  return tempDir;
-}
-
 export default DefaultBrowserLauncher;

packages/dev-middleware/src/utils/DefaultBrowserLauncher.js

@@ -9567,11 +9567,6 @@ tar@^6.1.11:
     mkdirp "^1.0.3"
     yallist "^4.0.0"
 
-temp-dir@^2.0.0:
-  version "2.0.0"
-  resolved "https://registry.yarnpkg.com/temp-dir/-/temp-dir-2.0.0.tgz#bde92b05bdfeb1516e804c9c00ad45177f31321e"
-  integrity sha512-aoBAniQmmwtcKp/7BzsH8Cxzv8OL736p7v1ihGb5e9DJ9kTwGWHrQrVB5+lfVDzfGrdRzXch+ig7LHaY1JTOrg==
-
 temp@^0.8.4:
   version "0.8.4"
   resolved "https://registry.yarnpkg.com/temp/-/temp-0.8.4.tgz#8c97a33a4770072e0a05f919396c7665a7dd59f2"