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
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");