Add blink::WebSocketHandshakeThrottle

Add blink::WebSocketHandshakeThrottle

Add a facility for WebSocket handshakes to be delayed or cancelled by the
embedder. By implementing blink::Platform::CreateWebSocketHandshakeThrottle()
the embedder can supply an implementation of blink::WebSocketHandshakeThrottle
to throttle WebSocket handshakes.

To avoid slowing down the normal case, the throttling runs in parallel with the
handshake. Only access to the new connection from Javascript is delayed.

The default behaviour is to have no throttling.

The implementation is in
blink::DocumentWebSocketChannel. DocumentWebSocketChannelTest has new tests for
the new functionality.

This is part of the implementation of Safe Browsing for WebSockets. See the
design doc at
https://docs.google.com/document/d/1iR3XMIQukqlXb6ajIHE91apHZAxyF_wvRoB5JGeJYPs/edit

BUG=644744
Review-Url: https://codereview.chromium.org/2930263002
Cr-Commit-Position: refs/heads/master@{#479709}
Committed: https://chromium.googlesource.com/chromium/src/+/bf9d1c2f8e5ea0366b4b086c7d0541bdc15ab176

[Adam Rice, 2017-06-12 07:12:07]

ricea@chromium.org changed reviewers:
+ yhirano@chromium.org

[yhirano, 2017-06-13 09:14:39]

https://codereview.chromium.org/2930263002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.cpp
(right):

https://codereview.chromium.org/2930263002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.cpp:146:
String selected_protocol;
const

https://codereview.chromium.org/2930263002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.cpp:147:
String extensions;
ditto

https://codereview.chromium.org/2930263002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.cpp:737:
void DocumentWebSocketChannel::OnSuccess() {
DCHECK(!throttle_passed);

https://codereview.chromium.org/2930263002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.cpp:747:
void DocumentWebSocketChannel::OnError(const WebString& console_message) {
DCHECK(!throttle_passed);

https://codereview.chromium.org/2930263002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.h
(right):

https://codereview.chromium.org/2930263002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.h:78:
WebSocketHandle* handle = 0) {
Please remove this parameter. It's only for tests, and you are introducing
ForTesting.

https://codereview.chromium.org/2930263002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.h:86:
WebSocketHandle* = 0);
ditto

[Adam Rice, 2017-06-13 12:53:42]

https://codereview.chromium.org/2930263002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.cpp
(right):

https://codereview.chromium.org/2930263002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.cpp:146:
String selected_protocol;
On 2017/06/13 09:14:39, yhirano wrote:
> const

Done.

https://codereview.chromium.org/2930263002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.cpp:147:
String extensions;
On 2017/06/13 09:14:39, yhirano wrote:
> ditto

Done.

https://codereview.chromium.org/2930263002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.cpp:737:
void DocumentWebSocketChannel::OnSuccess() {
On 2017/06/13 09:14:39, yhirano wrote:
> DCHECK(!throttle_passed);

Done. I also added a check that handshake_throttle_ is non-null.

https://codereview.chromium.org/2930263002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.cpp:747:
void DocumentWebSocketChannel::OnError(const WebString& console_message) {
On 2017/06/13 09:14:39, yhirano wrote:
> DCHECK(!throttle_passed);

Done.

https://codereview.chromium.org/2930263002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.h
(right):

https://codereview.chromium.org/2930263002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.h:78:
WebSocketHandle* handle = 0) {
On 2017/06/13 09:14:39, yhirano wrote:
> Please remove this parameter. It's only for tests, and you are introducing
> ForTesting.

Cool, thanks, done.

https://codereview.chromium.org/2930263002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/websockets/DocumentWebSocketChannel.h:86:
WebSocketHandle* = 0);
On 2017/06/13 09:14:39, yhirano wrote:
> ditto

Done.

[yhirano, 2017-06-14 11:55:39]

lgtm

[Adam Rice, 2017-06-15 09:36:47]

ricea@chromium.org changed reviewers:
+ haraken@chromium.org

[Adam Rice, 2017-06-15 09:36:48]

+haraken for blink platform.

[haraken, 2017-06-15 12:41:46]

platform/ LGTM

(I'm assuming that you'll add actual implementation of
WebSocketHandshakeThrottle to the content/ side in a follow-up CL.)

[Adam Rice, 2017-06-15 13:05:59]

The CQ bit was checked by ricea@chromium.org

[commit-bot: I haz the power, 2017-06-15 13:06:04]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Adam Rice, 2017-06-15 13:06:20]

Thanks for the review.

On 2017/06/15 12:41:46, haraken wrote:
> (I'm assuming that you'll add actual implementation of
> WebSocketHandshakeThrottle to the content/ side in a follow-up CL.)

Yes, I'm working on that now.

[commit-bot: I haz the power, 2017-06-15 15:19:57]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1497531959016240,
"parent_rev": "9895b1f7e22db3bcfe0743ad100383216c1d5b37", "commit_rev":
"bf9d1c2f8e5ea0366b4b086c7d0541bdc15ab176"}

[commit-bot: I haz the power, 2017-06-15 15:20:06]

Description was changed from

==========
Add blink::WebSocketHandshakeThrottle

Add a facility for WebSocket handshakes to be delayed or cancelled by the
embedder. By implementing blink::Platform::CreateWebSocketHandshakeThrottle()
the embedder can supply an implementation of blink::WebSocketHandshakeThrottle
to throttle WebSocket handshakes.

To avoid slowing down the normal case, the throttling runs in parallel with the
handshake. Only access to the new connection from Javascript is delayed.

The default behaviour is to have no throttling.

The implementation is in
blink::DocumentWebSocketChannel. DocumentWebSocketChannelTest has new tests for
the new functionality.

This is part of the implementation of Safe Browsing for WebSockets. See the
design doc at
https://docs.google.com/document/d/1iR3XMIQukqlXb6ajIHE91apHZAxyF_wvRoB5JGeJY...

BUG=644744
==========

to

==========
Add blink::WebSocketHandshakeThrottle

Add a facility for WebSocket handshakes to be delayed or cancelled by the
embedder. By implementing blink::Platform::CreateWebSocketHandshakeThrottle()
the embedder can supply an implementation of blink::WebSocketHandshakeThrottle
to throttle WebSocket handshakes.

To avoid slowing down the normal case, the throttling runs in parallel with the
handshake. Only access to the new connection from Javascript is delayed.

The default behaviour is to have no throttling.

The implementation is in
blink::DocumentWebSocketChannel. DocumentWebSocketChannelTest has new tests for
the new functionality.

This is part of the implementation of Safe Browsing for WebSockets. See the
design doc at
https://docs.google.com/document/d/1iR3XMIQukqlXb6ajIHE91apHZAxyF_wvRoB5JGeJY...

BUG=644744

Review-Url: https://codereview.chromium.org/2930263002
Cr-Commit-Position: refs/heads/master@{#479709}
Committed:
https://chromium.googlesource.com/chromium/src/+/bf9d1c2f8e5ea0366b4b086c7d05...
==========

[commit-bot: I haz the power, 2017-06-15 15:20:09]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/bf9d1c2f8e5ea0366b4b086c7d05...

[brettw, 2017-06-15 17:19:18]

brettw@chromium.org changed reviewers:
+ brettw@chromium.org

[brettw, 2017-06-15 17:19:18]

I am getting failures on the waterfall as a result of this patch because the new
header was not added to a BUILD file.
https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Ftryserver.chromium.linux...

[brettw, 2017-06-15 17:43:24]

This bot seems to be failing every time and is way backlogged now because it
runs everything again "without patch" and still fails. Will revert/

[brettw, 2017-06-15 17:43:48]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/2939123002/ by brettw@chromium.org.

The reason for reverting is: New header not added to build causes failures:

https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Ftryserver.chromium.linux....

[brettw, 2017-06-15 18:21:36]

The CQ missing this problem is being tracked here:
https://bugs.chromium.org/p/chromium/issues/detail?id=733500

[Adam Rice, 2017-06-16 05:54:41]

Sorry for the trouble. The problem is obvious in hindsight.

I am relanding with the addition of the new file to BUILD.gn in
https://codereview.chromium.org/2942173002.