Bug 1225921 - Have SessionStore keep a list of window data that might be saved during a flush. r=billm
authorMike Conley <mconley@mozilla.com>
Wed, 02 Dec 2015 13:48:19 -0500
changeset 275649 3d8926eef121ba1b6b003011e2cac09c00782657
parent 275648 26865d807d138011f9fd83a85aaab761e89be259
child 275650 378ef42875ed296adb790020ebeee26490c026d8
push id16572
push usermconley@mozilla.com
push dateFri, 04 Dec 2015 19:35:24 +0000
treeherderfx-team@3d8926eef121 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs1225921
milestone45.0a1
Bug 1225921 - Have SessionStore keep a list of window data that might be saved during a flush. r=billm This helps us keep track of what windows we've chosen to forget, and helps us avoid the problem of accidentally saving a window we've chosen to forget.
browser/components/sessionstore/SessionStore.jsm
--- a/browser/components/sessionstore/SessionStore.jsm
+++ b/browser/components/sessionstore/SessionStore.jsm
@@ -396,16 +396,30 @@ var SessionStoreInternal = {
   // properly handle final update message.
   _closedTabs: new WeakMap(),
 
   // A map (xul:browser -> object) that maps a browser associated with a
   // recently closed tab due to a window closure to the tab state information
   // that is being stored in _closedWindows for that tab.
   _closedWindowTabs: new WeakMap(),
 
+  // A set of window data that has the potential to be saved in the _closedWindows
+  // array for the session. We will remove window data from this set whenever
+  // forgetClosedWindow is called for the window, or when session history is
+  // purged, so that we don't accidentally save that data after the flush has
+  // completed. Closed tabs use a more complicated mechanism for this particular
+  // problem. When forgetClosedTab is called, the browser is removed from the
+  // _closedTabs map, so its data is not recorded. In the purge history case,
+  // the closedTabs array per window is overwritten so that once the flush is
+  // complete, the tab would only ever add itself to an array that SessionStore
+  // no longer cares about. Bug 1230636 has been filed to make the tab case
+  // work more like the window case, which is more explicit, and easier to
+  // reason about.
+  _saveableClosedWindowData: new WeakSet(),
+
   // A map (xul:browser -> object) that maps a browser that is switching
   // remoteness via navigateAndRestore, to the loadArguments that were
   // most recently passed when calling navigateAndRestore.
   _remotenessChangingBrowsers: new WeakMap(),
 
   // whether a setBrowserState call is in progress
   _browserSetState: false,
 
@@ -1265,16 +1279,20 @@ var SessionStoreInternal = {
       // we explicitly allow saving an "empty" window state.
       let isLastWindow =
         Object.keys(this._windows).length == 1 &&
         !this._closedWindows.some(win => win._shouldRestore || false);
 
       // clear this window from the list, since it has definitely been closed.
       delete this._windows[aWindow.__SSi];
 
+      // This window has the potential to be saved in the _closedWindows
+      // array (maybeSaveClosedWindows gets the final call on that).
+      this._saveableClosedWindowData.add(winData);
+
       // Now we have to figure out if this window is worth saving in the _closedWindows
       // Object.
       //
       // We're about to flush the tabs from this window, but it's possible that we
       // might never hear back from the content process(es) in time before the user
       // chooses to restore the closed window. So we do the following:
       //
       // 1) Use the tab state cache to determine synchronously if the window is
@@ -1347,16 +1365,17 @@ var SessionStoreInternal = {
     }
 
     // Cache the window state until it is completely gone.
     DyingWindowCache.set(aWindow, winData);
 
     let mm = aWindow.getGroupMessageManager("browsers");
     MESSAGES.forEach(msg => mm.removeMessageListener(msg, this));
 
+    this._saveableClosedWindowData.delete(winData);
     delete aWindow.__SSi;
   },
 
   /**
    * Decides whether or not a closed window should be put into the
    * _closedWindows Object. This might be called multiple times per
    * window, and will do the right thing of moving the window data
    * in or out of _closedWindows if the winData indicates that our
@@ -1368,17 +1387,19 @@ var SessionStoreInternal = {
    *        Whether or not the window being closed is the last
    *        browser window. Callers of this function should pass
    *        in the value of SessionStoreInternal.atLastWindow for
    *        this argument, and pass in the same value if they happen
    *        to call this method again asynchronously (for example, after
    *        a window flush).
    */
   maybeSaveClosedWindow(winData, isLastWindow) {
-    if (RunState.isRunning) {
+    // Make sure SessionStore is still running, and make sure that we
+    // haven't chosen to forget this window.
+    if (RunState.isRunning && this._saveableClosedWindowData.has(winData)) {
       // Determine whether the window has any tabs worth saving.
       let hasSaveableTabs = winData.tabs.some(this._shouldSaveTabState);
 
       // Note that we might already have this window stored in
       // _closedWindows from a previous call to this function.
       let winIndex = this._closedWindows.indexOf(winData);
       let alreadyStored = (winIndex != -1);
       let shouldStore = (hasSaveableTabs || isLastWindow);
@@ -1541,16 +1562,17 @@ var SessionStoreInternal = {
     var win = this._getMostRecentBrowserWindow();
     if (win) {
       win.setTimeout(() => SessionSaver.run(), 0);
     } else if (RunState.isRunning) {
       SessionSaver.run();
     }
 
     this._clearRestoringWindows();
+    this._saveableClosedWindowData.clear();
   },
 
   /**
    * On purge of domain data
    * @param aData
    *        String domain data
    */
   onPurgeDomainData: function ssi_onPurgeDomainData(aData) {
@@ -2185,17 +2207,19 @@ var SessionStoreInternal = {
   forgetClosedWindow: function ssi_forgetClosedWindow(aIndex) {
     // default to the most-recently closed window
     aIndex = aIndex || 0;
     if (!(aIndex in this._closedWindows)) {
       throw Components.Exception("Invalid index: not in the closed windows", Cr.NS_ERROR_INVALID_ARG);
     }
 
     // remove closed window from the array
+    let winData = this._closedWindows[aIndex];
     this._closedWindows.splice(aIndex, 1);
+    this._saveableClosedWindowData.delete(winData);
   },
 
   getWindowValue: function ssi_getWindowValue(aWindow, aKey) {
     if ("__SSi" in aWindow) {
       var data = this._windows[aWindow.__SSi].extData || {};
       return data[aKey] || "";
     }