Allow additional headers for WebSocket connect

sdk/lib/io/websocket.dart

Index: sdk/lib/io/websocket.dart
diff --git a/sdk/lib/io/websocket.dart b/sdk/lib/io/websocket.dart
index 327ef3aafff218300dd82a20dcf20ac8326d8305..5de7f13a4d1969d77074e0aa94ce88d78ed207aa 100644
--- a/sdk/lib/io/websocket.dart
+++ b/sdk/lib/io/websocket.dart
@@ -126,12 +126,30 @@ abstract class WebSocket implements Stream, StreamSink {
 
   /**
    * Create a new web socket connection. The URL supplied in [url]
-   * must use the scheme [:ws:] or [:wss:]. The [protocols] argument is
-   * specifying the subprotocols the client is willing to speak.
+   * must use the scheme `ws` or `wss`.
+   *
+   * The [protocols] argument is specifying the subprotocols the
+   * client is willing to speak.
+   *
+   * If [onRequest] is provided this function will be called with the

[Lasse Reichstein Nielsen, 2015/01/12 15:11:56]

I don't like the "onRequest" name. Its too vague.
I also don't understand the HTTP-to-WS upgrade protocol, so I'm hard pressed to
find a more suitable word.

It seems that the user of this function needs to understand the underlying
protocol in order to understand the parameter. That is: I read the
documentation, and don't get any smarter. What is this "HTTP upgrade" you are
talking about? 

Either specify as much of that protocol as necessary in the documentation, or
find a way to reference it somewhere else.

[Søren Gjesse, 2015/01/12 15:36:34]

Changed to pass a map of additional headers

+   * [HttpClientRequest] used to request the HTTP upgrade to the web
+   * socket protocol just before the request is actually sent. Use
+   * this if e.g. additional headers are required by the server. The

[Lasse Reichstein Nielsen, 2015/01/12 15:11:56]

Commas around "e.g.": "this if, e.g., additional"...

[Søren Gjesse, 2015/01/12 15:36:34]

Rephrased comment.

+   * following headers which are already set on the request should not
+   * be changed.
+   *
+   *     connection: Upgrade
+   *     sec-websocket-version: 13
+   *     sec-websocket-key: ...
+   *     upgrade: websocket

[Lasse Reichstein Nielsen, 2015/01/12 15:11:56]

Then make sure they aren't. If it's only those four, it should be simple to
check and throw if the user tries to change them. You might need a wrapper
object if the original request can't be locked down.

[Søren Gjesse, 2015/01/12 15:36:34]

Changed to pass a map of additional headers, and documented that these headers
are ignored.

+   *
+   * If `onRequest` returns a `Future` the request is not sent until that
+   * `Future` completes.
    */
   static Future<WebSocket> connect(String url,
-                                   {List<String> protocols: const []}) =>
-      _WebSocketImpl.connect(url, protocols);
+                                   {List<String> protocols: const [],

[Lasse Reichstein Nielsen, 2015/01/12 15:11:56]

Why is "protocols" a List and not an Iterable?
I could see the use for, e.g., a Set or the keys/values of a Map.

[Søren Gjesse, 2015/01/12 15:36:34]

No particular reason. Changed to Iterable.

+                                    Function onRequest}) =>

[kustermann, 2015/01/09 14:45:12]

change the signature to this:

onRequest(HttpClientRequest request)

?

[Søren Gjesse, 2015/01/12 15:36:34]

Changed to not use callback.

+      _WebSocketImpl.connect(url, protocols, onRequest);
 
   @Deprecated('This constructor will be removed in Dart 2.0. Use `implements`'
       ' instead of `extends` if implementing this abstract class.')