New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 801564 link

Starred by 3 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 2
Type: Bug

Blocked on:
issue 825232
issue 814982
issue 814985
issue 819101
issue 825231
issue 825263



Sign in to add a comment

Implement WebSockets over HTTP/2

Project Member Reported by b...@chromium.org, Jan 12 2018

Issue description

A draft proposal for WebSockets over HTTP/2 has recently been adopted by the IETF HTTP working group: https://tools.ietf.org/html/draft-ietf-httpbis-h2-websockets-00.  This should be implemented in Chromium.
 
Cc: ricea@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 15 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8cae04ed002baf0ff6cf20a7c76995792f9f4a51

commit 8cae04ed002baf0ff6cf20a7c76995792f9f4a51
Author: Bence Béky <bnc@chromium.org>
Date: Mon Jan 15 18:37:06 2018

Use the same HttpStreamFactoryImpl for Websockets.

Currently, HttpNetworkSession owns two HttpStreamFactoryImpl instances:
one for Websocket requests, one for other requests.  This is
unnecessary.  This CL makes HttpNetworkSession own and use a single
HttpStreamFactoryImpl for all requests.

HttpStreamFactoryImpl manages JobControllers, and JobController manages
Jobs.  Every JobController and Job corresponds to a request and is thus
either Websocket or non-Websocket.  Before this CL, Job called
JobController through the Job::Delegate interface to find out whether
it was for a Websocket request, and JobController called
HttpStreamFactoryImpl.  This CL removes these calls and instead passes a
Boolean in the constructors that is stored in const members of
JobController and Job.

Bug: 801564
Change-Id: I9e5784713f38dd5871455c96626e9e0d3354e160
Reviewed-on: https://chromium-review.googlesource.com/864562
Commit-Queue: Bence Béky <bnc@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529314}
[modify] https://crrev.com/8cae04ed002baf0ff6cf20a7c76995792f9f4a51/net/http/http_network_session.cc
[modify] https://crrev.com/8cae04ed002baf0ff6cf20a7c76995792f9f4a51/net/http/http_network_session.h
[modify] https://crrev.com/8cae04ed002baf0ff6cf20a7c76995792f9f4a51/net/http/http_network_session_peer.cc
[modify] https://crrev.com/8cae04ed002baf0ff6cf20a7c76995792f9f4a51/net/http/http_network_session_peer.h
[modify] https://crrev.com/8cae04ed002baf0ff6cf20a7c76995792f9f4a51/net/http/http_network_transaction.cc
[modify] https://crrev.com/8cae04ed002baf0ff6cf20a7c76995792f9f4a51/net/http/http_network_transaction_unittest.cc
[modify] https://crrev.com/8cae04ed002baf0ff6cf20a7c76995792f9f4a51/net/http/http_stream_factory_impl.cc
[modify] https://crrev.com/8cae04ed002baf0ff6cf20a7c76995792f9f4a51/net/http/http_stream_factory_impl.h
[modify] https://crrev.com/8cae04ed002baf0ff6cf20a7c76995792f9f4a51/net/http/http_stream_factory_impl_job.cc
[modify] https://crrev.com/8cae04ed002baf0ff6cf20a7c76995792f9f4a51/net/http/http_stream_factory_impl_job.h
[modify] https://crrev.com/8cae04ed002baf0ff6cf20a7c76995792f9f4a51/net/http/http_stream_factory_impl_job_controller.cc
[modify] https://crrev.com/8cae04ed002baf0ff6cf20a7c76995792f9f4a51/net/http/http_stream_factory_impl_job_controller.h
[modify] https://crrev.com/8cae04ed002baf0ff6cf20a7c76995792f9f4a51/net/http/http_stream_factory_impl_job_controller_unittest.cc
[modify] https://crrev.com/8cae04ed002baf0ff6cf20a7c76995792f9f4a51/net/http/http_stream_factory_impl_request_unittest.cc
[modify] https://crrev.com/8cae04ed002baf0ff6cf20a7c76995792f9f4a51/net/http/http_stream_factory_impl_unittest.cc
[modify] https://crrev.com/8cae04ed002baf0ff6cf20a7c76995792f9f4a51/net/http/http_stream_factory_test_util.cc
[modify] https://crrev.com/8cae04ed002baf0ff6cf20a7c76995792f9f4a51/net/http/http_stream_factory_test_util.h

Project Member

Comment 3 by bugdroid1@chromium.org, Jan 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/58b42322d7fd12e6909e95fe4bf63e29e83000da

commit 58b42322d7fd12e6909e95fe4bf63e29e83000da
Author: Bence Béky <bnc@chromium.org>
Date: Wed Jan 24 15:16:54 2018

Add field trial and command line flag for Websockets over HTTP/2.

The draft is in an early stage, and a lot of manual interop testing will
have to be done before this feature can be rolled out, hence the command
line flag in addition to the field trial parameter.

