Bug 1161928 - Require an epoch (managed in the parent) included in every message sent by the frame script to get rid of TabState.flush() calls in restoreTab() r=billm
authorTim Taubert <ttaubert@mozilla.com>
Wed, 06 May 2015 15:06:29 +0200
changeset 274298 133c21f8ab74cb847ba689547e15640949ce7093
parent 274297 360d53ac1556b3e92caa8ef05f0df0fbdfb347df
child 274299 f8ad92bbeea604d2cd367773ca7c21e63900cc8d
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
bugs1161928
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 1161928 - Require an epoch (managed in the parent) included in every message sent by the frame script to get rid of TabState.flush() calls in restoreTab() r=billm
browser/components/sessionstore/SessionStore.jsm
browser/components/sessionstore/content/content-sessionStore.js
--- a/browser/components/sessionstore/SessionStore.jsm
+++ b/browser/components/sessionstore/SessionStore.jsm
@@ -83,16 +83,26 @@ const NOTAB_MESSAGES = new Set([
 
   // For a description see above.
   "SessionStore:crashedTabRevived",
 
   // For a description see above.
   "SessionStore:update",
 ]);
 
+// The list of messages we accept without an "epoch" parameter.
+// See getCurrentEpoch() and friends to find out what an "epoch" is.
+const NOEPOCH_MESSAGES = new Set([
+  // For a description see above.
+  "SessionStore:setupSyncHandler",
+
+  // For a description see above.
+  "SessionStore:crashedTabRevived",
+]);
+
 // 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.
@@ -325,20 +335,17 @@ let SessionStoreInternal = {
   ]),
 
   _globalState: new GlobalState(),
 
   // During the initial restore and setBrowserState calls tracks the number of
   // windows yet to be restored
   _restoreCount: -1,
 
-  // This number gets incremented each time we start to restore a tab.
-  _nextRestoreEpoch: 1,
-
-  // For each <browser> element being restored, records the current epoch.
+  // For each <browser> element, records the current epoch.
   _browserEpochs: new WeakMap(),
 
   // Any browsers that fires the oop-browser-crashed event gets stored in
   // 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
