Issue 5184 - Support Firefox-specific webRequest types

lib/requestBlocker.js

Index: lib/requestBlocker.js
===================================================================
--- a/lib/requestBlocker.js
+++ b/lib/requestBlocker.js
@@ -24,30 +24,50 @@
 const {defaultMatcher} = require("matcher");
 const {FilterNotifier} = require("filterNotifier");
 const {Prefs} = require("prefs");
 const {checkWhitelisted, getKey} = require("whitelisting");
 const {stringifyURL, extractHostFromFrame, isThirdParty} = require("url");
 const {port} = require("messaging");
 const devtools = require("devtools");
 
-// Chrome can't distinguish between OBJECT_SUBREQUEST and OBJECT requests.
+// Chrome and Firefox (WebExtensions) can't distinguish between
+// OBJECT_SUBREQUEST and OBJECT requests.
 RegExpFilter.typeMap.OBJECT_SUBREQUEST = RegExpFilter.typeMap.OBJECT;
 
+let resourceTypeMapping = new Map([
+  ["beacon", "PING"],
+  ["imageset", "IMAGE"],
+  ["sub_frame", "SUBDOCUMENT"]
+]);

[Sebastian Noack, 2017/05/19 11:30:46]

How about using a mapping like below?

  let resourceTypes = new Map();
  for (let type in RegExpFilter.typeMap)
    resourceTypes.set(type.toLowerCase(), type);
  resourceTypes.set("beacon", "PING");
  resourceTypes.set("imageset", "IMAGE");
  resourceTypes.set("sub_frame", "SUBDOCUMENT");

Then in the onBeforeRequest handler:

  let mappedType = resourceTypes.get(type) || "OTHER";

  ...

  let filter = defaultMatcher.matchesAny(
    ...,
    RegExpFilter.typeMap[mappedType],
    ...
  );

  setTimeout(onBeforeRequestAsync, 0, ..., mappedType, ...);

This way we have two lookups again though, but:

* It seems to be overall less code.
* We don't have any duplicated logic (otherwise the code generating the
typeMasks mapping and the mapping for the devtools panel use redundant logic).
* We don't rely on chrome.webRequest.ResourceType. (Microsoft Edge actually
doesn't have ResourceType. It also seems to be generally more robust.)

What do you think?

[Manish Jethani, 2017/05/19 16:54:42]

Yes, that's two lookups now instead of one for the normal case, but it does
simplify the logic a bit. I've made the change.

+
+let typeMasks = new Map(
+  Object.keys(chrome.webRequest.ResourceType)
+  .map(typeKey => chrome.webRequest.ResourceType[typeKey])
+  .map(type => [type, RegExpFilter.typeMap[resourceTypeMapping.get(type)] ||
+                      RegExpFilter.typeMap[type.toUpperCase()] ||
+                      RegExpFilter.typeMap.OTHER])
+);
+
 function onBeforeRequestAsync(page, url, type, docDomain,
                               thirdParty, sitekey,
                               specificOnly, filter)
 {
   if (filter)
     FilterNotifier.emit("filter.hitCount", filter, 0, 0, page);
 
   if (devtools)
   {
+    let mappedType = resourceTypeMapping.get(type) || type.toUpperCase();
+    if (!RegExpFilter.typeMap[mappedType])
+      mappedType = "OTHER";
+
     devtools.logRequest(
-      page, url, type, docDomain,
+      page, url,
+      mappedType, docDomain,
       thirdParty, sitekey,
       specificOnly, filter
     );
   }
 }
 
 ext.webRequest.onBeforeRequest.addListener((url, type, page, frame) =>
 {
@@ -59,17 +79,17 @@
   let thirdParty = isThirdParty(url, docDomain);
   let sitekey = getKey(page, frame);
 
   let specificOnly = !!checkWhitelisted(
     page, frame, RegExpFilter.typeMap.GENERICBLOCK
   );
 
   let filter = defaultMatcher.matchesAny(
-    urlString, RegExpFilter.typeMap[type],
+    urlString, typeMasks.get(type),
     docDomain, thirdParty, sitekey, specificOnly
   );
 
   setTimeout(onBeforeRequestAsync, 0, page, urlString,
                                       type, docDomain,
                                       thirdParty, sitekey,
                                       specificOnly, filter);