Bug: 801564
Change-Id: I6fad7f0c8641f7ba4167302b121a971ddb41bd11
Reviewed-on: https://chromium-review.googlesource.com/864905
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Commit-Queue: Bence Béky <bnc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531532}
[modify] https://crrev.com/58b42322d7fd12e6909e95fe4bf63e29e83000da/components/network_session_configurator/browser/network_session_configurator.cc
[modify] https://crrev.com/58b42322d7fd12e6909e95fe4bf63e29e83000da/components/network_session_configurator/browser/network_session_configurator_unittest.cc
[modify] https://crrev.com/58b42322d7fd12e6909e95fe4bf63e29e83000da/components/network_session_configurator/common/network_switch_list.h
[modify] https://crrev.com/58b42322d7fd12e6909e95fe4bf63e29e83000da/net/http/http_network_session.h
[modify] https://crrev.com/58b42322d7fd12e6909e95fe4bf63e29e83000da/net/spdy/chromium/spdy_test_util_common.cc
[modify] https://crrev.com/58b42322d7fd12e6909e95fe4bf63e29e83000da/net/spdy/chromium/spdy_test_util_common.h

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/914a027690816a7c987c5a48930b8be175d7a622

commit 914a027690816a7c987c5a48930b8be175d7a622
Author: Bence Béky <bnc@chromium.org>
Date: Wed Jan 24 16:51:27 2018

Add SETTINGS_ENABLE_CONNECT_PROTOCOL.

Add SETTINGS_ENABLE_CONNECT_PROTOCOL to enum SpdySettingsIds and
add SpdySession::support_websocket_ with accessor and unittests.

This CL lands internal change 181778518.

Bug: 801564, 488484
Change-Id: I35b9361234672b1dc08c5c0a9cf2b780575fe344
Reviewed-on: https://chromium-review.googlesource.com/864745
Commit-Queue: Bence Béky <bnc@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531561}
[modify] https://crrev.com/914a027690816a7c987c5a48930b8be175d7a622/net/spdy/chromium/spdy_session.cc
[modify] https://crrev.com/914a027690816a7c987c5a48930b8be175d7a622/net/spdy/chromium/spdy_session.h
[modify] https://crrev.com/914a027690816a7c987c5a48930b8be175d7a622/net/spdy/chromium/spdy_session_unittest.cc
[modify] https://crrev.com/914a027690816a7c987c5a48930b8be175d7a622/net/spdy/core/spdy_protocol.cc
[modify] https://crrev.com/914a027690816a7c987c5a48930b8be175d7a622/net/spdy/core/spdy_protocol.h
[modify] https://crrev.com/914a027690816a7c987c5a48930b8be175d7a622/net/spdy/core/spdy_protocol_test.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 30 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dca6bd9172dbf71b6a00cd9ada2c49682ac84c15

commit dca6bd9172dbf71b6a00cd9ada2c49682ac84c15
Author: Bence Béky <bnc@chromium.org>
Date: Tue Jan 30 13:43:06 2018

Refactor Websocket test classes.

Factor out FakeWebSocketBasicHandshakeStream and
FakeWebSocketHandshakeStream into websocket_test_util.

Bug: 801564
Change-Id: I093667de806f7d4879144bac447e45663219c5a1
Reviewed-on: https://chromium-review.googlesource.com/891444
Commit-Queue: Bence Béky <bnc@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532853}
[modify] https://crrev.com/dca6bd9172dbf71b6a00cd9ada2c49682ac84c15/net/http/http_network_transaction_unittest.cc
[modify] https://crrev.com/dca6bd9172dbf71b6a00cd9ada2c49682ac84c15/net/url_request/url_request_http_job_unittest.cc
[modify] https://crrev.com/dca6bd9172dbf71b6a00cd9ada2c49682ac84c15/net/websockets/websocket_test_util.cc
[modify] https://crrev.com/dca6bd9172dbf71b6a00cd9ada2c49682ac84c15/net/websockets/websocket_test_util.h

Project Member

Comment 6 by bugdroid1@chromium.org, Jan 30 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/27a19b82052ea765f09611189f2efcfadd21a09c

commit 27a19b82052ea765f09611189f2efcfadd21a09c
Author: Bence Béky <bnc@chromium.org>
Date: Tue Jan 30 14:58:36 2018

Use base::Optional for next_protos_expected_in_ssl_config.

This will allow next_protos_expected_in_ssl_config to test for
ssl_config.alpn_protos being empty.

Bug: 801564
Change-Id: I05ee7803b7ffb5add8ef0a16f0a00c7572967041
Reviewed-on: https://chromium-review.googlesource.com/891445
Commit-Queue: Bence Béky <bnc@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532869}
[modify] https://crrev.com/27a19b82052ea765f09611189f2efcfadd21a09c/net/socket/socket_test_util.cc
[modify] https://crrev.com/27a19b82052ea765f09611189f2efcfadd21a09c/net/socket/socket_test_util.h
[modify] https://crrev.com/27a19b82052ea765f09611189f2efcfadd21a09c/net/spdy/chromium/spdy_network_transaction_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Jan 31 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0ca719f5ae20f05f645a14000631861c70114e86

commit 0ca719f5ae20f05f645a14000631861c70114e86
Author: Bence Béky <bnc@chromium.org>
Date: Wed Jan 31 13:41:19 2018

Add unittest to document Websocket behavior.

Add unittest to document existing Websocket behavior with existing
HTTP/2 connection: if there is an HTTP/2 connection open to a server, a
Websocket request should not use it, but instead open a new connection
with HTTP/2 disabled.

