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
--- a/mobile/android/chrome/content/browser.js
+++ b/mobile/android/chrome/content/browser.js
@@ -4531,49 +4531,51 @@ Tab.prototype = {
return null;
},
_updateZoomFromHistoryEvent: function(aHistoryEventName) {
// Restore zoom only when moving in session history, not for new page loads.
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(activeState) {
// If the media is active, we would check it's duration, because we don't
// want to show the media control interface for the short sound which
// duration is smaller than the threshold. The basic unit is second.
// Note : the streaming format's duration is infinite.
if (activeState === "inactive") {
--- a/mobile/android/components/SessionStore.js
+++ b/mobile/android/components/SessionStore.js
@@ -178,16 +178,17 @@ SessionStore.prototype = {
observerService.addObserver(this, "domwindowclosed", true);
observerService.addObserver(this, "browser:purge-session-history", true);
observerService.addObserver(this, "browser:purge-session-tabs", true);
observerService.addObserver(this, "quit-application-requested", true);
observerService.addObserver(this, "quit-application-proceeding", true);
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);
observerService.addObserver(this, "ClosedTabs:StartNotifications", true);
observerService.addObserver(this, "ClosedTabs:StopNotifications", true);
observerService.addObserver(this, "last-pb-context-exited", true);
observerService.addObserver(this, "Session:RestoreRecentTabs", true);
observerService.addObserver(this, "Tabs:OpenMultiple", true);
@@ -308,16 +309,37 @@ SessionStore.prototype = {
}
if (browser.__SS_restoreDataOnLocationChange) {
delete browser.__SS_restoreDataOnLocationChange;
this._restoreZoom(browser.__SS_data.scrolldata, browser);
}
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);
this._openTabs(data);
if (data.shouldNotifyTabsOpenedToJava) {
let window = Services.wm.getMostRecentWindow("navigator:browser");
window.WindowEventDispatcher.sendRequest({
@@ -614,16 +636,21 @@ SessionStore.prototype = {
aBrowser.removeEventListener("pageshow", this, true);
aBrowser.removeEventListener("AboutReaderContentReady", this, true);
aBrowser.removeEventListener("change", this, true);
aBrowser.removeEventListener("input", this, true);
aBrowser.removeEventListener("DOMAutoComplete", this, true);
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 +
", aNoNotification = " + aNoNotification);
if (!aNoNotification) {
this.saveStateDelayed();
}
},
@@ -1634,22 +1661,22 @@ SessionStore.prototype = {
if (parentId > -1) {
tab.parentId = parentId;
}
tab.browser.__SS_data = tabData;
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");
delete tab.browser.__SS_restore;
tab.browser.removeAttribute("pending");
} else {
// Mark the browser for delay loading
tab.browser.__SS_restore = true;