Issue 6586 - Fixed WebRTC wrapper after removing ext.webRequest.onBeforeRequest

lib/requestBlocker.js

Index: lib/requestBlocker.js
===================================================================
--- a/lib/requestBlocker.js
+++ b/lib/requestBlocker.js
@@ -76,6 +76,29 @@
   yield "CSP";
 }());
 
+function getDocumentInfo(page, frame, originUrl)
+{
+  return [
+    extractHostFromFrame(frame, originUrl),
+    getKey(page, frame, originUrl),
+    !!checkWhitelisted(page, frame, originUrl,
+                       RegExpFilter.typeMap.GENERICBLOCK)
+  ];
+}
+
+function matchRequest(url, type, docDomain, sitekey, specificOnly)
+{
+  let urlString = stringifyURL(url);
+  let thirdParty = isThirdParty(url, docDomain);
+
+  return [
+    defaultMatcher.matchesAny(urlString, RegExpFilter.typeMap[type],
+                              docDomain, thirdParty, sitekey, specificOnly),
+    urlString,
+    thirdParty
+  ];
+}
+
 function getRelatedTabIds(details)
 {
   // This is the common case, the request is associated with a single tab.
@@ -99,20 +122,17 @@
   return browser.tabs.query({url}).then(tabs => tabs.map(tab => tab.id));
 }
 
-function logRequest(details, url, type, docDomain, thirdParty,
+function logRequest(tabIds, url, type, docDomain, thirdParty,
                     sitekey, specificOnly, filter)
 {
-  getRelatedTabIds(details).then(tabIds =>
-  {
-    if (filter)
-      FilterNotifier.emit("filter.hitCount", filter, 0, 0, tabIds);
+  if (filter)
+    FilterNotifier.emit("filter.hitCount", filter, 0, 0, tabIds);
 
-    devtools.logRequest(
-      tabIds, url, type, docDomain,
-      thirdParty, sitekey,
-      specificOnly, filter
-    );
-  });
+  devtools.logRequest(
+    tabIds, url, type, docDomain,
+    thirdParty, sitekey,
+    specificOnly, filter
+  );
 }
 
 browser.webRequest.onBeforeRequest.addListener(details =>
@@ -174,21 +194,17 @@
   if (checkWhitelisted(page, frame, originUrl))
     return;
 
-  let urlString = stringifyURL(url);
   let type = resourceTypes.get(details.type) || "OTHER";
-  let docDomain = extractHostFromFrame(frame, originUrl);
-  let thirdParty = isThirdParty(url, docDomain);
-  let sitekey = getKey(page, frame, originUrl);
-  let specificOnly = !!checkWhitelisted(page, frame, originUrl,
-                                        RegExpFilter.typeMap.GENERICBLOCK);
+  let [docDomain, sitekey, specificOnly] = getDocumentInfo(page, frame,

[kzar, 2018/04/17 12:46:39]

Perhaps it would be a nicer abstraction to have a function called matchRequests
which takes a collection of URLs and returns an iterator of the matches? It
could do the work of getDocumentInfo then, as well as optionally calling
logRequest.

[Sebastian Noack, 2018/04/17 12:56:47]

FWIW, I'm not too happy with this abstraction (just slightly more than repeating
myself 3 times in the same file). However, at least I'm pretty sure that this
way, the code just gets inlined by the JIT. If I move the loop in here, I'm
mildly concerned that this might effect the performance in the onBeforeRequest
listener. In particular when turning it into a generator, I know for sure that
this will cause the JIT to struggle.

As for calling logRequest(), I don't see how this can be generalized (in a
reasonable way), since the onBeforeRequest listener must call getRelatedTabIds()
first, while the message listener for the WebRTC wrapper must pass in page.id.

[kzar, 2018/04/17 13:54:06]

Yea, fair enough.

+                                                           originUrl);
+  let [filter, urlString, thirdParty] = matchRequest(url, type, docDomain,
+                                                     sitekey, specificOnly);
 
-  let filter = defaultMatcher.matchesAny(
-    urlString, RegExpFilter.typeMap[type],
-    docDomain, thirdParty, sitekey, specificOnly
-  );
-
-  logRequest(details, urlString, type, docDomain,
-             thirdParty, sitekey, specificOnly, filter);
+  getRelatedTabIds(details).then(tabIds =>
+  {
+    logRequest(tabIds, urlString, type, docDomain,
+               thirdParty, sitekey, specificOnly, filter);
+  });
 
   if (filter instanceof BlockingFilter)
     return {cancel: true};
@@ -196,28 +212,19 @@
 
 port.on("filters.collapse", (message, sender) =>
 {
-  if (checkWhitelisted(sender.page, sender.frame))
+  let {page, frame} = sender;
+
+  if (checkWhitelisted(page, frame))
     return false;
 
-  let typeMask = RegExpFilter.typeMap[message.mediatype];
-  let documentHost = extractHostFromFrame(sender.frame);
-  let sitekey = getKey(sender.page, sender.frame);
   let blocked = false;
-
-  let specificOnly = checkWhitelisted(
-    sender.page, sender.frame, null,
-    RegExpFilter.typeMap.GENERICBLOCK
-  );
+  let [docDomain, sitekey, specificOnly] = getDocumentInfo(page, frame);
 
   for (let url of message.urls)
   {
-    let urlObj = new URL(url, message.baseURL);
-    let filter = defaultMatcher.matchesAny(
-      stringifyURL(urlObj),
-      typeMask, documentHost,
-      isThirdParty(urlObj, documentHost),
-      sitekey, specificOnly
-    );
+    let [filter] = matchRequest(new URL(url, message.baseURL),
+                                message.mediatype, docDomain,
+                                sitekey, specificOnly);
 
     if (filter instanceof BlockingFilter)
     {
@@ -230,6 +237,24 @@
   return blocked && Prefs.hidePlaceholders;
 });
 
+port.on("request.blockedByRTCWrapper", (msg, sender) =>
+{
+  let {page, frame} = sender;
+
+  if (checkWhitelisted(page, frame))
+    return false;
+
+  let [docDomain, sitekey, specificOnly] = getDocumentInfo(page, frame);
+  let [filter, url, thirdParty] = matchRequest(new URL(msg.url),
+                                               "WEBRTC", docDomain,
+                                               sitekey, specificOnly);
+
+  logRequest([sender.page.id], url, "WEBRTC", docDomain,
+             thirdParty, sitekey, specificOnly, filter);
+
+  return filter instanceof BlockingFilter;
+});
+
 let ignoreFilterNotifications = false;
 
 function onFilterChange(arg, isDisabledAction)
@@ -271,13 +296,3 @@
 FilterNotifier.on("filter.removed", onFilterChange);
 FilterNotifier.on("filter.disabled", arg => onFilterChange(arg, true));
 FilterNotifier.on("load", onFilterChange);
-
-port.on("request.blockedByRTCWrapper", (msg, sender) =>
-{
-  return ext.webRequest.onBeforeRequest._dispatch(
-     new URL(msg.url),
-     "webrtc",
-     sender.page,
-     sender.frame
-  ).includes(false);
-});