Bug: 801564
Change-Id: Ibff7278cb298491f8183ca0946acfef39c7e8309
Reviewed-on: https://chromium-review.googlesource.com/891738
Commit-Queue: Bence Béky <bnc@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533263}
[modify] https://crrev.com/0ca719f5ae20f05f645a14000631861c70114e86/net/spdy/chromium/spdy_network_transaction_unittest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Feb 3 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d885b2b7fdc09e95753301363e15be56a931ac1a

commit d885b2b7fdc09e95753301363e15be56a931ac1a
Author: Bence Béky <bnc@chromium.org>
Date: Sat Feb 03 00:58:31 2018

Add is_websocket argument to SpdySessionPool::FindAvailableSession().

This will be used by HttpStreamFactoryImpl::Job to find a SpdySession
that supports Websocket over HTTP/2.

Bug: 801564
Change-Id: Idac6a13743094c59a7ffa2a43e1fed7554361dd8
Reviewed-on: https://chromium-review.googlesource.com/896982
Commit-Queue: Bence Béky <bnc@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534222}
[modify] https://crrev.com/d885b2b7fdc09e95753301363e15be56a931ac1a/net/http/http_proxy_client_socket_wrapper.cc
[modify] https://crrev.com/d885b2b7fdc09e95753301363e15be56a931ac1a/net/http/http_stream_factory_impl_job.cc
[modify] https://crrev.com/d885b2b7fdc09e95753301363e15be56a931ac1a/net/spdy/chromium/spdy_network_transaction_unittest.cc
[modify] https://crrev.com/d885b2b7fdc09e95753301363e15be56a931ac1a/net/spdy/chromium/spdy_session_pool.cc
[modify] https://crrev.com/d885b2b7fdc09e95753301363e15be56a931ac1a/net/spdy/chromium/spdy_session_pool.h
[modify] https://crrev.com/d885b2b7fdc09e95753301363e15be56a931ac1a/net/spdy/chromium/spdy_session_pool_unittest.cc
[modify] https://crrev.com/d885b2b7fdc09e95753301363e15be56a931ac1a/net/spdy/chromium/spdy_session_unittest.cc
[modify] https://crrev.com/d885b2b7fdc09e95753301363e15be56a931ac1a/net/spdy/chromium/spdy_test_util_common.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Feb 7 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8d1c605a78d21df1b0c87dc7a7d14b319b3730c4

commit 8d1c605a78d21df1b0c87dc7a7d14b319b3730c4
Author: Bence Béky <bnc@chromium.org>
Date: Wed Feb 07 12:48:15 2018

Remove fake WebSocket test classes.

Create DummyConnectDelegate and DummyWebSocketStreamRequest.  Rename
DeterministicKeyWebSocketHandshakeStreamCreateHelper to
TestWebSocketHandshakeStreamCreateHelper, and add a constructor to use
these dummy classes.

Remove FakeWebSocketHandshakeStreamBase and its derived classes
FakeWebSocketHandshakeStream and AsyncFakeWebSocketHandshakeStream.
Also remove FakeWebSocketStreamCreateHelper that hands out
FakeWebSocketHandshakeStream instances.  Convert five tests to use
TestWebSocketHandshakeStreamCreateHelper, which is based on the real
WebSocketHandshakeStreamCreateHelper, and add/modify mock data
accordingly.

This change is not directly related to but done in preparation for
implementing WebSockets over HTTP/2.

Bug: 801564
Change-Id: I7a9fee80f0ce6b4dccf59f62570e52b5a64131f8
Reviewed-on: https://chromium-review.googlesource.com/904823
Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Bence Béky <bnc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534986}
[modify] https://crrev.com/8d1c605a78d21df1b0c87dc7a7d14b319b3730c4/net/http/http_network_transaction_unittest.cc
[modify] https://crrev.com/8d1c605a78d21df1b0c87dc7a7d14b319b3730c4/net/spdy/chromium/spdy_network_transaction_unittest.cc
[modify] https://crrev.com/8d1c605a78d21df1b0c87dc7a7d14b319b3730c4/net/url_request/url_request_http_job_unittest.cc
[modify] https://crrev.com/8d1c605a78d21df1b0c87dc7a7d14b319b3730c4/net/websockets/websocket_handshake_stream_create_helper.h
[modify] https://crrev.com/8d1c605a78d21df1b0c87dc7a7d14b319b3730c4/net/websockets/websocket_stream_create_test_base.cc
[modify] https://crrev.com/8d1c605a78d21df1b0c87dc7a7d14b319b3730c4/net/websockets/websocket_test_util.cc
[modify] https://crrev.com/8d1c605a78d21df1b0c87dc7a7d14b319b3730c4/net/websockets/websocket_test_util.h

Project Member

Comment 11 by bugdroid1@chromium.org, Feb 8 2018

Project Member

Comment 12 by bugdroid1@chromium.org, Feb 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/29f66160017e896af090b5fe3913578e1f3e50e3

commit 29f66160017e896af090b5fe3913578e1f3e50e3
Author: Bence Béky <bnc@chromium.org>
Date: Fri Feb 16 02:45:02 2018

Implement WebSocketSpdyStreamAdapter.

WebSocketSpdyStreamAdapter wraps a SpdyStream so that it can be used by
the future WebSocketHttp2HandshakeStream class to do the  WebSocket
handshake, then handed off to WebSocketBasicStream.

