Bug 967028 - Use a SHistoryListener to collect entries from history.pushState(). r=Yoric

browser/components/sessionstore/content/content-sessionStore.js

@@ -215,10 +215,20 @@ let SyncHandler = {
  */
 let SessionHistoryListener = {
   init: function () {
+    // The frame tree observer is needed to handle navigating away from
+    // an about page. Currently nsISHistoryListener does not have
+    // OnHistoryNewEntry() called for about pages because the history entry is
+    // modified to point at the new page. Once Bug 981900 lands the frame tree
+    // observer can be removed.
     gFrameTree.addObserver(this);
-    addEventListener("load", this, true);
-    addEventListener("hashchange", this, true);
-    Services.obs.addObserver(this, "browser:purge-session-history", false);
+
+    // By adding the SHistoryListener immediately, we will unfortunately be
+    // notified of every history entry as the tab is restored. We don't bother
+    // waiting to add the listener later because these notifications are cheap.
+    // We will likely only collect once since we are batching collection on
+    // a delay.
+    docShell.QueryInterface(Ci.nsIWebNavigation).sessionHistory.
+      addSHistoryListener(this);
 
     // Collect data if we start with a non-empty shistory.
     if (!SessionHistory.isEmpty(docShell)) {
@@ -227,22 +237,9 @@ let SessionHistoryListener = {
   },
 
   uninit: function () {
-    Services.obs.removeObserver(this, "browser:purge-session-history");
-  },
-
-  observe: function () {
-    // We need to use setTimeout() here because we listen for
-    // "browser:purge-session-history". When that is fired all observers are
-    // expected to purge their data. We can't expect to be called *after* the
-    // observer in browser.xml that clears session history so we need to wait
-    // a tick before actually collecting data.
-    setTimeout(() => this.collect(), 0);
-  },
-
-  handleEvent: function (event) {
-    // We are only interested in "load" events from subframes.
-    if (event.type == "hashchange" || event.target != content.document) {
-      this.collect();
+    let sessionHistory = docShell.QueryInterface(Ci.nsIWebNavigation).sessionHistory;
+    if (sessionHistory) {
+      sessionHistory.removeSHistoryListener(this);
     }
   },
 
@@ -258,7 +255,41 @@ let SessionHistoryListener = {
 
   onFrameTreeReset: function () {
     this.collect();
-  }
+  },
+
+  OnHistoryNewEntry: function (newURI) {
+    this.collect();
+  },
+
+  OnHistoryGoBack: function (backURI) {
+    this.collect();
+    return true;
+  },
+
+  OnHistoryGoForward: function (forwardURI) {
+    this.collect();
+    return true;
+  },
+
+  OnHistoryGotoIndex: function (index, gotoURI) {
+    this.collect();
+    return true;
+  },
+
+  OnHistoryPurge: function (numEntries) {
+    this.collect();
+    return true;
+  },
+
+  OnHistoryReload: function (reloadURI, reloadFlags) {
+    this.collect();
+    return true;
+  },
+
+  QueryInterface: XPCOMUtils.generateQI([
+    Ci.nsISHistoryListener,
+    Ci.nsISupportsWeakReference
+  ])
 };
 
 /**

browser/components/sessionstore/test/browser_sessionHistory.js

@@ -165,3 +165,71 @@ add_task(function test_subframes() {
   // Cleanup.
   gBrowser.removeTab(tab);
 });
+
+/**
+ * Ensure that navigating from an about page invalidates shistory.
+ */
+add_task(function test_about_page_navigate() {
+  // Create a new tab.
+  let tab = gBrowser.addTab("about:blank");
+  let browser = tab.linkedBrowser;
+  yield promiseBrowserLoaded(browser);
+
+  // Check that we have a single shistory entry.
+  SyncHandlers.get(browser).flush();
+  let {entries} = JSON.parse(ss.getTabState(tab));
+  is(entries.length, 1, "there is one shistory entry");
+  is(entries[0].url, "about:blank", "url is correct");
+
+  browser.loadURI("about:robots");
+  yield promiseBrowserLoaded(browser);
+
+  // Check that we have changed the history entry.
+  SyncHandlers.get(browser).flush();
+  let {entries} = JSON.parse(ss.getTabState(tab));
+  is(entries.length, 1, "there is one shistory entry");
+  is(entries[0].url, "about:robots", "url is correct");
+
+  // Cleanup.
+  gBrowser.removeTab(tab);
+});
+
+/**
+ * Ensure that history.pushState and history.replaceState invalidate shistory.
+ */
+add_task(function test_pushstate_replacestate() {
+  // Create a new tab.
+  let tab = gBrowser.addTab("http://example.com/1");
+  let browser = tab.linkedBrowser;
+  yield promiseBrowserLoaded(browser);
+
+  // Check that we have a single shistory entry.
+  SyncHandlers.get(browser).flush();
+  let {entries} = JSON.parse(ss.getTabState(tab));
+  is(entries.length, 1, "there is one shistory entry");
+  is(entries[0].url, "http://example.com/1", "url is correct");
+
+  browser.messageManager.
+    sendAsyncMessage("ss-test:historyPushState", {url: 'test-entry/'});
+  yield promiseContentMessage(browser, "ss-test:historyPushState");
+
+  // Check that we have added the history entry.
+  SyncHandlers.get(browser).flush();
+  let {entries} = JSON.parse(ss.getTabState(tab));
+  is(entries.length, 2, "there is another shistory entry");
+  is(entries[1].url, "http://example.com/test-entry/", "url is correct");
+
+  // Disabled until replaceState invalidation is supported. See Bug 967028.
+  // browser.messageManager.
+  //   sendAsyncMessage("ss-test:historyReplaceState", {url: 'test-entry2/'});
+  // yield promiseContentMessage(browser, "ss-test:historyReplaceState");
+
+  // // Check that we have modified the history entry.
+  // SyncHandlers.get(browser).flush();
+  // let {entries} = JSON.parse(ss.getTabState(tab));
+  // is(entries.length, 2, "there is still two shistory entries");
+  // is(entries[1].url, "http://example.com/test-entry/test-entry2/", "url is correct");
+
+  // Cleanup.
+  gBrowser.removeTab(tab);
+});

browser/components/sessionstore/test/content.js

@@ -163,3 +163,17 @@ addMessageListener("ss-test:click", function ({data}) {
   content.document.getElementById(data.id).click();
   sendAsyncMessage("ss-test:click");
 });
+
+addMessageListener("ss-test:historyPushState", function ({data}) {
+  content.window.history.
+    pushState(data.stateObj || {}, data.title || "", data.url);
+
+  sendAsyncMessage("ss-test:historyPushState");
+});
+
+addMessageListener("ss-test:historyReplaceState", function ({data}) {
+  content.window.history.
+    replaceState(data.stateObj || {}, data.title || "", data.url);
+
+  sendAsyncMessage("ss-test:historyReplaceState");
+});