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 209682 c98a31227412b993cdcacd8dcc8282f106c5f5f3
parent 209681 125453eb266c1e2d888c3470c6a3b39cb166a415
child 209683 98daafd906a9f7bd01d9ca71a8a433b56587e237
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersyoric
bugs1073992
milestone35.0a1
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);
+  }
+}