Bug: 801564
Change-Id: Icac1d4e19d12b390bd61562de9150698be81f5f7
Reviewed-on: https://chromium-review.googlesource.com/912009
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Bence Béky <bnc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537135}
[modify] https://crrev.com/29f66160017e896af090b5fe3913578e1f3e50e3/net/spdy/chromium/spdy_stream.h
[modify] https://crrev.com/29f66160017e896af090b5fe3913578e1f3e50e3/net/websockets/websocket_basic_stream.h
[modify] https://crrev.com/29f66160017e896af090b5fe3913578e1f3e50e3/net/websockets/websocket_basic_stream_adapters.cc
[modify] https://crrev.com/29f66160017e896af090b5fe3913578e1f3e50e3/net/websockets/websocket_basic_stream_adapters.h
[modify] https://crrev.com/29f66160017e896af090b5fe3913578e1f3e50e3/net/websockets/websocket_basic_stream_adapters_test.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Feb 21 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ed718f52c14b7678c6b1fbce8bb0c86375bbef60

commit ed718f52c14b7678c6b1fbce8bb0c86375bbef60
Author: Bence Béky <bnc@chromium.org>
Date: Wed Feb 21 13:18:00 2018

Change most WebSocketStreamCreate*Tests to use wss scheme.

Most of these tests will be adopted to test WebSockets over HTTP/2 as
well, but that only works with wss.  Also, change hosts and origins from
localhost to www.example.org instead so that it works with the test
certificate.

Also, change WebSocketStreamCreateUMATest to be a derived class of
WebSocketStreamCreateTest instead of creating an instance of it.  This
will also help with turning this into a parametrized class.

Bug: 801564
Change-Id: Ifff2262bcc15e0ee6ab67dea10cef605d47bd907
Reviewed-on: https://chromium-review.googlesource.com/927491
Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Bence Béky <bnc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538081}
[modify] https://crrev.com/ed718f52c14b7678c6b1fbce8bb0c86375bbef60/net/websockets/websocket_stream_test.cc

Project Member

Comment 14 by bugdroid1@chromium.org, Feb 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/46bfbc198e660fa3f6561f6babd7f3221ae1d091

commit 46bfbc198e660fa3f6561f6babd7f3221ae1d091
Author: Bence Béky <bnc@chromium.org>
Date: Thu Feb 22 19:28:20 2018

Add WebSocketHttp2HandshakeStream.

Add WebSocketHttp2HandshakeStream class to perform handshake according
to https://tools.ietf.org/html/draft-ietf-httpbis-h2-websockets-00.

Add WebSocketHandshakeStreamBase::CreateHttp2Stream() pure virtual
method and implement it in all derived classes.

Remove lowercase WebSocket-related headers that are not used after all
(they are leftover from early days of WebSockets over SPDY).

Bug: 801564
Change-Id: I02ca36c4ce3511de7f88b977083bc2b33c3bfcc1
Reviewed-on: https://chromium-review.googlesource.com/926769
Commit-Queue: Bence Béky <bnc@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538522}
[modify] https://crrev.com/46bfbc198e660fa3f6561f6babd7f3221ae1d091/net/BUILD.gn
[modify] https://crrev.com/46bfbc198e660fa3f6561f6babd7f3221ae1d091/net/http/http_cache_unittest.cc
[modify] https://crrev.com/46bfbc198e660fa3f6561f6babd7f3221ae1d091/net/http/http_stream_factory_impl_unittest.cc
[modify] https://crrev.com/46bfbc198e660fa3f6561f6babd7f3221ae1d091/net/spdy/chromium/spdy_http_utils.cc
[modify] https://crrev.com/46bfbc198e660fa3f6561f6babd7f3221ae1d091/net/spdy/chromium/spdy_http_utils.h
[modify] https://crrev.com/46bfbc198e660fa3f6561f6babd7f3221ae1d091/net/websockets/websocket_basic_stream_adapters_test.cc
[modify] https://crrev.com/46bfbc198e660fa3f6561f6babd7f3221ae1d091/net/websockets/websocket_handshake_constants.cc
[modify] https://crrev.com/46bfbc198e660fa3f6561f6babd7f3221ae1d091/net/websockets/websocket_handshake_constants.h
[modify] https://crrev.com/46bfbc198e660fa3f6561f6babd7f3221ae1d091/net/websockets/websocket_handshake_stream_base.h
[modify] https://crrev.com/46bfbc198e660fa3f6561f6babd7f3221ae1d091/net/websockets/websocket_handshake_stream_create_helper.cc
[modify] https://crrev.com/46bfbc198e660fa3f6561f6babd7f3221ae1d091/net/websockets/websocket_handshake_stream_create_helper.h
[modify] https://crrev.com/46bfbc198e660fa3f6561f6babd7f3221ae1d091/net/websockets/websocket_handshake_stream_create_helper_test.cc
[add] https://crrev.com/46bfbc198e660fa3f6561f6babd7f3221ae1d091/net/websockets/websocket_http2_handshake_stream.cc
[add] https://crrev.com/46bfbc198e660fa3f6561f6babd7f3221ae1d091/net/websockets/websocket_http2_handshake_stream.h
[modify] https://crrev.com/46bfbc198e660fa3f6561f6babd7f3221ae1d091/net/websockets/websocket_test_util.cc
[modify] https://crrev.com/46bfbc198e660fa3f6561f6babd7f3221ae1d091/net/websockets/websocket_test_util.h

