Issue 1727 - Fix WebSocket constructor without second argument in Chrome 47

include.preload.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 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 132 matching lines @@
       type: "filters.collapse",
       urls: urls,
       mediatype: mediatype,
       baseURL: document.location.href
     },
 
     function(collapse)
     {
       function collapseElement()
       {
+        var propertyName = "display";
+        var propertyValue = "none";
         if (element.localName == "frame")
-          element.style.setProperty("visibility", "hidden", "important");
-        else
-          element.style.setProperty("display", "none", "important");
-      }
-
-      if (collapse && !element._collapsed)
+        {
+          propertyName = "visibility";
+          propertyValue = "hidden";
+        }
+
+        if (element.style.getPropertyValue(propertyName) != propertyValue ||
+            element.style.getPropertyPriority(propertyName) != "important")
+          element.style.setProperty(propertyName, propertyValue, "important");
+      }
+
+      if (collapse)
       {
         collapseElement();
-        element._collapsed = true;
 
         if (MutationObserver)
           new MutationObserver(collapseElement).observe(
             element, {
               attributes: true,
               attributeFilter: ["style"]
             }
           );
       }
     }
@@ skipping 193 matching lines @@
         new CustomEvent(eventName + "-" + event.detail.url, {detail: block})
       );
     });
   });
 
   runInDocument(document, function(eventName)
   {
     // As far as possible we must track everything we use that could be
     // sabotaged by the website later in order to circumvent us.
     var RealWebSocket = WebSocket;
-    var bindWebSocket = Function.prototype.bind.apply.bind(Function.prototype.bind, RealWebSocket);
     var closeWebSocket = Function.prototype.call.bind(RealWebSocket.prototype.close);
     var addEventListener = document.addEventListener.bind(document);
     var removeEventListener = document.removeEventListener.bind(document);
     var dispatchEvent = document.dispatchEvent.bind(document);
     var CustomEvent = window.CustomEvent;
 
     function checkRequest(url, callback)
     {
       var incomingEventName = eventName + "-" + url;
       function listener(event)
       {
         callback(event.detail);
         removeEventListener(incomingEventName, listener);
       }
       addEventListener(incomingEventName, listener);
 
       dispatchEvent(new CustomEvent(eventName, {
         detail: {url: url}
       }));
     }
 
-    WebSocket = function WrappedWebSocket()
-    {
-      var args = [null];

[kzar, 2016/08/11 14:25:05]

I think this approach is extremely hard to follow, when something way simpler
would do:

```
if (!(this instanceof WrappedWebSocket)) return RealWebSocket();
if (arguments.length == 0) return new RealWebSocket();

var websocket;
if (arguments.length == 1) websocket = new RealWebSocket(url);
else if (arguments.length == 2) websocket = new RealWebSocket(url, protocols);
```

Sure it has a little duplication and isn't as clever, but is that such a bad
thing? I mean bindWebSocket for example is getting nearly impenetrable to
follow, good luck trying to figure out what
`Function.prototype.bind.apply.bind(Function.prototype.bind, RealWebSocket);`
does! The logic below with the creator is hardly easy to follow either.

[Sebastian Noack, 2016/08/11 14:38:46]

Well, if you'd write your logic compliant to our coding styles (add newlines
where appropriate) it wouldn't be any shorter, however it has quite a few
assumptions, and it's virtually impossible to make sure those hold true for all
current and future supported browser versions. The pure fact that we'd have to
extend that logic already, is a clear indication that this was a bad idea in the
first place.

My code on the other hand makes sure that the original WebSocket function is
called in the exact same way our wrapper is called, delegating the whole
behavior to the browser.

[kzar, 2016/08/11 14:54:16]

I didn't say anything about the code being shorter, but simpler. As far as I see
it we can fix the problem you spotted by simply adding a check to see if the
arguments' length is 1, I just don't agree about instead doing something so
complicated and hard to follow.

The WebSocket constructor takes two arguments url and protocol, we know this.
Maybe one day in the future that will change, but maybe one day in the future
Chrome will support interception of WebSocket connections anyway. Since when do
we do things in case it's one day useful?

[Sebastian Noack, 2016/08/11 15:11:35]

I don't think that I agree. But there you go.

-      for (var i = 0; i < arguments.length; i++)
-        args.push(arguments[i]);

[kzar, 2016/08/11 14:25:05]

This would allow for circumvention since we didn't keep track of
Array.prototype.push, but like I said above I think we should rather keep this
simple.

[Sebastian Noack, 2016/08/11 14:38:46]

Well spotted, fixed.

-
-      var ctor = bindWebSocket(args);
-      var websocket = this instanceof WrappedWebSocket ? new ctor : ctor();
+    WebSocket = function WrappedWebSocket(url, protocols)
+    {
+      // Throw correct exceptions if the constructor is used improperly.
+      if (!(this instanceof WrappedWebSocket)) return RealWebSocket();
+      if (arguments.length < 1) return new RealWebSocket();
+
+      var websocket;
+      if (arguments.length == 1)
+        websocket = new RealWebSocket(url);
+      else
+        websocket = new RealWebSocket(url, protocols);

[Wladimir Palant, 2016/08/15 12:33:24]

Why make assumptions about the arguments in the first place? What if a third
parameter is added to the spec?

  var websocket = Object.create(RealWebSocket.prototype);
  RealWebSocket.apply(websocket, arguments);

[Wladimir Palant, 2016/08/15 12:45:36]

This will also allow dropping protocols parameter from the function signature -
then WebSocket.length will match the real thing.

[Wladimir Palant, 2016/08/15 13:01:24]

Never mind, calling RealWebSocket.apply() won't work no matter what you pass in
- detecting function call vs. new there doesn't work by checking this pointer.
So avoiding assumptions about parameters here requires spread operator which
sadly isn't supported before Chrome 46 or Safari 7.1, not worth it.

[Sebastian Noack, 2016/08/15 13:06:10]

See patchset #2 (and the related discussin) for a compatible way to pass
variable arguments.

 
       checkRequest(websocket.url, function(blocked)
       {
         if (blocked)
           closeWebSocket(websocket);
       });
 
       return websocket;
     }.bind();
 
@@ skipping 194 matching lines @@
   }, true);
 
   return updateStylesheet;
 }
 
 if (document instanceof HTMLDocument)
 {
   checkSitekey();
   window.updateStylesheet = init(document);
 }