Issue 4599 - Update the page structure more eagerly

chrome/ext/background.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 105 matching lines @@
     if (!frames)
       frames = framesOfTabs[tabId] = Object.create(null);
 
     var frame = frames[frameId];
     if (!frame)
       frame = frames[frameId] = {};
 
     return frame;
   }
 
-  chrome.webNavigation.onBeforeNavigate.addListener(function(details)
-  {
-    // Capture parent frame here because onCommitted doesn't get this info.
-    var frame = createFrame(details.tabId, details.frameId);
-    frame.parent = framesOfTabs[details.tabId][details.parentFrameId] || null;
-  });
-
   var eagerlyUpdatedPages = new ext.PageMap();
 
-  ext._updatePageFrameStructure = function(frameId, tabId, url, eager)
+  ext._updatePageFrameStructure = function(frameId, tabId, url, eager,
+                                           parentFrameId)
   {
     if (frameId == 0)
     {
       let page = new Page({id: tabId, url: url});
 
       if (eagerlyUpdatedPages.get(page) != url)
       {
         ext._removeFromAllPageMaps(tabId);
 
         // When a sitekey header is received we must immediately update the page
@@ skipping 13 matching lines @@
           // when Chrome automatically resets them on navigation.
           if (chrome.runtime.lastError)
             ext.pages.onLoading._dispatch(page);
         });
       }
     }
 
     // Update frame URL in frame structure
     var frame = createFrame(tabId, frameId);
     frame.url = new URL(url);
+
+    // We only get the parentFrameId for onBeforeNavigate, not for onCommitted
+    if (typeof parentFrameId != "undefined")
+      frame.parent = framesOfTabs[tabId][parentFrameId] || null;
   };
 
+  chrome.webNavigation.onBeforeNavigate.addListener(function(details)
+  {
+    // Since requests are sometimes made by a page before its onCommitted event
+    // fires we also need to update the page structure here. Otherwise those
+    // requests will be considered third party since a URL for the page wasn't
+    // yet determined.
+    ext._updatePageFrameStructure(details.frameId, details.tabId,
+                                  details.url, false, details.parentFrameId);

[Wladimir Palant, 2016/11/14 08:18:44]

You are essentially reverting changes from issue 4386, the onCommitted handler
no longer serves any purpose. This is bad: onBeforeNavigate is often called when
no navigation occurs in the end. In this scenario you will end up misattributing
all requests from a tab to a page that never loaded.

+  });
+
   chrome.webNavigation.onCommitted.addListener(function(details)
   {
-    ext._updatePageFrameStructure(details.frameId, details.tabId, details.url);
+    ext._updatePageFrameStructure(details.frameId, details.tabId,
+                                  details.url, false);
   });
 
   function forgetTab(tabId)
   {
     ext.pages.onRemoved._dispatch(tabId);
 
     ext._removeFromAllPageMaps(tabId);
     delete framesOfTabs[tabId];
   }
 
@@ skipping 455 matching lines @@
   ext.windows = {
     create: function(createData, callback)
     {
       chrome.windows.create(createData, function(createdWindow)
       {
         afterTabLoaded(callback)(createdWindow.tabs[0]);
       });
     }
   };
 })();