Project Member

Comment 15 by bugdroid1@chromium.org, Feb 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/75fe7e99df3ced79f526f8e20919a3f2959dbbab

commit 75fe7e99df3ced79f526f8e20919a3f2959dbbab
Author: Bence Béky <bnc@chromium.org>
Date: Thu Feb 22 22:05:53 2018

Enable WebSockets over HTTP/2.

Change HttpStreamFactoryImpl::Job to allow secure WebSocket requests to
use an existing HTTP/2 connection if the server has already indicated
support.  If no such connection exists, a new non-HTTP/2 connection is
created for the WebSocket request just like before.

Bug: 801564
Change-Id: I72541fcb8c88906699bf85ff305f2b2d590095ca
Reviewed-on: https://chromium-review.googlesource.com/905344
Commit-Queue: Bence Béky <bnc@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538584}
[modify] https://crrev.com/75fe7e99df3ced79f526f8e20919a3f2959dbbab/net/http/http_stream_factory_impl_job.cc
[modify] https://crrev.com/75fe7e99df3ced79f526f8e20919a3f2959dbbab/net/http/http_stream_factory_impl_job.h
[modify] https://crrev.com/75fe7e99df3ced79f526f8e20919a3f2959dbbab/net/spdy/chromium/spdy_network_transaction_unittest.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Feb 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b2a0dd7bf3d155666b1ab309dfb4484724080d8f

commit b2a0dd7bf3d155666b1ab309dfb4484724080d8f
Author: Reilly Grant <reillyg@chromium.org>
Date: Fri Feb 23 00:24:52 2018

Revert "Enable WebSockets over HTTP/2."

This reverts commit 75fe7e99df3ced79f526f8e20919a3f2959dbbab.

Reason for revert: Failing on MSan bot:

https://uberchromegw.corp.google.com/i/chromium.memory/builders/Linux%20MSan%20Tests/builds/8103

Original change's description:
> Enable WebSockets over HTTP/2.
> 
> Change HttpStreamFactoryImpl::Job to allow secure WebSocket requests to
> use an existing HTTP/2 connection if the server has already indicated
> support.  If no such connection exists, a new non-HTTP/2 connection is
> created for the WebSocket request just like before.
> 
> Bug: 801564
> Change-Id: I72541fcb8c88906699bf85ff305f2b2d590095ca
> Reviewed-on: https://chromium-review.googlesource.com/905344
> Commit-Queue: Bence Béky <bnc@chromium.org>
> Reviewed-by: Adam Rice <ricea@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#538584}

TBR=ricea@chromium.org,bnc@chromium.org

Change-Id: I8ef9343117a4a0099f8dc15261e2ed6bdb277088
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 801564
Reviewed-on: https://chromium-review.googlesource.com/933303
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538637}
[modify] https://crrev.com/b2a0dd7bf3d155666b1ab309dfb4484724080d8f/net/http/http_stream_factory_impl_job.cc
[modify] https://crrev.com/b2a0dd7bf3d155666b1ab309dfb4484724080d8f/net/http/http_stream_factory_impl_job.h
[modify] https://crrev.com/b2a0dd7bf3d155666b1ab309dfb4484724080d8f/net/spdy/chromium/spdy_network_transaction_unittest.cc

Comment 17 by b...@chromium.org, Feb 23 2018

Blockedon: 814982 814985
Project Member

Comment 18 by bugdroid1@chromium.org, Feb 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8bfacd474c14c33e4942959a79345cb391d74e3d

commit 8bfacd474c14c33e4942959a79345cb391d74e3d
Author: Bence Béky <bnc@chromium.org>
Date: Fri Feb 23 13:05:13 2018

Enable WebSockets over HTTP/2.

Change HttpStreamFactoryImpl::Job to allow secure WebSocket requests to
use an existing HTTP/2 connection if the server has already indicated
support.  If no such connection exists, a new non-HTTP/2 connection is
created for the WebSocket request just like before.

This is a reland of https://crrev.com/c/905344, which got reverted at
https://crrev.com/c/933303, with an additional change in
http_network_session.cc to initialize
HttpNetworkSession::Params::enable_websocket_over_http2 to false.

Bug: 801564
Change-Id: I576a208e4f81b87efb56e116adf93bef9fc661ff
Reviewed-on: https://chromium-review.googlesource.com/933441
Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Bence Béky <bnc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538769}
[modify] https://crrev.com/8bfacd474c14c33e4942959a79345cb391d74e3d/net/http/http_network_session.cc
[modify] https://crrev.com/8bfacd474c14c33e4942959a79345cb391d74e3d/net/http/http_stream_factory_impl_job.cc
[modify] https://crrev.com/8bfacd474c14c33e4942959a79345cb391d74e3d/net/http/http_stream_factory_impl_job.h
[modify] https://crrev.com/8bfacd474c14c33e4942959a79345cb391d74e3d/net/spdy/chromium/spdy_network_transaction_unittest.cc

Project Member

Comment 19 by bugdroid1@chromium.org, Feb 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6c7c2520da894d75c518262a9281a8b4c4791cfa

commit 6c7c2520da894d75c518262a9281a8b4c4791cfa
Author: Bence Béky <bnc@chromium.org>
Date: Fri Feb 23 16:04:15 2018

