Issue 5042 - Adds handling for requests which are not associated with browser tab

lib/devtools.js

Index: lib/devtools.js
===================================================================
--- a/lib/devtools.js
+++ b/lib/devtools.js
@@ -29,20 +29,27 @@
 
 const nonRequestTypes = ["DOCUMENT", "ELEMHIDE", "GENERICBLOCK", "GENERICHIDE"];
 
 // Mapping of inspected tabs to their devpanel page
 // and recorded items. We can't use a PageMap here,
 // because data must persist after navigation/reload.
 let panels = new Map();
 
+function isActivePanel(panel)
+{
+  if (panel && !panel.reload && !panel.reloading)

[Sebastian Noack, 2017/05/30 11:10:49]

The if/else is redundant, You can just return the expression:

  return !!panel && !panel.reload && !panel.reloading;

[Jon Sonesen, 2017/05/31 08:28:17]

Acknowledged.

+    return true;
+  return false;
+}
+
 function getActivePanel(page)
 {
   let panel = panels.get(page.id);
-  if (panel && !panel.reload && !panel.reloading)
+  if (isActivePanel(panel))
     return panel;
   return null;
 }
 
 function getFilterInfo(filter)
 {
   if (!filter)
     return null;
@@ -126,22 +133,29 @@
  * @param {?boolean} specificOnly  Whether generic filters should be ignored

[Sebastian Noack, 2017/05/30 11:10:48]

Please replace {Page} with {?Page} above, in order to indicate that this
argument can be null, and update the description for the page argument to
consider that scenario.

[Jon Sonesen, 2017/05/31 08:28:17]

Sure, does this mean it should be moved down to where the other optional
parameters are defined as well?

[Sebastian Noack, 2017/05/31 08:35:01]

No. The arguments should be listed in the order they are passed to the function.

  * @param {?BlockingFilter} filter The matched filter or null if there is no
  *                                 match
  */
 exports.logRequest = function(page, url, type, docDomain,
                               thirdParty, sitekey,
                               specificOnly, filter)
 {
-  let panel = getActivePanel(page);
-  if (panel)
+  if (panels.size == 0)
+    return;
+  let request = {url, type, docDomain, thirdParty, sitekey, specificOnly};
+  if (page)
   {
-    let request = {url, type, docDomain, thirdParty, sitekey, specificOnly};
-    addRecord(panel, request, filter);
+    let panel = getActivePanel(page);
+    if (panel)
+      addRecord(panel, request, filter);
+    return;
   }
+  for (let panel of panels.values())
+    if (isActivePanel(panel))
+      addRecord(panel, request, filter);

[Sebastian Noack, 2017/05/30 11:10:49]

This would be somewhat less efficient, in case you have a lot of devtools panels
open, but simplifies the code and minimizes the diverge a bit:

  for (let [tabId, panel] of panels)
    if ((!page || page.id == tabId) && isActivePanel(panel))
      addRecord(panel, request, filter);

What do you think?

[Jon Sonesen, 2017/05/31 08:28:18]

Yeah I like this, will do. Thanks.

 };
 
 /**
  * Logs active element hiding filters to the devtools panel.
  *
  * @param {Page}     page       The page the elements were hidden on
  * @param {string[]} selectors  The selectors of applied ElemHideFilters
  * @param {string[]} filters    The text of applied ElemHideEmulationFilters