@@ -611,16 +618,29 @@ let SessionStoreInternal = {
 
     // Ensure we receive only specific messages from <xul:browser>s that
     // have no tab assigned, e.g. the ones that preload about:newtab pages.
     if (!tab && !NOTAB_MESSAGES.has(aMessage.name)) {
       throw new Error(`received unexpected message '${aMessage.name}' ` +
                       `from a browser that has no tab`);
     }
 
+    let data = aMessage.data || {};
+    let hasEpoch = data.hasOwnProperty("epoch");
+
+    // Most messages sent by frame scripts require to pass an epoch.
+    if (!hasEpoch && !NOEPOCH_MESSAGES.has(aMessage.name)) {
+      throw new Error(`received message '${aMessage.name}' without an epoch`);
+    }
+
+    // Ignore messages from previous epochs.
+    if (hasEpoch && !this.isCurrentEpoch(browser, data.epoch)) {
+      return;
+    }
+
     switch (aMessage.name) {
       case "SessionStore:setupSyncHandler":
         TabState.setSyncHandler(browser, aMessage.objects.handler);
         break;
       case "SessionStore:update":
         // Ignore messages from <browser> elements that have crashed
         // and not yet been revived.
         if (this._crashedBrowsers.has(browser.permanentKey)) {
@@ -676,87 +696,79 @@ let SessionStoreInternal = {
               // 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
-          let activePageData = tabData.entries[tabData.index - 1] || null;
-          let uri = activePageData ? activePageData.url || null : null;
-          browser.userTypedValue = uri;
-
-          // If the page has a title, set it.
-          if (activePageData) {
-            if (activePageData.title) {
-              tab.label = activePageData.title;
-              tab.crop = "end";
-            } else if (activePageData.url != "about:blank") {
-              tab.label = activePageData.url;
-              tab.crop = "center";
-            }
+        // 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
+        let activePageData = tabData.entries[tabData.index - 1] || null;
+        let uri = activePageData ? activePageData.url || null : null;
+        browser.userTypedValue = uri;
+
+        // If the page has a title, set it.
+        if (activePageData) {
+          if (activePageData.title) {
+            tab.label = activePageData.title;
+            tab.crop = "end";
+          } else if (activePageData.url != "about:blank") {
+            tab.label = activePageData.url;
+            tab.crop = "center";
           }
-
-          // Restore the tab icon.
-          if ("image" in tabData) {
-            win.gBrowser.setIcon(tab, tabData.image);
-          }
-
-          let event = win.document.createEvent("Events");
-          event.initEvent("SSTabRestoring", true, false);
-          tab.dispatchEvent(event);
         }
+
+        // Restore the tab icon.
+        if ("image" in tabData) {
+          win.gBrowser.setIcon(tab, tabData.image);
+        }
+
+        let event = win.document.createEvent("Events");
+        event.initEvent("SSTabRestoring", true, false);
+        tab.dispatchEvent(event);
         break;
       case "SessionStore:restoreTabContentStarted":
-        if (this.isCurrentEpoch(browser, aMessage.data.epoch)) {
-          if (browser.__SS_restoreState == TAB_STATE_NEEDS_RESTORE) {
-            // If a load not initiated by sessionstore was started in a
-            // previously pending tab. Mark the tab as no longer pending.
-            this.markTabAsRestoring(tab);
-          } else {
-            // If the user was typing into the URL bar when we crashed, but hadn't hit
-            // enter yet, then we just need to write that value to the URL bar without
-            // loading anything. This must happen after the load, since it will clear
-            // userTypedValue.
-            let tabData = browser.__SS_data;
-            if (tabData.userTypedValue && !tabData.userTypedClear) {
-              browser.userTypedValue = tabData.userTypedValue;
-              win.URLBarSetURI();
-            }
+        if (browser.__SS_restoreState == TAB_STATE_NEEDS_RESTORE) {
+          // If a load not initiated by sessionstore was started in a
+          // previously pending tab. Mark the tab as no longer pending.
+          this.markTabAsRestoring(tab);
+        } else {
+          // If the user was typing into the URL bar when we crashed, but hadn't hit
+          // enter yet, then we just need to write that value to the URL bar without
+          // loading anything. This must happen after the load, since it will clear
+          // userTypedValue.
+          let tabData = browser.__SS_data;
+          if (tabData.userTypedValue && !tabData.userTypedClear) {
+            browser.userTypedValue = tabData.userTypedValue;
+            win.URLBarSetURI();
           }
         }
         break;
       case "SessionStore:restoreTabContentComplete":
-        if (this.isCurrentEpoch(browser, aMessage.data.epoch)) {
-          // This callback is used exclusively by tests that want to
-          // monitor the progress of network loads.
-          if (gDebuggingEnabled) {
-            Services.obs.notifyObservers(browser, NOTIFY_TAB_RESTORED, null);
-          }
-
-          delete browser.__SS_data;
-
-          SessionStoreInternal._resetLocalTabRestoringState(tab);
-          SessionStoreInternal.restoreNextTab();
-
-          this._sendTabRestoredNotification(tab);
+        // This callback is used exclusively by tests that want to
+        // monitor the progress of network loads.
+        if (gDebuggingEnabled) {
+          Services.obs.notifyObservers(browser, NOTIFY_TAB_RESTORED, null);
         }
+
+        delete browser.__SS_data;
+
+        SessionStoreInternal._resetLocalTabRestoringState(tab);
+        SessionStoreInternal.restoreNextTab();
+
+        this._sendTabRestoredNotification(tab);
         break;
       case "SessionStore:reloadPendingTab":
-        if (this.isCurrentEpoch(browser, aMessage.data.epoch)) {
-          if (browser.__SS_restoreState == TAB_STATE_NEEDS_RESTORE) {
-            this.restoreTabContent(tab);
-          }
+        if (browser.__SS_restoreState == TAB_STATE_NEEDS_RESTORE) {
+          this.restoreTabContent(tab);
         }
         break;
       case "SessionStore:crashedTabRevived":
         this._crashedBrowsers.delete(browser.permanentKey);
         break;
       default:
         throw new Error(`received unknown message '${aMessage.name}'`);
         break;
@@ -2789,21 +2801,16 @@ let SessionStoreInternal = {
       tab.__SS_extdata = Cu.cloneInto(tabData.extData, {});
     } else {
       delete tab.__SS_extdata;
     }
 
     // Tab is now open.
     delete tabData.closedAt;
 
-    // Flush all data from the content script synchronously. This is done so
-    // that all async messages that are still on their way to chrome will
-    // be ignored and don't override any tab data set when restoring.
-    TabState.flush(browser);
-
     // Ensure the index is in bounds.
     let activeIndex = (tabData.index || tabData.entries.length) - 1;
     activeIndex = Math.min(activeIndex, tabData.entries.length - 1);
     activeIndex = Math.max(activeIndex, 0);
 
     // Save the index in case we updated it above.
     tabData.index = activeIndex + 1;
 
@@ -2811,21 +2818,20 @@ let SessionStoreInternal = {
     // attribute so that it runs in a content process.
     let activePageData = tabData.entries[activeIndex] || null;
     let uri = activePageData ? activePageData.url || null : null;
     if (loadArguments) {
       uri = loadArguments.uri;
     }
     tabbrowser.updateBrowserRemotenessByURL(browser, uri);
 
-    // Start a new epoch and include the epoch in the restoreHistory
-    // message. If a message is received that relates to a previous epoch, we
-    // discard it.
-    let epoch = this._nextRestoreEpoch++;
-    this._browserEpochs.set(browser.permanentKey, epoch);
+    // Start a new epoch to discard all frame script messages relating to a
+    // previous epoch. All async messages that are still on their way to chrome
+    // will be ignored and don't override any tab data set when restoring.
+    let epoch = this.startNextEpoch(browser);
 
     // keep the data around to prevent dataloss in case
     // a tab gets closed before it's been properly restored
     browser.__SS_data = tabData;
     browser.__SS_restoreState = TAB_STATE_NEEDS_RESTORE;
     browser.setAttribute("pending", "true");
     tab.setAttribute("pending", "true");
 
@@ -3602,17 +3608,16 @@ let SessionStoreInternal = {
     let window = aTab.ownerDocument.defaultView;
     let browser = aTab.linkedBrowser;
 
     // Keep the tab's previous state for later in this method
     let previousState = browser.__SS_restoreState;
 
     // The browser is no longer in any sort of restoring state.
     delete browser.__SS_restoreState;
-    this._browserEpochs.delete(browser.permanentKey);
 
     aTab.removeAttribute("pending");
     browser.removeAttribute("pending");
 
     if (previousState == TAB_STATE_RESTORING) {
       if (this._tabsRestoringCount)
         this._tabsRestoringCount--;
     } else if (previousState == TAB_STATE_NEEDS_RESTORE) {
@@ -3628,24 +3633,44 @@ let SessionStoreInternal = {
               "given tab is not restoring");
 
     let browser = tab.linkedBrowser;
     browser.messageManager.sendAsyncMessage("SessionStore:resetRestore", {});
     this._resetLocalTabRestoringState(tab);
   },
 
   /**
+   * Each fresh tab starts out with epoch=0. This function can be used to
+   * start a next epoch by incrementing the current value. It will enables us
+   * to ignore stale messages sent from previous epochs. The function returns
+   * the new epoch ID for the given |browser|.
+   */
+  startNextEpoch(browser) {
+    let next = this.getCurrentEpoch(browser) + 1;
+    this._browserEpochs.set(browser.permanentKey, next);
+    return next;
+  },
+
+  /**
+   * Returns the current epoch for the given <browser>. If we haven't assigned
+   * a new epoch this will default to zero for new tabs.
+   */
+  getCurrentEpoch(browser) {
+    return this._browserEpochs.get(browser.permanentKey) || 0;
+  },
+
+  /**
    * Each time a <browser> element is restored, we increment its "epoch". To
    * check if a message from content-sessionStore.js is out of date, we can
    * compare the epoch received with the message to the <browser> element's
    * epoch. This function does that, and returns true if |epoch| is up-to-date
    * with respect to |browser|.
    */
   isCurrentEpoch: function (browser, epoch) {
-    return (this._browserEpochs.get(browser.permanentKey) || 0) == epoch;
+    return this.getCurrentEpoch(browser) == epoch;
   },
 
 };
 
 /**
  * Priority queue that keeps track of a list of tabs to restore and returns
  * the tab we should restore next, based on priority rules. We decide between
  * pinned, visible and hidden tabs in that and FIFO order. Hidden tabs are only
--- a/browser/components/sessionstore/content/content-sessionStore.js
+++ b/browser/components/sessionstore/content/content-sessionStore.js
@@ -698,17 +698,18 @@ let MessageQueue = {
     durationMs = Date.now() - durationMs;
     let telemetry = {
       FX_SESSION_RESTORE_CONTENT_COLLECT_DATA_LONGEST_OP_MS: durationMs
     }
 
     // Send all data to the parent process.
     sendMessage("SessionStore:update", {
       id: this._id, data, telemetry,
-      isFinal: options.isFinal || false
+      isFinal: options.isFinal || false,
+      epoch: gCurrentEpoch
     });
 
     // Increase our unique message ID.
     this._id++;
   },
 
   /**
    * This function is used to make the message queue flush all queue data that