Bug 1445197 - part 3: Move about:debugging worker module to a shared module;r=ochameau draft
authorJulian Descottes <jdescottes@mozilla.com>
Fri, 20 Apr 2018 16:27:15 +0200
changeset 785649 a36c31ece12b53481edf63e178d64bf00bb9b6aa
parent 785644 419901fa6c9d503d460b878f555e1ef40cc19208
child 785650 b6e3933bd36c89524f707c528375afc410992622
push id107294
push userjdescottes@mozilla.com
push dateFri, 20 Apr 2018 15:52:30 +0000
reviewersochameau
bugs1445197
milestone61.0a1
Bug 1445197 - part 3: Move about:debugging worker module to a shared module;r=ochameau Extract all the logic that will be shared between about debugging and the application panel to a dedicated client module. MozReview-Commit-ID: Ccnmp3dCZpW
devtools/client/aboutdebugging/components/workers/Panel.js
devtools/client/aboutdebugging/components/workers/ServiceWorkerTarget.js
devtools/client/aboutdebugging/components/workers/Target.js
devtools/client/aboutdebugging/modules/moz.build
devtools/client/aboutdebugging/modules/worker.js
devtools/client/aboutdebugging/test/browser_service_workers_multi_content_process.js
devtools/client/framework/devtools-browser.js
devtools/shared/client/root-client.js
--- a/devtools/client/aboutdebugging/components/workers/Panel.js
+++ b/devtools/client/aboutdebugging/components/workers/Panel.js
@@ -2,21 +2,19 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 /* globals window */
 "use strict";
 
 loader.lazyImporter(this, "PrivateBrowsingUtils",
   "resource://gre/modules/PrivateBrowsingUtils.jsm");
 
-const { Ci } = require("chrome");
 const { Component, createFactory } = require("devtools/client/shared/vendor/react");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
-const { getWorkerForms } = require("../../modules/worker");
 const Services = require("Services");
 
 const PanelHeader = createFactory(require("../PanelHeader"));
 const TargetList = createFactory(require("../TargetList"));
 const WorkerTarget = createFactory(require("./Target"));
 const MultiE10SWarning = createFactory(require("./MultiE10sWarning"));
 const ServiceWorkerTarget = createFactory(require("./ServiceWorkerTarget"));
 
