Issue 4580 - Make filter/request logging use plain tabIds, prepare for multi-tab requests

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 55 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 onBeforeRequestAsync(page, url, type, docDomain,
+function onBeforeRequestAsync(tabId, url, type, docDomain,
                               thirdParty, sitekey,
                               specificOnly, filter)
 {
+  let tabIds = tabId != -1 ? [tabId] : [];

[kzar, 2018/04/03 17:14:03]

Nit: Seems like switching the order and using == would be slightly easier to
grok?

[Sebastian Noack, 2018/04/04 01:50:33]

If you think in numbers, yes, this way around seems slightly backwards:

  ~is(id, -1) => use(id)

However, if you think in terms of what "tabId != -1" actually means, the logic
seems simpler to follow this way:

  valid(id) => use(id)

Also it doesn't matter much, for the time being as my next patch under review is
already removing this line again anyway.

+
   if (filter)
-    FilterNotifier.emit("filter.hitCount", filter, 0, 0, page);
+    FilterNotifier.emit("filter.hitCount", filter, 0, 0, tabIds);
 
-  if (devtools)
-  {
-    devtools.logRequest(
-      page, 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 17 matching lines @@
 
   let frame = ext.getFrame(
     details.tabId,
     // We are looking for the frame that contains the element which
     // 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
   );
 
-  let page = null;
   let docDomain = null;
   let sitekey = null;
   let thirdParty = false;
   let specificOnly = false;
 
   if (frame)
   {
-    page = new ext.Page({id: details.tabId});
+    let page = new ext.Page({id: details.tabId});
 
     if (checkWhitelisted(page, frame))
       return;
 
     docDomain = extractHostFromFrame(frame);
     sitekey = getKey(page, frame);
     thirdParty = isThirdParty(url, docDomain);
     specificOnly = !!checkWhitelisted(page, frame,
                                       RegExpFilter.typeMap.GENERICBLOCK);
   }
 
   let urlString = stringifyURL(url);
   let type = resourceTypes.get(details.type) || "OTHER";
   let filter = defaultMatcher.matchesAny(
     urlString, RegExpFilter.typeMap[type],
     docDomain, thirdParty, sitekey, specificOnly
   );
 
-  setTimeout(onBeforeRequestAsync, 0, page, urlString,
+  setTimeout(onBeforeRequestAsync, 0, details.tabId, urlString,
                                       type, docDomain,
                                       thirdParty, sitekey,
                                       specificOnly, filter);
 
   if (filter instanceof BlockingFilter)
     return {cancel: true};
 }, {urls: ["<all_urls>"]}, ["blocking"]);
 
 port.on("filters.collapse", (message, sender) =>
 {
@@ skipping 75 matching lines @@
 
 port.on("request.blockedByRTCWrapper", (msg, sender) =>
 {
   return ext.webRequest.onBeforeRequest._dispatch(
      new URL(msg.url),
      "webrtc",
      sender.page,
      sender.frame
   ).includes(false);
 });