Bug 1445197 - part 3: Move about:debugging worker module to a shared module;r=ochameau
authorJulian Descottes <jdescottes@mozilla.com>
Fri, 20 Apr 2018 16:27:15 +0200
changeset 414698 42759c2366615c8d87ea74c8be00d2c8232af4b2
parent 414697 b4a9a31af41eb75ac0fb3b1d6806ca63d799ff3d
child 414699 f0e5fbd3927460e8de5ee92f93c978c1a58200f1
push id62995
push userjdescottes@mozilla.com
push dateFri, 20 Apr 2018 16:25:38 +0000
treeherderautoland@f0e5fbd39274 [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/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