Bug 1488502 - Disable service workers debugging when in multi e10s. r=jdescottes,daisuke

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

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

devtools/client/aboutdebugging-new/aboutdebugging.js

@@ -32,6 +32,12 @@ const {
   removeUSBRuntimesObserver,
 } = require("./src/modules/usb-runtimes");
 
+const {
+  addMultiE10sListener,
+  isMultiE10s,
+  removeMultiE10sListener,
+} = require("devtools/client/shared/multi-e10s-helper");
+
 loader.lazyRequireGetter(this, "adbAddon", "devtools/shared/adb/adb-addon", true);
 
 const Router = createFactory(require("devtools/client/shared/vendor/react-router-dom").HashRouter);
@@ -48,6 +54,7 @@ const AboutDebugging = {
     this.onAdbAddonUpdated = this.onAdbAddonUpdated.bind(this);
     this.onNetworkLocationsUpdated = this.onNetworkLocationsUpdated.bind(this);
     this.onUSBRuntimesUpdated = this.onUSBRuntimesUpdated.bind(this);
+    this.onMultiE10sUpdated = this.onMultiE10sUpdated.bind(this);
 
     this.store = configureStore();
     this.actions = bindActionCreators(actions, this.store.dispatch);
@@ -84,6 +91,12 @@ const AboutDebugging = {
 
     // Remove deprecated remote debugging extensions.
     await adbAddon.uninstallUnsupportedExtensions();
+
+    addMultiE10sListener(this.onMultiE10sUpdated);
+  },
+
+  onMultiE10sUpdated() {
+    this.actions.updateMultiE10sStatus(isMultiE10s());
   },
 
   onAdbAddonUpdated() {
@@ -111,6 +124,7 @@ const AboutDebugging = {
     // Remove all client listeners.
     this.actions.removeRuntimeListeners();
 
+    removeMultiE10sListener(this.onMultiE10sUpdated);
     removeNetworkLocationsObserver(this.onNetworkLocationsUpdated);
     removeUSBRuntimesObserver(this.onUSBRuntimesUpdated);
     adbAddon.off("update", this.onAdbAddonUpdated);

devtools/client/aboutdebugging-new/src/actions/ui.js

@@ -13,6 +13,7 @@ const {
   ADB_ADDON_UNINSTALL_FAILURE,
   ADB_ADDON_STATUS_UPDATED,
   DEBUG_TARGET_COLLAPSIBILITY_UPDATED,
+  MULTI_E10S_UPDATED,
   NETWORK_LOCATIONS_UPDATED,
   PAGE_SELECTED,
   PAGE_TYPES,
@@ -123,6 +124,12 @@ function scanUSBRuntimes() {
   };
 }
 
+function updateMultiE10sStatus(isMultiE10s) {
+  return (dispatch, getState) => {
+    dispatch({ type: MULTI_E10S_UPDATED, isMultiE10s});
+  };
+}
+
 module.exports = {
   addNetworkLocation,
   installAdbAddon,
@@ -132,5 +139,6 @@ module.exports = {
   uninstallAdbAddon,
   updateAdbAddonStatus,
   updateDebugTargetCollapsibility,
+  updateMultiE10sStatus,
   updateNetworkLocations,
 };

devtools/client/aboutdebugging-new/src/base.css

@@ -244,6 +244,10 @@ Form controls
   background: var(--box-background-hover)
 }
 
+.default-button:disabled {
+  opacity: 0.4;
+}
+
 /* smaller size for a default button */
 .default-button--micro {
   padding-inline-start: calc(2 * var(--base-distance));

devtools/client/aboutdebugging-new/src/components/debugtarget/InspectAction.js

@@ -22,6 +22,7 @@ class InspectAction extends PureComponent {
     return {
       dispatch: PropTypes.func.isRequired,
       target: Types.debugTarget.isRequired,
+      disabled: PropTypes.bool,
     };
   }
 
@@ -31,6 +32,8 @@ class InspectAction extends PureComponent {
   }
 
   render() {
+    const { disabled } = this.props;
+
     return Localized(
       {
         id: "about-debugging-debug-target-inspect-button",
@@ -39,6 +42,7 @@ class InspectAction extends PureComponent {
         {
           onClick: e => this.inspect(),
           className: "default-button  js-debug-target-inspect-button",
+          disabled,
         },
         "Inspect"
       )

devtools/client/aboutdebugging-new/src/components/debugtarget/ServiceWorkerAction.js

@@ -7,6 +7,7 @@
 const { createFactory, PureComponent } = require("devtools/client/shared/vendor/react");
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
+const { connect } = require("devtools/client/shared/vendor/react-redux");
 
 const FluentReact = require("devtools/client/shared/vendor/fluent-react");
 
@@ -24,6 +25,8 @@ class ServiceWorkerAction extends PureComponent {
       dispatch: PropTypes.func.isRequired,
       // Provided by wrapping the component with FluentReact.withLocalization.
       getString: PropTypes.func.isRequired,
+      // Provided by redux state
+      isMultiE10s: PropTypes.bool.isRequired,
       target: Types.debugTarget.isRequired,
     };
   }
@@ -39,13 +42,14 @@ class ServiceWorkerAction extends PureComponent {
   }
 
   _renderAction() {
-    const { dispatch, target } = this.props;
+    const { dispatch, isMultiE10s, target } = this.props;
     const { isActive, isRunning } = target.details;
 
     if (!isRunning) {
       const startLabel = this.props.getString("about-debugging-worker-action-start");
       return this._renderButton({
         className: "default-button",
+        disabled: isMultiE10s,
         label: startLabel,
         onClick: this.start.bind(this),
       });
@@ -53,24 +57,26 @@ class ServiceWorkerAction extends PureComponent {
 
     if (!isActive) {
       // Only debug button is available if the service worker is not active.
-      return InspectAction({ dispatch, target });
+      return InspectAction({ disabled: isMultiE10s, dispatch, target });
     }
 
     const pushLabel = this.props.getString("about-debugging-worker-action-push");
     return [
       this._renderButton({
         className: "default-button js-push-button",
+        disabled: isMultiE10s,
         label: pushLabel,
         onClick: this.push.bind(this),
       }),
-      InspectAction({ dispatch, target }),
+      InspectAction({ disabled: isMultiE10s, dispatch, target }),
     ];
   }
 
-  _renderButton({ className, label, onClick }) {
+  _renderButton({ className, disabled, label, onClick }) {
     return dom.button(
       {
         className,
+        disabled,
         onClick: e => onClick(),
       },
       label,
@@ -87,4 +93,11 @@ class ServiceWorkerAction extends PureComponent {
   }
 }
 
-module.exports = FluentReact.withLocalization(ServiceWorkerAction);
+const mapStateToProps = state => {
+  return {
+    isMultiE10s: state.ui.isMultiE10s,
+  };
+};
+
+module.exports = FluentReact.withLocalization(
+  connect(mapStateToProps)(ServiceWorkerAction));

devtools/client/aboutdebugging-new/src/constants.js

@@ -19,6 +19,7 @@ const actionTypes = {
   DISCONNECT_RUNTIME_FAILURE: "DISCONNECT_RUNTIME_FAILURE",
   DISCONNECT_RUNTIME_START: "DISCONNECT_RUNTIME_START",
   DISCONNECT_RUNTIME_SUCCESS: "DISCONNECT_RUNTIME_SUCCESS",
+  MULTI_E10S_UPDATED: "MULTI_E10S_UPDATED",
   NETWORK_LOCATIONS_UPDATED: "NETWORK_LOCATIONS_UPDATED",
   PAGE_SELECTED: "PAGE_SELECTED",
   REQUEST_EXTENSIONS_FAILURE: "REQUEST_EXTENSIONS_FAILURE",

devtools/client/aboutdebugging-new/src/create-store.js

@@ -9,6 +9,7 @@ const Services = require("Services");
 const { applyMiddleware, createStore } = require("devtools/client/shared/vendor/redux");
 const { thunk } = require("devtools/client/shared/redux/middleware/thunk.js");
 const { waitUntilService } = require("devtools/client/shared/redux/middleware/wait-service.js");
+const { isMultiE10s } = require("devtools/client/shared/multi-e10s-helper");
 
 const rootReducer = require("./reducers/index");
 const { DebugTargetsState } = require("./reducers/debug-targets-state");
@@ -50,7 +51,7 @@ function getUiState() {
   const showSystemAddons = Services.prefs.getBoolPref(PREFERENCES.SHOW_SYSTEM_ADDONS,
     false);
   return new UiState(locations, collapsibilities, networkEnabled, wifiEnabled,
-    showSystemAddons);
+    showSystemAddons, isMultiE10s());
 }
 
 exports.configureStore = configureStore;

devtools/client/aboutdebugging-new/src/reducers/ui-state.js

@@ -7,6 +7,7 @@
 const {
   ADB_ADDON_STATUS_UPDATED,
   DEBUG_TARGET_COLLAPSIBILITY_UPDATED,
+  MULTI_E10S_UPDATED,
   NETWORK_LOCATIONS_UPDATED,
   PAGE_SELECTED,
   TEMPORARY_EXTENSION_INSTALL_FAILURE,
@@ -16,10 +17,12 @@ const {
 } = require("../constants");
 
 function UiState(locations = [], debugTargetCollapsibilities = {},
-                 networkEnabled = false, wifiEnabled = false, showSystemAddons = false) {
+                 networkEnabled = false, wifiEnabled = false,
+                 showSystemAddons = false, isMultiE10s = false) {
   return {
     adbAddonStatus: null,
     debugTargetCollapsibilities,
+    isMultiE10s,
     isScanningUsb: false,
     networkEnabled,
     networkLocations: locations,
@@ -45,6 +48,11 @@ function uiReducer(state = UiState(), action) {
       return Object.assign({}, state, { debugTargetCollapsibilities });
     }
 
+    case MULTI_E10S_UPDATED: {
+      const { isMultiE10s } = action;
+      return Object.assign({}, state, { isMultiE10s });
+    }
+
     case NETWORK_LOCATIONS_UPDATED: {
       const { locations } = action;
       return Object.assign({}, state, { networkLocations: locations });

devtools/client/aboutdebugging/components/workers/Panel.js

@@ -11,6 +11,11 @@ const { Component, createFactory } = require("devtools/client/shared/vendor/reac
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
 const Services = require("Services");
+const {
+  addMultiE10sListener,
+  isMultiE10s,
+  removeMultiE10sListener,
+} = require("devtools/client/shared/multi-e10s-helper");
 
 const PanelHeader = createFactory(require("../PanelHeader"));
 const TargetList = createFactory(require("../TargetList"));
@@ -30,8 +35,6 @@ const Strings = Services.strings.createBundle(
 const WorkerIcon = "chrome://devtools/skin/images/debugging-workers.svg";
 const MORE_INFO_URL = "https://developer.mozilla.org/en-US/docs/Tools/about%3Adebugging" +
                       "#Service_workers_not_compatible";
-const PROCESS_COUNT_PREF = "dom.ipc.processCount";
-const MULTI_OPTOUT_PREF = "dom.ipc.multiOptOut";
 
 class WorkersPanel extends Component {
   static get propTypes() {
@@ -66,20 +69,7 @@ class WorkersPanel extends Component {
     client.mainRoot.on("processListChanged", this.updateWorkers);
     client.addListener("registration-changed", this.updateWorkers);
 
-    // Some notes about these observers:
-    // - nsIPrefBranch.addObserver observes prefixes. In reality, watching
-    //   PROCESS_COUNT_PREF watches two separate prefs:
-    //   dom.ipc.processCount *and* dom.ipc.processCount.web. Because these
-    //   are the two ways that we control the number of content processes,
-    //   that works perfectly fine.
-    // - The user might opt in or out of multi by setting the multi opt out
-    //   pref. That affects whether we need to show our warning, so we need to
-    //   update our state when that pref changes.
-    // - In all cases, we don't have to manually check which pref changed to
-    //   what. The platform code in nsIXULRuntime.maxWebProcessCount does all
-    //   of that for us.
-    Services.prefs.addObserver(PROCESS_COUNT_PREF, this.updateMultiE10S);
-    Services.prefs.addObserver(MULTI_OPTOUT_PREF, this.updateMultiE10S);
+    addMultiE10sListener(this.updateMultiE10S);
 
     this.updateMultiE10S();
     this.updateWorkers();
@@ -95,8 +85,7 @@ class WorkersPanel extends Component {
     }
     client.removeListener("registration-changed", this.updateWorkers);
 
-    Services.prefs.removeObserver(PROCESS_COUNT_PREF, this.updateMultiE10S);
-    Services.prefs.removeObserver(MULTI_OPTOUT_PREF, this.updateMultiE10S);
+    removeMultiE10sListener(this.updateMultiE10S);
   }
 
   get initialState() {
@@ -106,7 +95,7 @@ class WorkersPanel extends Component {
         shared: [],
         other: [],
       },
-      processCount: 1,
+      isMultiE10S: isMultiE10s(),
 
       // List of ContentProcessTargetFront registered from componentWillMount
       // from which we listen for worker list changes
@@ -115,10 +104,7 @@ class WorkersPanel extends Component {
   }
 
   updateMultiE10S() {
-    // We watch the pref but set the state based on
-    // nsIXULRuntime.maxWebProcessCount.
-    const processCount = Services.appinfo.maxWebProcessCount;
-    this.setState({ processCount });
+    this.setState({ isMultiE10S: isMultiE10s() });
   }
 
   updateWorkers() {
@@ -175,10 +161,7 @@ class WorkersPanel extends Component {
 
   render() {
     const { client, id } = this.props;
-    const { workers, processCount } = this.state;
-
-    const isE10S = Services.appinfo.browserTabsRemoteAutostart;
-    const isMultiE10S = isE10S && processCount > 1;
+    const { workers, isMultiE10S } = this.state;
 
     return dom.div(
       {

devtools/client/shared/moz.build

@@ -37,6 +37,7 @@ DevToolsModules(
     'key-shortcuts.js',
     'keycodes.js',
     'link.js',
+    'multi-e10s-helper.js',
     'natural-sort.js',
     'node-attribute-parser.js',
     'options-view.js',

devtools/client/shared/multi-e10s-helper.js

@@ -0,0 +1,53 @@
+/* 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";
+
+const Services = require("Services");
+
+/**
+ * This helper provides info on whether we are in multi e10s mode or not.
+ * Since this can be changed on the fly, there are subscribe/unsubscribe functions
+ * to get notified of this.
+ *
+ * The logic to handle this is borrowed from the (old) about:debugging code.
+ */
+
+const PROCESS_COUNT_PREF = "dom.ipc.processCount";
+const MULTI_OPTOUT_PREF = "dom.ipc.multiOptOut";
+
+function addMultiE10sListener(listener) {
+  // Some notes about these observers:
+  // - nsIPrefBranch.addObserver observes prefixes. In reality, watching
+  //   PROCESS_COUNT_PREF watches two separate prefs:
+  //   dom.ipc.processCount *and* dom.ipc.processCount.web. Because these
+  //   are the two ways that we control the number of content processes,
+  //   that works perfectly fine.
+  // - The user might opt in or out of multi by setting the multi opt out
+  //   pref. That affects whether we need to show our warning, so we need to
+  //   update our state when that pref changes.
+  // - In all cases, we don't have to manually check which pref changed to
+  //   what. The platform code in nsIXULRuntime.maxWebProcessCount does all
+  //   of that for us.
+  Services.prefs.addObserver(PROCESS_COUNT_PREF, listener);
+  Services.prefs.addObserver(MULTI_OPTOUT_PREF, listener);
+}
+
+function removeMultiE10sListener(listener) {
+  Services.prefs.removeObserver(PROCESS_COUNT_PREF, listener);
+  Services.prefs.removeObserver(MULTI_OPTOUT_PREF, listener);
+}
+
+function isMultiE10s() {
+  const isE10s = Services.appinfo.browserTabsRemoteAutostart;
+  const processCount = Services.appinfo.maxWebProcessCount;
+
+  return isE10s && processCount > 1;
+}
+
+module.exports = {
+  addMultiE10sListener,
+  isMultiE10s,
+  removeMultiE10sListener,
+};