Bug 1337940 - Part 1 - Capture session store tab data on history listener notifications. r=ahunt

So far we've simply used DOMTitleChanged as a proxy for navigation, since it's the earliest opportunity at which we have all necessary data for a new history entry (session history itself as well as tab URL and *title*) available.

However it turns out that this is not 100 % reliable, since some pages might e.g. implement their navigation in JS using the history API, which won't necessarily trigger any DOMTitleChanged events. In those case we'd fail to update the tab's session history in the session store unless the user eventually navigated to someplace else that actually triggers a title change event again - if the browser was closed before that, we'd fail to properly restore the user's state.

To fix this, we take a similar approach as the desktop session store and collect a tab's history data again when receiving any history change notification for that tab.

Because the OnHistory... notifications are mostly cancellable, the session history hasn't been actually updated yet at the point the history listener is being called. We therefore can't synchronously call onTabLoad() from within our history change notification handler and have to schedule an async timeout instead so as to give the session history a chance to complete updating its state.

MozReview-Commit-ID: LgHer940QwT

--HG--
extra : rebase_source : a9634be57f3f43e30f42431e8a28846d958534ee

mobile/android/chrome/content/browser.js

@@ -4540,39 +4540,41 @@ Tab.prototype = {
     this._restoreZoom = aHistoryEventName !== "New";
   },
 
-  OnHistoryNewEntry: function(aUri) {
+  OnHistoryNewEntry: function(newURI, oldIndex) {
+    Services.obs.notifyObservers(this.browser, "Content:HistoryChange", null);
     this._updateZoomFromHistoryEvent("New");
   },
 
-  OnHistoryGoBack: function(aUri) {
+  OnHistoryGoBack: function(backURI) {
+    Services.obs.notifyObservers(this.browser, "Content:HistoryChange", null);
     this._updateZoomFromHistoryEvent("Back");
     return true;
   },
 
-  OnHistoryGoForward: function(aUri) {
+  OnHistoryGoForward: function(forwardURI) {
+    Services.obs.notifyObservers(this.browser, "Content:HistoryChange", null);
     this._updateZoomFromHistoryEvent("Forward");
     return true;
   },
 
-  OnHistoryReload: function(aUri, aFlags) {
-    // we don't do anything with this, so don't propagate it
-    // for now anyway
+  OnHistoryReload: function(reloadURI, reloadFlags) {
+    Services.obs.notifyObservers(this.browser, "Content:HistoryChange", null);
     return true;
   },
 
-  OnHistoryGotoIndex: function(aIndex, aUri) {
+  OnHistoryGotoIndex: function(index, gotoURI) {
+    Services.obs.notifyObservers(this.browser, "Content:HistoryChange", null);
     this._updateZoomFromHistoryEvent("Goto");
     return true;
   },
 
-  OnHistoryPurge: function(aNumEntries) {
-    this._updateZoomFromHistoryEvent("Purge");
+  OnHistoryPurge: function(numEntries) {
+    Services.obs.notifyObservers(this.browser, "Content:HistoryChange", null);
     return true;
   },
 
-  OnHistoryReplaceEntry: function(aIndex) {
-    // we don't do anything with this, so don't propogate it
-    // for now anyway.
+  OnHistoryReplaceEntry: function(index) {
+    Services.obs.notifyObservers(this.browser, "Content:HistoryChange", null);
   },
 
   ShouldNotifyMediaPlaybackChange: function(inactive) {

mobile/android/components/SessionStore.js

@@ -183,6 +183,7 @@ SessionStore.prototype = {
         observerService.addObserver(this, "quit-application", true);
         observerService.addObserver(this, "Session:Restore", true);
         observerService.addObserver(this, "Session:NotifyLocationChange", true);
+        observerService.addObserver(this, "Content:HistoryChange", true);
         observerService.addObserver(this, "Tab:KeepZombified", true);
         observerService.addObserver(this, "application-background", true);
         observerService.addObserver(this, "application-foreground", true);
@@ -313,6 +314,27 @@ SessionStore.prototype = {
         }
         break;
       }
+      case "Content:HistoryChange": {
+        let browser = aSubject;
+        let window = browser.ownerGlobal;
+        log("Content:HistoryChange for tab " + window.BrowserApp.getTabForBrowser(browser).id);
+        // We want to ignore history changes which we caused ourselves when
+        // restoring the history of a delay-loaded tab.
+        if (!browser.__SS_restore && !browser.__SS_restoreReloadPending) {
+          // The OnHistory... notifications are called *before* the history changes
+          // are persisted. We therefore need to make our onTabLoad call async,
+          // so it can actually capture the new session history state.
+          if (browser.__SS_historyChange) {
+            window.clearTimeout(browser.__SS_historyChange);
+          }
+          browser.__SS_historyChange =
+            window.setTimeout(() => {
+              delete browser.__SS_historyChange;
+              this.onTabLoad(window, browser);
+            }, 0);
+        }
+        break;
+      }
       case "Tabs:OpenMultiple": {
         let data = JSON.parse(aData);
 
@@ -619,6 +641,11 @@ SessionStore.prototype = {
     aBrowser.removeEventListener("scroll", this, true);
     aBrowser.removeEventListener("resize", this, true);
 
+    if (aBrowser.__SS_historyChange) {
+      aWindow.clearTimeout(aBrowser.__SS_historyChange);
+      delete aBrowser.__SS_historyChange;
+    }
+
     delete aBrowser.__SS_data;
 
     log("onTabRemove() ran for tab " + aWindow.BrowserApp.getTabForBrowser(aBrowser).id +
@@ -1639,12 +1666,12 @@ SessionStore.prototype = {
       tab.browser.__SS_extdata = tabData.extData;
 
       if (window.BrowserApp.selectedTab == tab) {
-        this._restoreTab(tabData, tab.browser);
-
-        // We can now lift the general ban on tab data capturing,
-        // but we still need to protect the foreground tab until we're
+        // After we're done restoring, we can lift the general ban on tab data
+        // capturing, but we still need to protect the foreground tab until we're
         // sure it's actually reloading after history restoring has finished.
         tab.browser.__SS_restoreReloadPending = true;
+
+        this._restoreTab(tabData, tab.browser);
         this._startupRestoreFinished = true;
         log("startupRestoreFinished = true");