Issue 5027 - Use updated webRequest API for WebSocket blocking

lib/requestBlocker.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2017 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 134 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.websocket", (msg, sender) =>
+// Before Chrome 58, the webRequest API didn't intercept WebSocket connections,
+// (see https://crbug.com/129353). Hence we patched the WebSocket object as a
+// workaround. WebSocket connections detected by this workaround are handled
+// below.
+if (!("WEBSOCKET" in chrome.webRequest.ResourceType))
 {
-  if (ext.webRequest.hasOwnProperty("RequestType.WEBSOCKET"))

[Sebastian Noack, 2017/03/24 17:45:02]

Does that even work? I think this should rather be:

  'WEBSOCKET' in ext.webRequest.ResourceType

Also, do we even need to register that listener, if there is no WebSocket
support? How about this:

if ('WEBSOCKET' in ext.webRequest.ResourceType)
{
  port.on("request.websocket", (msg, sender) =>
  {
    ...
  });
}

[Jon Sonesen, 2017/03/26 12:38:55]

Thanks for pointing that out. I had tested this in a console but also, in
retrospect I did not check true and false expected results so it was always
giving me false but I never realized this. My apologies.

-    return;

[Jon Sonesen, 2017/03/24 13:04:43]

This will cause connections created in shared workers to no longer be blocked
due to the previous work around as discussed recently. This will probably need
to be concluded before landing these changes.

[Sebastian Noack, 2017/03/24 17:45:02]

Is that so? I'd rather assume that the code in lib/csp.js (that should be
changed as well) would cause WebSockets created in SharedWorkers to no longer
being blocked. As for the general question whether this is acceptable, let's
move this discussion to the issue tracker, where I just replied.

-  return ext.webRequest.onBeforeRequest._dispatch(
-    new URL(msg.url),
-    "WEBSOCKET",
-    sender.page,
-    sender.frame
-  ).indexOf(false) != -1;
-});
+  port.on("request.websocket", (msg, sender) =>
+  {
+    return ext.webRequest.onBeforeRequest._dispatch(
+      new URL(msg.url),
+      "WEBSOCKET",
+      sender.page,
+      sender.frame
+    ).indexOf(false) != -1;
+  });
+}