Bug 1388552 - Save Browser Console state in session store;r=mikedeboer,r=nchevobbe draft
authorBrian Grinstead <bgrinstead@mozilla.com>
Fri, 11 Aug 2017 09:16:59 -0700
changeset 644975 7f2f3300584d01c9807c159438e2f0668b990143
parent 644974 83612fd2c4edfde5c86cfc11a70682cc74ebfa12
child 725768 27a905cc92afd6c830903094511806c549132803
push id73609
push userbgrinstead@mozilla.com
push dateFri, 11 Aug 2017 16:29:30 +0000
reviewersmikedeboer, nchevobbe
bugs1388552
milestone57.0a1
Bug 1388552 - Save Browser Console state in session store;r=mikedeboer,r=nchevobbe This evolves restoreScratchpadSession into restoreDevToolsSession which can keep track of more than just scratchpad windows. In this case we also restore the Browser Console. MozReview-Commit-ID: D4vOGkpq8xH
browser/components/sessionstore/SessionStore.jsm
devtools/client/framework/devtools.js
devtools/client/scratchpad/scratchpad-manager.jsm
devtools/client/webconsole/hudservice.js
devtools/client/webconsole/test/browser.ini
devtools/client/webconsole/test/browser_console.js
devtools/client/webconsole/test/browser_console_restore.js
devtools/client/webconsole/test/head.js
devtools/shim/DevToolsShim.jsm
devtools/shim/tests/unit/test_devtools_shim.js
--- a/browser/components/sessionstore/SessionStore.jsm
+++ b/browser/components/sessionstore/SessionStore.jsm
@@ -2760,19 +2760,17 @@ var SessionStoreInternal = {
 
     // Merge closed windows from this session with ones from last session
     if (lastSessionState._closedWindows) {
       this._closedWindows = this._closedWindows.concat(lastSessionState._closedWindows);
       this._capClosedWindows();
       this._closedObjectsChanged = true;
     }
 
-    if (lastSessionState.scratchpads) {
-      DevToolsShim.restoreScratchpadSession(lastSessionState.scratchpads);
-    }
+    DevToolsShim.restoreDevToolsSession(lastSessionState);
 
     // Set data that persists between sessions
     this._recentCrashes = lastSessionState.session &&
                           lastSessionState.session.recentCrashes || 0;
 
     // Update the session start time using the restored session state.
     this._updateSessionStartTime(lastSessionState);
 
@@ -3129,20 +3127,17 @@ var SessionStoreInternal = {
       _closedWindows: lastClosedWindowsCopy,
       session,
       global: this._globalState.getState()
     };
 
     // Collect and store session cookies.
     state.cookies = SessionCookies.collect();
 
-    let scratchpads = DevToolsShim.getOpenedScratchpads();
-    if (scratchpads && scratchpads.length) {
-      state.scratchpads = scratchpads;
-    }
+    DevToolsShim.saveDevToolsSession(state);
 
     // Persist the last session if we deferred restoring it
     if (LastSession.canRestore) {
       state.lastSessionState = LastSession.getState();
     }
 
     // If we were called by the SessionSaver and started with only a private
     // window we want to pass the deferred initial state to not lose the
@@ -3525,19 +3520,17 @@ var SessionStoreInternal = {
         if (w == root.selectedWindow - 1) {
           this.windowToFocus = window;
         }
       }
     }
 
     this.restoreWindow(aWindow, root.windows[0], aOptions);
 
