Bug 1109875 - Don't flush state when closing tabs r=billm
authorTim Taubert <ttaubert@mozilla.com>
Thu, 30 Apr 2015 18:28:39 +0200
changeset 273741 98edf81629bd496d06580a07af58c7734704f500
parent 273740 7d540d97dfe4863977937c9b7db09736e084d39a
child 273742 e2ba32edadf3726f3bc42199c49ff55464b8c3cc
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs1109875
milestone40.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 1109875 - Don't flush state when closing tabs r=billm
browser/components/sessionstore/SessionStore.jsm
browser/components/sessionstore/TabState.jsm
--- a/browser/components/sessionstore/SessionStore.jsm
+++ b/browser/components/sessionstore/SessionStore.jsm
@@ -87,16 +87,19 @@ const NOTAB_MESSAGES = new Set([
 ]);
 
 // The list of messages we want to receive even during the short period after a
 // frame has been removed from the DOM and before its frame script has finished
 // unloading.
 const CLOSED_MESSAGES = new Set([
   // For a description see above.
   "SessionStore:crashedTabRevived",
+
+  // For a description see above.
+  "SessionStore:update",
 ]);
 
 // These are tab events that we listen to.
 const TAB_EVENTS = [
   "TabOpen", "TabClose", "TabSelect", "TabShow", "TabHide", "TabPinned",
   "TabUnpinned"
 ];
 
