Bug 1700909 - [devtools] Migrate descriptorFromURL to commandsFromURL. r=jdescottes

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

devtools/client/framework/commands-from-url.js

@@ -13,9 +13,12 @@ const {
 const {
   remoteClientManager,
 } = require("resource://devtools/client/shared/remote-debugging/remote-client-manager.js");
+const {
+  CommandsFactory,
+} = require("resource://devtools/shared/commands/commands-factory.js");
 
 /**
- * Construct a Target Descriptor for a given URL with various query parameters:
+ * Construct a commands object for a given URL with various query parameters:
  *
  * - host, port & ws: See the documentation for clientFromURL
  *
@@ -42,9 +45,9 @@ const {
  * @param {URL} url
  *        The url to fetch query params from.
  *
- * @return A target descriptor
+ * @return A commands object
  */
-exports.descriptorFromURL = async function descriptorFromURL(url) {
+exports.commandsFromURL = async function commandsFromURL(url) {
   const client = await clientFromURL(url);
   const params = url.searchParams;
 
@@ -58,9 +61,9 @@ exports.descriptorFromURL = async function descriptorFromURL(url) {
   const id = params.get("id");
   const type = params.get("type");
 
-  let descriptorFront;
+  let commands;
   try {
-    descriptorFront = await _descriptorFromURL(client, id, type);
+    commands = await _commandsFromURL(client, id, type);
   } catch (e) {
     if (!isCachedClient) {
       // If the client was not cached, then the client was created here. If the target
@@ -75,73 +78,71 @@ exports.descriptorFromURL = async function descriptorFromURL(url) {
   // In such case, force the Descriptor to destroy the client as soon as it gets
   // destroyed. This typically happens only for about:debugging toolboxes
   // opened for local Firefox's targets.
-  descriptorFront.shouldCloseClient = !isCachedClient;
+  commands.shouldCloseClient = !isCachedClient;
 
-  return descriptorFront;
+  return commands;
 };
 
-async function _descriptorFromURL(client, id, type) {
+async function _commandsFromURL(client, id, type) {
   if (!type) {
-    throw new Error("descriptorFromURL, missing type parameter");
+    throw new Error("commandsFromURL, missing type parameter");
   }
 
-  let descriptorFront;
+  let commands;
   if (type === "tab") {
     // Fetch target for a remote tab
     id = parseInt(id, 10);
     if (isNaN(id)) {
       throw new Error(
-        `descriptorFromURL, wrong tab id '${id}', should be a number`
+        `commandsFromURL, wrong tab id '${id}', should be a number`
       );
     }
     try {
-      descriptorFront = await client.mainRoot.getTab({ browserId: id });
+      commands = await CommandsFactory.forRemoteTab(id, { client });
     } catch (ex) {
       if (ex.message.startsWith("Protocol error (noTab)")) {
         throw new Error(
-          `descriptorFromURL, tab with browserId '${id}' doesn't exist`
+          `commandsFromURL, tab with browserId '${id}' doesn't exist`
         );
       }
       throw ex;
     }
   } else if (type === "extension") {
-    descriptorFront = await client.mainRoot.getAddon({ id });
+    commands = await CommandsFactory.forAddon(id, { client });
 
-    if (!descriptorFront) {
+    if (!commands) {
       throw new Error(
-        `descriptorFromURL, extension with id '${id}' doesn't exist`
+        `commandsFromURL, extension with id '${id}' doesn't exist`
       );
     }
   } else if (type === "worker") {
-    descriptorFront = await client.mainRoot.getWorker(id);
+    commands = await CommandsFactory.forWorker(id, { client });
 
-    if (!descriptorFront) {
-      throw new Error(
-        `descriptorFromURL, worker with id '${id}' doesn't exist`
-      );
+    if (!commands) {
+      throw new Error(`commandsFromURL, worker with id '${id}' doesn't exist`);
     }
   } else if (type == "process") {
-    // Fetch descriptor for a remote chrome actor
-    DevToolsServer.allowChromeProcess = true;
     try {
       id = parseInt(id, 10);
       if (isNaN(id)) {
         id = 0;
       }
-      descriptorFront = await client.mainRoot.getProcess(id);
+      // When debugging firefox itself, force the server to accept debugging processes.
+      DevToolsServer.allowChromeProcess = true;
+      commands = await CommandsFactory.forProcess(id, { client });
     } catch (ex) {
       if (ex.error == "noProcess") {
         throw new Error(
-          `descriptorFromURL, process with id '${id}' doesn't exist`
+          `commandsFromURL, process with id '${id}' doesn't exist`
         );
       }
       throw ex;
     }
   } else {
-    throw new Error(`descriptorFromURL, unsupported type '${type}' parameter`);
+    throw new Error(`commandsFromURL, unsupported type '${type}' parameter`);
   }
 
-  return descriptorFront;
+  return commands;
 }
 
 /**

devtools/client/framework/moz.build

@@ -27,7 +27,7 @@ DIRS += [
 
 DevToolsModules(
     "browser-menus.js",
-    "descriptor-from-url.js",
+    "commands-from-url.js",
     "devtools-browser.js",
     "devtools.js",
     "enable-devtools-popup.js",

devtools/client/framework/test/browser.ini

@@ -82,7 +82,7 @@ prefs =
 [browser_source_map-late-script.js]
 [browser_tab_commands_factory.js]
 [browser_tab_descriptor_fission.js]
-[browser_target_from_url.js]
+[browser_commands_from_url.js]
 [browser_target_cached-front.js]
 [browser_target_cached-resource.js]
 [browser_target_events.js]

devtools/client/framework/test/browser_commands_from_url.js

@@ -8,11 +8,8 @@ const { DevToolsLoader } = ChromeUtils.import(
   "resource://devtools/shared/loader/Loader.jsm"
 );
 const {
-  createCommandsDictionary,
-} = require("resource://devtools/shared/commands/index.js");
-const {
-  descriptorFromURL,
-} = require("resource://devtools/client/framework/descriptor-from-url.js");
+  commandsFromURL,
+} = require("resource://devtools/client/framework/commands-from-url.js");
 
 Services.prefs.setBoolPref("devtools.debugger.remote-enabled", true);
 Services.prefs.setBoolPref("devtools.debugger.prompt-connection", false);
@@ -32,44 +29,40 @@ function assertTarget(target, url, chrome = false) {
 add_task(async function() {
   const tab = await addTab(TEST_URI);
   const browser = tab.linkedBrowser;
-  let descriptor, target;
+  let commands, target;
 
   info("Test invalid type");
   try {
-    await descriptorFromURL(new URL("http://foo?type=x"));
+    await commandsFromURL(new URL("https://foo?type=x"));
     ok(false, "Shouldn't pass");
   } catch (e) {
-    is(e.message, "descriptorFromURL, unsupported type 'x' parameter");
+    is(e.message, "commandsFromURL, unsupported type 'x' parameter");
   }
 
   info("Test tab");
-  descriptor = await descriptorFromURL(
-    new URL("http://foo?type=tab&id=" + browser.browserId)
+  commands = await commandsFromURL(
+    new URL("https://foo?type=tab&id=" + browser.browserId)
   );
-  const commands = await createCommandsDictionary(descriptor);
   // Descriptor's getTarget will only work if the TargetCommand watches for the first top target
   await commands.targetCommand.startListening();
-  target = await descriptor.getTarget();
+  target = await commands.descriptorFront.getTarget();
   assertTarget(target, TEST_URI);
-  await descriptor.client.close();
+  await commands.destroy();
 
   info("Test invalid tab id");
   try {
-    await descriptorFromURL(new URL("http://foo?type=tab&id=10000"));
+    await commandsFromURL(new URL("https://foo?type=tab&id=10000"));
     ok(false, "Shouldn't pass");
   } catch (e) {
-    is(
-      e.message,
-      "descriptorFromURL, tab with browserId '10000' doesn't exist"
-    );
+    is(e.message, "commandsFromURL, tab with browserId '10000' doesn't exist");
   }
 
   info("Test parent process");
-  descriptor = await descriptorFromURL(new URL("http://foo?type=process"));
-  target = await descriptor.getTarget();
+  commands = await commandsFromURL(new URL("https://foo?type=process"));
+  target = await commands.descriptorFront.getTarget();
   const topWindow = Services.wm.getMostRecentWindow("navigator:browser");
   assertTarget(target, topWindow.location.href, true);
-  await descriptor.client.close();
+  await commands.destroy();
 
   await testRemoteTCP();
   await testRemoteWebSocket();
@@ -119,19 +112,19 @@ async function testRemoteTCP() {
   const server = await setupDevToolsServer(false);
 
   const { port } = server.listener;
-  const descriptor = await descriptorFromURL(
-    new URL("http://foo?type=process&host=127.0.0.1&port=" + port)
+  const commands = await commandsFromURL(
+    new URL("https://foo?type=process&host=127.0.0.1&port=" + port)
   );
-  const target = await descriptor.getTarget();
+  const target = await commands.descriptorFront.getTarget();
   const topWindow = Services.wm.getMostRecentWindow("navigator:browser");
   assertTarget(target, topWindow.location.href, true);
 
-  const settings = descriptor.client._transport.connectionSettings;
+  const settings = commands.client._transport.connectionSettings;
   is(settings.host, "127.0.0.1");
   is(parseInt(settings.port, 10), port);
   is(settings.webSocket, false);
 
-  await descriptor.client.close();
+  await commands.destroy();
 
   teardownDevToolsServer(server);
 }
@@ -142,18 +135,18 @@ async function testRemoteWebSocket() {
   const server = await setupDevToolsServer(true);
 
   const { port } = server.listener;
-  const descriptor = await descriptorFromURL(
-    new URL("http://foo?type=process&host=127.0.0.1&port=" + port + "&ws=true")
+  const commands = await commandsFromURL(
+    new URL("https://foo?type=process&host=127.0.0.1&port=" + port + "&ws=true")
   );
-  const target = await descriptor.getTarget();
+  const target = await commands.descriptorFront.getTarget();
   const topWindow = Services.wm.getMostRecentWindow("navigator:browser");
   assertTarget(target, topWindow.location.href, true);
 
-  const settings = descriptor.client._transport.connectionSettings;
+  const settings = commands.client._transport.connectionSettings;
   is(settings.host, "127.0.0.1");
   is(parseInt(settings.port, 10), port);
   is(settings.webSocket, true);
-  await descriptor.client.close();
+  await commands.destroy();
 
   teardownDevToolsServer(server);
 }

devtools/client/framework/toolbox-init.js

@@ -87,8 +87,8 @@ async function initToolbox(url, host) {
   } = require("resource://devtools/client/framework/devtools.js");
 
   const {
-    descriptorFromURL,
-  } = require("resource://devtools/client/framework/descriptor-from-url.js");
+    commandsFromURL,
+  } = require("resource://devtools/client/framework/commands-from-url.js");
   const {
     Toolbox,
   } = require("resource://devtools/client/framework/toolbox.js");
@@ -97,7 +97,8 @@ async function initToolbox(url, host) {
   const tool = url.searchParams.get("tool");
 
   try {
-    const descriptor = await descriptorFromURL(url);
+    const commands = await commandsFromURL(url);
+    const descriptor = commands.descriptorFront;
     const toolbox = gDevTools.getToolboxForDescriptor(descriptor);
     if (toolbox && toolbox.isDestroying()) {
       // If a toolbox already exists for the descriptor, wait for current

devtools/client/netmonitor/initializer.js

@@ -77,17 +77,13 @@ const url = new window.URL(href);
 // is running in standalone.
 if (window.location.protocol === "chrome:" && url.search.length > 1) {
   const {
-    descriptorFromURL,
-  } = require("resource://devtools/client/framework/descriptor-from-url.js");
-  const {
-    createCommandsDictionary,
-  } = require("resource://devtools/shared/commands/index.js");
+    commandsFromURL,
+  } = require("resource://devtools/client/framework/commands-from-url.js");
 
   (async function() {
     try {
-      const descriptor = await descriptorFromURL(url);
-      const target = await descriptor.getTarget();
-      const commands = await createCommandsDictionary(descriptor);
+      const commands = await commandsFromURL(url);
+      const target = await commands.descriptorFront.getTarget();
       // Create a fake toolbox object
       const toolbox = {
         target,

devtools/shared/commands/commands-factory.js

@@ -83,22 +83,18 @@ exports.CommandsFactory = {
   },
 
   /**
-   * For now, this method is only used by browser_target_command_various_descriptors.js
-   * in order to cover about:debugging codepath, where we connect to remote tabs via
-   * their current browserId.
-   * But:
-   *  1) this can also be used to debug local tab, but TabDescriptor.localTab/isLocalTab will be null/false.
-   *  2) beyond this test, this isn't used to connect to remote tab just yet.
-   * Bug 1700909 should start using this from toolbox-init/descriptor-from-url
-   * and will finaly be used to connect to remote tabs.
-
+   * Create commands for a given remote tab.
+   *
+   * Note that it can also be used for local tab, but isLocalTab attribute
+   * on commands.descriptorFront will be false.
+   *
+   * @param {Number} browserId: Identify which tab we should create commands for.
    * @param {Object} options
-   * @param {Number} options.browserId: Mandatory attribute, to identify which tab we should
-   *        create commands for.
    * @param {DevToolsClient} options.client: An optional DevToolsClient. If none is passed,
    *        a new one will be created.
+   * @returns {Object} Commands
    */
-  async forRemoteTabInTest({ browserId, client }) {
+  async forRemoteTab(browserId, { client } = {}) {
     if (!client) {
       client = await createLocalClient();
     }
@@ -109,12 +105,20 @@ exports.CommandsFactory = {
   },
 
   /**
-   * `id` is the WorkerDebugger's id, which is a unique ID computed by the platform code.
-   * These ids are exposed via WorkerDescriptor's id attributes.
-   * WorkerDescritpors can be retrieved via MainFront.listAllWorkers()/listWorkers().
+   * Create commands for a given main process worker.
+   *
+   * @param {String} id: WorkerDebugger's id, which is a unique ID computed by the platform code.
+   *        These ids are exposed via WorkerDescriptor's id attributes.
+   *        WorkerDescriptors can be retrieved via MainFront.listAllWorkers()/listWorkers().
+   * @param {Object} options
+   * @param {DevToolsClient} options.client: An optional DevToolsClient. If none is passed,
+   *        a new one will be created.
+   * @returns {Object} Commands
    */
-  async forWorker(id) {
-    const client = await createLocalClient();
+  async forWorker(id, { client } = {}) {
+    if (!client) {
+      client = await createLocalClient();
+    }
 
     const descriptor = await client.mainRoot.getWorker(id);
     const commands = await createCommandsDictionary(descriptor);
@@ -145,16 +149,39 @@ exports.CommandsFactory = {
     return commands;
   },
 
-  async forAddon(id) {
-    const client = await createLocalClient();
+  /**
+   * Create commands for a Web Extension.
+   *
+   * @param {String} id The Web Extension ID to debug.
+   * @param {Object} options
+   * @param {DevToolsClient} options.client: An optional DevToolsClient. If none is passed,
+   *        a new one will be created.
+   * @returns {Object} Commands
+   */
+  async forAddon(id, { client } = {}) {
+    if (!client) {
+      client = await createLocalClient();
+    }
 
     const descriptor = await client.mainRoot.getAddon({ id });
     const commands = await createCommandsDictionary(descriptor);
     return commands;
   },
 
-  async forProcess(osPid) {
-    const client = await createLocalClient();
+  /**
+   * Create commands for a given process
+   *
+   * @param {String} id: The process PID. Pass 0 if you want to debug the main process.
+   *        But ideally, CommandsFactory.forMainProcess should be used instead.
+   * @param {Object} options
+   * @param {DevToolsClient} options.client: An optional DevToolsClient. If none is passed,
+   *        a new one will be created.
+   * @returns {Object} Commands
+   */
+  async forProcess(osPid, { client } = {}) {
+    if (!client) {
+      client = await createLocalClient();
+    }
 
     const descriptor = await client.mainRoot.getProcess(osPid);
     const commands = await createCommandsDictionary(descriptor);

devtools/shared/commands/target/tests/browser_target_command_detach.js

@@ -19,9 +19,9 @@ add_task(async function() {
   const tab = await addTab(TEST_URL);
 
   info("Create a first commands, which will destroy its top target");
-  const commands = await CommandsFactory.forRemoteTabInTest({
-    browserId: tab.linkedBrowser.browserId,
-  });
+  const commands = await CommandsFactory.forRemoteTab(
+    tab.linkedBrowser.browserId
+  );
   const targetCommand = commands.targetCommand;
 
   // We have to start listening in order to ensure having a targetFront available
@@ -37,10 +37,12 @@ add_task(async function() {
   info(
     "Now create a second commands after destroy, to see if we can spawn a new, functional target"
   );
-  const secondCommands = await CommandsFactory.forRemoteTabInTest({
-    client: commands.client,
-    browserId: tab.linkedBrowser.browserId,
-  });
+  const secondCommands = await CommandsFactory.forRemoteTab(
+    tab.linkedBrowser.browserId,
+    {
+      client: commands.client,
+    }
+  );
   const secondTargetCommand = secondCommands.targetCommand;
 
   // We have to start listening in order to ensure having a targetFront available

devtools/shared/commands/target/tests/browser_target_command_various_descriptors.js

@@ -123,9 +123,9 @@ async function testRemoteTab() {
   );
 
   const tab = await addTab(TEST_URL);
-  const commands = await CommandsFactory.forRemoteTabInTest({
-    browserId: tab.linkedBrowser.browserId,
-  });
+  const commands = await CommandsFactory.forRemoteTab(
+    tab.linkedBrowser.browserId
+  );
   const { descriptorFront } = commands;
   is(
     descriptorFront.descriptorType,