Bug 1553110 - Part 2: Disable debugging/start links if sw cannot be debugged (multie10s) r=Ola,fluent-reviewers,flod

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

--HG--
extra : moz-landing-system : lando

devtools/client/application/initializer.js

@@ -22,6 +22,10 @@ const actions = require("./src/actions/index");
 const { WorkersListener } =
   require("devtools/client/shared/workers-listener");
 
+// NOTE: this API may change names for these functions. See Bug 1531349.
+const { addMultiE10sListener, isMultiE10s, removeMultiE10sListener } =
+  require("devtools/shared/multi-e10s-helper");
+
 const App = createFactory(require("./src/components/App"));
 
 /**
@@ -32,6 +36,7 @@ window.Application = {
   async bootstrap({ toolbox, panel }) {
     this.updateWorkers = this.updateWorkers.bind(this);
     this.updateDomain = this.updateDomain.bind(this);
+    this.updateCanDebugWorkers = this.updateCanDebugWorkers.bind(this);
 
     this.mount = document.querySelector("#mount");
     this.toolbox = toolbox;
@@ -50,8 +55,11 @@ window.Application = {
     this.workersListener = new WorkersListener(this.client.mainRoot);
     this.workersListener.addListener(this.updateWorkers);
     this.toolbox.target.on("navigate", this.updateDomain);
+    addMultiE10sListener(this.updateCanDebugWorkers);
 
+    // start up updates for the initial state
     this.updateDomain();
+    this.updateCanDebugWorkers();
     await this.updateWorkers();
 
     await l10n.init(["devtools/application.ftl"]);
@@ -74,9 +82,16 @@ window.Application = {
     this.actions.updateDomain(this.toolbox.target.url);
   },
 
+  updateCanDebugWorkers() {
+    // NOTE: this API may change names for this function. See Bug 1531349.
+    const canDebugWorkers = !isMultiE10s();
+    this.actions.updateCanDebugWorkers(canDebugWorkers);
+  },
+
   destroy() {
     this.workersListener.removeListener();
     this.toolbox.target.off("navigate", this.updateDomain);
+    removeMultiE10sListener(this.updateCanDebugWorkers);
 
     unmountComponentAtNode(this.mount);
     this.mount = null;

devtools/client/application/src/actions/workers.js

@@ -5,6 +5,7 @@
 "use strict";
 
 const {
+  UPDATE_CAN_DEBUG_WORKERS,
   UPDATE_WORKERS,
 } = require("../constants");
 
@@ -15,6 +16,14 @@ function updateWorkers(workers) {
   };
 }
 
+function updateCanDebugWorkers(canDebugWorkers) {
+  return {
+    type: UPDATE_CAN_DEBUG_WORKERS,
+    canDebugWorkers,
+  };
+}
+
 module.exports = {
+  updateCanDebugWorkers,
   updateWorkers,
 };

devtools/client/application/src/components/App.css

@@ -22,11 +22,11 @@ a:visited {
   cursor: pointer;
 }
 
-a.disabled,
-a.disabled:hover,
-a.disabled:visited {
+a.disabled-link,
+a.disabled-link:hover,
+a.disabled-link:visited {
   opacity: 0.5 !important;
-  cursor: default;
+  cursor: not-allowed;
 }
 
 /*
@@ -56,4 +56,4 @@ a.disabled:visited {
 
 .application:not(.application--empty) {
   grid-template-rows: 1fr auto;
-}
+}

devtools/client/application/src/components/App.js

@@ -21,36 +21,53 @@ const WorkerListEmpty = createFactory(require("./WorkerListEmpty"));
 class App extends Component {
   static get propTypes() {
     return {
+      // mapped from state
+      canDebugWorkers: PropTypes.bool.isRequired,
       client: PropTypes.object.isRequired,
-      workers: PropTypes.array.isRequired,
-      serviceContainer: PropTypes.object.isRequired,
+      // mapped from state
       domain: PropTypes.string.isRequired,
       fluentBundles: PropTypes.array.isRequired,
+      serviceContainer: PropTypes.object.isRequired,
+      // mapped from state
+      workers: PropTypes.array.isRequired,
     };
   }
 
   render() {
-    let { workers, domain, client, serviceContainer, fluentBundles } = this.props;
+    const {
+      canDebugWorkers,
+      client,
+      domain,
+      fluentBundles,
+      serviceContainer,
+      workers,
+    } = this.props;
 
     // Filter out workers from other domains
-    workers = workers.filter((x) => new URL(x.url).hostname === domain);
-    const isEmpty = workers.length === 0;
+    const domainWorkers = workers.filter((x) => new URL(x.url).hostname === domain);
+    const isWorkerListEmpty = domainWorkers.length === 0;
 
     return (
       LocalizationProvider(
         { messages: fluentBundles },
         main(
-          { className: `application ${isEmpty ? "application--empty" : ""}` },
-          isEmpty ? WorkerListEmpty({ serviceContainer })
-                  : WorkerList({ workers, client })
+          { className: `application ${isWorkerListEmpty ? "application--empty" : ""}` },
+          isWorkerListEmpty ? WorkerListEmpty({ serviceContainer })
+                  : WorkerList({ canDebugWorkers, client, workers: domainWorkers })
         )
       )
     );
   }
 }
 
+function mapStateToProps(state) {
+  return {
+    canDebugWorkers: state.workers.canDebugWorkers,
+    domain: state.page.domain,
+    workers: state.workers.list,
+  };
+}
+
 // Exports
 
-module.exports = connect(
-  (state) => ({ workers: state.workers.list, domain: state.page.domain }),
-)(App);
+module.exports = connect(mapStateToProps)(App);

devtools/client/application/src/components/Worker.js

@@ -28,7 +28,7 @@ class Worker extends Component {
   static get propTypes() {
     return {
       client: PropTypes.instanceOf(DebuggerClient).isRequired,
-      debugDisabled: PropTypes.bool,
+      isDebugEnabled: PropTypes.bool.isRequired,
       worker: PropTypes.shape({
         active: PropTypes.bool,
         name: PropTypes.string.isRequired,
@@ -59,6 +59,11 @@ class Worker extends Component {
   }
 
   start() {
+    if (!this.props.isDebugEnabled) {
+      console.log("Service workers cannot be started in multi-e10s");
+      return;
+    }
+
     if (!this.isActive() || this.isRunning()) {
       console.log("Running or inactive service workers cannot be started");
       return;
@@ -104,6 +109,56 @@ class Worker extends Component {
     return getUnicodeUrlPath(parts[parts.length - 1]);
   }
 
+  renderDebugLink() {
+    const { isDebugEnabled } = this.props;
+
+    const shallDisableLink = !this.isRunning() || !isDebugEnabled;
+    const linkClass = shallDisableLink ? "disabled-link" : "";
+
+    const localizationId = isDebugEnabled
+      ? "serviceworker-worker-debug"
+      : "serviceworker-worker-debug-forbidden";
+
+    const link = Localized(
+      {
+        id: localizationId,
+        // The localized title is only displayed if the debug link is disabled.
+        attrs: {
+          title: shallDisableLink,
+        },
+      },
+      a(
+        {
+          onClick: !shallDisableLink ? this.debug : null,
+          className: `${linkClass} worker__debug-link js-debug-link`,
+        }
+      )
+    );
+    return link;
+  }
+
+  renderStartLink() {
+    const { isDebugEnabled } = this.props;
+    const linkClass = !isDebugEnabled ? "disabled-link" : "";
+
+    const link = Localized(
+      {
+        id: "serviceworker-worker-start2",
+        // The localized title is only displayed if the debug link is disabled.
+        attrs: {
+          title: !isDebugEnabled,
+        },
+      },
+      a(
+        {
+          onClick: this.start,
+          className: `worker__start-link js-start-link ${linkClass}`,
+        }
+      )
+    );
+    return link;
+  }
+
   render() {
     const { worker } = this.props;
     const status = this.getServiceWorkerStatus();
@@ -118,28 +173,6 @@ class Worker extends Component {
         })
       ) : null;
 
-    const debugLinkDisabled = this.isRunning() ? "" : "disabled";
-
-    const debugLink = Localized({
-      id: "serviceworker-worker-debug",
-      // The localized title is only displayed if the debug link is disabled.
-      attrs: { title: !this.isRunning() },
-    },
-      a({
-        onClick: this.isRunning() ? this.debug : null,
-        className: `${debugLinkDisabled} worker__debug-link js-debug-link`,
-      })
-    );
-
-    const startLink = !this.isRunning() ?
-      Localized(
-        { id: "serviceworker-worker-start" },
-        a({
-          onClick: this.start,
-          className: "worker__start-link",
-        })
-      ) : null;
-
     const lastUpdated = worker.lastUpdateTime ?
       Localized(
         {
@@ -170,7 +203,7 @@ class Worker extends Component {
         dd({},
             span({ title: worker.scope, className: "worker__source-url js-source-url" },
               this.formatSource(worker.url)),
-            debugLink,
+            this.renderDebugLink(),
             lastUpdated ? br({}) : null,
             lastUpdated ? lastUpdated : null),
         Localized({ id: "serviceworker-worker-status" },
@@ -181,7 +214,7 @@ class Worker extends Component {
             { id: "serviceworker-worker-status-" + status },
             span({}),
           ),
-          startLink
+          !this.isRunning() ? this.renderStartLink() : null,
         )
       )
     );

devtools/client/application/src/components/WorkerList.js

@@ -21,13 +21,14 @@ const Localized = createFactory(FluentReact.Localized);
 class WorkerList extends Component {
   static get propTypes() {
     return {
+      canDebugWorkers: PropTypes.bool.isRequired,
       client: PropTypes.object.isRequired,
-      workers: PropTypes.object.isRequired,
+      workers: PropTypes.array.isRequired,
     };
   }
 
   render() {
-    const { workers, client } = this.props;
+    const { canDebugWorkers, client, workers } = this.props;
 
     return [
       article({ className: "workers-container" },
@@ -38,13 +39,13 @@ class WorkerList extends Component {
         ul({},
           workers.map(worker => Worker({
             client,
-            debugDisabled: false,
+            isDebugEnabled: canDebugWorkers,
             worker,
           })))
       ),
       Localized(
         {
-          id: "serviceworker-list-aboutdebugging",
+          id: "serviceworker-list-aboutdebugging2",
           a: a(
             {
               className: "aboutdebugging-plug__link",

devtools/client/application/src/constants.js

@@ -5,8 +5,9 @@
 "use strict";
 
 const actionTypes = {
-  UPDATE_WORKERS: "UPDATE_WORKERS",
   UPDATE_DOMAIN: "UPDATE_DOMAIN",
+  UPDATE_CAN_DEBUG_WORKERS: "UPDATE_CAN_DEBUG_WORKERS",
+  UPDATE_WORKERS: "UPDATE_WORKERS",
 };
 
 // flatten constants

devtools/client/application/src/reducers/workers-state.js

@@ -5,6 +5,7 @@
 "use strict";
 
 const {
+  UPDATE_CAN_DEBUG_WORKERS,
   UPDATE_WORKERS,
 } = require("../constants");
 
@@ -12,16 +13,19 @@ function WorkersState() {
   return {
     // Array of all service workers
     list: [],
+    canDebugWorkers: false,
   };
 }
 
 function workersReducer(state = WorkersState(), action) {
   switch (action.type) {
+    case UPDATE_CAN_DEBUG_WORKERS: {
+      return Object.assign({}, state, { canDebugWorkers: action.canDebugWorkers });
+    }
     case UPDATE_WORKERS: {
       const { workers } = action;
-      return { list: workers };
+      return Object.assign({}, state, { list: workers });
     }
-
     default:
       return state;
   }

devtools/client/application/test/browser_application_panel_list-domain-workers.js

@@ -30,6 +30,7 @@ add_task(async function() {
     "Navigate to another page for a different domain with no service worker");
 
   await navigate(target, EMPTY_URL);
+  info("Wait until the service worker list is updated");
   await waitUntil(() => doc.querySelector(".worker-list-empty") !== null);
   ok(true, "No service workers are shown for an empty page in a different domain.");
 

devtools/client/locales/en-US/application.ftl

@@ -26,9 +26,15 @@ serviceworker-worker-unregister = Unregister
 serviceworker-worker-debug = Debug
   .title = Only running service workers can be debugged
 
+# Text for the debug link displayed for an already started Service Worker, when we
+# are in multi e10s mode, which effectively disables this link.
+serviceworker-worker-debug-forbidden = Debug
+  .title = Can only debug service workers if multi e10s is disabled
+
 # Text for the start link displayed for a registered but not running Service Worker.
 # Clicking on the link will attempt to start the service worker.
-serviceworker-worker-start = Start
+serviceworker-worker-start2 = Start
+  .title = Can only start service workers if multi e10s is disabled
 
 # Text displayed for the updated time of the service worker. The <time> element will
 # display the last update time of the service worker script.