@@ -337,16 +340,21 @@ let SessionStoreInternal = {
   // here - that way we know which browsers to ignore messages from (until
   // they get restored).
   _crashedBrowsers: new WeakSet(),
 
   // A map (xul:browser -> nsIFrameLoader) that maps a browser to the last
   // associated frameLoader we heard about.
   _lastKnownFrameLoader: new WeakMap(),
 
+  // A map (xul:browser -> object) that maps a browser associated with a
+  // recently closed tab to all its necessary state information we need to
+  // properly handle final update message.
+  _closedTabs: new WeakMap(),
+
   // whether a setBrowserState call is in progress
   _browserSetState: false,
 
   // time in milliseconds when the session was started (saved across sessions),
   // defaults to now if no session was restored or timestamp doesn't exist
   _sessionStartTime: Date.now(),
 
   // states for all currently opened windows
@@ -627,19 +635,58 @@ let SessionStoreInternal = {
         // frameLoader we know about to compare.
         let frameLoader = browser.frameLoader ||
                           this._lastKnownFrameLoader.get(browser.permanentKey);
 
         // If the message isn't targeting the latest frameLoader discard it.
         if (frameLoader != aMessage.targetFrameLoader) {
           return;
         }
+
+        // Record telemetry measurements done in the child and update the tab's
+        // cached state. Mark the window as dirty and trigger a delayed write.
         this.recordTelemetry(aMessage.data.telemetry);
         TabState.update(browser, aMessage.data);
         this.saveStateDelayed(win);
+
+        // Handle any updates sent by the child after the tab was closed. This
+        // might be the final update as sent by the "unload" handler but also
+        // any async update message that was sent before the child unloaded.
+        if (this._closedTabs.has(browser.permanentKey)) {
+          let {closedTabs, tabData} = this._closedTabs.get(browser.permanentKey);
+
+          // Update the closed tab's state. This will be reflected in its
+          // window's list of closed tabs as that refers to the same object.
+          TabState.copyFromCache({linkedBrowser: browser}, tabData.state);
+
+          // Is this the tab's final message?
+          if (aMessage.data.isFinal) {
+            // We expect no further updates.
+            this._closedTabs.delete(browser.permanentKey);
+            // The tab state no longer needs this reference.
+            delete tabData.permanentKey;
+
+            // Determine whether the tab state is worth saving.
+            let shouldSave = this._shouldSaveTabState(tabData.state);
+            let index = closedTabs.indexOf(tabData);
+
+            if (shouldSave && index == -1) {
+              // If the tab state is worth saving and we didn't push it onto
+              // the list of closed tabs when it was closed (because we deemed
+              // the state not worth saving) then add it to the window's list
+              // of closed tabs now.
+              this.saveClosedTabData(closedTabs, tabData);
+            } else if (!shouldSave && index > -1) {
+              // Remove from the list of closed tabs. The update messages sent
+              // after the tab was closed changed enough state so that we no
+              // longer consider its data interesting enough to keep around.
+              this.removeClosedTabData(closedTabs, index);
+            }
+          }
+        }
         break;
       case "SessionStore:restoreHistoryComplete":
         if (this.isCurrentEpoch(browser, aMessage.data.epoch)) {
           // Notify the tabbrowser that the tab chrome has been restored.
           let tabData = browser.__SS_data;
 
           // wall-paper fix for bug 439675: make sure that the URL to be loaded
           // is always visible in the address bar
@@ -1393,45 +1440,115 @@ let SessionStoreInternal = {
     event.initEvent("SSTabClosing", true, false);
     aTab.dispatchEvent(event);
 
     // don't update our internal state if we don't have to
     if (this._max_tabs_undo == 0) {
       return;
     }
 
-    // Flush all data queued in the content script before the tab is gone.
-    TabState.flush(aTab.linkedBrowser);
-
     // Get the latest data for this tab (generally, from the cache)
     let tabState = TabState.collect(aTab);
 
     // Don't save private tabs
     let isPrivateWindow = PrivateBrowsingUtils.isWindowPrivate(aWindow);
     if (!isPrivateWindow && tabState.isPrivate) {
       return;
     }
 
-    // store closed-tab data for undo
+    // Store closed-tab data for undo.
+    let tabbrowser = aWindow.gBrowser;
+    let tabTitle = this._replaceLoadingTitle(aTab.label, tabbrowser, aTab);
+    let {permanentKey} = aTab.linkedBrowser;
+
+    let tabData = {
+      permanentKey,
+      state: tabState,
+      title: tabTitle,
+      image: tabbrowser.getIcon(aTab),
+      pos: aTab._tPos,
+      closedAt: Date.now()
+    };
+
+    let closedTabs = this._windows[aWindow.__SSi]._closedTabs;
+
+    // Determine whether the tab contains any information worth saving. Note
+    // that there might be pending state changes queued in the child that
+    // didn't reach the parent yet. If a tab is emptied before closing then we
+    // might still remove it from the list of closed tabs later.
     if (this._shouldSaveTabState(tabState)) {
-      let tabTitle = aTab.label;
-      let tabbrowser = aWindow.gBrowser;
-      tabTitle = this._replaceLoadingTitle(tabTitle, tabbrowser, aTab);
-
-      this._windows[aWindow.__SSi]._closedTabs.unshift({
-        state: tabState,
-        title: tabTitle,
-        image: tabbrowser.getIcon(aTab),
-        pos: aTab._tPos,
-        closedAt: Date.now()
-      });
-      var length = this._windows[aWindow.__SSi]._closedTabs.length;
-      if (length > this._max_tabs_undo)
-        this._windows[aWindow.__SSi]._closedTabs.splice(this._max_tabs_undo, length - this._max_tabs_undo);
+      // Save the tab state, for now. We might push a valid tab out
+      // of the list but those cases should be extremely rare and
+      // do probably never occur when using the browser normally.
+      // (Tests or add-ons might do weird things though.)
+      this.saveClosedTabData(closedTabs, tabData);
+    }
+
+    // Remember the closed tab to properly handle any last updates included in
+    // the final "update" message sent by the frame script's unload handler.
+    this._closedTabs.set(permanentKey, {closedTabs, tabData});
+  },
+
+  /**
+   * Insert a given |tabData| object into the list of |closedTabs|. We will
+   * determine the right insertion point based on the .closedAt properties of
+   * all tabs already in the list. The list will be truncated to contain a
+   * maximum of |this._max_tabs_undo| entries.
+   *
+   * @param closedTabs (array)
+   *        The list of closed tabs for a window.
+   * @param tabData (object)
+   *        The tabData to be inserted.
+   */
+  saveClosedTabData(closedTabs, tabData) {
+    // Find the index of the first tab in the list
+    // of closed tabs that was closed before our tab.
+    let index = closedTabs.findIndex(tab => {
+      return tab.closedAt < tabData.closedAt;
+    });
+
+    // If we found no tab closed before our
+    // tab then just append it to the list.
+    if (index == -1) {
+      index = closedTabs.length;
     }
+
+    // Insert tabData at the right position.
+    closedTabs.splice(index, 0, tabData);
+
+    // Truncate the list of closed tabs, if needed.
+    if (closedTabs.length > this._max_tabs_undo) {
+      closedTabs.splice(this._max_tabs_undo, closedTabs.length);
+    }
+  },
+
+  /**
+   * Remove the closed tab data at |index| from the list of |closedTabs|. If
+   * the tab's final message is still pending we will simply discard it when
+   * it arrives so that the tab doesn't reappear in the list.
+   *
+   * @param closedTabs (array)
+   *        The list of closed tabs for a window.
+   * @param index (uint)
+   *        The index of the tab to remove.
+   */
+  removeClosedTabData(closedTabs, index) {
+    // Remove the given index from the list.
+    let [closedTab] = closedTabs.splice(index, 1);
+
+    // If the closed tab's state still has a .permanentKey property then we
+    // haven't seen its final update message yet. Remove it from the map of
+    // closed tabs so that we will simply discard its last messages and will
+    // not add it back to the list of closed tabs again.
+    if (closedTab.permanentKey) {
+      this._closedTabs.delete(closedTab.permanentKey);
+      delete closedTab.permanentKey;
+    }
+
+    return closedTab;
   },
 
   /**
    * When a tab is selected, save session data
    * @param aWindow
    *        Window reference
    */
   onTabSelect: function ssi_onTabSelect(aWindow) {
@@ -1724,28 +1841,27 @@ let SessionStoreInternal = {
 
     // default to the most-recently closed tab
     aIndex = aIndex || 0;
     if (!(aIndex in closedTabs)) {
       throw Components.Exception("Invalid index: not in the closed tabs", Cr.NS_ERROR_INVALID_ARG);
     }
 
     // fetch the data of closed tab, while removing it from the array
-    let [closedTab] = closedTabs.splice(aIndex, 1);
-    let closedTabState = closedTab.state;
+    let {state, pos} = this.removeClosedTabData(closedTabs, aIndex);
 
     // create a new tab
     let tabbrowser = aWindow.gBrowser;
     let tab = tabbrowser.selectedTab = tabbrowser.addTab();
 
     // restore tab content
-    this.restoreTab(tab, closedTabState);
+    this.restoreTab(tab, state);
 
     // restore the tab's position
-    tabbrowser.moveTabTo(tab, closedTab.pos);
+    tabbrowser.moveTabTo(tab, pos);
 
     // focus the tab's content area (bug 342432)
     tab.linkedBrowser.focus();
 
     return tab;
   },
 
   forgetClosedTab: function ssi_forgetClosedTab(aWindow, aIndex) {
@@ -1757,17 +1873,17 @@ let SessionStoreInternal = {
 
     // default to the most-recently closed tab
     aIndex = aIndex || 0;
     if (!(aIndex in closedTabs)) {
       throw Components.Exception("Invalid index: not in the closed tabs", Cr.NS_ERROR_INVALID_ARG);
     }
 
     // remove closed tab from the array
-    closedTabs.splice(aIndex, 1);
+    this.removeClosedTabData(closedTabs, aIndex);
   },
 
   getClosedWindowCount: function ssi_getClosedWindowCount() {
     return this._closedWindows.length;
   },
 
   getClosedWindowData: function ssi_getClosedWindowData() {
     return JSON.stringify(this._closedWindows);
--- a/browser/components/sessionstore/TabState.jsm
+++ b/browser/components/sessionstore/TabState.jsm
@@ -48,16 +48,20 @@ this.TabState = Object.freeze({
   },
 
   collect: function (tab) {
     return TabStateInternal.collect(tab);
   },
 
   clone: function (tab) {
     return TabStateInternal.clone(tab);
+  },
+
+  copyFromCache: function (tab, tabData, options) {
+    TabStateInternal.copyFromCache(tab, tabData, options);
   }
 });
 
 let TabStateInternal = {
   // A map (xul:browser -> handler) that maps a tab to the
   // synchronous collection handler object for that tab.
   // See SyncHandler in content-sessionStore.js.
   _syncHandlers: new WeakMap(),
@@ -213,32 +217,32 @@ let TabStateInternal = {
 
     if (tab.__SS_extdata)
       tabData.extData = tab.__SS_extdata;
     else if (tabData.extData)
       delete tabData.extData;
 
     // Copy data from the tab state cache only if the tab has fully finished
     // restoring. We don't want to overwrite data contained in __SS_data.
-    this._copyFromCache(tab, tabData, options);
+    this.copyFromCache(tab, tabData, options);
 
     return tabData;
   },
 
   /**
    * Copy tab data for the given |tab| from the cache to |tabData|.
    *
    * @param tab (xul:tab)
    *        The tab belonging to the given |tabData| object.
    * @param tabData (object)
    *        The tab data belonging to the given |tab|.
    * @param options (object)
    *        {includePrivateData: true} to always include private data
    */
-  _copyFromCache: function (tab, tabData, options = {}) {
+  copyFromCache: function (tab, tabData, options = {}) {
     let data = TabStateCache.get(tab.linkedBrowser);
     if (!data) {
       return;
     }
 
     // The caller may explicitly request to omit privacy checks.
     let includePrivateData = options && options.includePrivateData;