Bug 1357909 - Tweak the strings explaining to the user what's happening. r=ochameau
authorBlake Kaplan <mrbkap@gmail.com>
Wed, 19 Apr 2017 15:51:44 -0700
changeset 566349 634530ff65a6bc3e249b538e7aba29124dd2293e
parent 566348 7e27cbd5c6990eb363ca5277e01b05c70fe3d705
child 566350 0f2c055d69b220875b8e8249bd20705ac0143d1f
push id55180
push userjjong@mozilla.com
push dateFri, 21 Apr 2017 09:36:13 +0000
reviewersochameau
bugs1357909
milestone55.0a1
Bug 1357909 - Tweak the strings explaining to the user what's happening. r=ochameau This patch also watches the dom.ipc.multiOptOut pref to make sure we update our state when all of the relevant prefs change as well as clarifies how the code works. MozReview-Commit-ID: 8qKymEth7C8
devtools/client/aboutdebugging/components/workers/multi-e10s-warning.js
devtools/client/aboutdebugging/components/workers/panel.js
devtools/client/locales/en-US/aboutdebugging.properties
--- a/devtools/client/aboutdebugging/components/workers/multi-e10s-warning.js
+++ b/devtools/client/aboutdebugging/components/workers/multi-e10s-warning.js
@@ -21,17 +21,17 @@ loader.lazyRequireGetter(this, "Debugger
 
 const Strings = Services.strings.createBundle("chrome://devtools/locale/aboutdebugging.properties");
 const MULTI_OPT_OUT_PREF = "dom.ipc.multiOptOut";
 
 module.exports = createClass({
   displayName: "multiE10SWarning",
 
   onUpdatePreferenceClick() {
-    let message = Strings.GetStringFromName("multiProcessWarningConfirmUpdate");
+    let message = Strings.GetStringFromName("multiProcessWarningConfirmUpdate2");
     if (window.confirm(message)) {
       // Disable multi until at least the next experiment.
       Services.prefs.setIntPref(MULTI_OPT_OUT_PREF,
                                 Services.appinfo.E10S_MULTI_EXPERIMENT);
       // Restart the browser.
       Services.startup.quit(Ci.nsIAppStartup.eAttemptQuit | Ci.nsIAppStartup.eRestart);
     }
   },
@@ -43,20 +43,20 @@ module.exports = createClass({
       },
       dom.div(
         {},
         dom.div({ className: "warning" }),
         dom.b({}, Strings.GetStringFromName("multiProcessWarningTitle"))
       ),
       dom.div(
         {},
-        Strings.GetStringFromName("multiProcessWarningMessage")
+        Strings.GetStringFromName("multiProcessWarningMessage2")
       ),
       dom.button(
         {
           className: "update-button",
           onClick: this.onUpdatePreferenceClick,
         },
-        Strings.GetStringFromName("multiProcessWarningUpdateLink")
+        Strings.GetStringFromName("multiProcessWarningUpdateLink2")
       )
     );
   },
 });
--- a/devtools/client/aboutdebugging/components/workers/panel.js
+++ b/devtools/client/aboutdebugging/components/workers/panel.js
@@ -26,16 +26,17 @@ loader.lazyRequireGetter(this, "Debugger
   "devtools/shared/client/main", true);
 
 const Strings = Services.strings.createBundle(
   "chrome://devtools/locale/aboutdebugging.properties");
 
 const WorkerIcon = "chrome://devtools/skin/images/debugging-workers.svg";
 const MORE_INFO_URL = "https://developer.mozilla.org/en-US/docs/Tools/about%3Adebugging";
 const PROCESS_COUNT_PREF = "dom.ipc.processCount";
+const MULTI_OPTOUT_PREF = "dom.ipc.multiOptOut";
 
 module.exports = createClass({
   displayName: "WorkersPanel",
 
   propTypes: {
     client: PropTypes.instanceOf(DebuggerClient).isRequired,
     id: PropTypes.string.isRequired
   },
@@ -53,35 +54,49 @@ module.exports = createClass({
 
   componentDidMount() {
     let client = this.props.client;
     client.addListener("workerListChanged", this.updateWorkers);
     client.addListener("serviceWorkerRegistrationListChanged", this.updateWorkers);
     client.addListener("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);
 
     this.updateMultiE10S();
     this.updateWorkers();
   },
 
   componentWillUnmount() {
     let client = this.props.client;
     client.removeListener("processListChanged", this.updateWorkers);
     client.removeListener("serviceWorkerRegistrationListChanged", this.updateWorkers);
     client.removeListener("workerListChanged", this.updateWorkers);
     client.removeListener("registration-changed", this.updateWorkers);
 
     Services.prefs.removeObserver(PROCESS_COUNT_PREF, this.updateMultiE10S);
+    Services.prefs.removeObserver(MULTI_OPTOUT_PREF, this.updateMultiE10S);
   },
 
   updateMultiE10S() {
     // We watch the pref but set the state based on
-    // nsIXULRuntime::maxWebProcessCount.
+    // nsIXULRuntime.maxWebProcessCount.
     let processCount = Services.appinfo.maxWebProcessCount;
     this.setState({ processCount });
   },
 
   updateWorkers() {
     let workers = this.getInitialState().workers;
 
     getWorkerForms(this.props.client).then(forms => {
--- a/devtools/client/locales/en-US/aboutdebugging.properties
+++ b/devtools/client/locales/en-US/aboutdebugging.properties
@@ -123,19 +123,19 @@ configurationIsNotCompatible = Your brow
 # LOCALIZATION NOTE (multiProcessWarningTitle):
 # This string is displayed as a warning message on top of the about:debugging#workers
 # page when multi-e10s is enabled
 multiProcessWarningTitle = Service Worker debugging is not compatible with multiple content processes at the moment.
 
 # LOCALIZATION NOTE (multiProcessWarningMessage):
 # This string is displayed in the warning section for multi-e10s in
 # about:debugging#workers
-multiProcessWarningMessage = The preference “dom.ipc.processCount” can be set to 1 to force a single content process.
+multiProcessWarningMessage2 = The preference “dom.ipc.multiOptOut” can be modified to force a single content process for the current version.
 
 # LOCALIZATION NOTE (multiProcessWarningLink):
 # This string is the text content of a link in the warning section for multi-e10s in
 # about:debugging#workers. The link updates the pref and restarts the browser.
-multiProcessWarningUpdateLink = Set dom.ipc.processCount to 1
+multiProcessWarningUpdateLink2 = Opt out of multiple content processes
 
 # LOCALIZATION NOTE (multiProcessWarningConfirmUpdate):
 # This string is displayed as a confirmation message when the user clicks on
 # the multiProcessWarningUpdateLink in about:debugging#workers
-multiProcessWarningConfirmUpdate = Set “dom.ipc.processCount” to 1 and restart the browser?
+multiProcessWarningConfirmUpdate2 = Opt out of multiple processes?