Skip to content

Commit

Permalink
Introduce URLRequest::OnConnected() callback
Browse files Browse the repository at this point in the history 
Originally based on a partial revert of crrev.com/c/2116892.

This CL lays some groundwork for CORS-RFC1918 code changes.

Bug: 986744
Change-Id: I26a10d07099fab39a77fdc6e12c1d4b2de324076
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2257752
Commit-Queue: Titouan Rigoudy <titouan@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#785723}
  • Loading branch information
@letitz
letitz authored and Commit Bot committed Jul 7, 2020
1 parent d86d92f commit 78af7da
Show file tree
Hide file tree
Showing 19 changed files with 407 additions and 8 deletions.
7 changes: 7 additions & 0 deletions net/http/http_cache_transaction.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,12 @@ void HttpCache::Transaction::SetBeforeNetworkStartCallback(
before_network_start_callback_ = callback;
}

void HttpCache::Transaction::SetConnectedCallback(
const ConnectedCallback& callback) {
DCHECK(!network_trans_);
connected_callback_ = callback;
}

void HttpCache::Transaction::SetRequestHeadersCallback(
RequestHeadersCallback callback) {
DCHECK(!network_trans_);
Expand Down Expand Up @@ -1685,6 +1691,7 @@ int HttpCache::Transaction::DoSendRequest() {
}

network_trans_->SetBeforeNetworkStartCallback(before_network_start_callback_);
network_trans_->SetConnectedCallback(connected_callback_);
network_trans_->SetRequestHeadersCallback(request_headers_callback_);
network_trans_->SetResponseHeadersCallback(response_headers_callback_);

Expand Down
2 changes: 2 additions & 0 deletions net/http/http_cache_transaction.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ class NET_EXPORT_PRIVATE HttpCache::Transaction : public HttpTransaction {
WebSocketHandshakeStreamBase::CreateHelper* create_helper) override;
void SetBeforeNetworkStartCallback(
const BeforeNetworkStartCallback& callback) override;
void SetConnectedCallback(const ConnectedCallback& callback) override;
void SetRequestHeadersCallback(RequestHeadersCallback callback) override;
void SetResponseHeadersCallback(ResponseHeadersCallback callback) override;
int ResumeNetworkStart() override;
Expand Down Expand Up @@ -667,6 +668,7 @@ class NET_EXPORT_PRIVATE HttpCache::Transaction : public HttpTransaction {
websocket_handshake_stream_base_create_helper_;

BeforeNetworkStartCallback before_network_start_callback_;
ConnectedCallback connected_callback_;
RequestHeadersCallback request_headers_callback_;
ResponseHeadersCallback response_headers_callback_;

Expand Down
21 changes: 18 additions & 3 deletions net/http/http_network_transaction.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,11 @@ void HttpNetworkTransaction::SetBeforeNetworkStartCallback(
before_network_start_callback_ = callback;
}

void HttpNetworkTransaction::SetConnectedCallback(
const ConnectedCallback& callback) {
connected_callback_ = callback;
}

void HttpNetworkTransaction::SetRequestHeadersCallback(
RequestHeadersCallback callback) {
DCHECK(!stream_);
Expand Down Expand Up @@ -857,9 +862,7 @@ int HttpNetworkTransaction::DoInitStream() {
}

int HttpNetworkTransaction::DoInitStreamComplete(int result) {
if (result == OK) {
next_state_ = STATE_GENERATE_PROXY_AUTH_TOKEN;
} else {
if (result != OK) {
if (result < 0)
result = HandleIOError(result);

Expand All @@ -869,8 +872,20 @@ int HttpNetworkTransaction::DoInitStreamComplete(int result) {
total_sent_bytes_ += stream_->GetTotalSentBytes();
}
CacheNetErrorDetailsAndResetStream();

return result;
}

// Fire off notification that we have successfully connected.
if (!connected_callback_.is_null()) {
result = connected_callback_.Run();
DCHECK_NE(result, ERR_IO_PENDING);
}

if (result == OK) {
// Only transition if we succeeded. Otherwise stop at STATE_NONE.
next_state_ = STATE_GENERATE_PROXY_AUTH_TOKEN;
}
return result;
}

Expand Down
2 changes: 2 additions & 0 deletions net/http/http_network_transaction.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ class NET_EXPORT_PRIVATE HttpNetworkTransaction
WebSocketHandshakeStreamBase::CreateHelper* create_helper) override;
void SetBeforeNetworkStartCallback(
const BeforeNetworkStartCallback& callback) override;
void SetConnectedCallback(const ConnectedCallback& callback) override;
void SetRequestHeadersCallback(RequestHeadersCallback callback) override;
void SetResponseHeadersCallback(ResponseHeadersCallback callback) override;

Expand Down Expand Up @@ -413,6 +414,7 @@ class NET_EXPORT_PRIVATE HttpNetworkTransaction
websocket_handshake_stream_base_create_helper_;

BeforeNetworkStartCallback before_network_start_callback_;
ConnectedCallback connected_callback_;
RequestHeadersCallback request_headers_callback_;
ResponseHeadersCallback response_headers_callback_;

Expand Down
200 changes: 200 additions & 0 deletions net/http/http_network_transaction_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,15 @@ class FailingProxyResolverFactory : public ProxyResolverFactory {
}
};

HttpRequestInfo DefaultRequestInfo() {
HttpRequestInfo info;
info.method = "GET";
info.url = GURL("http://www.example.org");
info.traffic_annotation =
net::MutableNetworkTrafficAnnotationTag(TRAFFIC_ANNOTATION_FOR_TESTS);
return info;
}

} // namespace

class HttpNetworkTransactionTest : public PlatformTest,
Expand Down Expand Up @@ -576,6 +585,40 @@ class HttpNetworkTransactionTest : public PlatformTest,

namespace {

// Used for injecting ConnectedCallback instances in HttpTransaction.
class ConnectedHandler {
public:
ConnectedHandler() = default;

ConnectedHandler(const ConnectedHandler&) = default;
ConnectedHandler& operator=(const ConnectedHandler&) = default;
ConnectedHandler(ConnectedHandler&&) = default;
ConnectedHandler& operator=(ConnectedHandler&&) = default;

// Returns a callback bound to this->OnConnected().
// The returned callback must not outlive this instance.
HttpTransaction::ConnectedCallback Callback() {
return base::BindRepeating(&ConnectedHandler::OnConnected,
base::Unretained(this));
}

// Compatible with HttpTransaction::ConnectedCallback.
int OnConnected() {
call_count_++;
return result_;
}

// Returns the number of times OnConnected() was called.
int call_count() const { return call_count_; }

// Sets the value to be returned by subsequent calls to OnConnected().
void set_result(int result) { result_ = result; }

private:
int call_count_ = 0;
int result_ = OK;
};

// Fill |str| with a long header list that consumes >= |size| bytes.
void FillLargeHeadersString(std::string* str, int size) {
const char row[] =
Expand Down Expand Up @@ -872,6 +915,154 @@ TEST_F(HttpNetworkTransactionTest, SimpleGETHostResolutionFailure) {
EXPECT_THAT(response->resolve_error_info.error, IsError(ERR_DNS_TIMED_OUT));
}

// This test verifies that if the transaction fails before even connecting to a
// remote endpoint, the ConnectedCallback is never called.
TEST_F(HttpNetworkTransactionTest, ConnectedCallbackNeverCalled) {
auto resolver = std::make_unique<MockHostResolver>();
resolver->rules()->AddSimulatedTimeoutFailure("www.example.org");
session_deps_.host_resolver = std::move(resolver);

ConnectedHandler connected_handler;
std::unique_ptr<HttpNetworkSession> session = CreateSession(&session_deps_);
auto request = DefaultRequestInfo();
HttpNetworkTransaction transaction(DEFAULT_PRIORITY, session.get());
transaction.SetConnectedCallback(connected_handler.Callback());

TestCompletionCallback callback;
transaction.Start(&request, callback.callback(), NetLogWithSource());
callback.WaitForResult();

EXPECT_EQ(connected_handler.call_count(), 0);
}

// This test verifies that if the ConnectedCallback returns an error, the
// entire transaction fails with that error.
TEST_F(HttpNetworkTransactionTest, ConnectedCallbackFailure) {
// The exact error code does not matter, as long as it is the same one
// returned by the transaction overall.
ConnectedHandler connected_handler;
connected_handler.set_result(ERR_NOT_IMPLEMENTED);

std::unique_ptr<HttpNetworkSession> session = CreateSession(&session_deps_);
auto request = DefaultRequestInfo();
HttpNetworkTransaction transaction(DEFAULT_PRIORITY, session.get());
transaction.SetConnectedCallback(connected_handler.Callback());

// We never get to writing any data, but we still need a socket.
StaticSocketDataProvider data;
session_deps_.socket_factory->AddSocketDataProvider(&data);

TestCompletionCallback callback;
EXPECT_THAT(
transaction.Start(&request, callback.callback(), NetLogWithSource()),
IsError(ERR_IO_PENDING));
EXPECT_THAT(callback.WaitForResult(), IsError(ERR_NOT_IMPLEMENTED));

EXPECT_EQ(connected_handler.call_count(), 1);
}

// This test verifies that the ConnectedCallback is called once in the case of
// simple requests.
TEST_F(HttpNetworkTransactionTest, ConnectedCallbackCalledOnce) {
MockRead data_reads[] = {
MockRead("HTTP/1.0 200 OK\r\n"),
MockRead(SYNCHRONOUS, OK),
};
StaticSocketDataProvider data(data_reads, base::span<MockWrite>());
session_deps_.socket_factory->AddSocketDataProvider(&data);

ConnectedHandler connected_handler;
std::unique_ptr<HttpNetworkSession> session = CreateSession(&session_deps_);
auto request = DefaultRequestInfo();
HttpNetworkTransaction transaction(DEFAULT_PRIORITY, session.get());
transaction.SetConnectedCallback(connected_handler.Callback());

TestCompletionCallback callback;
EXPECT_THAT(
transaction.Start(&request, callback.callback(), NetLogWithSource()),
IsError(ERR_IO_PENDING));
EXPECT_THAT(callback.WaitForResult(), IsOk());

EXPECT_EQ(connected_handler.call_count(), 1);
}

// This test verifies that the ConnectedCallback is called once more per
// authentication challenge.
TEST_F(HttpNetworkTransactionTest, ConnectedCallbackCalledOnEachAuthChallenge) {
ConnectedHandler connected_handler;
std::unique_ptr<HttpNetworkSession> session = CreateSession(&session_deps_);
auto request = DefaultRequestInfo();
HttpNetworkTransaction transaction(DEFAULT_PRIORITY, session.get());
transaction.SetConnectedCallback(connected_handler.Callback());

// First request receives an auth challenge.
MockRead data_reads1[] = {
MockRead("HTTP/1.0 401 Unauthorized\r\n"),
MockRead("WWW-Authenticate: Basic realm=\"MyRealm1\"\r\n\r\n"),
MockRead(SYNCHRONOUS, ERR_FAILED),
};
StaticSocketDataProvider data1(data_reads1, base::span<MockWrite>());
session_deps_.socket_factory->AddSocketDataProvider(&data1);

// Second request is allowed through.
MockRead data_reads2[] = {
MockRead("HTTP/1.0 200 OK\r\n\r\n"),
MockRead(SYNCHRONOUS, OK),
};
StaticSocketDataProvider data2(data_reads2, base::span<MockWrite>());
session_deps_.socket_factory->AddSocketDataProvider(&data2);

// First request, connects once.
TestCompletionCallback callback1;
EXPECT_THAT(
transaction.Start(&request, callback1.callback(), NetLogWithSource()),
IsError(ERR_IO_PENDING));
EXPECT_THAT(callback1.WaitForResult(), IsOk());

EXPECT_EQ(connected_handler.call_count(), 1);

// Second request, connects again.
TestCompletionCallback callback2;
EXPECT_THAT(transaction.RestartWithAuth(AuthCredentials(kFoo, kBar),
callback2.callback()),
IsError(ERR_IO_PENDING));
EXPECT_THAT(callback2.WaitForResult(), IsOk());

EXPECT_EQ(connected_handler.call_count(), 2);
}

// This test verifies that the ConnectedCallback is called once more per retry.
TEST_F(HttpNetworkTransactionTest, ConnectedCallbackCalledOnEachRetry) {
ConnectedHandler connected_handler;
std::unique_ptr<HttpNetworkSession> session = CreateSession(&session_deps_);
auto request = DefaultRequestInfo();
HttpNetworkTransaction transaction(DEFAULT_PRIORITY, session.get());
transaction.SetConnectedCallback(connected_handler.Callback());

// First request receives a retryable error.
MockRead data_reads1[] = {
MockRead(SYNCHRONOUS, ERR_HTTP2_SERVER_REFUSED_STREAM),
};
StaticSocketDataProvider data1(data_reads1, base::span<MockWrite>());
session_deps_.socket_factory->AddSocketDataProvider(&data1);

// Second request is allowed through.
MockRead data_reads2[] = {
MockRead("HTTP/1.0 200 OK\r\n\r\n"),
MockRead(SYNCHRONOUS, OK),
};
StaticSocketDataProvider data2(data_reads2, base::span<MockWrite>());
session_deps_.socket_factory->AddSocketDataProvider(&data2);

TestCompletionCallback callback1;
EXPECT_THAT(
transaction.Start(&request, callback1.callback(), NetLogWithSource()),
IsError(ERR_IO_PENDING));
EXPECT_THAT(callback1.WaitForResult(), IsOk());

EXPECT_EQ(connected_handler.call_count(), 2);
}

// Allow up to 4 bytes of junk to precede status line.
TEST_F(HttpNetworkTransactionTest, StatusLineJunk3Bytes) {
MockRead data_reads[] = {
Expand Down Expand Up @@ -1187,6 +1378,8 @@ TEST_F(HttpNetworkTransactionTest, Head) {

std::unique_ptr<HttpNetworkSession> session(CreateSession(&session_deps_));
HttpNetworkTransaction trans(DEFAULT_PRIORITY, session.get());
ConnectedHandler connected_handler;
trans.SetConnectedCallback(connected_handler.Callback());

MockWrite data_writes1[] = {
MockWrite("HEAD / HTTP/1.1\r\n"
Expand Down Expand Up @@ -1220,6 +1413,7 @@ TEST_F(HttpNetworkTransactionTest, Head) {
EXPECT_EQ(1234, response->headers->GetContentLength());
EXPECT_EQ("HTTP/1.1 404 Not Found", response->headers->GetStatusLine());
EXPECT_TRUE(response->proxy_server.is_direct());
EXPECT_EQ(connected_handler.call_count(), 1);

std::string server_header;
size_t iter = 0;
Expand Down Expand Up @@ -15836,6 +16030,8 @@ TEST_F(HttpNetworkTransactionTest, ProxyGet) {
TestCompletionCallback callback1;

HttpNetworkTransaction trans(DEFAULT_PRIORITY, session.get());
ConnectedHandler connected_handler;
trans.SetConnectedCallback(connected_handler.Callback());

int rv = trans.Start(&request, callback1.callback(), log.bound());
EXPECT_THAT(rv, IsError(ERR_IO_PENDING));
Expand All @@ -15853,6 +16049,7 @@ TEST_F(HttpNetworkTransactionTest, ProxyGet) {
EXPECT_EQ(ProxyServer(ProxyServer::SCHEME_HTTP,
HostPortPair::FromString("myproxy:70")),
response->proxy_server);
EXPECT_EQ(1, connected_handler.call_count());
EXPECT_TRUE(HttpVersion(1, 1) == response->headers->GetHttpVersion());

LoadTimingInfo load_timing_info;
Expand Down Expand Up @@ -15904,6 +16101,8 @@ TEST_F(HttpNetworkTransactionTest, ProxyTunnelGet) {
TestCompletionCallback callback1;

HttpNetworkTransaction trans(DEFAULT_PRIORITY, session.get());
ConnectedHandler connected_handler;
trans.SetConnectedCallback(connected_handler.Callback());

int rv = trans.Start(&request, callback1.callback(), log.bound());
EXPECT_THAT(rv, IsError(ERR_IO_PENDING));
Expand All @@ -15930,6 +16129,7 @@ TEST_F(HttpNetworkTransactionTest, ProxyTunnelGet) {
EXPECT_EQ(ProxyServer(ProxyServer::SCHEME_HTTP,
HostPortPair::FromString("myproxy:70")),
response->proxy_server);
EXPECT_EQ(1, connected_handler.call_count());

LoadTimingInfo load_timing_info;
EXPECT_TRUE(trans.GetLoadTimingInfo(&load_timing_info));
Expand Down
21 changes: 21 additions & 0 deletions net/http/http_transaction.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,24 @@ class NET_EXPORT_PRIVATE HttpTransaction {
// ResumeNetworkStart is called before establishing a connection.
typedef base::Callback<void(bool* defer)> BeforeNetworkStartCallback;

// Called each time a connection is obtained, before any data is sent.
//
// This can be called multiple times for a single transaction, in the case of
// retries, auth challenges, and split range requests.
// TODO(crbug.com/986744): Verify range request behavior with tests.
//
// The callee can call GetRemoteEndpoint() on the caller transaction to
// determine where the latest connection terminates.
//
// If this callback returns an error, the transaction fails with that error.
// Otherwise the transaction continues unimpeded.
// Must not return ERR_IO_PENDING.
//
// TODO(crbug.com/591068): Allow ERR_IO_PENDING, add a new state machine state
// to wait on a callback (either passed to this callback or a new explicit
// method like ResumeNetworkStart()) to be called before continuing.
typedef base::RepeatingCallback<int()> ConnectedCallback;

// Stops any pending IO and destroys the transaction object.
virtual ~HttpTransaction() {}

Expand Down Expand Up @@ -173,6 +191,9 @@ class NET_EXPORT_PRIVATE HttpTransaction {
virtual void SetBeforeNetworkStartCallback(
const BeforeNetworkStartCallback& callback) = 0;

// Sets the callback to receive a notification upon connection.
virtual void SetConnectedCallback(const ConnectedCallback& callback) = 0;

virtual void SetRequestHeadersCallback(RequestHeadersCallback callback) = 0;
virtual void SetResponseHeadersCallback(ResponseHeadersCallback callback) = 0;

Expand Down

0 comments on commit 78af7da

Please sign in to comment.