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

lib/requestBlocker.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 58 matching lines @@
   // WEBRTC gets addressed through a workaround, even if the webRequest API is
   // lacking support to block this kind of a request.
   yield "WEBRTC";
 
   // POPUP, CSP and ELEMHIDE filters aren't mapped to resource types.
   yield "POPUP";
   yield "ELEMHIDE";
   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.
   // If tabId is -1, its not (e.g. the request was sent by
   // a Service/Shared Worker) and we have to identify the related tabs.
   if (details.tabId != -1)
     return Promise.resolve([details.tabId]);
 
   let url;                    // Firefox provides "originUrl" indicating the
   if (details.originUrl)      // URL of the tab that caused this request.
     url = details.originUrl;  // In case of Service/Shared Worker, this is the
                               // URL of the tab that caused the worker to spawn.
 
   else if (details.initiator)        // Chromium >=63 provides "intiator" which
     url = details.initiator + "/*";  // is equivalent to "originUrl" on Firefox
                                      // except that its not a full URL but just
                                      // an origin (proto + host).
   else
     return Promise.resolve([]);
 
   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 =>
 {
   // Never block top-level documents.
   if (details.type == "main_frame")
     return;
 
   // Filter out requests from non web protocols. Ideally, we'd explicitly
   // specify the protocols we are interested in (i.e. http://, https://,
@@ skipping 41 matching lines @@
       // has triggered this request. For most requests (e.g. images) we
       // can just use the request's frame ID, but for subdocument requests
       // (e.g. iframes) we must instead use the request's parent frame ID.
       details.type == "sub_frame" ? details.parentFrameId : details.frameId
     );
   }
 
   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};
 }, {urls: ["<all_urls>"]}, ["blocking"]);
 
 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)
     {
       if (filter.collapse != null)
         return filter.collapse;
       blocked = true;
     }
   }
 
   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)
 {
   // Avoid triggering filters.behaviorChanged multiple times
   // when multiple filter hanges happen at the same time.
   if (ignoreFilterNotifications)
     return;
 
   // Ignore disabled subscriptions and filters, unless they just got
@@ skipping 21 matching lines @@
 }
 
 FilterNotifier.on("subscription.added", onFilterChange);
 FilterNotifier.on("subscription.removed", onFilterChange);
 FilterNotifier.on("subscription.updated", onFilterChange);
 FilterNotifier.on("subscription.disabled", arg => onFilterChange(arg, true));
 FilterNotifier.on("filter.added", onFilterChange);
 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);
-});