Revert "Change most WebSocketStreamCreate*Tests to use wss scheme."

This reverts commit ed718f52c14b7678c6b1fbce8bb0c86375bbef60.

Reason for revert:  https://crbug.com/814982 

Original change's description:
> Change most WebSocketStreamCreate*Tests to use wss scheme.
>
> Most of these tests will be adopted to test WebSockets over HTTP/2 as
> well, but that only works with wss.  Also, change hosts and origins from
> localhost to www.example.org instead so that it works with the test
> certificate.
>
> Also, change WebSocketStreamCreateUMATest to be a derived class of
> WebSocketStreamCreateTest instead of creating an instance of it.  This
> will also help with turning this into a parametrized class.
>
> Bug: 801564
> Change-Id: Ifff2262bcc15e0ee6ab67dea10cef605d47bd907
> Reviewed-on: https://chromium-review.googlesource.com/927491
> Reviewed-by: Adam Rice <ricea@chromium.org>
> Commit-Queue: Bence Béky <bnc@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#538081}

TBR=ricea@chromium.org,bnc@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 801564,  814982 
Change-Id: I27a752f3100d72087ca4ec436d8758915e71e4ec
Reviewed-on: https://chromium-review.googlesource.com/934601
Commit-Queue: Bence Béky <bnc@chromium.org>
Reviewed-by: Bence Béky <bnc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538790}
[modify] https://crrev.com/6c7c2520da894d75c518262a9281a8b4c4791cfa/net/websockets/websocket_stream_test.cc

Project Member

Comment 20 by bugdroid1@chromium.org, Mar 2 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3a0c4853739f401a0efa9eb378b46710f5b92365

commit 3a0c4853739f401a0efa9eb378b46710f5b92365
Author: Bence Béky <bnc@chromium.org>
Date: Fri Mar 02 13:38:51 2018

Misc minor changes in WebSockets tests.

This CL partially re-lands https://crrev.com/c/927491 among other
things.  This CL is mostly to prepare for adding tests for WebSockets
over HTTP/2.

Add missing include to spdy_network_transaction_unittest.cc for
HttpResponseInfo.

Change two WebSockets test file to use wildcard.pem instead of
spdy_pooling.pem.

Move WebSocketExtraHeadersToString from a test file anonymous namespace
to websocket_test_utils.h.  Also change argument type for extra headers
in websocket_stream_test.cc to WebSocketExtraHeaders and use this helper
function to convert them to a string.

In websocket_stream_test.cc:
* Change all tests to use www.example.org instead of localhost so that
  it is compatible with the test certificate wildcard.pem.  (This is
  only a requirement for wss, but it is cleaner if all tests use the
  same hostname.)
* Change tests that will be parametrized to test WebSockets over HTTP/2
  as well to use wss scheme.
* Change WebSocketStreamCreateUMATest to be a derived class of
  WebSocketStreamCreateTest instead of creating an instance of it, and
  use HistogramTester for simplicity.
* Change test base class methods and members from public to protected.
* Inline trivial AddRawExpectations() method.

Bug: 801564
Change-Id: I76a3ed145baa0f9171089765852e3165d43c9710
Reviewed-on: https://chromium-review.googlesource.com/942522
Commit-Queue: Bence Béky <bnc@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540509}
[modify] https://crrev.com/3a0c4853739f401a0efa9eb378b46710f5b92365/net/spdy/chromium/spdy_network_transaction_unittest.cc
[modify] https://crrev.com/3a0c4853739f401a0efa9eb378b46710f5b92365/net/websockets/websocket_basic_stream_adapters_test.cc
[modify] https://crrev.com/3a0c4853739f401a0efa9eb378b46710f5b92365/net/websockets/websocket_handshake_stream_create_helper_test.cc
[modify] https://crrev.com/3a0c4853739f401a0efa9eb378b46710f5b92365/net/websockets/websocket_stream_test.cc
[modify] https://crrev.com/3a0c4853739f401a0efa9eb378b46710f5b92365/net/websockets/websocket_test_util.cc
[modify] https://crrev.com/3a0c4853739f401a0efa9eb378b46710f5b92365/net/websockets/websocket_test_util.h

Project Member

Comment 21 by bugdroid1@chromium.org, Mar 2 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/70dd766a946c0635a2a487aed3d70b7d245623f3

commit 70dd766a946c0635a2a487aed3d70b7d245623f3
Author: Bence Béky <bnc@chromium.org>
Date: Fri Mar 02 17:13:24 2018

Implement HTTP/2 support in WebSocketStream.

Implement HTTP/2 support in websocket_stream.cc::(anonymous
namespace)::Delegate::OnResponseStarted().

Convert many WebSocketStream tests to test over HTTP/2 as well.

Relax requirement for non-empty header keys in SpdyTestUtil, since
URLRequest-based tests send an empty User-Agent header.

Enable WebSockets over HTTP/2 and set some other
HttpNetworkSession::Params flags customary for tests in
WebSocketTestURLRequestContextHost constructor (copied from
SpdySessionDependencies constructor).

