From 7415553a35648f31cfd11df912fc3c9c098e218a Mon Sep 17 00:00:00 2001 From: Kenichi Ishibashi Date: Sat, 13 Mar 2021 01:38:06 +0000 Subject: [PATCH] Plumb 103 responses from SpdyStream to the network service MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL makes SpdyStream pass 103 Early Hints responses to the network service (i.e., URLLoader). It works in the following way: SpdyStream --(delegate)--> SpdyHttpStream --(response_callback)--> HttpNetworkTransaction --(early_response_headers_callback)--> URLLoader The callback for early responses is passed from the network service to HttpNetworkTransaction in the following way: URLLoader -> URLRequest -> URLRequestHttpJob -> HttpNetworkTransaction Alternative option: We can use existing `request_headers_callback_` to plumb 103 responses from //net to the network service. However, existing callbacks may not expect to be called multiple times. Calling them with 1xx responses may result in unexpected behavior. Having dedicated callback for 103 responses avoids such risks. Bug: 671310 Change-Id: Ib688e891e0ed525db5f55a9b38eba258f72d4aed Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2734060 Commit-Queue: Kenichi Ishibashi Reviewed-by: David Schinazi Reviewed-by: Yutaka Hirano Reviewed-by: Bence Béky Reviewed-by: Kinuko Yasuda Cr-Commit-Position: refs/heads/master@{#862630} --- net/http/http_cache_transaction.cc | 8 ++ net/http/http_cache_transaction.h | 3 + net/http/http_network_transaction.cc | 29 +++++++ net/http/http_network_transaction.h | 3 + net/http/http_status_code_list.h | 1 + net/http/http_transaction.h | 2 + net/http/http_transaction_test_util.h | 1 + net/log/net_log_event_type_list.h | 7 ++ net/spdy/bidirectional_stream_spdy_impl.cc | 6 ++ net/spdy/bidirectional_stream_spdy_impl.h | 1 + net/spdy/spdy_http_stream.cc | 14 ++++ net/spdy/spdy_http_stream.h | 1 + net/spdy/spdy_proxy_client_socket.cc | 3 + net/spdy/spdy_proxy_client_socket.h | 1 + net/spdy/spdy_session_fuzzer.cc | 1 + net/spdy/spdy_session_pool_unittest.cc | 2 + net/spdy/spdy_stream.cc | 51 +++++++++--- net/spdy/spdy_stream.h | 13 ++- net/spdy/spdy_stream_test_util.cc | 9 ++ net/spdy/spdy_stream_test_util.h | 8 ++ net/spdy/spdy_stream_unittest.cc | 82 ++++++++++++++----- net/url_request/url_request.cc | 8 ++ net/url_request/url_request.h | 7 +- net/url_request/url_request_http_job.cc | 9 ++ net/url_request/url_request_http_job.h | 3 + net/url_request/url_request_job.h | 6 ++ .../websocket_basic_stream_adapters.cc | 6 ++ .../websocket_basic_stream_adapters.h | 1 + .../throttling_network_transaction.cc | 5 ++ .../throttling_network_transaction.h | 2 + services/network/url_loader.cc | 16 ++++ services/network/url_loader.h | 1 + 32 files changed, 276 insertions(+), 34 deletions(-) diff --git a/net/http/http_cache_transaction.cc b/net/http/http_cache_transaction.cc index bb0b938e71f92c..d7d9280e9ba484 100644 --- a/net/http/http_cache_transaction.cc +++ b/net/http/http_cache_transaction.cc @@ -596,6 +596,12 @@ void HttpCache::Transaction::SetResponseHeadersCallback( response_headers_callback_ = std::move(callback); } +void HttpCache::Transaction::SetEarlyResponseHeadersCallback( + ResponseHeadersCallback callback) { + DCHECK(!network_trans_); + early_response_headers_callback_ = std::move(callback); +} + int HttpCache::Transaction::ResumeNetworkStart() { if (network_trans_) return network_trans_->ResumeNetworkStart(); @@ -1705,6 +1711,8 @@ int HttpCache::Transaction::DoSendRequest() { std::move(before_network_start_callback_)); network_trans_->SetConnectedCallback(connected_callback_); network_trans_->SetRequestHeadersCallback(request_headers_callback_); + network_trans_->SetEarlyResponseHeadersCallback( + early_response_headers_callback_); network_trans_->SetResponseHeadersCallback(response_headers_callback_); // Old load timing information, if any, is now obsolete. diff --git a/net/http/http_cache_transaction.h b/net/http/http_cache_transaction.h index 63baba59e61d6c..a23378ade1ae04 100644 --- a/net/http/http_cache_transaction.h +++ b/net/http/http_cache_transaction.h @@ -158,6 +158,8 @@ class NET_EXPORT_PRIVATE HttpCache::Transaction : public HttpTransaction { void SetConnectedCallback(const ConnectedCallback& callback) override; void SetRequestHeadersCallback(RequestHeadersCallback callback) override; void SetResponseHeadersCallback(ResponseHeadersCallback callback) override; + void SetEarlyResponseHeadersCallback( + ResponseHeadersCallback callback) override; int ResumeNetworkStart() override; void GetConnectionAttempts(ConnectionAttempts* out) const override; void CloseConnectionOnDestruction() override; @@ -674,6 +676,7 @@ class NET_EXPORT_PRIVATE HttpCache::Transaction : public HttpTransaction { BeforeNetworkStartCallback before_network_start_callback_; ConnectedCallback connected_callback_; RequestHeadersCallback request_headers_callback_; + ResponseHeadersCallback early_response_headers_callback_; ResponseHeadersCallback response_headers_callback_; // True if the Transaction is currently processing the DoLoop. diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc index 8939aee83ab5b1..9672aedfce32ae 100644 --- a/net/http/http_network_transaction.cc +++ b/net/http/http_network_transaction.cc @@ -532,6 +532,12 @@ void HttpNetworkTransaction::SetRequestHeadersCallback( request_headers_callback_ = std::move(callback); } +void HttpNetworkTransaction::SetEarlyResponseHeadersCallback( + ResponseHeadersCallback callback) { + DCHECK(!stream_); + early_response_headers_callback_ = std::move(callback); +} + void HttpNetworkTransaction::SetResponseHeadersCallback( ResponseHeadersCallback callback) { DCHECK(!stream_); @@ -1127,6 +1133,29 @@ int HttpNetworkTransaction::DoReadHeadersComplete(int result) { DCHECK(response_.headers.get()); + // Check for a 103 Early Hints response. + if (response_.headers->response_code() == HTTP_EARLY_HINTS) { + NetLogResponseHeaders( + net_log_, + NetLogEventType::HTTP_TRANSACTION_READ_EARLY_HINTS_RESPONSE_HEADERS, + response_.headers.get()); + + // Early Hints does not make sense for a WebSocket handshake. + if (ForWebSocketHandshake()) + return ERR_FAILED; + + // TODO(crbug.com/671310): Validate headers? It seems that + // "Content-Encoding" etc should not appear. + + if (early_response_headers_callback_) + early_response_headers_callback_.Run(std::move(response_.headers)); + + response_.headers = + base::MakeRefCounted(std::string()); + next_state_ = STATE_READ_HEADERS; + return OK; + } + if (!ContentEncodingsValid()) return ERR_CONTENT_DECODING_FAILED; diff --git a/net/http/http_network_transaction.h b/net/http/http_network_transaction.h index bf7c586a096a7c..947322b51d257e 100644 --- a/net/http/http_network_transaction.h +++ b/net/http/http_network_transaction.h @@ -87,6 +87,8 @@ class NET_EXPORT_PRIVATE HttpNetworkTransaction BeforeNetworkStartCallback callback) override; void SetConnectedCallback(const ConnectedCallback& callback) override; void SetRequestHeadersCallback(RequestHeadersCallback callback) override; + void SetEarlyResponseHeadersCallback( + ResponseHeadersCallback callback) override; void SetResponseHeadersCallback(ResponseHeadersCallback callback) override; int ResumeNetworkStart() override; void CloseConnectionOnDestruction() override; @@ -420,6 +422,7 @@ class NET_EXPORT_PRIVATE HttpNetworkTransaction BeforeNetworkStartCallback before_network_start_callback_; ConnectedCallback connected_callback_; RequestHeadersCallback request_headers_callback_; + ResponseHeadersCallback early_response_headers_callback_; ResponseHeadersCallback response_headers_callback_; ConnectionAttempts connection_attempts_; diff --git a/net/http/http_status_code_list.h b/net/http/http_status_code_list.h index c08791031a242e..669b3724340857 100644 --- a/net/http/http_status_code_list.h +++ b/net/http/http_status_code_list.h @@ -19,6 +19,7 @@ // Informational 1xx HTTP_STATUS(CONTINUE, 100, "Continue") HTTP_STATUS(SWITCHING_PROTOCOLS, 101, "Switching Protocols") +HTTP_STATUS(EARLY_HINTS, 103, "Early Hints") // Successful 2xx HTTP_STATUS(OK, 200, "OK") diff --git a/net/http/http_transaction.h b/net/http/http_transaction.h index 924344fb92ad54..f575e1dc4c12d1 100644 --- a/net/http/http_transaction.h +++ b/net/http/http_transaction.h @@ -198,6 +198,8 @@ class NET_EXPORT_PRIVATE HttpTransaction { virtual void SetConnectedCallback(const ConnectedCallback& callback) = 0; virtual void SetRequestHeadersCallback(RequestHeadersCallback callback) = 0; + virtual void SetEarlyResponseHeadersCallback( + ResponseHeadersCallback callback) = 0; virtual void SetResponseHeadersCallback(ResponseHeadersCallback callback) = 0; // Resumes the transaction after being deferred. diff --git a/net/http/http_transaction_test_util.h b/net/http/http_transaction_test_util.h index fee5ca670fd804..397c59c97e219f 100644 --- a/net/http/http_transaction_test_util.h +++ b/net/http/http_transaction_test_util.h @@ -249,6 +249,7 @@ class MockNetworkTransaction void SetRequestHeadersCallback(RequestHeadersCallback callback) override {} void SetResponseHeadersCallback(ResponseHeadersCallback) override {} + void SetEarlyResponseHeadersCallback(ResponseHeadersCallback) override {} int ResumeNetworkStart() override; diff --git a/net/log/net_log_event_type_list.h b/net/log/net_log_event_type_list.h index f905169aee5912..722925214aa09b 100644 --- a/net/log/net_log_event_type_list.h +++ b/net/log/net_log_event_type_list.h @@ -1252,6 +1252,13 @@ EVENT_TYPE(HTTP_TRANSACTION_READ_HEADERS) // } EVENT_TYPE(HTTP_TRANSACTION_READ_RESPONSE_HEADERS) +// This event is sent on receipt of a 103 Early Hints response headers. +// The following parameters are attached: +// { +// "headers": , +// } +EVENT_TYPE(HTTP_TRANSACTION_READ_EARLY_HINTS_RESPONSE_HEADERS) + // Measures the time to read the entity body from the server. EVENT_TYPE(HTTP_TRANSACTION_READ_BODY) diff --git a/net/spdy/bidirectional_stream_spdy_impl.cc b/net/spdy/bidirectional_stream_spdy_impl.cc index 193a3fdf880543..654d9b3ec8d8e9 100644 --- a/net/spdy/bidirectional_stream_spdy_impl.cc +++ b/net/spdy/bidirectional_stream_spdy_impl.cc @@ -206,6 +206,12 @@ void BidirectionalStreamSpdyImpl::OnHeadersSent() { delegate_->OnStreamReady(/*request_headers_sent=*/true); } +void BidirectionalStreamSpdyImpl::OnEarlyHintsReceived( + const spdy::Http2HeaderBlock& headers) { + DCHECK(stream_); + // TODO(crbug.com/671310): Plumb Early Hints to `delegate_` if needed. +} + void BidirectionalStreamSpdyImpl::OnHeadersReceived( const spdy::Http2HeaderBlock& response_headers, const spdy::Http2HeaderBlock* pushed_request_headers) { diff --git a/net/spdy/bidirectional_stream_spdy_impl.h b/net/spdy/bidirectional_stream_spdy_impl.h index 2059147dc3f274..44ff6fd3884790 100644 --- a/net/spdy/bidirectional_stream_spdy_impl.h +++ b/net/spdy/bidirectional_stream_spdy_impl.h @@ -62,6 +62,7 @@ class NET_EXPORT_PRIVATE BidirectionalStreamSpdyImpl // SpdyStream::Delegate implementation: void OnHeadersSent() override; + void OnEarlyHintsReceived(const spdy::Http2HeaderBlock& headers) override; void OnHeadersReceived( const spdy::Http2HeaderBlock& response_headers, const spdy::Http2HeaderBlock* pushed_request_headers) override; diff --git a/net/spdy/spdy_http_stream.cc b/net/spdy/spdy_http_stream.cc index 5cd3d5516a6065..e3af6e9fef09fd 100644 --- a/net/spdy/spdy_http_stream.cc +++ b/net/spdy/spdy_http_stream.cc @@ -392,6 +392,20 @@ void SpdyHttpStream::OnHeadersSent() { } } +void SpdyHttpStream::OnEarlyHintsReceived( + const spdy::Http2HeaderBlock& headers) { + DCHECK(!response_headers_complete_); + DCHECK(response_info_); + DCHECK_EQ(stream_->type(), SPDY_REQUEST_RESPONSE_STREAM); + + const bool headers_valid = SpdyHeadersToHttpResponse(headers, response_info_); + CHECK(headers_valid); + + if (!response_callback_.is_null()) { + DoResponseCallback(OK); + } +} + void SpdyHttpStream::OnHeadersReceived( const spdy::Http2HeaderBlock& response_headers, const spdy::Http2HeaderBlock* pushed_request_headers) { diff --git a/net/spdy/spdy_http_stream.h b/net/spdy/spdy_http_stream.h index 50006b157f1838..11ec965cf3e8c6 100644 --- a/net/spdy/spdy_http_stream.h +++ b/net/spdy/spdy_http_stream.h @@ -90,6 +90,7 @@ class NET_EXPORT_PRIVATE SpdyHttpStream : public SpdyStream::Delegate, // SpdyStream::Delegate implementation. void OnHeadersSent() override; + void OnEarlyHintsReceived(const spdy::Http2HeaderBlock& headers) override; void OnHeadersReceived( const spdy::Http2HeaderBlock& response_headers, const spdy::Http2HeaderBlock* pushed_request_headers) override; diff --git a/net/spdy/spdy_proxy_client_socket.cc b/net/spdy/spdy_proxy_client_socket.cc index 94ab2675b5f58e..bd4fc213d95426 100644 --- a/net/spdy/spdy_proxy_client_socket.cc +++ b/net/spdy/spdy_proxy_client_socket.cc @@ -450,6 +450,9 @@ void SpdyProxyClientSocket::OnHeadersSent() { OnIOComplete(OK); } +void SpdyProxyClientSocket::OnEarlyHintsReceived( + const spdy::Http2HeaderBlock& headers) {} + void SpdyProxyClientSocket::OnHeadersReceived( const spdy::Http2HeaderBlock& response_headers, const spdy::Http2HeaderBlock* pushed_request_headers) { diff --git a/net/spdy/spdy_proxy_client_socket.h b/net/spdy/spdy_proxy_client_socket.h index e1d89a262459c6..ca0e3c538f53d0 100644 --- a/net/spdy/spdy_proxy_client_socket.h +++ b/net/spdy/spdy_proxy_client_socket.h @@ -100,6 +100,7 @@ class NET_EXPORT_PRIVATE SpdyProxyClientSocket : public ProxyClientSocket, // SpdyStream::Delegate implementation. void OnHeadersSent() override; + void OnEarlyHintsReceived(const spdy::Http2HeaderBlock& headers) override; void OnHeadersReceived( const spdy::Http2HeaderBlock& response_headers, const spdy::Http2HeaderBlock* pushed_request_headers) override; diff --git a/net/spdy/spdy_session_fuzzer.cc b/net/spdy/spdy_session_fuzzer.cc index c5a3c10ffa8b39..fada2ab15e52d7 100644 --- a/net/spdy/spdy_session_fuzzer.cc +++ b/net/spdy/spdy_session_fuzzer.cc @@ -33,6 +33,7 @@ class FuzzerDelegate : public net::SpdyStream::Delegate { : done_closure_(std::move(done_closure)) {} void OnHeadersSent() override {} + void OnEarlyHintsReceived(const spdy::Http2HeaderBlock& headers) override {} void OnHeadersReceived( const spdy::Http2HeaderBlock& response_headers, const spdy::Http2HeaderBlock* pushed_request_headers) override {} diff --git a/net/spdy/spdy_session_pool_unittest.cc b/net/spdy/spdy_session_pool_unittest.cc index 2c1c2e4a742fa9..cacc43ec70e7dd 100644 --- a/net/spdy/spdy_session_pool_unittest.cc +++ b/net/spdy/spdy_session_pool_unittest.cc @@ -176,6 +176,8 @@ class SessionOpeningDelegate : public SpdyStream::Delegate { void OnHeadersSent() override {} + void OnEarlyHintsReceived(const spdy::Http2HeaderBlock& headers) override {} + void OnHeadersReceived( const spdy::Http2HeaderBlock& response_headers, const spdy::Http2HeaderBlock* pushed_request_headers) override {} diff --git a/net/spdy/spdy_stream.cc b/net/spdy/spdy_stream.cc index c75181fd69a82b..138e23d692ef55 100644 --- a/net/spdy/spdy_stream.cc +++ b/net/spdy/spdy_stream.cc @@ -23,6 +23,7 @@ #include "base/trace_event/memory_usage_estimator.h" #include "base/values.h" #include "net/base/load_timing_info.h" +#include "net/http/http_status_code.h" #include "net/log/net_log.h" #include "net/log/net_log_capture_mode.h" #include "net/log/net_log_event_type.h" @@ -423,17 +424,16 @@ void SpdyStream::OnHeadersReceived( recv_first_byte_time; } - // Ignore informational headers like 103 Early Hints. - // TODO(bnc): Add support for 103 Early Hints, https://crbug.com/671310. - // However, do not ignore 101 Switching Protocols, because broken - // servers might send this as a response to a WebSocket request, - // in which case it needs to pass through so that the WebSocket layer - // can signal an error. - if (status / 100 == 1 && status != 101) { - // Record the timing of the 103 Early Hints response for the experiment - // (https://crbug.com/1093693). - if (status == 103 && first_early_hints_time_.is_null()) - first_early_hints_time_ = recv_first_byte_time; + // Handle informational responses (1xx): + // * Pass through 101 Switching Protocols, because broken servers might + // send this as a response to a WebSocket request, in which case it + // needs to pass through so that the WebSocket layer can signal an + // error. + // * Plumb 103 Early Hints to the delegate. + // * Ignore other informational responses. + if (status / 100 == 1 && status != HTTP_SWITCHING_PROTOCOLS) { + if (status == HTTP_EARLY_HINTS) + OnEarlyHintsReceived(response_headers, recv_first_byte_time); return; } @@ -903,6 +903,35 @@ void SpdyStream::QueueNextDataFrame() { std::make_unique(std::move(data_buffer))); } +void SpdyStream::OnEarlyHintsReceived( + const spdy::Http2HeaderBlock& response_headers, + base::TimeTicks recv_first_byte_time) { + // Record the timing of the 103 Early Hints response for the experiment + // (https://crbug.com/1093693). + if (first_early_hints_time_.is_null()) + first_early_hints_time_ = recv_first_byte_time; + + // Transfer-encoding is a connection specific header. + if (response_headers.find("transfer-encoding") != response_headers.end()) { + const char error[] = "Received transfer-encoding header"; + LogStreamError(ERR_HTTP2_PROTOCOL_ERROR, error); + session_->ResetStream(stream_id_, ERR_HTTP2_PROTOCOL_ERROR, error); + return; + } + + if (type_ != SPDY_REQUEST_RESPONSE_STREAM || io_state_ == STATE_IDLE) { + const char error[] = "Early Hints received before request sent."; + LogStreamError(ERR_HTTP2_PROTOCOL_ERROR, error); + session_->ResetStream(stream_id_, ERR_HTTP2_PROTOCOL_ERROR, error); + return; + } + + // `delegate_` must be attached at this point when `type_` is + // SPDY_REQUEST_RESPONSE_STREAM. + CHECK(delegate_); + delegate_->OnEarlyHintsReceived(response_headers); +} + void SpdyStream::SaveResponseHeaders( const spdy::Http2HeaderBlock& response_headers, int status) { diff --git a/net/spdy/spdy_stream.h b/net/spdy/spdy_stream.h index 1f826d2dd7a650..0a1fdcabfd004a 100644 --- a/net/spdy/spdy_stream.h +++ b/net/spdy/spdy_stream.h @@ -75,13 +75,19 @@ class NET_EXPORT_PRIVATE SpdyStream { // for push streams. Must not cause the stream to be closed. virtual void OnHeadersSent() = 0; - // OnHeadersReceived(), OnDataReceived(), OnTrailers(), and OnClose() - // are guaranteed to be called in the following order: + // OnEarlyHintsReceived(), OnHeadersReceived(), OnDataReceived(), + // OnTrailers(), and OnClose() are guaranteed to be called in the following + // order: + // - OnEarlyHintsReceived() zero or more times; // - OnHeadersReceived() exactly once; // - OnDataReceived() zero or more times; // - OnTrailers() zero or one times; // - OnClose() exactly once. + // Called when a 103 Early Hints response is received. + virtual void OnEarlyHintsReceived( + const spdy::Http2HeaderBlock& headers) = 0; + // Called when response headers have been received. In case of a pushed // stream, the pushed request headers are also passed. virtual void OnHeadersReceived( @@ -447,6 +453,9 @@ class NET_EXPORT_PRIVATE SpdyStream { // |pending_send_data_| is set. void QueueNextDataFrame(); + void OnEarlyHintsReceived(const spdy::Http2HeaderBlock& response_headers, + base::TimeTicks recv_first_byte_time); + // Saves the given headers into |response_headers_| and calls // OnHeadersReceived() on the delegate if attached. void SaveResponseHeaders(const spdy::Http2HeaderBlock& response_headers, diff --git a/net/spdy/spdy_stream_test_util.cc b/net/spdy/spdy_stream_test_util.cc index 5c6085b12a5679..a8d17c7f96b5ab 100644 --- a/net/spdy/spdy_stream_test_util.cc +++ b/net/spdy/spdy_stream_test_util.cc @@ -25,6 +25,9 @@ ClosingDelegate::~ClosingDelegate() = default; void ClosingDelegate::OnHeadersSent() {} +void ClosingDelegate::OnEarlyHintsReceived( + const spdy::Http2HeaderBlock& headers) {} + void ClosingDelegate::OnHeadersReceived( const spdy::Http2HeaderBlock& response_headers, const spdy::Http2HeaderBlock* pushed_request_headers) {} @@ -64,6 +67,12 @@ void StreamDelegateBase::OnHeadersSent() { send_headers_completed_ = true; } +void StreamDelegateBase::OnEarlyHintsReceived( + const spdy::Http2HeaderBlock& headers) { + EXPECT_EQ(stream_->type() != SPDY_PUSH_STREAM, send_headers_completed_); + early_hints_.push_back(headers.Clone()); +} + void StreamDelegateBase::OnHeadersReceived( const spdy::Http2HeaderBlock& response_headers, const spdy::Http2HeaderBlock* pushed_request_headers) { diff --git a/net/spdy/spdy_stream_test_util.h b/net/spdy/spdy_stream_test_util.h index c8092ec05fee8d..0e10cb34ac5f91 100644 --- a/net/spdy/spdy_stream_test_util.h +++ b/net/spdy/spdy_stream_test_util.h @@ -30,6 +30,7 @@ class ClosingDelegate : public SpdyStream::Delegate { ~ClosingDelegate() override; // SpdyStream::Delegate implementation. + void OnEarlyHintsReceived(const spdy::Http2HeaderBlock& headers) override; void OnHeadersSent() override; void OnHeadersReceived( const spdy::Http2HeaderBlock& response_headers, @@ -56,6 +57,7 @@ class StreamDelegateBase : public SpdyStream::Delegate { ~StreamDelegateBase() override; void OnHeadersSent() override; + void OnEarlyHintsReceived(const spdy::Http2HeaderBlock& headers) override; void OnHeadersReceived( const spdy::Http2HeaderBlock& response_headers, const spdy::Http2HeaderBlock* pushed_request_headers) override; @@ -81,6 +83,11 @@ class StreamDelegateBase : public SpdyStream::Delegate { // returns the stream's ID when it was open. spdy::SpdyStreamId stream_id() const { return stream_id_; } + // Returns 103 Early Hints response headers. + const std::vector& early_hints() const { + return early_hints_; + } + std::string GetResponseHeaderValue(const std::string& name) const; bool send_headers_completed() const { return send_headers_completed_; } @@ -96,6 +103,7 @@ class StreamDelegateBase : public SpdyStream::Delegate { spdy::SpdyStreamId stream_id_; TestCompletionCallback callback_; bool send_headers_completed_; + std::vector early_hints_; spdy::Http2HeaderBlock response_headers_; SpdyReadQueue received_data_queue_; LoadTimingInfo load_timing_info_; diff --git a/net/spdy/spdy_stream_unittest.cc b/net/spdy/spdy_stream_unittest.cc index bba60fa5741603..09544c50d01d31 100644 --- a/net/spdy/spdy_stream_unittest.cc +++ b/net/spdy/spdy_stream_unittest.cc @@ -1246,32 +1246,46 @@ TEST_F(SpdyStreamTestWithMockClock, InformationalHeaders) { load_timing_info.receive_non_informational_headers_start); } -// 103 Early Hints hasn't been implemented yet, but we collect timing -// information for the experiment (https://crbug.com/1093693). This tests it. +// Tests that timing information of 103 Eary Hints responses are collected and +// callbacks are called as expected. TEST_F(SpdyStreamTestWithMockClock, EarlyHints) { // Set up the request. spdy::SpdySerializedFrame req( spdy_util_.ConstructSpdyGet(nullptr, 0, 1, LOWEST)); AddWrite(req); - // Set up the early hints response headers. - spdy::Http2HeaderBlock informational_headers; - informational_headers[":status"] = "103"; - spdy::SpdySerializedFrame informational_response( + // Set up two early hints response headers. + const char kLinkHeaderValue1[] = "; rel=preload; as=image"; + spdy::Http2HeaderBlock informational_headers1; + informational_headers1[":status"] = "103"; + informational_headers1["link"] = kLinkHeaderValue1; + spdy::SpdySerializedFrame informational_response1( spdy_util_.ConstructSpdyResponseHeaders( - 1, std::move(informational_headers), false)); - // Add the headers twice to make sure that multiple informational responses - // don't confuse the timing information. + 1, std::move(informational_headers1), false)); + + const char kLinkHeaderValue2[] = "; rel=preload; as=stylesheet"; + spdy::Http2HeaderBlock informational_headers2; + informational_headers2[":status"] = "103"; + informational_headers2["link"] = kLinkHeaderValue2; + spdy::SpdySerializedFrame informational_response2( + spdy_util_.ConstructSpdyResponseHeaders( + 1, std::move(informational_headers2), false)); + + // Add the headers to make sure that multiple informational responses don't + // confuse the timing information. const int kNumberOfInformationalResponses = 2; - for (int i = 0; i < kNumberOfInformationalResponses; ++i) { - // Separate the headers into 2 fragments and add pauses between the - // fragments so that the test runner can advance the mock clock to test - // timing information. - AddMockRead(ReadFrameExceptForLastByte(informational_response)); - AddReadPause(); - AddMockRead(LastByteOfReadFrame(informational_response)); - AddReadPause(); - } + // Separate the headers into 2 fragments and add pauses between the + // fragments so that the test runner can advance the mock clock to test + // timing information. + AddMockRead(ReadFrameExceptForLastByte(informational_response1)); + AddReadPause(); + AddMockRead(LastByteOfReadFrame(informational_response1)); + AddReadPause(); + + AddMockRead(ReadFrameExceptForLastByte(informational_response2)); + AddReadPause(); + AddMockRead(LastByteOfReadFrame(informational_response2)); + AddReadPause(); // Set up the non-informational response headers and body. spdy::SpdySerializedFrame reply( @@ -1303,8 +1317,36 @@ TEST_F(SpdyStreamTestWithMockClock, EarlyHints) { RunUntilNextPause(); AdvanceClock(base::TimeDelta::FromSeconds(1)); } - // We don't check the status code of the informational headers here because - // SpdyStream doesn't propagate it to the delegate. + + // Check the callback was called twice with 103 status code. + const std::vector& early_hints = + delegate().early_hints(); + EXPECT_EQ(early_hints.size(), + static_cast(kNumberOfInformationalResponses)); + { + const spdy::Http2HeaderBlock& hint = delegate().early_hints()[0]; + spdy::Http2HeaderBlock::const_iterator status_iterator = + hint.find(spdy::kHttp2StatusHeader); + ASSERT_TRUE(status_iterator != hint.end()); + EXPECT_EQ(status_iterator->second, "103"); + + spdy::Http2HeaderBlock::const_iterator link_header_iterator = + hint.find("link"); + ASSERT_TRUE(link_header_iterator != hint.end()); + EXPECT_EQ(link_header_iterator->second, kLinkHeaderValue1); + } + { + const spdy::Http2HeaderBlock& hint = delegate().early_hints()[1]; + spdy::Http2HeaderBlock::const_iterator status_iterator = + hint.find(spdy::kHttp2StatusHeader); + ASSERT_TRUE(status_iterator != hint.end()); + EXPECT_EQ(status_iterator->second, "103"); + + spdy::Http2HeaderBlock::const_iterator link_header_iterator = + hint.find("link"); + ASSERT_TRUE(link_header_iterator != hint.end()); + EXPECT_EQ(link_header_iterator->second, kLinkHeaderValue2); + } // The receive non-informational headers start time should be captured at this // time. diff --git a/net/url_request/url_request.cc b/net/url_request/url_request.cc index 9e4626b4edbfc9..865fb4db6a4390 100644 --- a/net/url_request/url_request.cc +++ b/net/url_request/url_request.cc @@ -626,6 +626,7 @@ void URLRequest::StartJob(std::unique_ptr job) { job_->SetExtraRequestHeaders(extra_request_headers_); job_->SetPriority(priority_); job_->SetRequestHeadersCallback(request_headers_callback_); + job_->SetEarlyResponseHeadersCallback(early_response_headers_callback_); job_->SetResponseHeadersCallback(response_headers_callback_); if (upload_data_stream_.get()) @@ -1182,6 +1183,13 @@ void URLRequest::SetResponseHeadersCallback(ResponseHeadersCallback callback) { response_headers_callback_ = std::move(callback); } +void URLRequest::SetEarlyResponseHeadersCallback( + ResponseHeadersCallback callback) { + DCHECK(!job_.get()); + DCHECK(early_response_headers_callback_.is_null()); + early_response_headers_callback_ = std::move(callback); +} + void URLRequest::set_socket_tag(const SocketTag& socket_tag) { DCHECK(!is_pending_); DCHECK(url().SchemeIsHTTPOrHTTPS()); diff --git a/net/url_request/url_request.h b/net/url_request/url_request.h index 8842d2e17fdb05..0ad11d35f36f5a 100644 --- a/net/url_request/url_request.h +++ b/net/url_request/url_request.h @@ -693,6 +693,10 @@ class NET_EXPORT URLRequest : public base::SupportsUserData { // called with a response from the server. void SetResponseHeadersCallback(ResponseHeadersCallback callback); + // Sets a callback that will be invoked each time a 103 Early Hints response + // is received from the remote party. + void SetEarlyResponseHeadersCallback(ResponseHeadersCallback callback); + // Sets socket tag to be applied to all sockets used to execute this request. // Must be set before Start() is called. Only currently supported for HTTP // and HTTPS requests on Android; UID tagging requires @@ -956,8 +960,9 @@ class NET_EXPORT URLRequest : public base::SupportsUserData { SocketTag socket_tag_; - // See Set{Request|Response}HeadersCallback() above for details. + // See Set{Request|Response,EarlyResponse}HeadersCallback() above for details. RequestHeadersCallback request_headers_callback_; + ResponseHeadersCallback early_response_headers_callback_; ResponseHeadersCallback response_headers_callback_; bool upgrade_if_insecure_; diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc index 5f465591083087..dbf5ffae2bec29 100644 --- a/net/url_request/url_request_http_job.cc +++ b/net/url_request/url_request_http_job.cc @@ -463,6 +463,8 @@ void URLRequestHttpJob::StartTransactionInternal() { transaction_->SetConnectedCallback(base::BindRepeating( &URLRequestHttpJob::NotifyConnectedCallback, base::Unretained(this))); transaction_->SetRequestHeadersCallback(request_headers_callback_); + transaction_->SetEarlyResponseHeadersCallback( + early_response_headers_callback_); transaction_->SetResponseHeadersCallback(response_headers_callback_); if (!throttling_entry_.get() || @@ -1405,6 +1407,13 @@ void URLRequestHttpJob::SetRequestHeadersCallback( request_headers_callback_ = std::move(callback); } +void URLRequestHttpJob::SetEarlyResponseHeadersCallback( + ResponseHeadersCallback callback) { + DCHECK(!transaction_); + DCHECK(!early_response_headers_callback_); + early_response_headers_callback_ = std::move(callback); +} + void URLRequestHttpJob::SetResponseHeadersCallback( ResponseHeadersCallback callback) { DCHECK(!transaction_); diff --git a/net/url_request/url_request_http_job.h b/net/url_request/url_request_http_job.h index 1ad9a378e26502..4b09404d87fd61 100644 --- a/net/url_request/url_request_http_job.h +++ b/net/url_request/url_request_http_job.h @@ -50,6 +50,8 @@ class NET_EXPORT_PRIVATE URLRequestHttpJob : public URLRequestJob { static std::unique_ptr Create(URLRequest* request); void SetRequestHeadersCallback(RequestHeadersCallback callback) override; + void SetEarlyResponseHeadersCallback( + ResponseHeadersCallback callback) override; void SetResponseHeadersCallback(ResponseHeadersCallback callback) override; protected: @@ -259,6 +261,7 @@ class NET_EXPORT_PRIVATE URLRequestHttpJob : public URLRequestJob { int64_t total_sent_bytes_from_previous_transactions_; RequestHeadersCallback request_headers_callback_; + ResponseHeadersCallback early_response_headers_callback_; ResponseHeadersCallback response_headers_callback_; base::WeakPtrFactory weak_factory_{this}; diff --git a/net/url_request/url_request_job.h b/net/url_request/url_request_job.h index 29b381de17dc75..9d952e24e18e32 100644 --- a/net/url_request/url_request_job.h +++ b/net/url_request/url_request_job.h @@ -241,6 +241,12 @@ class NET_EXPORT URLRequestJob { // from the remote party with the actual response headers recieved. virtual void SetResponseHeadersCallback(ResponseHeadersCallback callback) {} + // Sets a callback that will be invoked each time a 103 Early Hints response + // is received from the remote party with the actual response headers + // received. + virtual void SetEarlyResponseHeadersCallback( + ResponseHeadersCallback callback) {} + // Causes the current transaction always close its active socket on // destruction. Does not close H2/H3 sessions. virtual void CloseConnectionOnDestruction(); diff --git a/net/websockets/websocket_basic_stream_adapters.cc b/net/websockets/websocket_basic_stream_adapters.cc index e995869e49364f..398fd240ecc8e4 100644 --- a/net/websockets/websocket_basic_stream_adapters.cc +++ b/net/websockets/websocket_basic_stream_adapters.cc @@ -130,6 +130,12 @@ void WebSocketSpdyStreamAdapter::OnHeadersSent() { delegate_->OnHeadersSent(); } +void WebSocketSpdyStreamAdapter::OnEarlyHintsReceived( + const spdy::Http2HeaderBlock& headers) { + // This callback should not be called for a WebSocket handshake. + NOTREACHED(); +} + void WebSocketSpdyStreamAdapter::OnHeadersReceived( const spdy::Http2HeaderBlock& response_headers, const spdy::Http2HeaderBlock* pushed_request_headers) { diff --git a/net/websockets/websocket_basic_stream_adapters.h b/net/websockets/websocket_basic_stream_adapters.h index 23b6a90e9b73aa..e4bb64e4414898 100644 --- a/net/websockets/websocket_basic_stream_adapters.h +++ b/net/websockets/websocket_basic_stream_adapters.h @@ -94,6 +94,7 @@ class NET_EXPORT_PRIVATE WebSocketSpdyStreamAdapter // SpdyStream::Delegate methods. void OnHeadersSent() override; + void OnEarlyHintsReceived(const spdy::Http2HeaderBlock& headers) override; void OnHeadersReceived( const spdy::Http2HeaderBlock& response_headers, const spdy::Http2HeaderBlock* pushed_request_headers) override; diff --git a/services/network/throttling/throttling_network_transaction.cc b/services/network/throttling/throttling_network_transaction.cc index ea61af7ed35d69..47567ac4bd87c5 100644 --- a/services/network/throttling/throttling_network_transaction.cc +++ b/services/network/throttling/throttling_network_transaction.cc @@ -288,6 +288,11 @@ void ThrottlingNetworkTransaction::SetResponseHeadersCallback( network_transaction_->SetResponseHeadersCallback(std::move(callback)); } +void ThrottlingNetworkTransaction::SetEarlyResponseHeadersCallback( + net::ResponseHeadersCallback callback) { + network_transaction_->SetEarlyResponseHeadersCallback(std::move(callback)); +} + void ThrottlingNetworkTransaction::SetConnectedCallback( const ConnectedCallback& callback) { network_transaction_->SetConnectedCallback(callback); diff --git a/services/network/throttling/throttling_network_transaction.h b/services/network/throttling/throttling_network_transaction.h index 83b510a8014cae..b6099528e9219f 100644 --- a/services/network/throttling/throttling_network_transaction.h +++ b/services/network/throttling/throttling_network_transaction.h @@ -83,6 +83,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) ThrottlingNetworkTransaction void SetRequestHeadersCallback(net::RequestHeadersCallback callback) override; void SetResponseHeadersCallback( net::ResponseHeadersCallback callback) override; + void SetEarlyResponseHeadersCallback( + net::ResponseHeadersCallback callback) override; int ResumeNetworkStart() override; void GetConnectionAttempts(net::ConnectionAttempts* out) const override; void CloseConnectionOnDestruction() override; diff --git a/services/network/url_loader.cc b/services/network/url_loader.cc index cdc72082443395..136c4ec4841235 100644 --- a/services/network/url_loader.cc +++ b/services/network/url_loader.cc @@ -648,6 +648,12 @@ URLLoader::URLLoader( &URLLoader::SetRawResponseHeaders, base::Unretained(this))); } + // TODO(crbug.com/671310): We may want to add + // ResourceRequest::report_early_response so that we can bypass setting + // EarlyResponseHeadersCallback if the client can't handle Early Hints. + url_request_->SetEarlyResponseHeadersCallback(base::BindRepeating( + &URLLoader::NotifyEarlyResponse, base::Unretained(this))); + if (keepalive_ && keepalive_statistics_recorder_) { keepalive_statistics_recorder_->OnLoadStarted( *factory_params_->top_frame_id, keepalive_request_size_); @@ -1913,6 +1919,16 @@ void URLLoader::SetRawResponseHeaders( raw_response_headers_ = headers; } +void URLLoader::NotifyEarlyResponse( + scoped_refptr headers) { + DCHECK(!has_received_response_); + DCHECK(url_loader_client_); + DCHECK(headers); + DCHECK_EQ(headers->response_code(), 103); + // TODO(crbug.com/671310): Notify the early response to `url_loader_client_` + // after https://crrev.com/c/2725403 is landed. +} + void URLLoader::SetRawRequestHeadersAndNotify( net::HttpRawRequestHeaders headers) { auto* devtools_observer = GetDevToolsObserver(); diff --git a/services/network/url_loader.h b/services/network/url_loader.h index 7c21a65ab6222d..f8c6112335ea43 100644 --- a/services/network/url_loader.h +++ b/services/network/url_loader.h @@ -324,6 +324,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) URLLoader void SendResponseToClient(); void CompletePendingWrite(bool success); void SetRawResponseHeaders(scoped_refptr); + void NotifyEarlyResponse(scoped_refptr); void SetRawRequestHeadersAndNotify(net::HttpRawRequestHeaders); void SendUploadProgress(const net::UploadProgress& progress); void OnUploadProgressACK();