Issue 3453 - Adapt indistinguishable request types for changes in Chrome 49

chrome/ext/background.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 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 336 matching lines @@
     onBeforeRequest: new ext._EventTarget(),
     handlerBehaviorChanged: function()
     {
       // Defer handlerBehaviorChanged() until navigation occurs.
       // There wouldn't be any visible effect when calling it earlier,
       // but it's an expensive operation and that way we avoid to call
       // it multiple times, if multiple filters are added/removed.
       var onBeforeNavigate = chrome.webNavigation.onBeforeNavigate;
       if (!onBeforeNavigate.hasListener(propagateHandlerBehaviorChange))
         onBeforeNavigate.addListener(propagateHandlerBehaviorChange);
+    },
+    getIndistinguishableTypes: function()
+    {
+      // Chrome 38-48 mistakenly reports requests of type `object`
+      // (e.g. requests initiated by Flash) with the type `other`.
+      // https://code.google.com/p/chromium/issues/detail?id=410382
+      var match = navigator.userAgent.match(/\bChrome\/(\d+)/);

[kzar, 2015/12/23 14:43:20]

We already parse the application name and version in the info module,  how come
we're doing it again here?

[Sebastian Noack, 2015/12/23 14:46:56]

adblockplus.js (including the info module) depends on ext, and as ext is loaded
first, the adblockplus.js might not be avialable yet. Moreover, circular
dependencies are generally bad practice.

[kzar, 2015/12/23 14:56:42]

Acknowledged.

+      if (match)
+      {
+        var version = parseInt(match[1], 10);
+        if (version >= 38 && version <= 48)
+          return [["OTHER", "OBJECT", "OBJECT_SUBREQUEST"]];
+      }
+
+      // Before Chrome 49, requests of the type `font` and `ping`
+      // have been reported with the type `other`.
+      // https://code.google.com/p/chromium/issues/detail?id=410382
+      var otherTypes = ["OTHER", "MEDIA"];
+      if (!("FONT" in chrome.webRequest.ResourceType))
+        otherTypes.push("FONT");
+      if (!("PING" in chrome.webRequest.ResourceType))
+        otherTypes.push("PING");
+
+      return [["OBJECT", "OBJECT_SUBREQUEST"], otherTypes];

[Wladimir Palant, 2015/12/23 12:50:51]

This logic is very hard to follow, and I'm not really sure it is correct either.
I suggest you structure it like this:

  var types = [];
  var version = (match ? parseInt(match[1], 10) : Infinity);

  // bla-bla objects
  if (version >= 38 || version <= 48)
    types.push(["OTHER", "OBJECT", "OBJECT_SUBREQUEST"]);
  else
    types.push(["OBJECT", "OBJECT_SUBREQUEST"]);

  // bla-bla font and ping
  ...

  return types;

[Sebastian Noack, 2015/12/23 13:08:43]

This won't work. If the workaround for Chrome 38-48 is applied the
indistinguishable types should simply be [["OTHER", "OBJECT",
"OBJECT_SUBREQUEST"]]. Yes, that means that other types that cannot be
distinguished (i.e. media and font) are ignored rather than mapped. However,
this has been the behavior since we used that workaround. And changing that
behavior would bear the risk of introducing false positives due to overly greedy
matching.

Also to be honest, I think that my implementation makes it more obvious that we
never apply the workaround if we cannot detect the Chromium version. That is
important for Kitt.

     }
   };
 
-  // Since Chrome 38 requests of type 'object' (e.g. requests
-  // initiated by Flash) are mistakenly reported with the type 'other'.
-  // https://code.google.com/p/chromium/issues/detail?id=410382
-  var match = navigator.userAgent.match(/\bChrome\/(\d+)/);
-  if (match && parseInt(match[1], 10) >= 38)
-  {
-    ext.webRequest.indistinguishableTypes = [
-      ["OTHER", "OBJECT", "OBJECT_SUBREQUEST"]
-    ];
-  }
-  else
-  {
-    ext.webRequest.indistinguishableTypes = [
-      ["OBJECT", "OBJECT_SUBREQUEST"],
-      ["OTHER", "MEDIA", "FONT"]
-    ];
-  }
-
   chrome.tabs.query({}, function(tabs)
   {
     tabs.forEach(function(tab)
     {
       chrome.webNavigation.getAllFrames({tabId: tab.id}, function(details)
       {
         if (details && details.length > 0)
         {
           var frames = framesOfTabs[tab.id] = Object.create(null);
 
@@ skipping 174 matching lines @@
             callback(new Page(tab));
         }
         else
         {
           ext.pages.open(optionsUrl, callback);
         }
       });
     });
   };
 })();