Bug: 801564
Change-Id: I24f5bc999c4e62dc1e8feb4b7db94d0f5b38323c
Reviewed-on: https://chromium-review.googlesource.com/940463
Commit-Queue: Bence Béky <bnc@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540540}
[modify] https://crrev.com/70dd766a946c0635a2a487aed3d70b7d245623f3/net/spdy/chromium/spdy_test_util_common.cc
[modify] https://crrev.com/70dd766a946c0635a2a487aed3d70b7d245623f3/net/websockets/websocket_stream.cc
[modify] https://crrev.com/70dd766a946c0635a2a487aed3d70b7d245623f3/net/websockets/websocket_stream_test.cc
[modify] https://crrev.com/70dd766a946c0635a2a487aed3d70b7d245623f3/net/websockets/websocket_test_util.cc

Project Member

Comment 22 by bugdroid1@chromium.org, Mar 6 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b28709c2c9152a798564e29b67f977f8a60512f8

commit b28709c2c9152a798564e29b67f977f8a60512f8
Author: Bence Béky <bnc@chromium.org>
Date: Tue Mar 06 13:03:44 2018

Refactor WebSocket handshake helpers into base class.

Refactor helper functions shared by WebSocketBasicHandshakeStream and
WebSocketHttp2HandshakeStream from anonymous namespace into
WebSocketHandshakeStreamBase as static methods, protected if only used
by derived classes, public if used by other helper functions remaining
in anonymous namespace.

Also remove out-of-line WebSocketHandshakeStreamBase constructor and
destructor and remove obsolete comment, as WebSocket is not compiled on
iOS.

Also remove kSecWebSocketProtocolLowercase and
kSecWebSocketExtensionsLowercase.  There is no need for these because
HttpResponseHeaders::EnumerateHeader() is case-insensitive.

This is a follow-up to
https://crrev.com/c/926769/2/net/websockets/websocket_http2_handshake_stream.cc#61

Bug: 801564
Change-Id: I0afdc5c66f743af4df8a1b7151b40e7a8e08e8bd
Reviewed-on: https://chromium-review.googlesource.com/949282
Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Bence Béky <bnc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541084}
[modify] https://crrev.com/b28709c2c9152a798564e29b67f977f8a60512f8/net/BUILD.gn
[modify] https://crrev.com/b28709c2c9152a798564e29b67f977f8a60512f8/net/websockets/websocket_basic_handshake_stream.cc
[modify] https://crrev.com/b28709c2c9152a798564e29b67f977f8a60512f8/net/websockets/websocket_handshake_constants.cc
[modify] https://crrev.com/b28709c2c9152a798564e29b67f977f8a60512f8/net/websockets/websocket_handshake_constants.h
[add] https://crrev.com/b28709c2c9152a798564e29b67f977f8a60512f8/net/websockets/websocket_handshake_stream_base.cc
[modify] https://crrev.com/b28709c2c9152a798564e29b67f977f8a60512f8/net/websockets/websocket_handshake_stream_base.h
[modify] https://crrev.com/b28709c2c9152a798564e29b67f977f8a60512f8/net/websockets/websocket_http2_handshake_stream.cc
[modify] https://crrev.com/b28709c2c9152a798564e29b67f977f8a60512f8/net/websockets/websocket_stream_test.cc

Project Member

Comment 23 by bugdroid1@chromium.org, Mar 6 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f377580d14c74a21827fdea5bc05bb14984f3a41

commit f377580d14c74a21827fdea5bc05bb14984f3a41
Author: Bence Béky <bnc@chromium.org>
Date: Tue Mar 06 13:03:55 2018

Add four more tests for WebSockets over HTTP/2.

Test that response status code 302 results in failure.  Also
test that complete, incomplete, and failed requests are logged to UMA
appropriately.

Since this further increases the number of optional test parameters that
are only changed from their default value in a small number of tests,
default arguments are increasingly inelegant for this purpose.  Instead,
introduce private members and setters for Timer, additional response
data, and HTTP/2 response status code.

Bug: 801564
Change-Id: I79e8b7648351e4dfa26b31c87ac14eefa93ccb36
Reviewed-on: https://chromium-review.googlesource.com/949365
Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Bence Béky <bnc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541085}
[modify] https://crrev.com/f377580d14c74a21827fdea5bc05bb14984f3a41/net/websockets/websocket_stream_test.cc

Project Member

Comment 24 by bugdroid1@chromium.org, Mar 7 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ff2b79df6f94b83257ba742c2aa239a5db10c516

commit ff2b79df6f94b83257ba742c2aa239a5db10c516
Author: Bence Béky <bnc@chromium.org>
Date: Wed Mar 07 04:15:25 2018

Signal error on 101 response to an HTTP/2 WebSocket request.

Bug: 801564
Change-Id: If1dc07926da68ca7981fba3527ab26ff2cd7e72d
Reviewed-on: https://chromium-review.googlesource.com/951931
Commit-Queue: Ryan Hamilton <rch@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541322}
[modify] https://crrev.com/ff2b79df6f94b83257ba742c2aa239a5db10c516/net/spdy/chromium/spdy_stream.cc
[modify] https://crrev.com/ff2b79df6f94b83257ba742c2aa239a5db10c516/net/websockets/websocket_stream_test.cc

Comment 25 by b...@chromium.org, Mar 8 2018

Labels: -OS-iOS M-67
Project Member

Comment 26 by bugdroid1@chromium.org, Mar 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/de0be3103958f4cdc02d56e69c0e57e423b1ed61

