From 1b513e4677c44554b1644f982abda6f9c2362c6d Mon Sep 17 00:00:00 2001 From: Tim Taubert Date: Mon, 13 Oct 2014 14:58:19 +0200 Subject: [PATCH] Backed out changeset c98a31227412 (bug 1073992) --- .../sessionstore/RevivableWindows.jsm | 45 ------ .../components/sessionstore/SessionSaver.jsm | 33 ++-- .../components/sessionstore/SessionStore.jsm | 40 +++-- browser/components/sessionstore/moz.build | 1 - .../components/sessionstore/test/browser.ini | 1 - .../test/browser_revive_windows.js | 149 ------------------ 6 files changed, 41 insertions(+), 228 deletions(-) delete mode 100644 browser/components/sessionstore/RevivableWindows.jsm delete mode 100644 browser/components/sessionstore/test/browser_revive_windows.js diff --git a/browser/components/sessionstore/RevivableWindows.jsm b/browser/components/sessionstore/RevivableWindows.jsm deleted file mode 100644 index 7ccd8023b1b..00000000000 --- 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; - } -}); diff --git a/browser/components/sessionstore/SessionSaver.jsm b/browser/components/sessionstore/SessionSaver.jsm index a087c9f8350..7eabb7863ef 100644 --- a/browser/components/sessionstore/SessionSaver.jsm +++ b/browser/components/sessionstore/SessionSaver.jsm @@ -19,8 +19,6 @@ 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", @@ -207,24 +205,23 @@ let SessionSaverInternal = { 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); diff --git a/browser/components/sessionstore/SessionStore.jsm b/browser/components/sessionstore/SessionStore.jsm index 14d47555335..9336b827c4c 100644 --- a/browser/components/sessionstore/SessionStore.jsm +++ b/browser/components/sessionstore/SessionStore.jsm @@ -107,8 +107,6 @@ 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", @@ -703,9 +701,7 @@ let SessionStoreInternal = { this.saveStateDelayed(win); break; } - - // Any event handled here indicates a user action. - RevivableWindows.clear(); + this._clearRestoringWindows(); }, /** @@ -1017,9 +1013,11 @@ let SessionStoreInternal = { SessionCookies.update([winData]); } +#ifndef XP_MACOSX // Until we decide otherwise elsewhere, this window is part of a series // of closing windows to quit. - RevivableWindows.add(winData); + 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 @@ -1041,8 +1039,9 @@ let SessionStoreInternal = { // 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 @@ -1156,11 +1155,8 @@ let SessionStoreInternal = { 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) { @@ -1168,6 +1164,8 @@ let SessionStoreInternal = { } else if (RunState.isRunning) { SessionSaver.run(); } + + this._clearRestoringWindows(); }, /** @@ -1221,12 +1219,11 @@ let SessionStoreInternal = { } } - // Purging domain data indicates a user action. - RevivableWindows.clear(); - if (RunState.isRunning) { SessionSaver.run(); } + + this._clearRestoringWindows(); }, /** @@ -3349,6 +3346,21 @@ let SessionStoreInternal = { 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. */ diff --git a/browser/components/sessionstore/moz.build b/browser/components/sessionstore/moz.build index ec5d17639d8..723a13489fb 100644 --- a/browser/components/sessionstore/moz.build +++ b/browser/components/sessionstore/moz.build @@ -46,7 +46,6 @@ EXTRA_JS_MODULES.sessionstore = [ ] EXTRA_PP_JS_MODULES.sessionstore += [ - 'RevivableWindows.jsm', 'SessionSaver.jsm', 'SessionStore.jsm', ] diff --git a/browser/components/sessionstore/test/browser.ini b/browser/components/sessionstore/test/browser.ini index 3157078ff7c..5c6966155c8 100644 --- a/browser/components/sessionstore/test/browser.ini +++ b/browser/components/sessionstore/test/browser.ini @@ -81,7 +81,6 @@ skip-if = buildapp == 'mulet' [browser_merge_closed_tabs.js] [browser_pageStyle.js] [browser_privatetabs.js] -[browser_revive_windows.js] [browser_scrollPositions.js] [browser_sessionHistory.js] skip-if = e10s diff --git a/browser/components/sessionstore/test/browser_revive_windows.js b/browser/components/sessionstore/test/browser_revive_windows.js deleted file mode 100644 index 51906552c3e..00000000000 --- a/browser/components/sessionstore/test/browser_revive_windows.js +++ /dev/null @@ -1,149 +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); - } - - // 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); - } -}