Bug 1073992 - Keep track of revivable windows separately to allow reviving more windows than the max_undo_windows pref allows r=yoric
authorTim Taubert <ttaubert@mozilla.com>
Sat, 27 Sep 2014 09:34:08 +0200
changeset 232897 c98a31227412b993cdcacd8dcc8282f106c5f5f3
parent 232896 125453eb266c1e2d888c3470c6a3b39cb166a415
child 232898 98daafd906a9f7bd01d9ca71a8a433b56587e237
push id4187
push userbhearsum@mozilla.com
push dateFri, 28 Nov 2014 15:29:12 +0000
treeherdermozilla-beta@f23cc6a30c11 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyoric
bugs1073992
milestone35.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1073992 - Keep track of revivable windows separately to allow reviving more windows than the max_undo_windows pref allows r=yoric
browser/components/sessionstore/RevivableWindows.jsm
browser/components/sessionstore/SessionSaver.jsm
browser/components/sessionstore/SessionStore.jsm
browser/components/sessionstore/moz.build
browser/components/sessionstore/test/browser.ini
browser/components/sessionstore/test/browser_revive_windows.js
new file mode 100644
--- /dev/null
+++ b/browser/components/sessionstore/RevivableWindows.jsm
@@ -0,0 +1,45 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+this.EXPORTED_SYMBOLS = ["RevivableWindows"];
+
+// List of closed windows that we can revive when closing
+// windows in succession until the browser quits.
+let closedWindows = [];
+
+/**
+ * This module keeps track of closed windows that are revivable. On Windows
+ * and Linux we can revive windows before saving to disk - i.e. moving them
+ * from state._closedWindows[] to state.windows[] so that they're opened
+ * automatically on next startup. This feature lets us properly support
+ * closing windows in succession until the browser quits.
+ *
+ * The length of the list is not capped by max_undo_windows unlike
+ * state._closedWindows[].
+ */
+this.RevivableWindows = Object.freeze({
+  // Returns whether there are windows to revive.
+  get isEmpty() {
+    return closedWindows.length == 0;
+  },
+
+  // Add a window to the list.
+  add(winState) {
+#ifndef XP_MACOSX
+    closedWindows.push(winState);
+#endif
+  },
+
+  // Get the list of revivable windows.
+  get() {
+    return [...closedWindows];
+  },
+
+  // Clear the list of revivable windows.
+  clear() {
+    closedWindows.length = 0;
+  }
+});
--- a/browser/components/sessionstore/SessionSaver.jsm
+++ b/browser/components/sessionstore/SessionSaver.jsm
@@ -14,16 +14,18 @@ Cu.import("resource://gre/modules/Timer.
 Cu.import("resource://gre/modules/Services.jsm", this);
 Cu.import("resource://gre/modules/XPCOMUtils.jsm", this);
 Cu.import("resource://gre/modules/TelemetryStopwatch.jsm", this);
 
 XPCOMUtils.defineLazyModuleGetter(this, "console",
   "resource://gre/modules/devtools/Console.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "PrivacyFilter",
   "resource:///modules/sessionstore/PrivacyFilter.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "RevivableWindows",
+  "resource:///modules/sessionstore/RevivableWindows.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "SessionStore",
   "resource:///modules/sessionstore/SessionStore.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "SessionFile",
   "resource:///modules/sessionstore/SessionFile.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "PrivateBrowsingUtils",
   "resource://gre/modules/PrivateBrowsingUtils.jsm");
 
 // Minimal interval between two save operations (in milliseconds).
@@ -200,33 +202,34 @@ let SessionSaverInternal = {
 
     // Make sure that we keep the previous session if we started with a single
     // private window and no non-private windows have been opened, yet.
     if (state.deferredInitialState) {
       state.windows = state.deferredInitialState.windows || [];
       delete state.deferredInitialState;
     }
 
-#ifndef XP_MACOSX
-    // We want to restore closed windows that are marked with _shouldRestore.
-    // We're doing this here because we want to control this only when saving
-    // the file.
-    while (state._closedWindows.length) {
-      let i = state._closedWindows.length - 1;
+    // We want to revive closed windows that have been closed in succession
+    // without any user action in between closing those. This happens here in
+    // the SessionSaver because we only want to revive when saving to disk.
+    // On Mac OS X this list will always be empty.
+    let windowsToRevive = RevivableWindows.get();
+    state.windows.unshift(...windowsToRevive);
+    let revivedWindows = state._closedWindows.splice(0, windowsToRevive.length);
+#ifdef DEBUG
+    // Check that the windows to revive equal the windows
+    // that we removed from the list of closed windows.
+    let match = revivedWindows.every((win, idx) => {
+      return win == windowsToRevive[windowsToRevive.length - 1 - idx];
+    });
 
-      if (!state._closedWindows[i]._shouldRestore) {
-        // We only need to go until _shouldRestore
-        // is falsy since we're going in reverse.
-        break;
-      }
-
-      delete state._closedWindows[i]._shouldRestore;
-      state.windows.unshift(state._closedWindows.pop());
+    if (!match) {
+      throw new Error("SessionStore: revived windows didn't match closed windows");
     }
-#endif
+#endif DEBUG
 
     stopWatchFinish("COLLECT_DATA_MS", "COLLECT_DATA_LONGEST_OP_MS");
     return this._writeState(state);
   },
 
   /**
    * Saves the current session state. Collects data asynchronously and calls
    * _saveState() to collect data again (with a cache hit rate of hopefully
--- a/browser/components/sessionstore/SessionStore.jsm
+++ b/browser/components/sessionstore/SessionStore.jsm
@@ -102,16 +102,18 @@ XPCOMUtils.defineLazyModuleGetter(this, 
   "resource://gre/modules/devtools/Console.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "RecentWindow",
   "resource:///modules/RecentWindow.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "GlobalState",
   "resource:///modules/sessionstore/GlobalState.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "PrivacyFilter",
   "resource:///modules/sessionstore/PrivacyFilter.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "RevivableWindows",
+  "resource:///modules/sessionstore/RevivableWindows.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "RunState",
   "resource:///modules/sessionstore/RunState.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "ScratchpadManager",
   "resource:///modules/devtools/scratchpad-manager.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "SessionSaver",
   "resource:///modules/sessionstore/SessionSaver.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "SessionCookies",
   "resource:///modules/sessionstore/SessionCookies.jsm");
@@ -696,17 +698,19 @@ let SessionStoreInternal = {
         this.onTabHide(win, aEvent.originalTarget);
         break;
       case "TabPinned":
       case "TabUnpinned":
       case "SwapDocShells":
         this.saveStateDelayed(win);
         break;
     }
-    this._clearRestoringWindows();
+
+    // Any event handled here indicates a user action.
+    RevivableWindows.clear();
   },
 
   /**
    * Generate a unique window identifier
    * @return string
    *         A unique string to identify a window
    */
   _generateWindowID: function ssi_generateWindowID() {
@@ -1008,21 +1012,19 @@ let SessionStoreInternal = {
 
       if (isFullyLoaded) {
         winData.title = tabbrowser.selectedBrowser.contentTitle || tabbrowser.selectedTab.label;
         winData.title = this._replaceLoadingTitle(winData.title, tabbrowser,
                                                   tabbrowser.selectedTab);
         SessionCookies.update([winData]);
       }
 
-#ifndef XP_MACOSX
       // Until we decide otherwise elsewhere, this window is part of a series
       // of closing windows to quit.
-      winData._shouldRestore = true;
-#endif
+      RevivableWindows.add(winData);
 
       // Store the window's close date to figure out when each individual tab
       // was closed. This timestamp should allow re-arranging data based on how
       // recently something was closed.
       winData.closedAt = Date.now();
 
       // Save non-private windows if they have at
       // least one saveable tab or are the last window.
@@ -1034,19 +1036,18 @@ let SessionStoreInternal = {
         let hasSaveableTabs = winData.tabs.some(this._shouldSaveTabState);
 
         // When closing windows one after the other until Firefox quits, we
         // will move those closed in series back to the "open windows" bucket
         // before writing to disk. If however there is only a single window
         // with tabs we deem not worth saving then we might end up with a
         // random closed or even a pop-up window re-opened. To prevent that
         // we explicitly allow saving an "empty" window state.
-        let isLastWindow =
-          Object.keys(this._windows).length == 1 &&
-          !this._closedWindows.some(win => win._shouldRestore || false);
+        let numOpenWindows = Object.keys(this._windows).length;
+        let isLastWindow = numOpenWindows == 1 && RevivableWindows.isEmpty;
 
         if (hasSaveableTabs || isLastWindow) {
           // we don't want to save the busy state
           delete winData.busy;
 
           this._closedWindows.unshift(winData);
           this._capClosedWindows();
         }
@@ -1150,27 +1151,28 @@ let SessionStoreInternal = {
     // also clear all data about closed tabs and windows
     for (let ix in this._windows) {
       if (ix in openWindows) {
         this._windows[ix]._closedTabs = [];
       } else {
         delete this._windows[ix];
       }
     }
+
     // also clear all data about closed windows
     this._closedWindows = [];
+    RevivableWindows.clear();
+
     // give the tabbrowsers a chance to clear their histories first
     var win = this._getMostRecentBrowserWindow();
     if (win) {
       win.setTimeout(() => SessionSaver.run(), 0);
     } else if (RunState.isRunning) {
       SessionSaver.run();
     }
-
-    this._clearRestoringWindows();
   },
 
   /**
    * On purge of domain data
    * @param aData
    *        String domain data
    */
   onPurgeDomainData: function ssi_onPurgeDomainData(aData) {
@@ -1214,21 +1216,22 @@ let SessionStoreInternal = {
         // some duplication from restoreHistory - make sure we get the correct title
         let activeIndex = (selectedTab.index || selectedTab.entries.length) - 1;
         if (activeIndex >= selectedTab.entries.length)
           activeIndex = selectedTab.entries.length - 1;
         this._closedWindows[ix].title = selectedTab.entries[activeIndex].title;
       }
     }
 
+    // Purging domain data indicates a user action.
+    RevivableWindows.clear();
+
     if (RunState.isRunning) {
       SessionSaver.run();
     }
-
-    this._clearRestoringWindows();
   },
 
   /**
    * On preference change
    * @param aData
    *        String preference changed
    */
   onPrefChange: function ssi_onPrefChange(aData) {
@@ -3342,31 +3345,16 @@ let SessionStoreInternal = {
       normalWindowIndex++;
     if (normalWindowIndex >= this._max_windows_undo)
       spliceTo = normalWindowIndex + 1;
 #endif
     this._closedWindows.splice(spliceTo, this._closedWindows.length);
   },
 
   /**
-   * Clears the set of windows that are "resurrected" before writing to disk to
-   * make closing windows one after the other until shutdown work as expected.
-   *
-   * This function should only be called when we are sure that there has been
-   * a user action that indicates the browser is actively being used and all
-   * windows that have been closed before are not part of a series of closing
-   * windows.
-   */
-  _clearRestoringWindows: function ssi_clearRestoringWindows() {
-    for (let i = 0; i < this._closedWindows.length; i++) {
-      delete this._closedWindows[i]._shouldRestore;
-    }
-  },
-
-  /**
    * Reset state to prepare for a new session state to be restored.
    */
   _resetRestoringState: function ssi_initRestoringState() {
     TabRestoreQueue.reset();
     this._tabsRestoringCount = 0;
   },
 
   /**
--- a/browser/components/sessionstore/moz.build
+++ b/browser/components/sessionstore/moz.build
@@ -41,12 +41,13 @@ EXTRA_JS_MODULES.sessionstore = [
     'SessionWorker.jsm',
     'TabAttributes.jsm',
     'TabState.jsm',
     'TabStateCache.jsm',
     'Utils.jsm',
 ]
 
 EXTRA_PP_JS_MODULES.sessionstore += [
+    'RevivableWindows.jsm',
     'SessionSaver.jsm',
     'SessionStore.jsm',
 ]
 
--- a/browser/components/sessionstore/test/browser.ini
+++ b/browser/components/sessionstore/test/browser.ini
@@ -76,16 +76,17 @@ skip-if = buildapp == 'mulet'
 [browser_frametree.js]
 [browser_frame_history.js]
 [browser_global_store.js]
 [browser_history_cap.js]
 [browser_label_and_icon.js]
 [browser_merge_closed_tabs.js]
 [browser_pageStyle.js]
 [browser_privatetabs.js]
+[browser_revive_windows.js]
 [browser_scrollPositions.js]
 [browser_sessionHistory.js]
 skip-if = e10s
 [browser_sessionStorage.js]
 skip-if = e10s
 [browser_swapDocShells.js]
 skip-if = e10s # See bug 918634
 [browser_telemetry.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/sessionstore/test/browser_revive_windows.js
@@ -0,0 +1,149 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+const IS_MAC = ("nsILocalFileMac" in Ci);
+const URL_PREFIX = "about:mozilla?t=browser_revive_windows&r=";
+const PREF_MAX_UNDO = "browser.sessionstore.max_windows_undo";
+
+const URL_MAIN_WINDOW = URL_PREFIX + Math.random();
+const URL_ADD_WINDOW1 = URL_PREFIX + Math.random();
+const URL_ADD_WINDOW2 = URL_PREFIX + Math.random();
+const URL_CLOSED_WINDOW = URL_PREFIX + Math.random();
+
+add_task(function* setup() {
+  registerCleanupFunction(() => Services.prefs.clearUserPref(PREF_MAX_UNDO));
+});
+
+/**
+ * This test ensure that when closing windows in succession until the browser
+ * quits we are able to revive more windows than we keep around for the
+ * "Undo Close Window" feature.
+ */
+add_task(function* test_revive_windows() {
+  // We can restore a single window max.
+  Services.prefs.setIntPref(PREF_MAX_UNDO, 1);
+
+  // Clear list of closed windows.
+  forgetClosedWindows();
+
+  let windows = [];
+
+  // Create three windows.
+  for (let i = 0; i < 3; i++) {
+    let win = yield promiseNewWindow();
+    windows.push(win);
+
+    let tab = win.gBrowser.addTab("about:mozilla");
+    yield promiseBrowserLoaded(tab.linkedBrowser);
+  }
+
+  // Close all windows.
+  for (let win of windows) {
+    yield promiseWindowClosed(win);
+  }
+
+  is(ss.getClosedWindowCount(), 1, "one window restorable");
+
+  // Save to disk and read.
+  let state = JSON.parse(yield promiseRecoveryFileContents());
+
+  // Check number of windows.
+  if (IS_MAC) {
+    is(state.windows.length, 1, "one open window");
+    is(state._closedWindows.length, 1, "one closed window");
+  } else {
+    is(state.windows.length, 4, "four open windows");
+    is(state._closedWindows.length, 0, "closed windows");
+  }
+});
+
+/**
+ * This test ensures that when closing windows one after the other until the
+ * browser shuts down (on Windows and Linux) we revive closed windows in the
+ * right order.
+ */
+add_task(function* test_revive_windows_order() {
+  // We can restore up to three windows max.
+  Services.prefs.setIntPref(PREF_MAX_UNDO, 3);
+
+  // Clear list of closed windows.
+  forgetClosedWindows();
+
+  let tab = gBrowser.addTab(URL_MAIN_WINDOW);
+  yield promiseBrowserLoaded(tab.linkedBrowser);
+  registerCleanupFunction(() => gBrowser.removeTab(tab));
+
+  let win0 = yield promiseNewWindow();
+  let tab0 = win0.gBrowser.addTab(URL_CLOSED_WINDOW);
+  yield promiseBrowserLoaded(tab0.linkedBrowser);
+
+  yield promiseWindowClosed(win0);
+  let data = ss.getClosedWindowData();
+  ok(data.contains(URL_CLOSED_WINDOW), "window is restorable");
+
+  let win1 = yield promiseNewWindow();
+  let tab1 = win1.gBrowser.addTab(URL_ADD_WINDOW1);
+  yield promiseBrowserLoaded(tab1.linkedBrowser);
+
+  let win2 = yield promiseNewWindow();
+  let tab2 = win2.gBrowser.addTab(URL_ADD_WINDOW2);
+  yield promiseBrowserLoaded(tab2.linkedBrowser);
+
+  // Close both windows so that |win1| will be opened first and would be
+  // behind |win2| that was closed later.
+  yield promiseWindowClosed(win1);
+  yield promiseWindowClosed(win2);
+
+  // Repeat the checks once.
+  for (let i = 0; i < 2; i++) {
+    info(`checking window data, iteration #${i}`);
+    let contents = yield promiseRecoveryFileContents();
+    let {windows, _closedWindows: closedWindows} = JSON.parse(contents);
+
+    if (IS_MAC) {
+      // Check number of windows.
+      is(windows.length, 1, "one open window");
+      is(closedWindows.length, 3, "three closed windows");
+
+      // Check open window.
+      ok(JSON.stringify(windows).contains(URL_MAIN_WINDOW),
+        "open window is correct");
+
+      // Check closed windows.
+      ok(JSON.stringify(closedWindows[0]).contains(URL_ADD_WINDOW2),
+        "correct first additional window");
+      ok(JSON.stringify(closedWindows[1]).contains(URL_ADD_WINDOW1),
+        "correct second additional window");
+      ok(JSON.stringify(closedWindows[2]).contains(URL_CLOSED_WINDOW),
+        "correct main window");
+    } else {
+      // Check number of windows.
+      is(windows.length, 3, "three open windows");
+      is(closedWindows.length, 1, "one closed window");
+
+      // Check closed window.
+      ok(JSON.stringify(closedWindows).contains(URL_CLOSED_WINDOW),
+        "closed window is correct");
+
+      // Check that windows are in the right order.
+      ok(JSON.stringify(windows[0]).contains(URL_ADD_WINDOW1),
+        "correct first additional window");
+      ok(JSON.stringify(windows[1]).contains(URL_ADD_WINDOW2),
+        "correct second additional window");
+      ok(JSON.stringify(windows[2]).contains(URL_MAIN_WINDOW),
+        "correct main window");
+    }
+  }
+});
+
+function promiseNewWindow() {
+  return new Promise(resolve => whenNewWindowLoaded({private: false}, resolve));
+}
+
+function forgetClosedWindows() {
+  while (ss.getClosedWindowCount()) {
+    ss.forgetClosedWindow(0);
+  }
+}