From 39a6f12a13bdb5e4439ad1b957fb4eb782f72843 Mon Sep 17 00:00:00 2001 From: Tim Taubert Date: Tue, 21 May 2013 15:02:46 +0200 Subject: [PATCH] Bug 867142 - Remove browser.__SS_restoreState and use a WeakMap instead; r=yoric --- .../sessionstore/src/SessionStore.jsm | 103 +++++++++++++----- .../test/browser_595601-restore_hidden.js | 6 +- .../sessionstore/test/browser_607016.js | 18 +-- .../sessionstore/test/browser_636279.js | 56 +--------- .../sessionstore/test/browser_739805.js | 2 +- browser/components/sessionstore/test/head.js | 9 +- .../tabview/test/browser_tabview_bug595601.js | 12 +- browser/components/tabview/test/head.js | 9 +- 8 files changed, 98 insertions(+), 117 deletions(-) diff --git a/browser/components/sessionstore/src/SessionStore.jsm b/browser/components/sessionstore/src/SessionStore.jsm index 86c80b6e99c9..d1dc51247259 100644 --- a/browser/components/sessionstore/src/SessionStore.jsm +++ b/browser/components/sessionstore/src/SessionStore.jsm @@ -16,9 +16,6 @@ const STATE_QUITTING = -1; const STATE_STOPPED_STR = "stopped"; const STATE_RUNNING_STR = "running"; -const TAB_STATE_NEEDS_RESTORE = 1; -const TAB_STATE_RESTORING = 2; - const PRIVACY_NONE = 0; const PRIVACY_ENCRYPTED = 1; const PRIVACY_FULL = 2; @@ -235,6 +232,29 @@ this.SessionStore = { checkPrivacyLevel: function ss_checkPrivacyLevel(aIsHTTPS, aUseDefaultPref) { return SessionStoreInternal.checkPrivacyLevel(aIsHTTPS, aUseDefaultPref); + }, + + /** + * Returns whether a given browser is waiting to be restored. That means its + * history and state is ready but we wait until it's higher up in the priority + * queue or until it's made visible (if restore_on_demand=true). + * + * @param aBrowser Browser reference + * @returns bool + */ + isTabStateNeedsRestore: function ss_isTabStateNeedsRestore(aBrowser) { + return TabRestoreStates.isNeedsRestore(aBrowser); + }, + + /** + * Returns whether a given browser is currently restoring, i.e. we wait for + * the actual page to load and will restore form data when it's finished. + * + * @param aBrowser Browser reference + * @returns bool + */ + isTabStateRestoring: function ss_isTabStateRestoring(aBrowser) { + return TabRestoreStates.isRestoring(aBrowser); } }; @@ -1057,7 +1077,7 @@ let SessionStoreInternal = { RestoringTabsData.remove(aTab.linkedBrowser); delete aTab.linkedBrowser.__SS_formDataSaved; delete aTab.linkedBrowser.__SS_hostSchemeData; - if (aTab.linkedBrowser.__SS_restoreState) + if (TabRestoreStates.has(aTab.linkedBrowser)) this._resetTabRestoringState(aTab); }); openWindows[aWindow.__SSi] = true; @@ -1252,10 +1272,10 @@ let SessionStoreInternal = { // If this tab was in the middle of restoring or still needs to be restored, // we need to reset that state. If the tab was restoring, we will attempt to // restore the next tab. - let previousState = browser.__SS_restoreState; - if (previousState) { + if (TabRestoreStates.has(browser)) { + let wasRestoring = TabRestoreStates.isRestoring(browser); this._resetTabRestoringState(aTab); - if (previousState == TAB_STATE_RESTORING) + if (wasRestoring) this.restoreNextTab(); } @@ -1317,8 +1337,7 @@ let SessionStoreInternal = { // following "load" is too late for deleting the data caches) // It's possible to get a load event after calling stop on a browser (when // overwriting tabs). We want to return early if the tab hasn't been restored yet. - if (aBrowser.__SS_restoreState && - aBrowser.__SS_restoreState == TAB_STATE_NEEDS_RESTORE) { + if (TabRestoreStates.isNeedsRestore(aBrowser)) { return; } @@ -1354,11 +1373,8 @@ let SessionStoreInternal = { this._windows[aWindow.__SSi].selected = aWindow.gBrowser.tabContainer.selectedIndex; let tab = aWindow.gBrowser.selectedTab; - // If __SS_restoreState is still on the browser and it is - // TAB_STATE_NEEDS_RESTORE, then then we haven't restored - // this tab yet. Explicitly call restoreTab to kick off the restore. - if (tab.linkedBrowser.__SS_restoreState && - tab.linkedBrowser.__SS_restoreState == TAB_STATE_NEEDS_RESTORE) + // Explicitly call restoreTab() to to restore the tab if we need to. + if (TabRestoreStates.isNeedsRestore(tab.linkedBrowser)) this.restoreTab(tab); // attempt to update the current URL we send in a crash report @@ -1368,8 +1384,7 @@ let SessionStoreInternal = { onTabShow: function ssi_onTabShow(aWindow, aTab) { // If the tab hasn't been restored yet, move it into the right bucket - if (aTab.linkedBrowser.__SS_restoreState && - aTab.linkedBrowser.__SS_restoreState == TAB_STATE_NEEDS_RESTORE) { + if (TabRestoreStates.isNeedsRestore(aTab.linkedBrowser)) { TabRestoreQueue.hiddenToVisible(aTab); // let's kick off tab restoration again to ensure this tab gets restored @@ -1384,8 +1399,7 @@ let SessionStoreInternal = { onTabHide: function ssi_onTabHide(aWindow, aTab) { // If the tab hasn't been restored yet, move it into the right bucket - if (aTab.linkedBrowser.__SS_restoreState && - aTab.linkedBrowser.__SS_restoreState == TAB_STATE_NEEDS_RESTORE) { + if (TabRestoreStates.isNeedsRestore(aTab.linkedBrowser)) { TabRestoreQueue.visibleToHidden(aTab); } @@ -2756,7 +2770,7 @@ let SessionStoreInternal = { // state (in restoreHistoryPrecursor). if (aOverwriteTabs) { for (let i = 0; i < tabbrowser.tabs.length; i++) { - if (tabbrowser.browsers[i].__SS_restoreState) + if (TabRestoreStates.has(tabbrowser.browsers[i])) this._resetTabRestoringState(tabbrowser.tabs[i]); } } @@ -2986,7 +3000,7 @@ let SessionStoreInternal = { // keep the data around to prevent dataloss in case // a tab gets closed before it's been properly restored RestoringTabsData.set(browser, tabData); - browser.__SS_restoreState = TAB_STATE_NEEDS_RESTORE; + TabRestoreStates.setNeedsRestore(browser); browser.setAttribute("pending", "true"); tab.setAttribute("pending", "true"); @@ -3176,7 +3190,7 @@ let SessionStoreInternal = { this._tabsRestoringCount++; // Set this tab's state to restoring - browser.__SS_restoreState = TAB_STATE_RESTORING; + TabRestoreStates.setIsRestoring(browser); browser.removeAttribute("pending"); aTab.removeAttribute("pending"); @@ -4398,10 +4412,11 @@ let SessionStoreInternal = { let browser = aTab.linkedBrowser; // Keep the tab's previous state for later in this method - let previousState = browser.__SS_restoreState; + let wasRestoring = TabRestoreStates.isRestoring(browser); + let wasNeedsRestore = TabRestoreStates.isNeedsRestore(browser); // The browser is no longer in any sort of restoring state. - delete browser.__SS_restoreState; + TabRestoreStates.remove(browser); aTab.removeAttribute("pending"); browser.removeAttribute("pending"); @@ -4413,11 +4428,11 @@ let SessionStoreInternal = { // Remove the progress listener if we should. this._removeTabsProgressListener(window); - if (previousState == TAB_STATE_RESTORING) { + if (wasRestoring) { if (this._tabsRestoringCount) this._tabsRestoringCount--; } - else if (previousState == TAB_STATE_NEEDS_RESTORE) { + else if (wasNeedsRestore) { // Make sure the session history listener is removed. This is normally // done in restoreTab, but this tab is being removed before that gets called. this._removeSHistoryListener(aTab); @@ -4670,6 +4685,41 @@ let TabAttributes = { } }; +// A map keeping track of all tab restore states. A tab might be 'needs-restore' +// if it waits until the restoration process is kicked off. This might start +// when the tab reaches a higher position in the priority queue or when it's +// made visible (when restore_on_demand=true). If a tab is 'restoring' we wait +// for its actual page to load and will then restore form data etc. If has() +// returns false the tab has not been restored from previous data or it has +// already finished restoring and is thus now seen as a valid and complete tab. +let TabRestoreStates = { + _states: new WeakMap(), + + has: function (browser) { + return this._states.has(browser); + }, + + isNeedsRestore: function ss_isNeedsRestore(browser) { + return this._states.get(browser) === "needs-restore"; + }, + + setNeedsRestore: function (browser) { + this._states.set(browser, "needs-restore"); + }, + + isRestoring: function ss_isRestoring(browser) { + return this._states.get(browser) === "restoring"; + }, + + setIsRestoring: function (browser) { + this._states.set(browser, "restoring"); + }, + + remove: function (browser) { + this._states.delete(browser); + } +}; + // This is used to help meter the number of restoring tabs. This is the control // point for telling the next tab to restore. It gets attached to each gBrowser // via gBrowser.addTabsProgressListener @@ -4677,8 +4727,7 @@ let gRestoreTabsProgressListener = { onStateChange: function(aBrowser, aWebProgress, aRequest, aStateFlags, aStatus) { // Ignore state changes on browsers that we've already restored and state // changes that aren't applicable. - if (aBrowser.__SS_restoreState && - aBrowser.__SS_restoreState == TAB_STATE_RESTORING && + if (TabRestoreStates.isRestoring(aBrowser) && aStateFlags & Ci.nsIWebProgressListener.STATE_STOP && aStateFlags & Ci.nsIWebProgressListener.STATE_IS_NETWORK && aStateFlags & Ci.nsIWebProgressListener.STATE_IS_WINDOW) { diff --git a/browser/components/sessionstore/test/browser_595601-restore_hidden.js b/browser/components/sessionstore/test/browser_595601-restore_hidden.js index cff2d9c5d370..73447e1610d7 100644 --- a/browser/components/sessionstore/test/browser_595601-restore_hidden.js +++ b/browser/components/sessionstore/test/browser_595601-restore_hidden.js @@ -73,7 +73,7 @@ let TabsProgressListener = { }, onStateChange: function (aBrowser, aWebProgress, aRequest, aStateFlags, aStatus) { - if (this.callback && aBrowser.__SS_restoreState == TAB_STATE_RESTORING && + if (this.callback && SessionStore.isTabStateRestoring(aBrowser) && aStateFlags & Ci.nsIWebProgressListener.STATE_STOP && aStateFlags & Ci.nsIWebProgressListener.STATE_IS_NETWORK && aStateFlags & Ci.nsIWebProgressListener.STATE_IS_WINDOW) @@ -85,9 +85,9 @@ let TabsProgressListener = { for (let i = 0; i < this.window.gBrowser.tabs.length; i++) { let browser = this.window.gBrowser.tabs[i].linkedBrowser; - if (browser.__SS_restoreState == TAB_STATE_RESTORING) + if (SessionStore.isTabStateRestoring(browser)) isRestoring++; - else if (browser.__SS_restoreState == TAB_STATE_NEEDS_RESTORE) + else if (SessionStore.isTabStateNeedsRestore(browser)) needsRestore++; } diff --git a/browser/components/sessionstore/test/browser_607016.js b/browser/components/sessionstore/test/browser_607016.js index 6534b6e457a4..934465a6fbe5 100644 --- a/browser/components/sessionstore/test/browser_607016.js +++ b/browser/components/sessionstore/test/browser_607016.js @@ -22,17 +22,6 @@ function test() { // any given time. This guarantees that a finishing load won't start another. Services.prefs.setBoolPref("browser.sessionstore.restore_on_demand", true); - // We have our own progress listener for this test, which we'll attach before our state is set - let progressListener = { - onStateChange: function (aBrowser, aWebProgress, aRequest, aStateFlags, aStatus) { - if (aBrowser.__SS_restoreState == TAB_STATE_RESTORING && - aStateFlags & Ci.nsIWebProgressListener.STATE_STOP && - aStateFlags & Ci.nsIWebProgressListener.STATE_IS_NETWORK && - aStateFlags & Ci.nsIWebProgressListener.STATE_IS_WINDOW) - progressCallback(aBrowser); - } - } - let state = { windows: [{ tabs: [ { entries: [{ url: "http://example.org#1" }], extData: { "uniq": r() } }, { entries: [{ url: "http://example.org#2" }], extData: { "uniq": r() } }, // overwriting @@ -42,10 +31,10 @@ function test() { { entries: [{ url: "http://example.org#6" }] } // creating ], selected: 1 }] }; - function progressCallback(aBrowser) { + gProgressListener.setCallback(function progressCallback(aBrowser) { // We'll remove the progress listener after the first one because we aren't // loading any other tabs - window.gBrowser.removeTabsProgressListener(progressListener); + gProgressListener.unsetCallback(); let curState = JSON.parse(ss.getBrowserState()); for (let i = 0; i < curState.windows[0].tabs.length; i++) { @@ -109,9 +98,8 @@ function test() { "(creating) new data is stored in extData where there was none"); cleanup(); - } + }); - window.gBrowser.addTabsProgressListener(progressListener); ss.setBrowserState(JSON.stringify(state)); } diff --git a/browser/components/sessionstore/test/browser_636279.js b/browser/components/sessionstore/test/browser_636279.js index 653199209080..8605c9dd4af0 100644 --- a/browser/components/sessionstore/test/browser_636279.js +++ b/browser/components/sessionstore/test/browser_636279.js @@ -18,19 +18,15 @@ function test() { waitForExplicitFinish(); registerCleanupFunction(function () { - TabsProgressListener.uninit(); ss.setBrowserState(stateBackup); }); - - TabsProgressListener.init(); - window.addEventListener("SSWindowStateReady", function onReady() { window.removeEventListener("SSWindowStateReady", onReady, false); let firstProgress = true; - TabsProgressListener.setCallback(function (needsRestore, isRestoring) { + gProgressListener.setCallback(function (browser, needsRestore, isRestoring) { if (firstProgress) { firstProgress = false; is(isRestoring, 3, "restoring 3 tabs concurrently"); @@ -39,7 +35,7 @@ function test() { } if (0 == needsRestore) { - TabsProgressListener.unsetCallback(); + gProgressListener.unsetCallback(); waitForFocus(finish); } }); @@ -49,51 +45,3 @@ function test() { ss.setBrowserState(JSON.stringify(statePinned)); } - -function countTabs() { - let needsRestore = 0, isRestoring = 0; - let windowsEnum = Services.wm.getEnumerator("navigator:browser"); - - while (windowsEnum.hasMoreElements()) { - let window = windowsEnum.getNext(); - if (window.closed) - continue; - - for (let i = 0; i < window.gBrowser.tabs.length; i++) { - let browser = window.gBrowser.tabs[i].linkedBrowser; - if (browser.__SS_restoreState == TAB_STATE_RESTORING) - isRestoring++; - else if (browser.__SS_restoreState == TAB_STATE_NEEDS_RESTORE) - needsRestore++; - } - } - - return [needsRestore, isRestoring]; -} - -let TabsProgressListener = { - init: function () { - gBrowser.addTabsProgressListener(this); - }, - - uninit: function () { - this.unsetCallback(); - gBrowser.removeTabsProgressListener(this); - }, - - setCallback: function (callback) { - this.callback = callback; - }, - - unsetCallback: function () { - delete this.callback; - }, - - onStateChange: function (aBrowser, aWebProgress, aRequest, aStateFlags, aStatus) { - if (this.callback && aBrowser.__SS_restoreState == TAB_STATE_RESTORING && - aStateFlags & Ci.nsIWebProgressListener.STATE_STOP && - aStateFlags & Ci.nsIWebProgressListener.STATE_IS_NETWORK && - aStateFlags & Ci.nsIWebProgressListener.STATE_IS_WINDOW) - this.callback.apply(null, countTabs()); - } -} diff --git a/browser/components/sessionstore/test/browser_739805.js b/browser/components/sessionstore/test/browser_739805.js index 077916456ee9..43eff87bd2b7 100644 --- a/browser/components/sessionstore/test/browser_739805.js +++ b/browser/components/sessionstore/test/browser_739805.js @@ -22,7 +22,7 @@ function test() { isnot(gBrowser.selectedTab, tab, "newly created tab is not selected"); ss.setTabState(tab, JSON.stringify(tabState)); - is(browser.__SS_restoreState, TAB_STATE_NEEDS_RESTORE, "tab needs restoring"); + ok(SessionStore.isTabStateNeedsRestore(browser), "tab needs restoring"); let state = JSON.parse(ss.getTabState(tab)); let formdata = state.entries[0].formdata; diff --git a/browser/components/sessionstore/test/head.js b/browser/components/sessionstore/test/head.js index 86d741d2d483..4f4e386e49ec 100644 --- a/browser/components/sessionstore/test/head.js +++ b/browser/components/sessionstore/test/head.js @@ -2,9 +2,6 @@ * 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/. */ -const TAB_STATE_NEEDS_RESTORE = 1; -const TAB_STATE_RESTORING = 2; - let tmp = {}; Cu.import("resource:///modules/sessionstore/SessionStore.jsm", tmp); let SessionStore = tmp.SessionStore; @@ -241,7 +238,7 @@ let gProgressListener = { function gProgressListener_onStateChange(aBrowser, aWebProgress, aRequest, aStateFlags, aStatus) { if ((!this._checkRestoreState || - aBrowser.__SS_restoreState == TAB_STATE_RESTORING) && + SessionStore.isTabStateRestoring(aBrowser)) && aStateFlags & Ci.nsIWebProgressListener.STATE_STOP && aStateFlags & Ci.nsIWebProgressListener.STATE_IS_NETWORK && aStateFlags & Ci.nsIWebProgressListener.STATE_IS_WINDOW) { @@ -256,9 +253,9 @@ let gProgressListener = { for (let win in BrowserWindowIterator()) { for (let i = 0; i < win.gBrowser.tabs.length; i++) { let browser = win.gBrowser.tabs[i].linkedBrowser; - if (browser.__SS_restoreState == TAB_STATE_RESTORING) + if (SessionStore.isTabStateRestoring(browser)) isRestoring++; - else if (browser.__SS_restoreState == TAB_STATE_NEEDS_RESTORE) + else if (SessionStore.isTabStateNeedsRestore(browser)) needsRestore++; else wasRestored++; diff --git a/browser/components/tabview/test/browser_tabview_bug595601.js b/browser/components/tabview/test/browser_tabview_bug595601.js index c833a7ac0854..a38d0fd23a96 100644 --- a/browser/components/tabview/test/browser_tabview_bug595601.js +++ b/browser/components/tabview/test/browser_tabview_bug595601.js @@ -3,9 +3,6 @@ let ss = Cc["@mozilla.org/browser/sessionstore;1"].getService(Ci.nsISessionStore); -const TAB_STATE_NEEDS_RESTORE = 1; -const TAB_STATE_RESTORING = 2; - let stateBackup = ss.getBrowserState(); let state = {windows:[{tabs:[ @@ -125,9 +122,9 @@ function countTabs() { for (let i = 0; i < window.gBrowser.tabs.length; i++) { let browser = window.gBrowser.tabs[i].linkedBrowser; - if (browser.__SS_restoreState == TAB_STATE_RESTORING) + if (SessionStore.isTabStateRestoring(browser)) isRestoring++; - else if (browser.__SS_restoreState == TAB_STATE_NEEDS_RESTORE) + else if (SessionStore.isTabStateNeedsRestore(browser)) needsRestore++; } } @@ -168,8 +165,9 @@ let TabsProgressListener = { self.callback.apply(null, countTabs()); }; - let isRestoring = aBrowser.__SS_restoreState == TAB_STATE_RESTORING; - let wasRestoring = !aBrowser.__SS_restoreState && aBrowser.__wasRestoring; + let isRestoring = SessionStore.isTabStateRestoring(aBrowser); + let needsRestore = SessionStore.isTabStateNeedsRestore(aBrowser); + let wasRestoring = !isRestoring && !needsRestore && aBrowser.__wasRestoring; let hasStopped = aStateFlags & Ci.nsIWebProgressListener.STATE_STOP; if (isRestoring && !hasStopped) diff --git a/browser/components/tabview/test/head.js b/browser/components/tabview/test/head.js index ec4e9cd9b04f..cf4f5a178e0f 100644 --- a/browser/components/tabview/test/head.js +++ b/browser/components/tabview/test/head.js @@ -1,6 +1,10 @@ /* Any copyright is dedicated to the Public Domain. http://creativecommons.org/publicdomain/zero/1.0/ */ +let tmp = {}; +Cu.import("resource:///modules/sessionstore/SessionStore.jsm", tmp); +let SessionStore = tmp.SessionStore; + // Some tests here assume that all restored tabs are loaded without waiting for // the user to bring them to the foreground. We ensure this by resetting the // related preference (see the "firefox.js" defaults file for details). @@ -117,8 +121,6 @@ function newWindowWithTabView(shownCallback, loadCallback, width, height) { // ---------- function afterAllTabsLoaded(callback, win) { - const TAB_STATE_NEEDS_RESTORE = 1; - win = win || window; let stillToLoad = 0; @@ -137,8 +139,7 @@ function afterAllTabsLoaded(callback, win) { let browser = tab.linkedBrowser; let isRestorable = !(tab.hidden && !restoreHiddenTabs && - browser.__SS_restoreState && - browser.__SS_restoreState == TAB_STATE_NEEDS_RESTORE); + SessionStore.isTabStateNeedsRestore(browser)); if (isRestorable && browser.contentDocument.readyState != "complete" || browser.webProgress.isLoadingDocument) {