Bug 867118 - Remove browser.__SS_data and use a WeakMap instead; r=yoric
authorTim Taubert <ttaubert@mozilla.com>
Fri, 03 May 2013 09:41:00 +0200
changeset 130908 6ddd5fb7f041
parent 130907 e8c1bde62025
child 130909 253e46507c57
push id24638
push userttaubert@mozilla.com
push date2013-05-06 11:30 +0000
treeherdermozilla-central@dae38fc0aeb4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyoric
bugs867118
milestone23.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 867118 - Remove browser.__SS_data and use a WeakMap instead; r=yoric
browser/components/sessionstore/src/SessionStore.jsm
browser/components/sessionstore/test/browser_579868.js
--- a/browser/components/sessionstore/src/SessionStore.jsm
+++ b/browser/components/sessionstore/src/SessionStore.jsm
@@ -1052,18 +1052,18 @@ let SessionStoreInternal = {
     // If the browser is shutting down, simply return after clearing the
     // session data on disk as this notification fires after the
     // quit-application notification so the browser is about to exit.
     if (this._loadState == STATE_QUITTING)
       return;
     this._lastSessionState = null;
     let openWindows = {};
     this._forEachBrowserWindow(function(aWindow) {
-      Array.forEach(aWindow.gBrowser.tabs, function(aTab) {
-        delete aTab.linkedBrowser.__SS_data;
+      Array.forEach(aWindow.gBrowser.tabs, aTab => {
+        RestoringTabsData.remove(aTab.linkedBrowser);
         delete aTab.linkedBrowser.__SS_formDataSaved;
         delete aTab.linkedBrowser.__SS_hostSchemeData;
         if (aTab.linkedBrowser.__SS_restoreState)
           this._resetTabRestoringState(aTab);
       });
       openWindows[aWindow.__SSi] = true;
     });
     // also clear all data about closed tabs and windows
@@ -1244,17 +1244,17 @@ let SessionStoreInternal = {
    */
   onTabRemove: function ssi_onTabRemove(aWindow, aTab, aNoNotification) {
     let browser = aTab.linkedBrowser;
     browser.removeEventListener("load", this, true);
 
     let mm = browser.messageManager;
     MESSAGES.forEach(msg => mm.removeMessageListener(msg, this));
 
-    delete browser.__SS_data;
+    RestoringTabsData.remove(aTab.linkedBrowser);
     delete browser.__SS_formDataSaved;
     delete browser.__SS_hostSchemeData;
 
     // 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) {
@@ -1321,17 +1321,17 @@ 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) {
       return;
     }
 
-    delete aBrowser.__SS_data;
+    RestoringTabsData.remove(aBrowser);
     delete aBrowser.__SS_formDataSaved;
     this.saveStateDelayed(aWindow);
 
     // attempt to update the current URL we send in a crash report
     this._updateCrashReportURL(aWindow);
   },
 
   /**
@@ -1630,52 +1630,58 @@ let SessionStoreInternal = {
         this._windows[aWindow.__SSi].extData[aKey])
       delete this._windows[aWindow.__SSi].extData[aKey];
   },
 
   getTabValue: function ssi_getTabValue(aTab, aKey) {
     let data = {};
     if (aTab.__SS_extdata) {
       data = aTab.__SS_extdata;
-    }
-    else if (aTab.linkedBrowser.__SS_data && aTab.linkedBrowser.__SS_data.extData) {
-      // If the tab hasn't been fully restored, get the data from the to-be-restored data
-      data = aTab.linkedBrowser.__SS_data.extData;
+    } else {
+      let tabData = RestoringTabsData.get(aTab.linkedBrowser);
+      if (tabData && "extData" in tabData) {
+        // If the tab hasn't been fully restored yet,
+        // get the data from the to-be-restored data.
+        data = tabData.extData;
+      }
     }
     return data[aKey] || "";
   },
 
   setTabValue: function ssi_setTabValue(aTab, aKey, aStringValue) {
     // If the tab hasn't been restored, then set the data there, otherwise we
     // could lose newly added data.
     let saveTo;
     if (aTab.__SS_extdata) {
       saveTo = aTab.__SS_extdata;
-    }
-    else if (aTab.linkedBrowser.__SS_data && aTab.linkedBrowser.__SS_data.extData) {
-      saveTo = aTab.linkedBrowser.__SS_data.extData;
-    }
-    else {
-      aTab.__SS_extdata = {};
-      saveTo = aTab.__SS_extdata;
+    } else {
+      let tabData = RestoringTabsData.get(aTab.linkedBrowser);
+      if (tabData && "extData" in tabData) {
+        saveTo = tabData.extData;
+      } else {
+        aTab.__SS_extdata = {};
+        saveTo = aTab.__SS_extdata;
+      }
     }
     saveTo[aKey] = aStringValue;
     this.saveStateDelayed(aTab.ownerDocument.defaultView);
   },
 
   deleteTabValue: function ssi_deleteTabValue(aTab, aKey) {
     // We want to make sure that if data is accessed early, we attempt to delete
-    // that data from __SS_data as well. Otherwise we'll throw in cases where
-    // data can be set or read.
+    // that data from to-be-restored data as well. Otherwise we'll throw in
+    // cases where data can be set or read.
     let deleteFrom;
     if (aTab.__SS_extdata) {
       deleteFrom = aTab.__SS_extdata;
-    }
-    else if (aTab.linkedBrowser.__SS_data && aTab.linkedBrowser.__SS_data.extData) {
-      deleteFrom = aTab.linkedBrowser.__SS_data.extData;
+    } else {
+      let tabData = RestoringTabsData.get(aTab.linkedBrowser);
+      if (tabData && "extData" in tabData) {
+        deleteFrom = tabData.extData;
+      }
     }
 
     if (deleteFrom && deleteFrom[aKey])
       delete deleteFrom[aKey];
   },
 
   persistTabAttribute: function ssi_persistTabAttribute(aName) {
     if (aName in this.xulAttributes)
@@ -1879,19 +1885,19 @@ let SessionStoreInternal = {
    */
   _collectTabData: function ssi_collectTabData(aTab, aFullData) {
     var tabData = { entries: [], lastAccessed: aTab.lastAccessed };
     var browser = aTab.linkedBrowser;
 
     if (!browser || !browser.currentURI)
       // can happen when calling this function right after .addTab()
       return tabData;
-    else if (browser.__SS_data) {
+    else if (RestoringTabsData.has(browser)) {
       // use the data to be restored when the tab hasn't been completely loaded
-      tabData = browser.__SS_data;
+      tabData = RestoringTabsData.get(browser);
       if (aTab.pinned)
         tabData.pinned = true;
       else
         delete tabData.pinned;
       tabData.hidden = aTab.hidden;
 
       // If __SS_extdata is set then we'll use that since it might be newer.
       if (aTab.__SS_extdata)
@@ -2159,17 +2165,17 @@ let SessionStoreInternal = {
    * @param aTabData
    *        tabData object to add the information to
    * @param aFullData
    *        always return privacy sensitive data (use with care)
    */
   _updateTextAndScrollDataForTab:
     function ssi_updateTextAndScrollDataForTab(aWindow, aBrowser, aTabData, aFullData) {
     // we shouldn't update data for incompletely initialized tabs
-    if (aBrowser.__SS_data)
+    if (RestoringTabsData.has(aBrowser))
       return;
 
     var tabIndex = (aTabData.index || aTabData.entries.length) - 1;
     // entry data needn't exist for tabs just initialized with an incomplete session state
     if (!aTabData.entries[tabIndex])
       return;
 
     let selectedPageStyle = aBrowser.markupDocumentViewer.authorStyleDisabled ? "_nostyle" :
@@ -2965,17 +2971,17 @@ let SessionStoreInternal = {
       else
         tabbrowser.showTab(tab);
 
       for (let name in tabData.attributes)
         this.xulAttributes[name] = true;
 
       // keep the data around to prevent dataloss in case
       // a tab gets closed before it's been properly restored
-      browser.__SS_data = tabData;
+      RestoringTabsData.set(browser, tabData);
       browser.__SS_restoreState = TAB_STATE_NEEDS_RESTORE;
       browser.setAttribute("pending", "true");
       tab.setAttribute("pending", "true");
 
       // Make sure that set/getTabValue will set/read the correct data by
       // wiping out any current value in tab.__SS_extdata.
       delete tab.__SS_extdata;
 
@@ -3134,17 +3140,17 @@ let SessionStoreInternal = {
    * @param aTab
    *        the tab to restore
    *
    * @returns true/false indicating whether or not a load actually happened
    */
   restoreTab: function ssi_restoreTab(aTab) {
     let window = aTab.ownerDocument.defaultView;
     let browser = aTab.linkedBrowser;
-    let tabData = browser.__SS_data;
+    let tabData = RestoringTabsData.get(browser);
 
     // There are cases within where we haven't actually started a load. In that
     // that case we'll reset state changes we made and return false to the caller
     // can handle appropriately.
     let didStartLoad = false;
 
     // Make sure that the tabs progress listener is attached to this window
     this._ensureTabsProgressListener(window);
@@ -4551,16 +4557,41 @@ let TabRestoreQueue = {
       visible.splice(index, 1);
       hidden.push(tab);
     } else {
       throw new Error("restore queue: visible tab not found");
     }
   }
 };
 
+// A map storing tabData belonging to xul:browsers of tabs. This will hold data
+// while a tab is restoring (i.e. loading). Because we can't query or use the
+// incomplete state of a loading tab we'll use data stored in the map if browser
+// state is collected while a tab is still restoring or if it's closed before
+// having restored fully.
+let RestoringTabsData = {
+  _data: new WeakMap(),
+
+  has: function (browser) {
+    return this._data.has(browser);
+  },
+
+  get: function (browser) {
+    return this._data.get(browser);
+  },
+
+  set: function (browser, data) {
+    this._data.set(browser, data);
+  },
+
+  remove: function (browser) {
+    this._data.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
 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 &&
--- a/browser/components/sessionstore/test/browser_579868.js
+++ b/browser/components/sessionstore/test/browser_579868.js
@@ -12,17 +12,16 @@ function test() {
     tab1.linkedBrowser.removeEventListener("load", mainPart, true);
 
     // Tell the session storer that the tab is pinned
     let newTabState = '{"entries":[{"url":"about:rights"}],"pinned":true,"userTypedValue":"Hello World!"}';
     ss.setTabState(tab1, newTabState);
 
     // Undo pinning
     gBrowser.unpinTab(tab1);
-    ok("__SS_data" in tab1.linkedBrowser, "tab should still be loading");
 
     // Close and restore tab
     gBrowser.removeTab(tab1);
     let savedState = JSON.parse(ss.getClosedTabData(window))[0].state;
     isnot(savedState.pinned, true, "Pinned should not be true");
     tab1 = ss.undoCloseTab(window, 0);
 
     isnot(tab1.pinned, true, "Should not be pinned");