Backed out changeset 44cb72be622d (bug 1081135) a=sylvestre
authorTim Taubert <ttaubert@mozilla.com>
Thu, 16 Oct 2014 15:25:16 +0200
changeset 223445 a7da5ed1451dbac9cbe999c7e21439bd3a2787fd
parent 223444 5d8833bdfccd391340e8f760abf5f403b7125b08
child 223446 94a97b7115cbddb1301614c1fdc82f8c067ec7b6
push id5
push usergszorc@mozilla.com
push dateWed, 29 Oct 2014 02:51:31 +0000
reviewerssylvestre
bugs1081135, 1073992
milestone35.0a2
backs out44cb72be622dc5581a944eab309738232ab104c3
Backed out changeset 44cb72be622d (bug 1081135) a=sylvestre * * * Backed out changeset 82df8ad2c609 (bug 1073992) * * * Backed out changeset c98a31227412 (bug 1073992)
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
deleted file mode 100644
--- a/browser/components/sessionstore/RevivableWindows.jsm
+++ /dev/null
@@ -1,45 +0,0 @@
-/* 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,18 +14,16 @@ 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).
@@ -202,34 +200,33 @@ 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;
     }
 
-    // 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];
-    });
+#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;
 
-    if (!match) {
-      throw new Error("SessionStore: revived windows didn't match closed windows");
+      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());
     }
-#endif DEBUG
+#endif
 
     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,18 +102,16 @@ 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");
@@ -698,19 +696,17 @@ let SessionStoreInternal = {
         this.onTabHide(win, aEvent.originalTarget);
         break;
       case "TabPinned":
       case "TabUnpinned":
       case "SwapDocShells":
         this.saveStateDelayed(win);
         break;
     }
-
-    // Any event handled here indicates a user action.
-    RevivableWindows.clear();
+    this._clearRestoringWindows();
   },
 
   /**
    * Generate a unique window identifier
    * @return string
    *         A unique string to identify a window
    */
   _generateWindowID: function ssi_generateWindowID() {
@@ -1012,16 +1008,22 @@ 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
+
       // 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.
       if (!winData.isPrivate) {
@@ -1032,30 +1034,27 @@ 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 numOpenWindows = Object.keys(this._windows).length;
-        let isLastWindow = numOpenWindows == 1 && RevivableWindows.isEmpty;
+        let isLastWindow =
+          Object.keys(this._windows).length == 1 &&
+          !this._closedWindows.some(win => win._shouldRestore || false);
 
         if (hasSaveableTabs || isLastWindow) {
           // we don't want to save the busy state
           delete winData.busy;
 
           this._closedWindows.unshift(winData);
           this._capClosedWindows();
         }
-
-        // Until we decide otherwise elsewhere, this window
-        // is part of a series of closing windows to quit.
-        RevivableWindows.add(winData);
       }
 
       // clear this window from the list
       delete this._windows[aWindow.__SSi];
 
       // save the state without this window to disk
       this.saveStateDelayed();
     }
@@ -1151,28 +1150,27 @@ 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) {
@@ -1216,22 +1214,21 @@ 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) {
@@ -3345,16 +3342,31 @@ 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,13 +41,12 @@ 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,17 +76,16 @@ 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]
deleted file mode 100644
--- a/browser/components/sessionstore/test/browser_revive_windows.js
+++ /dev/null
@@ -1,160 +0,0 @@
-/* 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);
-  }
-
-  // Create a private window.
-  // This window must not be revived.
-  {
-    let win = yield promiseNewWindow({private: true});
-    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);
-  TabState.flush(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(opts = {private: false}) {
-  return new Promise(resolve => whenNewWindowLoaded(opts, resolve));
-}
-
-function forgetClosedWindows() {
-  while (ss.getClosedWindowCount()) {
-    ss.forgetClosedWindow(0);
-  }
-}