-    if (aState.scratchpads) {
-      DevToolsShim.restoreScratchpadSession(aState.scratchpads);
-    }
+    DevToolsShim.restoreDevToolsSession(aState);
   },
 
   /**
    * Manage history restoration for a window
    * @param aWindow
    *        Window to restore the tabs into
    * @param aTabs
    *        Array of tab references
--- a/devtools/client/framework/devtools.js
+++ b/devtools/client/framework/devtools.js
@@ -9,16 +9,17 @@ const Services = require("Services");
 
 const {DevToolsShim} = Cu.import("chrome://devtools-shim/content/DevToolsShim.jsm", {});
 
 // Load gDevToolsBrowser toolbox lazily as they need gDevTools to be fully initialized
 loader.lazyRequireGetter(this, "TargetFactory", "devtools/client/framework/target", true);
 loader.lazyRequireGetter(this, "Toolbox", "devtools/client/framework/toolbox", true);
 loader.lazyRequireGetter(this, "ToolboxHostManager", "devtools/client/framework/toolbox-host-manager", true);
 loader.lazyRequireGetter(this, "gDevToolsBrowser", "devtools/client/framework/devtools-browser", true);
+loader.lazyRequireGetter(this, "HUDService", "devtools/client/webconsole/hudservice", true);
 loader.lazyImporter(this, "ScratchpadManager", "resource://devtools/client/scratchpad/scratchpad-manager.jsm");
 
 // Dependencies required for addon sdk compatibility layer.
 loader.lazyRequireGetter(this, "DebuggerServer", "devtools/server/main", true);
 loader.lazyRequireGetter(this, "DebuggerClient", "devtools/shared/client/main", true);
 loader.lazyImporter(this, "BrowserToolboxProcess", "resource://devtools/client/framework/ToolboxProcess.jsm");
 
 const {defaultTools: DefaultTools, defaultThemes: DefaultThemes} =
@@ -392,34 +393,42 @@ DevTools.prototype = {
         definitions.push(definition);
       }
     }
 
     return definitions.sort(this.ordinalSort);
   },
 
   /**
-   * Get the array of currently opened scratchpad windows.
+   * Called from SessionStore.jsm in mozilla-central when saving the current state.
    *
-   * @return {Array} array of currently opened scratchpad windows.
-   *         Empty array if the scratchpad manager is not loaded.
+   * @param {Object} state
+   *                 A SessionStore state object that gets modified by reference
    */
