Bug 1357909 - Tweak the strings explaining to the user what's happening. r=ochameau draft
authorBlake Kaplan <mrbkap@gmail.com>
Wed, 19 Apr 2017 15:51:44 -0700
changeset 566146 5e15e3332d9fa1366d22b60e8b4bf031a066d2a4
parent 565261 a34919b3d942cfd4f0737d432742b0ffa9929389
child 625209 69eba542524464573173da59037e88bbb7da1cf6
push id55103
push userbmo:mrbkap@mozilla.com
push dateThu, 20 Apr 2017 23:27:43 +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?