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

lib/devtools.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 12 matching lines @@
 const {SpecialSubscription} = require("subscriptionClasses");
 const {FilterStorage} = require("filterStorage");
 const {defaultMatcher} = require("matcher");
 const {FilterNotifier} = require("filterNotifier");
 const {extractHostFromFrame} = require("url");
 const {port} = require("messaging");
 
 const nonRequestTypes = ["DOCUMENT", "ELEMHIDE",
                          "GENERICBLOCK", "GENERICHIDE", "CSP"];
 
-// Mapping of inspected tabs to their devpanel page
-// and recorded items. We can't use a PageMap here,

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

Maybe remove/change this comment since it wouldn't be consistent to use a
PageMap anymore anyway?

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

Well spotted. Done.

-// because data must persist after navigation/reload.
 let panels = new Map();
 
 function isActivePanel(panel)
 {
   return panel && !panel.reload && !panel.reloading;
 }
 
 function getActivePanel(tabId)
 {
   let panel = panels.get(tabId);
@@ skipping 301 matching lines @@
     localOnBeforeRequest,
     {
       urls: ["http://*/*", "https://*/*"],
       types: ["main_frame"],
       tabId: inspectedTabId
     }
   );
 
   if (panels.size == 0)
   {
     ext.pages.onLoading.addListener(onLoading);

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

What about these? Well I don't mind if we remove them later, but in which case I
guess the codereview title (commit message) should be tweaked?

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

There is quite some magic around the ext.pages.onLoading event. Hence there is
no trivial replacement using the plain browser.* API. It might be possible
though that not all of that magic is necessary for the devtools panel, but I
would have to verify that. So I'd rather get rid of ext.page.onLoading
(potentially not only here) with a separate change. But I updated the review
title (and commit message) to be more specific, as you requested.

     FilterNotifier.on("filter.added", onFilterAdded);
     FilterNotifier.on("filter.removed", onFilterRemoved);
     FilterNotifier.on("subscription.added", onSubscriptionAdded);
   }
 
   newPort.onDisconnect.addListener(() =>
   {
     panels.delete(inspectedTabId);
     browser.webRequest.onBeforeRequest.removeListener(localOnBeforeRequest);
 
     if (panels.size == 0)
     {
       ext.pages.onLoading.removeListener(onLoading);
       FilterNotifier.off("filter.added", onFilterAdded);
       FilterNotifier.off("filter.removed", onFilterRemoved);
       FilterNotifier.off("subscription.added", onSubscriptionAdded);
     }
   });
 
   panels.set(inspectedTabId, {port: newPort, records: []});
 });
 
 port.on("devtools.traceElemHide", (message, sender) =>
 {
   logHiddenElements(
     sender.page.id, message.selectors, message.filters,
     extractHostFromFrame(sender.frame)
   );
 });