-  getOpenedScratchpads: function () {
+  saveDevToolsSession: function (state) {
+    state.browserConsole = HUDService.getBrowserConsoleSessionState();
+
     // Check if the module is loaded to avoid loading ScratchpadManager for no reason.
-    if (!Cu.isModuleLoaded("resource://devtools/client/scratchpad/scratchpad-manager.jsm")) {
-      return [];
+    state.scratchpads = [];
+    if (Cu.isModuleLoaded("resource://devtools/client/scratchpad/scratchpad-manager.jsm")) {
+      state.scratchpads = ScratchpadManager.getSessionState();
     }
-    return ScratchpadManager.getSessionState();
   },
 
   /**
-   * Restore the provided array of scratchpad window states.
+   * Restore the devtools session state as provided by SessionStore.
    */
-  restoreScratchpadSession: function (scratchpads) {
-    ScratchpadManager.restoreSession(scratchpads);
+  restoreDevToolsSession: function ({scratchpads, browserConsole}) {
+    if (scratchpads) {
+      ScratchpadManager.restoreSession(scratchpads);
+    }
+
+    if (browserConsole && !HUDService.getBrowserConsole()) {
+      HUDService.toggleBrowserConsole();
+    }
   },
 
   /**
    * Show a Toolbox for a target (either by creating a new one, or if a toolbox
    * already exists for the target, by bring to the front the existing one)
    * If |toolId| is specified then the displayed toolbox will have the
    * specified tool selected.
    * If |hostType| is specified then the toolbox will be displayed using the
--- a/devtools/client/scratchpad/scratchpad-manager.jsm
+++ b/devtools/client/scratchpad/scratchpad-manager.jsm
@@ -153,33 +153,29 @@ this.ScratchpadManager = {
 
 
 /**
  * The ShutdownObserver listens for app shutdown and saves the current state
  * of the scratchpads for session restore.
  */
 var ShutdownObserver = {
   _initialized: false,
-
-  init: function SDO_init()
-  {
+  init() {
     if (this._initialized) {
       return;
     }
 
     Services.obs.addObserver(this, "quit-application-granted");
 
     this._initialized = true;
   },
 
-  observe: function SDO_observe(aMessage, aTopic, aData)
-  {
-    if (aTopic == "quit-application-granted") {
+  observe(message, topic) {
+    if (topic == "quit-application-granted") {
       ScratchpadManager.saveOpenWindows();
       this.uninit();
     }
   },
 
-  uninit: function SDO_uninit()
-  {
+  uninit() {
     Services.obs.removeObserver(this, "quit-application-granted");
   }
 };
--- a/devtools/client/webconsole/hudservice.js
+++ b/devtools/client/webconsole/hudservice.js
@@ -43,16 +43,24 @@ HUD_SERVICE.prototype =
   _browserConsoleDefer: null,
 
   /**
    * Keeps a reference for each Web Console / Browser Console that is created.
    * @type Map
    */
   consoles: null,
 
+  _browerConsoleSessionState: false,
+  storeBrowserConsoleSessionState() {
+    this._browerConsoleSessionState = !!this.getBrowserConsole();
+  },
+  getBrowserConsoleSessionState() {
+    return this._browerConsoleSessionState;
+  },
+
   /**
    * Assign a function to this property to listen for every request that
    * completes. Used by unit tests. The callback takes one argument: the HTTP
    * activity object as received from the remote Web Console.
    *
    * @type object
    *       Includes a property named |callback|. Assign the function to the
    *       |callback| property of this object.
@@ -633,16 +641,19 @@ BrowserConsole.prototype = extend(WebCon
    *         A promise for the initialization.
    */
   init: function BC_init()
   {
     if (this._bc_init) {
       return this._bc_init;
     }
 
+    // Only add the shutdown observer if we've opened a Browser Console window.
+    ShutdownObserver.init();
+
     this.ui._filterPrefsPrefix = BROWSER_CONSOLE_FILTER_PREFS_PREFIX;
 
     let window = this.iframeWindow;
 
     // Make sure that the closing of the Browser Console window destroys this
     // instance.
     let onClose = () => {
       window.removeEventListener("unload", onClose);
@@ -690,8 +701,36 @@ BrowserConsole.prototype = extend(WebCon
       }));
 
     return this._bc_destroyer.promise;
   },
 });
 
 const HUDService = new HUD_SERVICE();
 exports.HUDService = HUDService;
+
+/**
+ * The ShutdownObserver listens for app shutdown and saves the current state
+ * of the Browser Console for session restore.
+ */
+var ShutdownObserver = {
+  _initialized: false,
+  init() {
+    if (this._initialized) {
+      return;
+    }
+
+    Services.obs.addObserver(this, "quit-application-granted");
+
+    this._initialized = true;
+  },
+
+  observe(message, topic) {
+    if (topic == "quit-application-granted") {
+      HUDService.storeBrowserConsoleSessionState();
+      this.uninit();
+    }
+  },
+
+  uninit() {
+    Services.obs.removeObserver(this, "quit-application-granted");
+  }
+};
--- a/devtools/client/webconsole/test/browser.ini
+++ b/devtools/client/webconsole/test/browser.ini
@@ -180,16 +180,17 @@ skip-if = (os == 'linux' && bits == 32 &
 [browser_console_log_inspectable_object.js]
 [browser_console_native_getters.js]
 [browser_console_navigation_marker.js]
 [browser_console_netlogging.js]
 [browser_console_nsiconsolemessage.js]
 [browser_console_optimized_out_vars.js]
 [browser_console_private_browsing.js]
 skip-if = e10s # Bug 1042253 - webconsole e10s tests
+[browser_console_restore.js]
 [browser_console_server_logging.js]
 [browser_console_variables_view.js]
 [browser_console_variables_view_filter.js]
 [browser_console_variables_view_dom_nodes.js]
 [browser_console_variables_view_dont_sort_non_sortable_classes_properties.js]
 [browser_console_variables_view_special_names.js]
 [browser_console_variables_view_while_debugging.js]
 [browser_console_variables_view_while_debugging_and_inspecting.js]
--- a/devtools/client/webconsole/test/browser_console.js
+++ b/devtools/client/webconsole/test/browser_console.js
@@ -17,17 +17,17 @@ const TEST_XHR_ERROR_URI = `http://examp
 const TEST_IMAGE = "http://example.com/browser/devtools/client/webconsole/" +
                    "test/test-image.png";
 
 const {ObjectClient} = require("devtools/shared/client/main");
 
 add_task(function* () {
   yield loadTab(TEST_URI);
 
-  let opened = waitForConsole();
+  let opened = waitForBrowserConsole();
 
   let hud = HUDService.getBrowserConsole();
   ok(!hud, "browser console is not open");
   info("wait for the browser console to open with ctrl-shift-j");
   EventUtils.synthesizeKey("j", { accelKey: true, shiftKey: true }, window);
 
   hud = yield opened;
   ok(hud, "browser console opened");
@@ -193,25 +193,8 @@ function* testCPOWInspection(hud) {
   // Before the fix for Bug 1382833, this wouldn't resolve due to a CPOW error
   // in the ObjectActor.
   let prototypeAndProperties = yield objectClient.getPrototypeAndProperties();
 
   // Just a sanity check to make sure a valid packet came back
   is(prototypeAndProperties.prototype.class, "XBL prototype JSClass",
     "Looks like a valid response");
 }
-
-function waitForConsole() {
-  let deferred = defer();
-
-  Services.obs.addObserver(function observer(aSubject) {
-    Services.obs.removeObserver(observer, "web-console-created");
-    aSubject.QueryInterface(Ci.nsISupportsString);
-
-    let hud = HUDService.getBrowserConsole();
-    ok(hud, "browser console is open");
-    is(aSubject.data, hud.hudId, "notification hudId is correct");
-
-    executeSoon(() => deferred.resolve(hud));
-  }, "web-console-created");
-
-  return deferred.promise;
-}
new file mode 100644
--- /dev/null
+++ b/devtools/client/webconsole/test/browser_console_restore.js
@@ -0,0 +1,32 @@
+/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim: set ft=javascript ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Check that the browser console gets session state is set correctly, and that
+// it re-opens when restore is requested.
+
+"use strict";
+
+add_task(async function() {
+  is(HUDService.getBrowserConsoleSessionState(), false, "Session state false by default");
+  HUDService.storeBrowserConsoleSessionState();
+  is(HUDService.getBrowserConsoleSessionState(), false,
+    "Session state still not true even after setting (since Browser Console is closed)");
+
+  await HUDService.toggleBrowserConsole();
+  HUDService.storeBrowserConsoleSessionState();
+  is(HUDService.getBrowserConsoleSessionState(), true,
+    "Session state true (since Browser Console is opened)");
+
+  info("Closing the browser console and waiting for the session restore to reopen it")
+  await HUDService.toggleBrowserConsole();
+
+  let opened = waitForBrowserConsole();
+  gDevTools.restoreDevToolsSession({
+    browserConsole: true
+  });
+
+  info("Waiting for the console to open after session restore")
+  await opened;
+});
--- a/devtools/client/webconsole/test/head.js
+++ b/devtools/client/webconsole/test/head.js
@@ -1802,8 +1802,23 @@ function simulateMessageLinkClick(elemen
 function getRenderedSource(root) {
   let location = root.querySelector(".message-location .frame-link");
   return location ? {
     url: location.getAttribute("data-url"),
     line: location.getAttribute("data-line"),
     column: location.getAttribute("data-column"),
   } : null;
 }
+
+function waitForBrowserConsole() {
+  return new Promise(resolve => {
+    Services.obs.addObserver(function observer(subject) {
+      Services.obs.removeObserver(observer, "web-console-created");
+      subject.QueryInterface(Ci.nsISupportsString);
+
+      let hud = HUDService.getBrowserConsole();
+      ok(hud, "browser console is open");
+      is(subject.data, hud.hudId, "notification hudId is correct");
+
+      executeSoon(() => resolve(hud));
+    }, "web-console-created");
+  });
+}
--- a/devtools/shim/DevToolsShim.jsm
+++ b/devtools/shim/DevToolsShim.jsm
@@ -184,37 +184,36 @@ this.DevToolsShim = {
     } else {
       removeItem(this.themes, t => t === theme);
     }
   },
 
   /**
    * Called from SessionStore.jsm in mozilla-central when saving the current state.
    *
-   * @return {Array} array of currently opened scratchpad windows. Empty array if devtools
-   *         are not installed
+   * @param {Object} state
+   *                 A SessionStore state object that gets modified by reference
    */
-  getOpenedScratchpads: function () {
+  saveDevToolsSession: function (state) {
     if (!this.isInitialized()) {
-      return [];
+      return;
     }
-
-    return this._gDevTools.getOpenedScratchpads();
+    this._gDevTools.saveDevToolsSession(state);
   },
 
   /**
    * Called from SessionStore.jsm in mozilla-central when restoring a state that contained
-   * opened scratchpad windows.
+   * opened scratchpad windows and browser console.
    */
-  restoreScratchpadSession: function (scratchpads) {
+  restoreDevToolsSession: function (session) {
     if (!this.isInstalled()) {
       return;
     }
 
-    this.gDevTools.restoreScratchpadSession(scratchpads);
+    this.gDevTools.restoreDevToolsSession(session);
   },
 
   /**
    * Called from nsContextMenu.js in mozilla-central when using the Inspect Element
    * context menu item.
    *
    * @param {XULTab} tab
    *        The browser tab on which inspect node was used.
--- a/devtools/shim/tests/unit/test_devtools_shim.js
+++ b/devtools/shim/tests/unit/test_devtools_shim.js
@@ -17,18 +17,18 @@ function createMockDevTools() {
   let methods = [
     "on",
     "off",
     "registerTool",
     "registerTheme",
     "unregisterTool",
     "unregisterTheme",
     "emit",
-    "getOpenedScratchpads",
-    "restoreScratchpadSession",
+    "saveDevToolsSession",
+    "restoreDevToolsSession",
   ];
 
   let mock = {
     callLog: {}
   };
 
   for (let method of methods) {
     // Create a stub for method, that only pushes its arguments in the inner callLog
@@ -226,49 +226,51 @@ function test_events() {
 }
 
 function test_scratchpad_apis() {
   mockDevToolsInstalled(false);
 
   ok(!DevToolsShim.isInstalled(), "DevTools are not installed");
   ok(!DevToolsShim.isInitialized(), "DevTools are not initialized");
 
-  // Ensure that getOpenedScratchpads doesn't initialize the tools
-  DevToolsShim.getOpenedScratchpads();
+  // Ensure that saveDevToolsSession doesn't initialize the tools
+  DevToolsShim.saveDevToolsSession({});
 
   ok(!DevToolsShim.isInstalled(), "DevTools are not installed");
   ok(!DevToolsShim.isInitialized(), "DevTools are not initialized");
 
-  // Check that restoreScratchpadSession doesn't crash.
-  DevToolsShim.restoreScratchpadSession([{}]);
+  // Check that restoreDevToolsSession doesn't crash.
+  DevToolsShim.restoreDevToolsSession({
+    scratchpads: [{}],
+    browserConsole: true,
+  });
 
-  let scratchpads = DevToolsShim.getOpenedScratchpads();
-  equal(scratchpads.length, 0,
-      "getOpenedScratchpads returns [] when DevTools are not installed");
+  // Check that saveDevToolsSession doesn't crash.
+  DevToolsShim.saveDevToolsSession({});
 
   mockDevToolsInstalled(true);
 
   ok(DevToolsShim.isInstalled(), "DevTools are installed");
   ok(!DevToolsShim.isInitialized(), "DevTools are not initialized");
 
   let mock = createMockDevTools();
   DevToolsShim._initDevTools = () => {
-    // Next call to restoreScratchpadSession is expected to initialize DevTools, which we
+    // Next call to restoreDevToolsSession is expected to initialize DevTools, which we
     // simulate here by registering our mock.
     DevToolsShim.register(mock);
   };
 
   let scratchpadSessions = [{}];
-  DevToolsShim.restoreScratchpadSession(scratchpadSessions);
-  checkCalls(mock, "restoreScratchpadSession", 1, [scratchpadSessions]);
+  DevToolsShim.restoreDevToolsSession(scratchpadSessions);
+  checkCalls(mock, "restoreDevToolsSession", 1, [scratchpadSessions]);
 
   ok(DevToolsShim.isInitialized(), "DevTools are initialized");
 
-  DevToolsShim.getOpenedScratchpads();
-  checkCalls(mock, "getOpenedScratchpads", 1, []);
+  DevToolsShim.saveDevToolsSession({});
+  checkCalls(mock, "saveDevToolsSession", 1, []);
 
   restoreDevToolsInstalled();
 }
 
 function run_test() {
   test_register_unregister();
   DevToolsShim.unregister();