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 309775 3d8926eef121ba1b6b003011e2cac09c00782657
parent 309774 26865d807d138011f9fd83a85aaab761e89be259
child 309776 378ef42875ed296adb790020ebeee26490c026d8
push id5513
push userraliiev@mozilla.com
push dateMon, 25 Jan 2016 13:55:34 +0000
treeherdermozilla-beta@5ee97dd05b5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs1225921
milestone45.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 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] || "";
     }