commit de0be3103958f4cdc02d56e69c0e57e423b1ed61
Author: Bence Béky <bnc@chromium.org>
Date: Tue Mar 13 17:51:58 2018

Fix WebSocket handshake result UMA.

Net.WebSocket.HandshakeResult histogram has been broken since
https://crrev.com/304093003, because
WebSocketStream::Delegate::OnResponseStarted() has not been called upon
failure any longer.  This CL restores this histogram by moving logging
into WebSocketHandshakeStreamBase and its two derived classes.  It also
refines the histogram by protocol (whether the handshake is over HTTP/2
or not), and provides a more gradual reason for failure as well.

Bug: 801564
Change-Id: Iefa977703124df2bfcf77bc48e05f3ac594c2f50
Reviewed-on: https://chromium-review.googlesource.com/951746
Commit-Queue: Bence Béky <bnc@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542845}
[modify] https://crrev.com/de0be3103958f4cdc02d56e69c0e57e423b1ed61/net/websockets/websocket_basic_handshake_stream.cc
[modify] https://crrev.com/de0be3103958f4cdc02d56e69c0e57e423b1ed61/net/websockets/websocket_basic_handshake_stream.h
[modify] https://crrev.com/de0be3103958f4cdc02d56e69c0e57e423b1ed61/net/websockets/websocket_handshake_stream_base.cc
[modify] https://crrev.com/de0be3103958f4cdc02d56e69c0e57e423b1ed61/net/websockets/websocket_handshake_stream_base.h
[modify] https://crrev.com/de0be3103958f4cdc02d56e69c0e57e423b1ed61/net/websockets/websocket_http2_handshake_stream.cc
[modify] https://crrev.com/de0be3103958f4cdc02d56e69c0e57e423b1ed61/net/websockets/websocket_http2_handshake_stream.h
[modify] https://crrev.com/de0be3103958f4cdc02d56e69c0e57e423b1ed61/net/websockets/websocket_stream.cc
[modify] https://crrev.com/de0be3103958f4cdc02d56e69c0e57e423b1ed61/net/websockets/websocket_stream_test.cc
[modify] https://crrev.com/de0be3103958f4cdc02d56e69c0e57e423b1ed61/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/de0be3103958f4cdc02d56e69c0e57e423b1ed61/tools/metrics/histograms/histograms.xml

Comment 27 by b...@chromium.org, Mar 23 2018

Blockedon: 825231

Comment 28 by b...@chromium.org, Mar 23 2018

Blockedon: 825232

Comment 29 by b...@chromium.org, Mar 23 2018

Blockedon: 825263
Blockedon: 819101
Project Member

Comment 31 by bugdroid1@chromium.org, Jun 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d611ebfba50d215d4257ee89b4ef890a016d31b2

commit d611ebfba50d215d4257ee89b4ef890a016d31b2
Author: Bence Béky <bnc@chromium.org>
Date: Thu Jun 07 04:34:34 2018

Add Net.SpdyResponseCode histogram to track HTTP/2 response codes.

This would be useful to see how often servers send a 103 Early Hints
response (to be able to better prioritize implementing support), and to
see how often servers send a 101 Switching Protocols response (which
means their WebSocket over HTTP/2 implementation is broken), and a whole
bunch of other things.

Bug: 671310, 801564
Change-Id: I9e66acb6405793c4b3c12cf9d5b32edb2262c70b
Reviewed-on: https://chromium-review.googlesource.com/1087583
Reviewed-by: Zhongyi Shi <zhongyi@chromium.org>
Reviewed-by: Gayane Petrosyan <gayane@chromium.org>
Commit-Queue: Bence Béky <bnc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565179}
[modify] https://crrev.com/d611ebfba50d215d4257ee89b4ef890a016d31b2/net/spdy/spdy_stream.cc
[modify] https://crrev.com/d611ebfba50d215d4257ee89b4ef890a016d31b2/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/d611ebfba50d215d4257ee89b4ef890a016d31b2/tools/metrics/histograms/histograms.xml

Project Member

Comment 32 by bugdroid1@chromium.org, Jul 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b09dddf1cbd1e6cc54d5de3ac5f4e2a0aec48409

commit b09dddf1cbd1e6cc54d5de3ac5f4e2a0aec48409
Author: Bence Béky <bnc@chromium.org>
Date: Tue Jul 31 18:52:04 2018

Add histogram to track server HTTP/2 WebSocket support.

Add Net.SpdySession.ServerSupportsWebSocket histogram to track server
support of WebSockets over HTTP/2.

Bug: 801564
Change-Id: I1cd63e5bdca17106487c48ce6c1734e4d6591985
Reviewed-on: https://chromium-review.googlesource.com/1152486
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Commit-Queue: Bence Béky <bnc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579500}
[modify] https://crrev.com/b09dddf1cbd1e6cc54d5de3ac5f4e2a0aec48409/net/spdy/spdy_network_transaction_unittest.cc
[modify] https://crrev.com/b09dddf1cbd1e6cc54d5de3ac5f4e2a0aec48409/net/spdy/spdy_session.cc
[modify] https://crrev.com/b09dddf1cbd1e6cc54d5de3ac5f4e2a0aec48409/tools/metrics/histograms/histograms.xml

Sign in to add a comment