Bug 1412050 - avoid devtools onboarding flow when restoring session;r=ochameau

MozReview-Commit-ID: 8fJOPaOnkQe

--HG--
extra : rebase_source : 533d576fc094290e89feb7a4feb77b83054d458c

devtools/shim/DevToolsShim.jsm

@@ -14,6 +14,8 @@ XPCOMUtils.defineLazyGetter(this, "DevtoolsStartup", () => {
             .wrappedJSObject;
 });
 
+const DEVTOOLS_ENABLED_PREF = "devtools.enabled";
+
 this.EXPORTED_SYMBOLS = [
   "DevToolsShim",
 ];
@@ -54,6 +56,15 @@ this.DevToolsShim = {
              .hasSubstitution("devtools");
   },
 
+  /**
+   * Returns true if DevTools are enabled for the current profile. If devtools are not
+   * enabled, initializing DevTools will open the onboarding page. Some entry points
+   * should no-op in this case.
+   */
+  isEnabled: function () {
+    return Services.prefs.getBoolPref(DEVTOOLS_ENABLED_PREF);
+  },
+
   /**
    * Check if DevTools have already been initialized.
    *
@@ -134,15 +145,15 @@ this.DevToolsShim = {
   },
 
   /**
-   * Called from SessionStore.jsm in mozilla-central when restoring a state that contained
-   * opened scratchpad windows and browser console.
+   * Called from SessionStore.jsm in mozilla-central when restoring a previous session.
+   * Will always be called, even if the session does not contain DevTools related items.
    */
   restoreDevToolsSession: function (session) {
-    let devtoolsReady = this._maybeInitializeDevTools();
-    if (!devtoolsReady) {
+    if (!this.isEnabled()) {
       return;
     }
 
+    this.initDevTools();
     this._gDevTools.restoreDevToolsSession(session);
   },
 
@@ -160,16 +171,18 @@ this.DevToolsShim = {
    *         markup view or that resolves immediately if DevTools are not installed.
    */
   inspectNode: function (tab, selectors) {
+    if (!this.isEnabled()) {
+      DevtoolsStartup.openInstallPage("ContextMenu");
+      return Promise.resolve();
+    }
+
     // Record the timing at which this event started in order to compute later in
     // gDevTools.showToolbox, the complete time it takes to open the toolbox.
     // i.e. especially take `DevtoolsStartup.initDevTools` into account.
     let { performance } = Services.appShell.hiddenDOMWindow;
     let startTime = performance.now();
 
-    let devtoolsReady = this._maybeInitializeDevTools("ContextMenu");
-    if (!devtoolsReady) {
-      return Promise.resolve();
-    }
+    this.initDevTools("ContextMenu");
 
     return this._gDevTools.inspectNode(tab, selectors, startTime);
   },
@@ -184,27 +197,22 @@ this.DevToolsShim = {
   },
 
   /**
-   * Should be called if a shim method attempts to initialize devtools.
-   * - if DevTools are already initialized, returns true.
-   * - if DevTools are not initialized, call initDevTools from devtools-startup:
-   *   - if devtools.enabled is true, DevTools will synchronously initialize and the
-   *     method will return true.
-   *   - if devtools.enabled is false, DevTools installation flow will start and the
-   *     method will return false
+   * Initialize DevTools via DevToolsStartup if needed. This method throws if DevTools are
+   * not enabled.. If the entry point is supposed to trigger the onboarding, call it
+   * explicitly via DevtoolsStartup.openInstallPage().
    *
    * @param {String} reason
    *        optional, if provided should be a valid entry point for DEVTOOLS_ENTRY_POINT
    *        in toolkit/components/telemetry/Histograms.json
    */
-  _maybeInitializeDevTools: function (reason) {
-    // Attempt to initialize DevTools, which should be synchronous.
+  initDevTools: function (reason) {
+    if (!this.isEnabled()) {
+      throw new Error("DevTools are not enabled and can not be initialized.");
+    }
+
     if (!this.isInitialized()) {
       DevtoolsStartup.initDevTools(reason);
     }
-
-    // The initialization process can lead to show the user installation screen, in this
-    // case this.isInitialized() will still be false after calling initDevTools().
-    return this.isInitialized();
   }
 };
 
@@ -224,11 +232,12 @@ let webExtensionsMethods = [
 
 for (let method of webExtensionsMethods) {
   this.DevToolsShim[method] = function () {
-    let devtoolsReady = this._maybeInitializeDevTools();
-    if (!devtoolsReady) {
+    if (!this.isEnabled()) {
       throw new Error("Could not call a DevToolsShim webextension method ('" + method +
         "'): DevTools are not initialized.");
     }
+
+    this.initDevTools();
     return this._gDevTools[method].apply(this._gDevTools, arguments);
   };
 }

devtools/shim/tests/unit/test_devtools_shim.js

@@ -6,6 +6,7 @@
 
 const { DevToolsShim } =
     Components.utils.import("chrome://devtools-shim/content/DevToolsShim.jsm", {});
+const { Services } = Components.utils.import("resource://gre/modules/Services.jsm", {});
 
 // Test the DevToolsShim
 
@@ -128,13 +129,14 @@ function test_events() {
   checkCalls(mock, "emit", 2, ["devtools-unregistered"]);
 }
 
-function test_scratchpad_apis() {
-  let backupMaybeInitializeDevTools = DevToolsShim._maybeInitializeDevTools;
-  DevToolsShim._maybeInitializeDevTools = () => {
-    return false;
-  };
+function test_restore_session_apis() {
+  // Backup method and preferences that will be updated for the test.
+  let initDevToolsBackup = DevToolsShim.initDevTools;
+  let devtoolsEnabledValue = Services.prefs.getBoolPref("devtools.enabled");
 
+  Services.prefs.setBoolPref("devtools.enabled", false);
   ok(!DevToolsShim.isInitialized(), "DevTools are not initialized");
+  ok(!DevToolsShim.isEnabled(), "DevTools are not enabled");
 
   // Ensure that save & restore DevToolsSession don't initialize the tools and don't crash
   DevToolsShim.saveDevToolsSession({});
@@ -143,14 +145,15 @@ function test_scratchpad_apis() {
     browserConsole: true,
   });
 
+  Services.prefs.setBoolPref("devtools.enabled", true);
+  ok(DevToolsShim.isEnabled(), "DevTools are enabled");
   ok(!DevToolsShim.isInitialized(), "DevTools are not initialized");
 
   let mock = createMockDevTools();
-  DevToolsShim._maybeInitializeDevTools = () => {
+  DevToolsShim.initDevTools = () => {
     // Next call to restoreDevToolsSession is expected to initialize DevTools, which we
     // simulate here by registering our mock.
     DevToolsShim.register(mock);
-    return true;
   };
 
   let scratchpadSessions = [{}];
@@ -162,7 +165,9 @@ function test_scratchpad_apis() {
   DevToolsShim.saveDevToolsSession({});
   checkCalls(mock, "saveDevToolsSession", 1, []);
 
-  DevToolsShim._maybeInitializeDevTools = backupMaybeInitializeDevTools;
+  // Restore initial backups.
+  DevToolsShim.initDevTools = initDevToolsBackup;
+  Services.prefs.setBoolPref("devtools.enabled", devtoolsEnabledValue);
 }
 
 function run_test() {
@@ -178,7 +183,7 @@ function run_test() {
   test_off_called_before_with_bad_callback();
   DevToolsShim.unregister();
 
-  test_scratchpad_apis();
+  test_restore_session_apis();
   DevToolsShim.unregister();
 
   test_events();