Bug 1898719 - [remote] Use private properties and methods in Remote Agent and WebDriver BiDi classes. r=webdriver-reviewers,jdescottes

Differential Revision: https://phabricator.services.mozilla.com/D212668

remote/cdp/CDP.sys.mjs

@@ -97,6 +97,7 @@ export class CDP {
 
     Cu.printStderr(`DevTools listening on ${this.address}\n`);
 
+    try {
       // Write connection details to DevToolsActivePort file within the profile.
       this._activePortPath = PathUtils.join(
         PathUtils.profileDir,
@@ -104,7 +105,7 @@ export class CDP {
       );
 
       const data = `${this.agent.port}\n${this.mainTargetPath}`;
-    try {
+
       await IOUtils.write(this._activePortPath, lazy.textEncoder.encode(data));
     } catch (e) {
       lazy.logger.warn(

remote/components/RemoteAgent.sys.mjs

@@ -148,6 +148,59 @@ class RemoteAgentParentProcess {
     return this.#webDriverBiDi;
   }
 
+  /**
+   * Handle the --remote-debugging-port command line argument.
+   *
+   * @param {nsICommandLine} cmdLine
+   *     Instance of the command line interface.
+   *
+   * @returns {boolean}
+   *     Return `true` if the command line argument has been found.
+   */
+  #handleRemoteDebuggingPortFlag(cmdLine) {
+    let enabled = false;
+
+    try {
+      // Catch cases when the argument, and a port have been specified.
+      const port = cmdLine.handleFlagWithParam("remote-debugging-port", false);
+      if (port !== null) {
+        enabled = true;
+
+        // In case of an invalid port keep the default port
+        const parsed = Number(port);
+        if (!isNaN(parsed)) {
+          this.#port = parsed;
+        }
+      }
+    } catch (e) {
+      // If no port has been given check for the existence of the argument.
+      enabled = cmdLine.handleFlag("remote-debugging-port", false);
+    }
+
+    return enabled;
+  }
+
+  #handleAllowHostsFlag(cmdLine) {
+    try {
+      const hosts = cmdLine.handleFlagWithParam("remote-allow-hosts", false);
+      return hosts.split(",");
+    } catch (e) {
+      return null;
+    }
+  }
+
+  #handleAllowOriginsFlag(cmdLine) {
+    try {
+      const origins = cmdLine.handleFlagWithParam(
+        "remote-allow-origins",
+        false
+      );
+      return origins.split(",");
+    } catch (e) {
+      return null;
+    }
+  }
+
   /**
    * Check if the provided URI's host is an IP address.
    *
@@ -165,17 +218,6 @@ class RemoteAgentParentProcess {
     return false;
   }
 
-  handle(cmdLine) {
-    // remote-debugging-port has to be consumed in nsICommandLineHandler:handle
-    // to avoid issues on macos. See Marionette.sys.mjs::handle() for more details.
-    // TODO: remove after Bug 1724251 is fixed.
-    try {
-      cmdLine.handleFlagWithParam("remote-debugging-port", false);
-    } catch (e) {
-      cmdLine.handleFlag("remote-debugging-port", false);
-    }
-  }
-
   async #listen(port) {
     if (Services.appinfo.processType != Ci.nsIXULRuntime.PROCESS_TYPE_DEFAULT) {
       throw Components.Exception(
@@ -317,56 +359,14 @@ class RemoteAgentParentProcess {
     }
   }
 
-  /**
+  handle(cmdLine) {
-   * Handle the --remote-debugging-port command line argument.
+    // remote-debugging-port has to be consumed in nsICommandLineHandler:handle
-   *
+    // to avoid issues on macos. See Marionette.sys.mjs::handle() for more details.
-   * @param {nsICommandLine} cmdLine
+    // TODO: remove after Bug 1724251 is fixed.
-   *     Instance of the command line interface.
-   *
-   * @returns {boolean}
-   *     Return `true` if the command line argument has been found.
-   */
-  handleRemoteDebuggingPortFlag(cmdLine) {
-    let enabled = false;
-
     try {
-      // Catch cases when the argument, and a port have been specified.
+      cmdLine.handleFlagWithParam("remote-debugging-port", false);
-      const port = cmdLine.handleFlagWithParam("remote-debugging-port", false);
-      if (port !== null) {
-        enabled = true;
-
-        // In case of an invalid port keep the default port
-        const parsed = Number(port);
-        if (!isNaN(parsed)) {
-          this.#port = parsed;
-        }
-      }
     } catch (e) {
-      // If no port has been given check for the existence of the argument.
+      cmdLine.handleFlag("remote-debugging-port", false);
-      enabled = cmdLine.handleFlag("remote-debugging-port", false);
-    }
-
-    return enabled;
-  }
-
-  handleAllowHostsFlag(cmdLine) {
-    try {
-      const hosts = cmdLine.handleFlagWithParam("remote-allow-hosts", false);
-      return hosts.split(",");
-    } catch (e) {
-      return null;
-    }
-  }
-
-  handleAllowOriginsFlag(cmdLine) {
-    try {
-      const origins = cmdLine.handleFlagWithParam(
-        "remote-allow-origins",
-        false
-      );
-      return origins.split(",");
-    } catch (e) {
-      return null;
     }
   }
 
@@ -383,14 +383,13 @@ class RemoteAgentParentProcess {
       case "command-line-startup":
         Services.obs.removeObserver(this, topic);
 
-        this.#enabled = this.handleRemoteDebuggingPortFlag(subject);
+        this.#enabled = this.#handleRemoteDebuggingPortFlag(subject);
 
         if (this.#enabled) {
+          this.#allowHosts = this.#handleAllowHostsFlag(subject);
+          this.#allowOrigins = this.#handleAllowOriginsFlag(subject);
+
           Services.obs.addObserver(this, "final-ui-startup");
-
-          this.#allowHosts = this.handleAllowHostsFlag(subject);
-          this.#allowOrigins = this.handleAllowOriginsFlag(subject);
-
           Services.obs.addObserver(this, "browser-idle-startup-tasks-finished");
           Services.obs.addObserver(this, "mail-idle-startup-tasks-finished");
           Services.obs.addObserver(this, "quit-application");

remote/webdriver-bidi/WebDriverBiDi.sys.mjs

@@ -31,6 +31,12 @@ const RECOMMENDED_PREFS = new Map([
  * @see https://w3c.github.io/webdriver-bidi
  */
 export class WebDriverBiDi {
+  #agent;
+  #bidiServerPath;
+  #running;
+  #session;
+  #sessionlessConnections;
+
   /**
    * Creates a new instance of the WebDriverBiDi class.
    *
@@ -38,19 +44,20 @@ export class WebDriverBiDi {
    *     Reference to the Remote Agent instance.
    */
   constructor(agent) {
-    this.agent = agent;
+    this.#agent = agent;
-    this._running = false;
+    this.#running = false;
 
-    this._session = null;
+    this.#bidiServerPath;
-    this._sessionlessConnections = new Set();
+    this.#session = null;
+    this.#sessionlessConnections = new Set();
   }
 
   get address() {
-    return `ws://${this.agent.host}:${this.agent.port}`;
+    return `ws://${this.#agent.host}:${this.#agent.port}`;
   }
 
   get session() {
-    return this._session;
+    return this.#session;
   }
 
   /**
@@ -60,7 +67,7 @@ export class WebDriverBiDi {
    *     The connection without an accociated WebDriver session.
    */
   addSessionlessConnection(connection) {
-    this._sessionlessConnections.add(connection);
+    this.#sessionlessConnections.add(connection);
   }
 
   /**
@@ -82,7 +89,7 @@ export class WebDriverBiDi {
    *     If, for whatever reason, a session could not be created.
    */
   async createSession(capabilities, flags, sessionlessConnection) {
-    if (this.session) {
+    if (this.#session) {
       throw new lazy.error.SessionNotCreatedError(
         "Maximum number of active sessions"
       );
@@ -98,7 +105,7 @@ export class WebDriverBiDi {
     // has been requested, register a path handler for the session.
     let webSocketUrl = null;
     if (
-      this.agent.running &&
+      this.#agent.running &&
       (session.capabilities.get("webSocketUrl") || sessionlessConnection)
     ) {
       // Creating a WebDriver BiDi session too early can cause issues with
@@ -107,16 +114,16 @@ export class WebDriverBiDi {
       // cause shutdown hangs. As such WebDriver BiDi will return a new session
       // once the initial application window has finished initializing.
       lazy.logger.debug(`Waiting for initial application window`);
-      await this.agent.browserStartupFinished;
+      await this.#agent.browserStartupFinished;
 
-      this.agent.server.registerPathHandler(session.path, session);
+      this.#agent.server.registerPathHandler(session.path, session);
       webSocketUrl = `${this.address}${session.path}`;
 
       lazy.logger.debug(`Registered session handler: ${session.path}`);
 
       if (sessionlessConnection) {
         // Remove temporary session-less connection
-        this._sessionlessConnections.delete(sessionlessConnection);
+        this.#sessionlessConnections.delete(sessionlessConnection);
       }
     }
 
@@ -124,11 +131,11 @@ export class WebDriverBiDi {
     // a path handler has been registered. Otherwise set its value to null.
     session.capabilities.set("webSocketUrl", webSocketUrl);
 
-    this._session = session;
+    this.#session = session;
 
     return {
-      sessionId: this.session.id,
+      sessionId: this.#session.id,
-      capabilities: this.session.capabilities,
+      capabilities: this.#session.capabilities,
     };
   }
 
@@ -136,19 +143,19 @@ export class WebDriverBiDi {
    * Delete the current WebDriver session.
    */
   deleteSession() {
-    if (!this.session) {
+    if (!this.#session) {
       return;
     }
 
     // When the Remote Agent is listening, and a BiDi WebSocket is active,
     // unregister the path handler for the session.
-    if (this.agent.running && this.session.capabilities.get("webSocketUrl")) {
+    if (this.#agent.running && this.#session.capabilities.get("webSocketUrl")) {
-      this.agent.server.registerPathHandler(this.session.path, null);
+      this.#agent.server.registerPathHandler(this.#session.path, null);
-      lazy.logger.debug(`Unregistered session handler: ${this.session.path}`);
+      lazy.logger.debug(`Unregistered session handler: ${this.#session.path}`);
     }
 
-    this.session.destroy();
+    this.#session.destroy();
-    this._session = null;
+    this.#session = null;
   }
 
   /**
@@ -161,7 +168,7 @@ export class WebDriverBiDi {
    *     The readiness state.
    */
   getSessionReadinessStatus() {
-    if (this.session) {
+    if (this.#session) {
       // We currently only support one session, see Bug 1720707.
       return {
         ready: false,
@@ -179,42 +186,42 @@ export class WebDriverBiDi {
    * Starts the WebDriver BiDi support.
    */
   async start() {
-    if (this._running) {
+    if (this.#running) {
       return;
     }
 
-    this._running = true;
+    this.#running = true;
 
     lazy.RecommendedPreferences.applyPreferences(RECOMMENDED_PREFS);
 
     // Install a HTTP handler for direct WebDriver BiDi connection requests.
-    this.agent.server.registerPathHandler(
+    this.#agent.server.registerPathHandler(
       "/session",
       new lazy.WebDriverNewSessionHandler(this)
     );
 
     Cu.printStderr(`WebDriver BiDi listening on ${this.address}\n`);
 
+    try {
       // Write WebSocket connection details to the WebDriverBiDiServer.json file
       // located within the application's profile.
-    this._bidiServerPath = PathUtils.join(
+      this.#bidiServerPath = PathUtils.join(
         PathUtils.profileDir,
         "WebDriverBiDiServer.json"
       );
 
       const data = {
-      ws_host: this.agent.host,
+        ws_host: this.#agent.host,
-      ws_port: this.agent.port,
+        ws_port: this.#agent.port,
       };
 
-    try {
       await IOUtils.write(
-        this._bidiServerPath,
+        this.#bidiServerPath,
         lazy.textEncoder.encode(JSON.stringify(data, undefined, "  "))
       );
     } catch (e) {
       lazy.logger.warn(
-        `Failed to create ${this._bidiServerPath} (${e.message})`
+        `Failed to create ${this.#bidiServerPath} (${e.message})`
       );
     }
   }
@@ -223,30 +230,30 @@ export class WebDriverBiDi {
    * Stops the WebDriver BiDi support.
    */
   async stop() {
-    if (!this._running) {
+    if (!this.#running) {
       return;
     }
 
     try {
-      await IOUtils.remove(this._bidiServerPath);
+      await IOUtils.remove(this.#bidiServerPath);
     } catch (e) {
       lazy.logger.warn(
-        `Failed to remove ${this._bidiServerPath} (${e.message})`
+        `Failed to remove ${this.#bidiServerPath} (${e.message})`
       );
     }
 
     try {
       // Close open session
       this.deleteSession();
-      this.agent.server.registerPathHandler("/session", null);
+      this.#agent.server.registerPathHandler("/session", null);
 
       // Close all open session-less connections
-      this._sessionlessConnections.forEach(connection => connection.close());
+      this.#sessionlessConnections.forEach(connection => connection.close());
-      this._sessionlessConnections.clear();
+      this.#sessionlessConnections.clear();
     } catch (e) {
       lazy.logger.error("Failed to stop protocol", e);
     } finally {
-      this._running = false;
+      this.#running = false;
     }
   }
 }