Bug 1445197 - part 3: Move about:debugging worker module to a shared module;r=ochameau
☠☠ backed out by 03c304d9325b ☠ ☠
authorJulian Descottes <jdescottes@mozilla.com>
Thu, 05 Apr 2018 21:45:15 +0200
changeset 414316 ba9ffea6d21695bdfff41bd32baa4a67c0a504cf
parent 414315 a0345830e1cfa65674eed51a56d98cccf7352f49
child 414317 4b6f77cfd686930e3be525c51d10b825549c5eae
push id62832
push userjdescottes@mozilla.com
push dateWed, 18 Apr 2018 16:03:30 +0000
treeherderautoland@4b6f77cfd686 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersochameau
bugs1445197
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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/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
@@ -6,17 +6,16 @@
 
 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 +42,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;
@@ -105,84 +103,41 @@ class WorkersPanel extends Component {
 
   updateMultiE10S() {
     // We watch the pref but set the state based on
     // nsIXULRuntime.maxWebProcessCount.
     let processCount = Services.appinfo.maxWebProcessCount;
     this.setState({ processCount });
   }
 
-  updateWorkers() {
+  async 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
-        });
-      });
+    let forms = await this.props.client.mainRoot.listAllWorkers();
 
-      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;
+    workers.service = forms.serviceWorkers
+      .map(form => Object.assign({ icon: WorkerIcon, name: form.url }, form));
 
-              // 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);
-        }
-      });
+    // Lookup shared and regular workers in forms.workers.
+    forms.workers.forEach(form => {
+      let worker = {
+        icon: WorkerIcon,
+        name: form.url,
+        url: form.url,
+        workerActor: form.actor
+      };
 
-      // 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 });
+      if (form.type === Ci.nsIWorkerDebugger.TYPE_DEDICATED) {
+        workers.other.push(worker);
+      } else if (form.type === Ci.nsIWorkerDebugger.TYPE_SHARED) {
+        workers.shared.push(worker);
+      }
     });
-  }
 
-  getRegistrationForWorker(form, registrations) {
-    for (let registration of registrations) {
-      if (registration.scope === form.scope) {
-        return registration;
-      }
-    }
-    return null;
+    this.setState({ workers });
   }
 
   isE10S() {
     return Services.appinfo.browserTabsRemoteAutostart;
   }
 
   renderServiceWorkersError() {
     let isWindowPrivate = PrivateBrowsingUtils.isContentWindowPrivate(window);
--- 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/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
@@ -2,16 +2,18 @@
  * 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";
 
 const { Ci } = require("chrome");
 const { arg, DebuggerClient } = require("devtools/shared/client/debugger-client");
 
+const TYPE_SERVICE = Ci.nsIWorkerDebugger.TYPE_SERVICE;
+
 /**
  * A RootClient object represents a root actor on the server. Each
  * DebuggerClient keeps a RootClient instance representing the root actor
  * for the initial connection; DebuggerClient's 'listTabs' and
  * 'listChildProcesses' methods forward to that root actor.
  *
  * @param client object
  *      The client connection to which this actor belongs.
@@ -155,16 +157,135 @@ RootClient.prototype = {
       type: "getWindow",
       outerWindowID,
     };
 
     return this.request(packet);
   },
 
   /**
+   * Retrieve the service worker registrations currently available for the this client.
+   *
+   * @param  {Form} forms
+   *         All available worker forms
+   * @return {Array} array of form-like objects dedicated to service workers:
+   *         - {String} url: url of the worker
+   *         - {String} scope: scope controlled by this worker
+   *         - {String} fetch: fetch url if available
+   *         - {Boolean} active: is the service worker active
+   *         - {String} registrationActor: id of the registration actor, needed to start,
+   *           unregister and get push information for the registation. Available on
+   *           active service workers only in e10s.
+   *         - {String} workerActor: id of the worker actor, needed to debug and send push
+   *           notifications.
+   */
+  _mergeServiceWorkerForms: async function(workers) {
+    // List service worker registrations
+    let { registrations } = await this.listServiceWorkerRegistrations();
+
+    // registrations contain only service worker registrations, add each registration
+    // to the workers array.
+    let fromRegistrations = registrations.map(form => {
+      return {
+        url: form.url,
+        scope: form.scope,
+        fetch: form.fetch,
+        active: form.active,
+        registrationActor: form.actor,
+      };
+    });
+
+    // Loop on workers to retrieve the worker actor corresponding to the current
+    // instance of the service worker if available.
+    let fromWorkers = [];
+    workers
+      .filter(form => form.type === TYPE_SERVICE)
+      .forEach(form => {
+        let registration = fromRegistrations.find(w => w.scope === form.scope);
+        if (registration) {
+          if (!registration.url) {
+            // XXX: Race, sometimes a ServiceWorkerRegistrationInfo doesn't
+            // have a scriptSpec, but its associated WorkerDebugger does.
+            registration.url = form.url;
+          }
+          // Add the worker actor as "workerActor" on the registration.
+          registration.workerActor = form.actor;
+        } else {
+          // 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.
+          fromWorkers.push({
+            url: form.url,
+            scope: form.scope,
+            fetch: form.fetch,
+            // Infer active=false since the service worker is not supposed to be
+            // registered yet in this edge case.
+            active: false,
+            workerActor: form.actor,
+          });
+        }
+      });
+
+    // Concatenate service workers found in forms.registrations and forms.workers.
+    let combined = [...fromRegistrations, ...fromWorkers];
+
+    // Filter out service workers missing a url.
+    return combined.filter(form => !!form.url);
+  },
+
+  /**
+   * 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} serviceWorkers
+   *           array of form-like objects for serviceworkers (cf _mergeServiceWorkerForms)
+   *         - {Array} workers
+   *           Array of WorkerActor forms, containing all possible workers.
+   */
+  listAllWorkers: async function() {
+    let workers = [];
+
+    try {
+      // 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);
+      }
+    } catch (e) {
+      console.warn("Error while listing all workers, is the client disconnected?", e);
+    }
+
+    // Combine information from registrations and workers to create a usable list of
+    // workers for consumer code.
+    let serviceWorkers = await this._mergeServiceWorkerForms(workers);
+
+    return {
+      serviceWorkers,
+      workers
+    };
+  },
+
+  /**
    * Description of protocol's actors and methods.
    *
    * @param function onResponse
    *        Called with the response packet.
    */
   protocolDescription: DebuggerClient.requester({ type: "protocolDescription" }),
 
   /*