diff --git a/browser/components/BrowserGlue.jsm b/browser/components/BrowserGlue.jsm index 6173fe571a6a..d91ce15f4fd1 100644 --- a/browser/components/BrowserGlue.jsm +++ b/browser/components/BrowserGlue.jsm @@ -2745,7 +2745,7 @@ BrowserGlue.prototype = { }, }, - // WebDriver components (Remote Agent and Marionette) need to be + // WebDriver components (Marionette) need to be // initialized as very last step. { condition: AppConstants.ENABLE_WEBDRIVER, @@ -2758,9 +2758,6 @@ BrowserGlue.prototype = { "browser-startup-idle-tasks-finished" ); - // Request startup of the Remote Agent (support for WebDriver BiDi - // and the partial Chrome DevTools protocol) before Marionette. - Services.obs.notifyObservers(null, "remote-startup-requested"); Services.obs.notifyObservers(null, "marionette-startup-requested"); }); }, diff --git a/mobile/android/chrome/geckoview/geckoview.js b/mobile/android/chrome/geckoview/geckoview.js index 3e6451884fb8..0e26f04cb610 100644 --- a/mobile/android/chrome/geckoview/geckoview.js +++ b/mobile/android/chrome/geckoview/geckoview.js @@ -853,15 +853,13 @@ function startup() { ); InitLater(() => { - // This lets Marionette and the Remote Agent (used for our CDP and the - // upcoming WebDriver BiDi implementation) start listening (when enabled). - // Both GeckoView and these two remote protocols do most of their + // This lets Marionette start listening (when enabled). + // Both GeckoView and this remote protocol do most of their // initialization in "profile-after-change", and there is no order enforced - // between them. Therefore we defer asking both components to startup - // until after all "profile-after-change" handlers (including this one) - // have completed. + // between them. Therefore we defer asking the Marionette component to + // startup until after all "profile-after-change" handlers (including this + // one) have completed. Services.obs.notifyObservers(null, "marionette-startup-requested"); - Services.obs.notifyObservers(null, "remote-startup-requested"); }); }); diff --git a/remote/.gitignore b/remote/.gitignore index 74e8609ab2c4..912df7a0acd2 100644 --- a/remote/.gitignore +++ b/remote/.gitignore @@ -1,3 +1,4 @@ -test/puppeteer/package-lock.json -test/puppeteer/node_modules/ test/puppeteer/.local-chromium/ +test/puppeteer/lib/ +test/puppeteer/node_modules/ +test/puppeteer/package-lock.json diff --git a/remote/cdp/CDP.jsm b/remote/cdp/CDP.jsm index e091a78771ab..0c1cfee49944 100644 --- a/remote/cdp/CDP.jsm +++ b/remote/cdp/CDP.jsm @@ -75,6 +75,13 @@ class CDP { return; } + // Starting CDP too early can cause issues with clients like Puppeteer. + // Especially when closing the browser while it's still starting up, which + // can cause shutdown hangs. As such CDP will be started when all the + // browser windows are restored (workaround for bug 1764420). + logger.debug(`Delay start-up until all windows have been restored`); + await this.agent.browserStartupFinished; + // Note: Ideally this would only be set at the end of the method. However // since start() is async, we prefer to set the flag early in order to // avoid potential race conditions. diff --git a/remote/components/RemoteAgent.jsm b/remote/components/RemoteAgent.jsm index 3e6c836902b5..200f29e1fb1a 100644 --- a/remote/components/RemoteAgent.jsm +++ b/remote/components/RemoteAgent.jsm @@ -14,6 +14,7 @@ XPCOMUtils.defineLazyModuleGetters(this, { Services: "resource://gre/modules/Services.jsm", CDP: "chrome://remote/content/cdp/CDP.jsm", + Deferred: "chrome://remote/content/shared/Sync.jsm", HttpServer: "chrome://remote/content/server/HTTPD.jsm", Log: "chrome://remote/content/shared/Log.jsm", WebDriverBiDi: "chrome://remote/content/webdriver-bidi/WebDriverBiDi.jsm", @@ -44,6 +45,7 @@ class RemoteAgentClass { #enabled; #port; #server; + #browserStartupFinished; #cdp; #webDriverBiDi; @@ -55,6 +57,7 @@ class RemoteAgentClass { this.#enabled = false; this.#port = DEFAULT_PORT; this.#server = null; + this.#browserStartupFinished = Deferred(); // Supported protocols this.#cdp = null; @@ -94,6 +97,17 @@ class RemoteAgentClass { return this.#allowOrigins; } + /** + * A promise resolved once initialization of the browser has completed and + * all the windows restored. + * + * @returns {Promise} + * Promise that resolves when the application startup has finished. + */ + get browserStartupFinished() { + return this.#browserStartupFinished.promise; + } + get cdp() { return this.#cdp; } @@ -201,8 +215,7 @@ class RemoteAgentClass { Services.obs.notifyObservers(null, "remote-listening", true); - await this.cdp?.start(); - await this.webDriverBiDi?.start(); + await Promise.all([this.webDriverBiDi?.start(), this.cdp?.start()]); } catch (e) { await this.close(); logger.error(`Unable to start remote agent: ${e.message}`, e); @@ -298,19 +311,19 @@ class RemoteAgentClass { this.#enabled = this.handleRemoteDebuggingPortFlag(subject); if (this.enabled) { - Services.obs.addObserver(this, "remote-startup-requested"); + Services.obs.addObserver(this, "final-ui-startup"); this.#allowHosts = this.handleAllowHostsFlag(subject); this.#allowOrigins = this.handleAllowOriginsFlag(subject); } - // Ideally we should only enable the Remote Agent when the command + // Ideally we should only initialize the Remote Agent when the command // line argument has been specified. But to allow Browser Chrome tests - // to run the Remote Agent and the supported protocols also need to be - // initialized. + // to run certain states have to be set and listeners registered. + // Once bug 1762647 is fixed all the next lines can be moved to + // "final-ui-startup". - // Listen for application shutdown to also shutdown the Remote Agent - // and a possible running instance of httpd.js. + Services.obs.addObserver(this, "sessionstore-windows-restored"); Services.obs.addObserver(this, "quit-application"); // With Bug 1717899 we will extend the lifetime of the Remote Agent to @@ -335,7 +348,7 @@ class RemoteAgentClass { break; - case "remote-startup-requested": + case "final-ui-startup": Services.obs.removeObserver(this, topic); try { @@ -347,6 +360,15 @@ class RemoteAgentClass { break; + // For now only used in CDP to wait until all the application windows + // have been opened and fully restored. + case "sessionstore-windows-restored": + Services.obs.removeObserver(this, topic); + this.#browserStartupFinished.resolve(); + break; + + // Listen for application shutdown to also shutdown the Remote Agent + // and a possible running instance of httpd.js. case "quit-application": Services.obs.removeObserver(this, "quit-application");