Skip to content

Commit

Permalink
Plumb 103 responses from SpdyStream to the network service
Browse files Browse the repository at this point in the history
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 <bashi@chromium.org>
Reviewed-by: David Schinazi <dschinazi@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Bence Béky <bnc@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#862630}
  • Loading branch information
bashi authored and Chromium LUCI CQ committed Mar 13, 2021
1 parent 006a5c2 commit 7415553
Show file tree
Hide file tree
Showing 32 changed files with 276 additions and 34 deletions.
8 changes: 8 additions & 0 deletions net/http/http_cache_transaction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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.
Expand Down
3 changes: 3 additions & 0 deletions net/http/http_cache_transaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down
29 changes: 29 additions & 0 deletions net/http/http_network_transaction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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_);
Expand Down Expand Up @@ -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<HttpResponseHeaders>(std::string());
next_state_ = STATE_READ_HEADERS;
return OK;
}

if (!ContentEncodingsValid())
return ERR_CONTENT_DECODING_FAILED;

Expand Down
3 changes: 3 additions & 0 deletions net/http/http_network_transaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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_;
Expand Down
1 change: 1 addition & 0 deletions net/http/http_status_code_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 2 additions & 0 deletions net/http/http_transaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions net/http/http_transaction_test_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ class MockNetworkTransaction

void SetRequestHeadersCallback(RequestHeadersCallback callback) override {}
void SetResponseHeadersCallback(ResponseHeadersCallback) override {}
void SetEarlyResponseHeadersCallback(ResponseHeadersCallback) override {}

int ResumeNetworkStart() override;

Expand Down
7 changes: 7 additions & 0 deletions net/log/net_log_event_type_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -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": <The list of header:value pairs>,
// }
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)

Expand Down
6 changes: 6 additions & 0 deletions net/spdy/bidirectional_stream_spdy_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions net/spdy/bidirectional_stream_spdy_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
14 changes: 14 additions & 0 deletions net/spdy/spdy_http_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions net/spdy/spdy_http_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 3 additions & 0 deletions net/spdy/spdy_proxy_client_socket.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions net/spdy/spdy_proxy_client_socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions net/spdy/spdy_session_fuzzer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
Expand Down
2 changes: 2 additions & 0 deletions net/spdy/spdy_session_pool_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
Expand Down
51 changes: 40 additions & 11 deletions net/spdy/spdy_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -903,6 +903,35 @@ void SpdyStream::QueueNextDataFrame() {
std::make_unique<SimpleBufferProducer>(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) {
Expand Down
13 changes: 11 additions & 2 deletions net/spdy/spdy_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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,
Expand Down
9 changes: 9 additions & 0 deletions net/spdy/spdy_stream_test_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}
Expand Down Expand Up @@ -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) {
Expand Down
8 changes: 8 additions & 0 deletions net/spdy/spdy_stream_test_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
Expand All @@ -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<spdy::Http2HeaderBlock>& early_hints() const {
return early_hints_;
}

std::string GetResponseHeaderValue(const std::string& name) const;
bool send_headers_completed() const { return send_headers_completed_; }

Expand All @@ -96,6 +103,7 @@ class StreamDelegateBase : public SpdyStream::Delegate {
spdy::SpdyStreamId stream_id_;
TestCompletionCallback callback_;
bool send_headers_completed_;
std::vector<spdy::Http2HeaderBlock> early_hints_;
spdy::Http2HeaderBlock response_headers_;
SpdyReadQueue received_data_queue_;
LoadTimingInfo load_timing_info_;
Expand Down
Loading

0 comments on commit 7415553

Please sign in to comment.