@@ -43,17 +41,16 @@ class WorkersPanel extends Component {
     };
   }
 
   constructor(props) {
     super(props);
 
     this.updateMultiE10S = this.updateMultiE10S.bind(this);
     this.updateWorkers = this.updateWorkers.bind(this);
-    this.getRegistrationForWorker = this.getRegistrationForWorker.bind(this);
     this.isE10S = this.isE10S.bind(this);
     this.renderServiceWorkersError = this.renderServiceWorkersError.bind(this);
 
     this.state = this.initialState;
   }
 
   componentDidMount() {
     let client = this.props.client;
@@ -108,83 +105,29 @@ class WorkersPanel extends Component {
     // nsIXULRuntime.maxWebProcessCount.
     let processCount = Services.appinfo.maxWebProcessCount;
     this.setState({ processCount });
   }
 
   updateWorkers() {
     let workers = this.initialState.workers;
 
-    getWorkerForms(this.props.client).then(forms => {
-      forms.registrations.forEach(form => {
-        workers.service.push({
-          icon: WorkerIcon,
-          name: form.url,
-          url: form.url,
-          scope: form.scope,
-          fetch: form.fetch,
-          registrationActor: form.actor,
-          active: form.active
-        });
-      });
-
-      forms.workers.forEach(form => {
-        let worker = {
-          icon: WorkerIcon,
-          name: form.url,
-          url: form.url,
-          workerActor: form.actor
-        };
-        switch (form.type) {
-          case Ci.nsIWorkerDebugger.TYPE_SERVICE:
-            let registration = this.getRegistrationForWorker(form, workers.service);
-            if (registration) {
-              // XXX: Race, sometimes a ServiceWorkerRegistrationInfo doesn't
-              // have a scriptSpec, but its associated WorkerDebugger does.
-              if (!registration.url) {
-                registration.name = registration.url = form.url;
-              }
-              registration.workerActor = form.actor;
-            } else {
-              worker.fetch = form.fetch;
-
-              // If a service worker registration could not be found, this means we are in
-              // e10s, and registrations are not forwarded to other processes until they
-              // reach the activated state. Augment the worker as a registration worker to
-              // display it in aboutdebugging.
-              worker.scope = form.scope;
-              worker.active = false;
-              workers.service.push(worker);
-            }
-            break;
-          case Ci.nsIWorkerDebugger.TYPE_SHARED:
-            workers.shared.push(worker);
-            break;
-          default:
-            workers.other.push(worker);
-        }
-      });
+    this.props.client.mainRoot.listAllWorkers().then(({service, other, shared}) => {
+      workers.service = service.map(f => Object.assign({ icon: WorkerIcon }, f));
+      workers.other = other.map(f => Object.assign({ icon: WorkerIcon }, f));
+      workers.shared = shared.map(f => Object.assign({ icon: WorkerIcon }, f));
 
       // XXX: Filter out the service worker registrations for which we couldn't
       // find the scriptSpec.
       workers.service = workers.service.filter(reg => !!reg.url);
 
       this.setState({ workers });
     });
   }
 
-  getRegistrationForWorker(form, registrations) {
-    for (let registration of registrations) {
-      if (registration.scope === form.scope) {
-        return registration;
-      }
-    }
-    return null;
-  }
-
   isE10S() {
     return Services.appinfo.browserTabsRemoteAutostart;
   }
 
   renderServiceWorkersError() {
     let isWindowPrivate = PrivateBrowsingUtils.isContentWindowPrivate(window);
     let isPrivateBrowsingMode = PrivateBrowsingUtils.permanentPrivateBrowsing;
     let isServiceWorkerDisabled = !Services.prefs
--- a/devtools/client/aboutdebugging/components/workers/ServiceWorkerTarget.js
+++ b/devtools/client/aboutdebugging/components/workers/ServiceWorkerTarget.js
@@ -4,21 +4,22 @@
 
 /* eslint-env browser */
 
 "use strict";
 
 const { Component } = require("devtools/client/shared/vendor/react");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
-const { debugWorker } = require("../../modules/worker");
 const Services = require("Services");
 
 loader.lazyRequireGetter(this, "DebuggerClient",
   "devtools/shared/client/debugger-client", true);
+loader.lazyRequireGetter(this, "gDevToolsBrowser",
+  "devtools/client/framework/devtools-browser", true);
 
 const Strings = Services.strings.createBundle(
   "chrome://devtools/locale/aboutdebugging.properties");
 
 class ServiceWorkerTarget extends Component {
   static get propTypes() {
     return {
       client: PropTypes.instanceOf(DebuggerClient).isRequired,
@@ -80,17 +81,17 @@ class ServiceWorkerTarget extends Compon
 
   debug() {
     if (!this.isRunning()) {
       // If the worker is not running, we can't debug it.
       return;
     }
 
     let { client, target } = this.props;
-    debugWorker(client, target.workerActor);
+    gDevToolsBrowser.openWorkerToolbox(client, target.workerActor);
   }
 
   push() {
     if (!this.isActive() || !this.isRunning()) {
       // If the worker is not running, we can't push to it.
       // If the worker is not active, the registration might be unavailable and the
       // push will not succeed.
       return;
--- a/devtools/client/aboutdebugging/components/workers/Target.js
+++ b/devtools/client/aboutdebugging/components/workers/Target.js
@@ -4,21 +4,22 @@
 
 /* eslint-env browser */
 
 "use strict";
 
 const { Component } = require("devtools/client/shared/vendor/react");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
-const { debugWorker } = require("../../modules/worker");
 const Services = require("Services");
 
 loader.lazyRequireGetter(this, "DebuggerClient",
   "devtools/shared/client/debugger-client", true);
+loader.lazyRequireGetter(this, "gDevToolsBrowser",
+  "devtools/client/framework/devtools-browser", true);
 
 const Strings = Services.strings.createBundle(
   "chrome://devtools/locale/aboutdebugging.properties");
 
 class WorkerTarget extends Component {
   static get propTypes() {
     return {
       client: PropTypes.instanceOf(DebuggerClient).isRequired,
@@ -33,17 +34,17 @@ class WorkerTarget extends Component {
 
   constructor(props) {
     super(props);
     this.debug = this.debug.bind(this);
   }
 
   debug() {
     let { client, target } = this.props;
-    debugWorker(client, target.workerActor);
+    gDevToolsBrowser.openWorkerToolbox(client, target.workerActor);
   }
 
   render() {
     let { target, debugDisabled } = this.props;
 
     return dom.li({ className: "target-container" },
       dom.img({
         className: "target-icon",
--- a/devtools/client/aboutdebugging/modules/moz.build
+++ b/devtools/client/aboutdebugging/modules/moz.build
@@ -1,9 +1,8 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 DevToolsModules(
     'addon.js',
     'connect.js',
-    'worker.js',
 )
deleted file mode 100644
--- a/devtools/client/aboutdebugging/modules/worker.js
+++ /dev/null
@@ -1,75 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-"use strict";
-
-loader.lazyRequireGetter(this, "gDevTools",
-  "devtools/client/framework/devtools", true);
-loader.lazyRequireGetter(this, "TargetFactory",
-  "devtools/client/framework/target", true);
-loader.lazyRequireGetter(this, "Toolbox",
-  "devtools/client/framework/toolbox", true);
-
-/**
- * Open a window-hosted toolbox to debug the worker associated to the provided
- * worker actor.
- *
- * @param {DebuggerClient} client
- * @param {Object} workerActor
- *        worker actor form to debug
- */
-exports.debugWorker = function(client, workerActor) {
-  client.attachWorker(workerActor, (response, workerClient) => {
-    let workerTarget = TargetFactory.forWorker(workerClient);
-    gDevTools.showToolbox(workerTarget, "jsdebugger", Toolbox.HostType.WINDOW)
-      .then(toolbox => {
-        toolbox.once("destroy", () => workerClient.detach());
-      });
-  });
-};
-
-/**
- * Retrieve all service worker registrations as well as workers from the parent
- * and child processes.
- *
- * @param {DebuggerClient} client
- * @return {Object}
- *         - {Array} registrations
- *           Array of ServiceWorkerRegistrationActor forms
- *         - {Array} workers
- *           Array of WorkerActor forms
- */
-exports.getWorkerForms = async function(client) {
-  let registrations = [];
-  let workers = [];
-
-  try {
-    // List service worker registrations
-    ({ registrations } =
-      await client.mainRoot.listServiceWorkerRegistrations());
-
-    // List workers from the Parent process
-    ({ workers } = await client.mainRoot.listWorkers());
-
-    // And then from the Child processes
-    let { processes } = await client.mainRoot.listProcesses();
-    for (let process of processes) {
-      // Ignore parent process
-      if (process.parent) {
-        continue;
-      }
-      let { form } = await client.getProcess(process.id);
-      let processActor = form.actor;
-      let response = await client.request({
-        to: processActor,
-        type: "listWorkers"
-      });
-      workers = workers.concat(response.workers);
-    }
-  } catch (e) {
-    // Something went wrong, maybe our client is disconnected?
-  }
-
-  return { registrations, workers };
-};
--- a/devtools/client/aboutdebugging/test/browser_service_workers_multi_content_process.js
+++ b/devtools/client/aboutdebugging/test/browser_service_workers_multi_content_process.js
@@ -25,16 +25,21 @@ add_task(async function() {
 
   let swTab = await addTab(TAB_URL, { background: true });
 
   info("Wait for service worker to appear in the list");
   // Check that the service worker appears in the UI
   let serviceWorkerContainer =
     await waitUntilServiceWorkerContainer(SERVICE_WORKER, document);
 
+  info("Wait until the service worker is running and the Debug button appears");
+  await waitUntil(() => {
+    return !!getDebugButton(serviceWorkerContainer);
+  }, 100);
+
   info("Check that service worker buttons are disabled.");
   let debugButton = getDebugButton(serviceWorkerContainer);
   ok(debugButton.disabled, "Start/Debug button is disabled");
 
   info("Update the preference to 1");
   let onWarningCleared = waitUntil(() => {
     let hasWarning = document.querySelector(".service-worker-multi-process");
     return !hasWarning && !debugButton.disabled;
--- a/devtools/client/framework/devtools-browser.js
+++ b/devtools/client/framework/devtools-browser.js
@@ -384,16 +384,34 @@ var gDevToolsBrowser = exports.gDevTools
     }
 
     let msg = L10N.getStr("toolbox.noContentProcessForTab.message");
     Services.prompt.alert(null, "", msg);
     return Promise.reject(msg);
   },
 
   /**
+   * Open a window-hosted toolbox to debug the worker associated to the provided
+   * worker actor.
+   *
+   * @param  {DebuggerClient} client
+   * @param  {Object} workerActor
+   *         worker actor form to debug
+   */
+  openWorkerToolbox(client, workerActor) {
+    client.attachWorker(workerActor, (response, workerClient) => {
+      let workerTarget = TargetFactory.forWorker(workerClient);
+      gDevTools.showToolbox(workerTarget, null, Toolbox.HostType.WINDOW)
+        .then(toolbox => {
+          toolbox.once("destroy", () => workerClient.detach());
+        });
+    });
+  },
+
+  /**
    * Install WebIDE widget
    */
   // Used by itself
   installWebIDEWidget() {
     if (this.isWebIDEWidgetInstalled()) {
       return;
     }
 
--- a/devtools/shared/client/root-client.js
+++ b/devtools/shared/client/root-client.js
@@ -86,16 +86,117 @@ RootClient.prototype = {
    * List the running processes.
    *
    * @param function onResponse
    *        Called with the response packet.
    */
   listProcesses: DebuggerClient.requester({ type: "listProcesses" }),
 
   /**
+   * Retrieve all service worker registrations as well as workers from the parent
+   * and child processes. Listing service workers involves merging information coming from
+   * registrations and workers, this method will combine this information to present a
+   * unified array of serviceWorkers. If you are only interested in other workers, use
+   * listWorkers.
+   *
+   * @return {Object}
+   *         - {Array} service
+   *           array of form-like objects for serviceworkers
+   *         - {Array} shared
+   *           Array of WorkerActor forms, containing shared workers.
+   *         - {Array} other
+   *           Array of WorkerActor forms, containing other workers.
+   */
+  listAllWorkers: async function() {
+    let result = {
+      service: [],
+      shared: [],
+      other: []
+    };
+
+    try {
+      let registrations = [];
+      let workers = [];
+
+      // List service worker registrations
+      ({ registrations } = await this.listServiceWorkerRegistrations());
+
+      // List workers from the Parent process
+      ({ workers } = await this.listWorkers());
+
+      // And then from the Child processes
+      let { processes } = await this.listProcesses();
+      for (let process of processes) {
+        // Ignore parent process
+        if (process.parent) {
+          continue;
+        }
+        let { form } = await this._client.getProcess(process.id);
+        let processActor = form.actor;
+        let response = await this._client.request({
+          to: processActor,
+          type: "listWorkers"
+        });
+        workers = workers.concat(response.workers);
+      }
+
+      registrations.forEach(form => {
+        result.service.push({
+          name: form.url,
+          url: form.url,
+          scope: form.scope,
+          fetch: form.fetch,
+          registrationActor: form.actor,
+          active: form.active
+        });
+      });
+
+      workers.forEach(form => {
+        let worker = {
+          name: form.url,
+          url: form.url,
+          workerActor: form.actor
+        };
+        switch (form.type) {
+          case Ci.nsIWorkerDebugger.TYPE_SERVICE:
+            let registration = result.service.find(r => r.scope === form.scope);
+            if (registration) {
+              // XXX: Race, sometimes a ServiceWorkerRegistrationInfo doesn't
+              // have a scriptSpec, but its associated WorkerDebugger does.
+              if (!registration.url) {
+                registration.name = registration.url = form.url;
+              }
+              registration.workerActor = form.actor;
+            } else {
+              worker.fetch = form.fetch;
+
+              // If a service worker registration could not be found, this means we are in
+              // e10s, and registrations are not forwarded to other processes until they
+              // reach the activated state. Augment the worker as a registration worker to
+              // display it in aboutdebugging.
+              worker.scope = form.scope;
+              worker.active = false;
+              result.service.push(worker);
+            }
+            break;
+          case Ci.nsIWorkerDebugger.TYPE_SHARED:
+            result.shared.push(worker);
+            break;
+          default:
+            result.other.push(worker);
+        }
+      });
+    } catch (e) {
+      // Something went wrong, maybe our client is disconnected?
+    }
+
+    return result;
+  },
+
+  /**
    * Fetch the TabActor for the currently selected tab, or for a specific
    * tab given as first parameter.
    *
    * @param [optional] object filter
    *        A dictionary object with following optional attributes:
    *         - outerWindowID: used to match tabs in parent process
    *         - tabId: used to match tabs in child processes
    *         - tab: a reference to xul:tab element