Bug 1412050 - session restore should not initialize devtools when unnecessary;r=bgrins draft
authorJulian Descottes <jdescottes@mozilla.com>
Fri, 27 Oct 2017 10:13:27 +0200
changeset 687444 137ffca2e8ebdee6056e6746f54afa3e0c4539aa
parent 687443 533d576fc094290e89feb7a4feb77b83054d458c
child 687703 c6f40181110067d81e016b477f6d04a6b9fa3210
push id86510
push userjdescottes@mozilla.com
push dateFri, 27 Oct 2017 08:48:05 +0000
reviewersbgrins
bugs1412050
milestone58.0a1
Bug 1412050 - session restore should not initialize devtools when unnecessary;r=bgrins MozReview-Commit-ID: BEQBZsXREoQ
devtools/shim/DevToolsShim.jsm
devtools/shim/tests/unit/test_devtools_shim.js
--- a/devtools/shim/DevToolsShim.jsm
+++ b/devtools/shim/DevToolsShim.jsm
@@ -148,16 +148,23 @@ this.DevToolsShim = {
    * 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) {
     if (!this.isEnabled()) {
       return;
     }
 
+    let {scratchpads, browserConsole} = session;
+    let hasDevToolsData = browserConsole || (scratchpads && scratchpads.length);
+    if (!hasDevToolsData) {
+      // Do not initialize DevTools unless there is DevTools specific data in the session.
+      return;
+    }
+
     this.initDevTools();
     this._gDevTools.restoreDevToolsSession(session);
   },
 
   /**
    * Called from nsContextMenu.js in mozilla-central when using the Inspect Element
    * context menu item.
    *
--- a/devtools/shim/tests/unit/test_devtools_shim.js
+++ b/devtools/shim/tests/unit/test_devtools_shim.js
@@ -129,41 +129,49 @@ function test_events() {
   checkCalls(mock, "emit", 2, ["devtools-unregistered"]);
 }
 
 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");
 
+  // Create fake session objects to restore.
+  let sessionWithoutDevTools = {};
+  let sessionWithDevTools = {
+    scratchpads: [{}],
+    browserConsole: true,
+  };
+
   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
+  // Check that save & restore DevToolsSession don't initialize the tools and don't crash.
   DevToolsShim.saveDevToolsSession({});
-  DevToolsShim.restoreDevToolsSession({
-    scratchpads: [{}],
-    browserConsole: true,
-  });
+  DevToolsShim.restoreDevToolsSession(sessionWithDevTools);
 
   Services.prefs.setBoolPref("devtools.enabled", true);
   ok(DevToolsShim.isEnabled(), "DevTools are enabled");
   ok(!DevToolsShim.isInitialized(), "DevTools are not initialized");
 
+  // Check that DevTools are not initialized when calling restoreDevToolsSession without
+  // DevTools related data.
+  DevToolsShim.restoreDevToolsSession(sessionWithoutDevTools);
+  ok(!DevToolsShim.isInitialized(), "DevTools are still not initialized");
+
   let mock = createMockDevTools();
   DevToolsShim.initDevTools = () => {
     // Next call to restoreDevToolsSession is expected to initialize DevTools, which we
     // simulate here by registering our mock.
     DevToolsShim.register(mock);
   };
 
-  let scratchpadSessions = [{}];
-  DevToolsShim.restoreDevToolsSession(scratchpadSessions);
-  checkCalls(mock, "restoreDevToolsSession", 1, [scratchpadSessions]);
+  DevToolsShim.restoreDevToolsSession(sessionWithDevTools);
+  checkCalls(mock, "restoreDevToolsSession", 1, [sessionWithDevTools]);
 
   ok(DevToolsShim.isInitialized(), "DevTools are initialized");
 
   DevToolsShim.saveDevToolsSession({});
   checkCalls(mock, "saveDevToolsSession", 1, []);
 
   // Restore initial backups.
   DevToolsShim.initDevTools = initDevToolsBackup;