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

ext/background.js

Index: ext/background.js
===================================================================
--- a/ext/background.js
+++ b/ext/background.js
@@ -561,50 +561,60 @@
     });
   });
 
   chrome.webRequest.onBeforeRequest.addListener(details =>
   {
     // The high-level code isn't interested in requests that aren't
     // related to a tab or requests loading a top-level document,

[Sebastian Noack, 2017/04/21 08:39:53]

This comment needs to be adapted too.

[Jon Sonesen, 2017/04/24 08:40:55]

Acknowledged.

     // those should never be blocked.
-    if (details.tabId == -1 || details.type == "main_frame")
+    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://,
     // ws:// and wss://) with the url patterns, given below, when adding this
     // listener. But unfortunately, Chrome <=57 doesn't support the WebSocket
     // protocol and is causing an error if it is given.
     let url = new URL(details.url);
     if (url.protocol != "http:" && url.protocol != "https:" &&
         url.protocol != "ws:" && url.protocol != "wss:")
       return;
 
     // 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.
-    let {frameId, type} = details;

[Sebastian Noack, 2017/04/21 08:39:53]

If you remove the variable declaration here, the type == "sub_frame" branch
below, will define frameId and type as global variables, which then get shadowed
when you declare these variables below.

[Jon Sonesen, 2017/04/24 08:40:55]

Acknowledged.

     if (type == "sub_frame")
     {
       frameId = details.parentFrameId;
       type = "SUBDOCUMENT";
     }
 
-    let frame = ext.getFrame(details.tabId, frameId);
-    if (frame)
+    let {frameId, type} = details;
+
+    // Sometimes requests are not associated with a browser tab and
+    // in this case we want to still be able to view the url being called.
+    // However, since tabId's are assumed to be unique elsewhere in the code

[Sebastian Noack, 2017/04/21 08:39:53]

IMO, the second sentence is rather confusing, and not necessary here.

[Jon Sonesen, 2017/04/24 08:40:55]

Acknowledged.

+    // it seems safer to initialize frame and page as null and then only
+    // initiate a page, or fetch a frame if we know it will have a unique tabId.
+    let frame = null;
+    let page = null;
+    if (details.tabId != -1)
     {
-      let results = ext.webRequest.onBeforeRequest._dispatch(
-        url, type.toUpperCase(), new Page({id: details.tabId}), frame
-      );
+      frame = ext.getFrame(details.tabId, frameId);
+      page = new Page({id: details.tabId});
+    }
 
-      if (results.indexOf(false) != -1)
-        return {cancel: true};
-    }
+    let results = ext.webRequest.onBeforeRequest._dispatch(
+      url, type.toUpperCase(), page, frame
+    );
+
+    if (results.indexOf(false) != -1)

[Sebastian Noack, 2017/04/21 08:39:53]

Please use results.includes(false), which is equivalent to, but more readable
than results.indexOf(false) != -1. You might also be able to get rid of the
temporary variable then:

  if (ext.webRequest.onBeforeRequest._dispatch(url, type.toUpperCase(),
                                               page, frame).includes(false))

[Jon Sonesen, 2017/04/24 08:40:56]

Acknowledged.

+      return {cancel: true};
   }, {urls: ["<all_urls>"]}, ["blocking"]);
 
 
   /* Message passing */
 
   chrome.runtime.onMessage.addListener((message, rawSender, sendResponse) =>
   {
     let sender = {};