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

chrome/ext/background.js

Index: chrome/ext/background.js
===================================================================
--- a/chrome/ext/background.js
+++ b/chrome/ext/background.js
@@ -354,27 +354,33 @@
       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)