Issue 4804 - Avoid trashing pagemaps prematurely

chrome/ext/background.js

Index: chrome/ext/background.js
diff --git a/chrome/ext/background.js b/chrome/ext/background.js
index 8cec8296408b0cafb5452c7eb0afb6e67d59035b..8ab10ad920cf5d163d30d78c45adcefb05c8f4ea 100644
--- a/chrome/ext/background.js
+++ b/chrome/ext/background.js
@@ -118,44 +118,23 @@
     return frame;
   }
 
-  chrome.webNavigation.onBeforeNavigate.addListener(details =>
-  {
-    // Capture parent frame here because onCommitted doesn't get this info.
-    let frame = createFrame(details.tabId, details.frameId);
-    frame.parent = framesOfTabs[details.tabId][details.parentFrameId] || null;
-  });
-
-  let eagerlyUpdatedPages = new ext.PageMap();
-
-  ext._updatePageFrameStructure = (frameId, tabId, url, eager) =>
+  function updatePageFrameStructure(frameId, tabId, url)
   {
     if (frameId == 0)
     {
       let page = new Page({id: tabId, url: url});
 
-      if (eagerlyUpdatedPages.get(page) != url)
+      chrome.tabs.get(tabId, () =>
       {
-        ext._removeFromAllPageMaps(tabId);
-
-        // When a sitekey header is received we must immediately update the page
-        // structure in order to record and use the key. We want to avoid
-        // trashing the page structure if the onCommitted event is then fired
-        // for the page.
-        if (eager)
-          eagerlyUpdatedPages.set(page, url);
-
-        chrome.tabs.get(tabId, () =>
-        {
-          // If the tab is prerendered, chrome.tabs.get() sets
-          // chrome.runtime.lastError and we have to dispatch the onLoading event,
-          // since the onUpdated event isn't dispatched for prerendered tabs.
-          // However, we have to keep relying on the unUpdated event for tabs that
-          // are already visible. Otherwise browser action changes get overridden
-          // when Chrome automatically resets them on navigation.
-          if (chrome.runtime.lastError)
-            ext.pages.onLoading._dispatch(page);
-        });
-      }
+        // If the tab is prerendered, chrome.tabs.get() sets
+        // chrome.runtime.lastError and we have to dispatch the onLoading event,
+        // since the onUpdated event isn't dispatched for prerendered tabs.
+        // However, we have to keep relying on the unUpdated event for tabs that
+        // are already visible. Otherwise browser action changes get overridden
+        // when Chrome automatically resets them on navigation.
+        if (chrome.runtime.lastError)
+          ext.pages.onLoading._dispatch(page);
+      });
     }
 
     // Update frame URL in frame structure
@@ -165,7 +144,36 @@
 
   chrome.webNavigation.onCommitted.addListener(details =>
   {
-    ext._updatePageFrameStructure(details.frameId, details.tabId, details.url);
+    updatePageFrameStructure(details.frameId, details.tabId, details.url);
+  });
+
+  chrome.webRequest.onHeadersReceived.addListener(details =>
+  {
+    // Ideally we would only need the above chrome.webNavigation.onCommitted
+    // listener to update the page state but unfortunately pages can make web
+    // requests before their onCommitted event fires[1].
+    //   So for HTTP/S requests we can also use onHeadersReceived, being careful
+    // to ignore responses which won't directly result in a navigation, for
+    // example redirections.
+    // [1] - https://bugs.chromium.org/p/chromium/issues/detail?id=665843
+    /****    FIXME - Check Chromium code to ensure we got these right!     ****/
+    if (!(details.statusCode > 299 && details.statusCode < 400 &&
+         details.statusCode != 304) || details.statusCode == 204)
+      updatePageFrameStructure(details.frameId, details.tabId, details.url);
+  }, {types: ["main_frame", "sub_frame"], urls: ["http://*/*", "https://*/*"]});
+
+  chrome.webNavigation.onBeforeNavigate.addListener(details =>
+  {
+    // Capture parent frame here because onCommitted doesn't get this info.
+    let frame = createFrame(details.tabId, details.frameId);
+    frame.parent = framesOfTabs[details.tabId][details.parentFrameId] || null;
+
+    // Since we can only listen for HTTP/S requests with onHeadersReceived we
+    // must update the page structure here for other navigations which might
+    // result in further requests.
+    let url = new URL(details.url);
+    if (url.protocol == "about:" || url.protocol == "data:")
+      updatePageFrameStructure(details.frameId, details.tabId, details.url);
   });
 
   function forgetTab(tabId)
@@ -234,7 +242,7 @@
     },
     _queueChanges()
     {
-      chrome.tabs.get(this._tabId, function()
+      chrome.tabs.get(this._tabId, () =>
       {
         // If the tab is prerendered, chrome.tabs.get() sets
         // chrome.runtime.lastError and we have to delay our changes
@@ -256,7 +264,7 @@
         {
           this._applyChanges();
         }
-      }.bind(this));
+      });
     },
     _addChange(name, value)
     {