Issue 6191 - Update frame structure for Chrome Web Store pages

ext/background.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present 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 245 matching lines @@
                              details.parentFrameId);
   },
   {types: ["main_frame", "sub_frame"], urls: ["http://*/*", "https://*/*"]},
   ["responseHeaders"]);
 
   browser.webNavigation.onBeforeNavigate.addListener(details =>
   {
     // Since we can only listen for HTTP(S) responses using
     // webRequest.onHeadersReceived we must update the page structure here for
     // other navigations.
-    let url = new URL(details.url);
-    if ((url.protocol != "http:" && url.protocol != "https:") ||
-        // Chrome doesn't dispatch webRequest.onHeadersReceived for
-        // Web Store URLs.
-        url.href.startsWith("https://chrome.google.com/webstore/"))

[kzar, 2018/01/05 16:46:20]

I guess we should only do this for Chrome (or Chromium)? I wonder if there
should be a similar exception for Firefox for addons.mozilla.org?

I also worry if checking if the URL starts with that string for every navigation
could slow the extension down, what do you think?

[Manish Jethani, 2018/01/12 07:25:03]

Ideally yes, but it's probably OK if the frame structure is updated twice
(onBeforeNavigate and onHeadersReceived) on non-Chromium browser only for this
one URL. When we move this code into a module then we can use the info module to
narrow this check.
I checked and apparently it's not required.
I did some benchmarks using this code:

{ 
  let u = [];
  let n = 100000;
  let c = 0;
  for (let i = 0; i < n; i++)
    u.push("https://" + Math.random().toString(36).slice(2));
  let s = performance.now();
  for (let i = 0; i < n; i++)
  {
    let url = new URL(u[i]);
    if (url.protocol != "http:" && url.protocol != "https:" ||
        url.hostname.startsWith("ab"))
    //if (!/^https?:/.test(u[i]) || /^https:\/\/ab/.test(u[i]))
    //if (!u[i].startsWith("http:") && !u[i].startsWith("https:") ||
    //    u[i].startsWith("https://ab"))
      c++;
  }     
  let t = (performance.now() - s) | 0;
  console.log(c + " matches found in " + t + "ms");
}   

Using a URL object is too slow. If we use regular expressions, it's ten times as
fast. It gets even faster if we simply do a string comparison (twice as fast on
Firefox, about ten percent faster on Chrome).

So my most recent change would end up doing the additional comparison for every
single HTTP(S) URL, but at the same time it removes the URL object, which was
also created for every single URL and hence slowed things down. It's a good
bargain, I think.

[Manish Jethani, 2018/01/12 07:31:57]

By the way, if it wasn't for Edge, we could have used the additional "filter"
option [1] which I expect would be faster.

[1]:
https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/webNavigation/o...

-    {
-      updatePageFrameStructure(details.frameId, details.tabId, details.url,
+    let {url} = details;
+    if (!(url.startsWith("http:") ||
+          url.startsWith("https:") &&
+          // Chrome doesn't dispatch webRequest.onHeadersReceived
+          // for Web Store URLs.
+          !url.startsWith("https://chrome.google.com/webstore/")))
+    {
+      updatePageFrameStructure(details.frameId, details.tabId, url,
                                details.parentFrameId);
     }
   });
 
   function forgetTab(tabId)
   {
     ext.pages.onRemoved._dispatch(tabId);
 
     ext._removeFromAllPageMaps(tabId);
     framesOfTabs.delete(tabId);
@@ skipping 406 matching lines @@
   ext.windows = {
     create(createData, callback)
     {
       browser.windows.create(createData, createdWindow =>
       {
         afterTabLoaded(callback)(createdWindow.tabs[0]);
       });
     }
   };
 }