diff options
| -rw-r--r-- | net/http/http_pipelined_connection.h | 12 | ||||
| -rw-r--r-- | net/http/http_pipelined_connection_impl.cc | 44 | ||||
| -rw-r--r-- | net/http/http_pipelined_connection_impl.h | 4 | ||||
| -rw-r--r-- | net/http/http_pipelined_connection_impl_unittest.cc | 140 | ||||
| -rw-r--r-- | net/http/http_pipelined_host.cc | 108 | ||||
| -rw-r--r-- | net/http/http_pipelined_host.h | 70 | ||||
| -rw-r--r-- | net/http/http_pipelined_host_impl.cc | 183 | ||||
| -rw-r--r-- | net/http/http_pipelined_host_impl.h | 99 | ||||
| -rw-r--r-- | net/http/http_pipelined_host_impl_unittest.cc | 301 | ||||
| -rw-r--r-- | net/http/http_pipelined_host_pool.cc | 78 | ||||
| -rw-r--r-- | net/http/http_pipelined_host_pool.h | 36 | ||||
| -rw-r--r-- | net/http/http_pipelined_host_pool_unittest.cc | 185 | ||||
| -rw-r--r-- | net/http/http_pipelined_host_unittest.cc | 201 | ||||
| -rw-r--r-- | net/http/http_pipelined_network_transaction_unittest.cc | 224 | ||||
| -rw-r--r-- | net/http/http_stream_factory_impl.cc | 2 | ||||
| -rw-r--r-- | net/http/http_stream_factory_impl.h | 11 | ||||
| -rw-r--r-- | net/http/http_stream_factory_impl_job.cc | 8 | ||||
| -rw-r--r-- | net/http/http_stream_factory_impl_job.h | 2 | ||||
| -rw-r--r-- | net/net.gyp | 6 |
19 files changed, 1288 insertions, 426 deletions
diff --git a/net/http/http_pipelined_connection.h b/net/http/http_pipelined_connection.h index 59e61d6..e8c332b 100644 --- a/net/http/http_pipelined_connection.h +++ b/net/http/http_pipelined_connection.h @@ -19,12 +19,24 @@ struct SSLConfig; class NET_EXPORT_PRIVATE HttpPipelinedConnection { public: + enum Feedback { + OK, + PIPELINE_SOCKET_ERROR, + OLD_HTTP_VERSION, + MUST_CLOSE_CONNECTION, + }; + class Delegate { public: // Called when a pipeline has newly available capacity. This may be because // the first request has been sent and the pipeline is now active. Or, it // may be because a request successfully completed. virtual void OnPipelineHasCapacity(HttpPipelinedConnection* pipeline) = 0; + + // Called every time a pipeline receives headers. Lets the delegate know if + // the headers indicate that pipelining can be used. + virtual void OnPipelineFeedback(HttpPipelinedConnection* pipeline, + Feedback feedback) = 0; }; class Factory { diff --git a/net/http/http_pipelined_connection_impl.cc b/net/http/http_pipelined_connection_impl.cc index 7792970..95d27a6 100644 --- a/net/http/http_pipelined_connection_impl.cc +++ b/net/http/http_pipelined_connection_impl.cc @@ -12,6 +12,7 @@ #include "net/http/http_response_body_drainer.h" #include "net/http/http_response_headers.h" #include "net/http/http_stream_parser.h" +#include "net/http/http_version.h" #include "net/socket/client_socket_handle.h" namespace net { @@ -167,7 +168,7 @@ int HttpPipelinedConnectionImpl::DoSendRequestLoop(int result) { rv = DoEvictPendingSendRequests(rv); break; default: - NOTREACHED() << "bad send state: " << state; + CHECK(false) << "bad send state: " << state; rv = ERR_FAILED; break; } @@ -337,7 +338,7 @@ int HttpPipelinedConnectionImpl::DoReadHeadersLoop(int result) { case READ_STATE_NONE: break; default: - NOTREACHED() << "bad read state"; + CHECK(false) << "bad read state"; rv = ERR_FAILED; break; } @@ -394,7 +395,7 @@ int HttpPipelinedConnectionImpl::DoStartNextDeferredRead(int result) { break; default: - NOTREACHED() << "Unexpected read state: " + CHECK(false) << "Unexpected read state: " << stream_info_map_[next_id].state; } @@ -425,6 +426,8 @@ int HttpPipelinedConnectionImpl::DoReadHeadersComplete(int result) { usable_ = false; } + CheckHeadersForPipelineCompatibility(result, active_read_id_); + if (!read_still_on_call_stack_) { QueueUserCallback(active_read_id_, stream_info_map_[active_read_id_].read_headers_callback, @@ -445,6 +448,7 @@ int HttpPipelinedConnectionImpl::DoReadStreamClosed() { CHECK_EQ(stream_info_map_[active_read_id_].state, STREAM_CLOSED); active_read_id_ = 0; if (!usable_) { + // TODO(simonjam): Don't wait this long to evict. read_next_state_ = READ_STATE_EVICT_PENDING_READS; return OK; } @@ -527,7 +531,7 @@ void HttpPipelinedConnectionImpl::Close(int pipeline_id, break; default: - NOTREACHED(); + CHECK(false); break; } } @@ -624,6 +628,38 @@ void HttpPipelinedConnectionImpl::Drain(HttpPipelinedStream* stream, // |drainer| will delete itself when done. } +void HttpPipelinedConnectionImpl::CheckHeadersForPipelineCompatibility( + int result, + int pipeline_id) { + if (result < OK) { + switch (result) { + // TODO(simonjam): Ignoring specific errors like this may not work. + // Collect metrics to see if this code is useful. + case ERR_ABORTED: + case ERR_INTERNET_DISCONNECTED: + // These errors are no fault of the server. + break; + + default: + delegate_->OnPipelineFeedback(this, PIPELINE_SOCKET_ERROR); + return; + } + } + HttpResponseInfo* info = GetResponseInfo(pipeline_id); + const HttpVersion required_version(1, 1); + if (info->headers->GetParsedHttpVersion() < required_version) { + delegate_->OnPipelineFeedback(this, OLD_HTTP_VERSION); + return; + } + if (!info->headers->IsKeepAlive() || !CanFindEndOfResponse(pipeline_id)) { + usable_ = false; + delegate_->OnPipelineFeedback(this, MUST_CLOSE_CONNECTION); + return; + } + // TODO(simonjam): We should also check for, and work around, authentication. + delegate_->OnPipelineFeedback(this, OK); +} + void HttpPipelinedConnectionImpl::QueueUserCallback( int pipeline_id, OldCompletionCallback* callback, diff --git a/net/http/http_pipelined_connection_impl.h b/net/http/http_pipelined_connection_impl.h index a63aabd..64577b7 100644 --- a/net/http/http_pipelined_connection_impl.h +++ b/net/http/http_pipelined_connection_impl.h @@ -256,6 +256,10 @@ class NET_EXPORT_PRIVATE HttpPipelinedConnectionImpl // HttpPipelinedSockets indicates the connection was suddenly closed. int DoEvictPendingReadHeaders(int result); + // Reports back to |delegate_| whether pipelining will work. This is called + // every time we receive headers. + void CheckHeadersForPipelineCompatibility(int result, int pipeline_id); + // Posts a task to fire the user's callback in response to SendRequest() or // ReadResponseHeaders() completing on an underlying parser. This might be // invoked in response to our own IO callbacks, or it may be invoked if the diff --git a/net/http/http_pipelined_connection_impl_unittest.cc b/net/http/http_pipelined_connection_impl_unittest.cc index 2c9beec..2c0d2df 100644 --- a/net/http/http_pipelined_connection_impl_unittest.cc +++ b/net/http/http_pipelined_connection_impl_unittest.cc @@ -31,9 +31,14 @@ class DummySocketParams : public base::RefCounted<DummySocketParams> { REGISTER_SOCKET_PARAMS_FOR_POOL(MockTransportClientSocketPool, DummySocketParams); -class MockPipelineDelegate : public HttpPipelinedConnectionImpl::Delegate { +namespace { + +class MockPipelineDelegate : public HttpPipelinedConnection::Delegate { public: MOCK_METHOD1(OnPipelineHasCapacity, void(HttpPipelinedConnection* pipeline)); + MOCK_METHOD2(OnPipelineFeedback, void( + HttpPipelinedConnection* pipeline, + HttpPipelinedConnection::Feedback feedback)); }; class SuddenCloseObserver : public MessageLoop::TaskObserver { @@ -1211,6 +1216,137 @@ TEST_F(HttpPipelinedConnectionImplTest, EvictIfDrainingChunkedEncoding) { stream2->Close(false); } +TEST_F(HttpPipelinedConnectionImplTest, EvictionDueToMissingContentLength) { + MockWrite writes[] = { + MockWrite(false, 0, "GET /ok.html HTTP/1.1\r\n\r\n"), + MockWrite(false, 1, "GET /evicted.html HTTP/1.1\r\n\r\n"), + MockWrite(false, 2, "GET /rejected.html HTTP/1.1\r\n\r\n"), + }; + MockRead reads[] = { + MockRead(true, 3, "HTTP/1.1 200 OK\r\n\r\n"), + MockRead(false, 4, "ok.html"), + MockRead(false, OK, 5), + }; + Initialize(reads, arraysize(reads), writes, arraysize(writes)); + + scoped_ptr<HttpStream> ok_stream(NewTestStream("ok.html")); + scoped_ptr<HttpStream> evicted_stream(NewTestStream("evicted.html")); + scoped_ptr<HttpStream> rejected_stream(NewTestStream("rejected.html")); + + HttpRequestHeaders headers; + HttpResponseInfo response; + EXPECT_EQ(OK, ok_stream->SendRequest(headers, NULL, &response, &callback_)); + EXPECT_EQ(OK, evicted_stream->SendRequest(headers, NULL, &response, + &callback_)); + EXPECT_EQ(OK, rejected_stream->SendRequest(headers, NULL, &response, + &callback_)); + + TestOldCompletionCallback ok_callback; + EXPECT_EQ(ERR_IO_PENDING, ok_stream->ReadResponseHeaders(&ok_callback)); + + TestOldCompletionCallback evicted_callback; + EXPECT_EQ(ERR_IO_PENDING, + evicted_stream->ReadResponseHeaders(&evicted_callback)); + + data_->RunFor(1); + EXPECT_LE(OK, ok_callback.WaitForResult()); + data_->StopAfter(10); + + ExpectResponse("ok.html", ok_stream, false); + ok_stream->Close(false); + + EXPECT_EQ(ERR_PIPELINE_EVICTION, + rejected_stream->ReadResponseHeaders(&callback_)); + rejected_stream->Close(true); + EXPECT_EQ(ERR_PIPELINE_EVICTION, evicted_callback.WaitForResult()); + evicted_stream->Close(true); +} + +TEST_F(HttpPipelinedConnectionImplTest, FeedbackOnSocketError) { + MockWrite writes[] = { + MockWrite(false, 0, "GET /ok.html HTTP/1.1\r\n\r\n"), + }; + MockRead reads[] = { + MockRead(false, ERR_FAILED, 1), + }; + Initialize(reads, arraysize(reads), writes, arraysize(writes)); + + EXPECT_CALL(delegate_, + OnPipelineFeedback( + pipeline_.get(), + HttpPipelinedConnection::PIPELINE_SOCKET_ERROR)) + .Times(1); + + scoped_ptr<HttpStream> stream(NewTestStream("ok.html")); + HttpRequestHeaders headers; + HttpResponseInfo response; + EXPECT_EQ(OK, stream->SendRequest(headers, NULL, &response, &callback_)); + EXPECT_EQ(ERR_FAILED, stream->ReadResponseHeaders(&callback_)); +} + +TEST_F(HttpPipelinedConnectionImplTest, FeedbackOnHttp10) { + MockWrite writes[] = { + MockWrite(false, 0, "GET /ok.html HTTP/1.1\r\n\r\n"), + }; + MockRead reads[] = { + MockRead(false, 1, "HTTP/1.0 200 OK\r\n"), + MockRead(false, 2, "Content-Length: 7\r\n"), + MockRead(false, 3, "Connection: keep-alive\r\n\r\n"), + MockRead(false, 4, "ok.html"), + }; + Initialize(reads, arraysize(reads), writes, arraysize(writes)); + + EXPECT_CALL(delegate_, + OnPipelineFeedback(pipeline_.get(), + HttpPipelinedConnection::OLD_HTTP_VERSION)) + .Times(1); + + scoped_ptr<HttpStream> stream(NewTestStream("ok.html")); + TestSyncRequest(stream, "ok.html"); +} + +TEST_F(HttpPipelinedConnectionImplTest, FeedbackOnMustClose) { + MockWrite writes[] = { + MockWrite(false, 0, "GET /ok.html HTTP/1.1\r\n\r\n"), + }; + MockRead reads[] = { + MockRead(false, 1, "HTTP/1.1 200 OK\r\n"), + MockRead(false, 2, "Content-Length: 7\r\n"), + MockRead(false, 3, "Connection: close\r\n\r\n"), + MockRead(false, 4, "ok.html"), + }; + Initialize(reads, arraysize(reads), writes, arraysize(writes)); + + EXPECT_CALL(delegate_, + OnPipelineFeedback( + pipeline_.get(), + HttpPipelinedConnection::MUST_CLOSE_CONNECTION)) + .Times(1); + + scoped_ptr<HttpStream> stream(NewTestStream("ok.html")); + TestSyncRequest(stream, "ok.html"); +} + +TEST_F(HttpPipelinedConnectionImplTest, FeedbackOnNoContentLength) { + MockWrite writes[] = { + MockWrite(false, 0, "GET /ok.html HTTP/1.1\r\n\r\n"), + }; + MockRead reads[] = { + MockRead(false, 1, "HTTP/1.1 200 OK\r\n\r\n"), + MockRead(false, 2, "ok.html"), + }; + Initialize(reads, arraysize(reads), writes, arraysize(writes)); + + EXPECT_CALL(delegate_, + OnPipelineFeedback( + pipeline_.get(), + HttpPipelinedConnection::MUST_CLOSE_CONNECTION)) + .Times(1); + + scoped_ptr<HttpStream> stream(NewTestStream("ok.html")); + TestSyncRequest(stream, "ok.html"); +} + TEST_F(HttpPipelinedConnectionImplTest, OnPipelineHasCapacity) { MockWrite writes[] = { MockWrite(false, 0, "GET /ok.html HTTP/1.1\r\n\r\n"), @@ -1250,4 +1386,6 @@ TEST_F(HttpPipelinedConnectionImplTest, OnPipelineHasCapacityWithoutSend) { stream.reset(NULL); } +} // anonymous namespace + } // namespace net diff --git a/net/http/http_pipelined_host.cc b/net/http/http_pipelined_host.cc deleted file mode 100644 index 32d023d..0000000 --- a/net/http/http_pipelined_host.cc +++ /dev/null @@ -1,108 +0,0 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "net/http/http_pipelined_host.h" - -#include "base/stl_util.h" -#include "net/http/http_pipelined_connection_impl.h" -#include "net/http/http_pipelined_stream.h" - -namespace net { - -class HttpPipelinedConnectionImplFactory : - public HttpPipelinedConnection::Factory { - public: - HttpPipelinedConnection* CreateNewPipeline( - ClientSocketHandle* connection, - HttpPipelinedConnection::Delegate* delegate, - const SSLConfig& used_ssl_config, - const ProxyInfo& used_proxy_info, - const BoundNetLog& net_log, - bool was_npn_negotiated) OVERRIDE { - return new HttpPipelinedConnectionImpl(connection, delegate, - used_ssl_config, used_proxy_info, - net_log, was_npn_negotiated); - } -}; - -HttpPipelinedHost::HttpPipelinedHost( - HttpPipelinedHost::Delegate* delegate, - const HostPortPair& origin, - HttpPipelinedConnection::Factory* factory) - : delegate_(delegate), - origin_(origin), - factory_(factory) { - if (!factory) { - factory_.reset(new HttpPipelinedConnectionImplFactory()); - } -} - -HttpPipelinedHost::~HttpPipelinedHost() { - CHECK(pipelines_.empty()); -} - -HttpPipelinedStream* HttpPipelinedHost::CreateStreamOnNewPipeline( - ClientSocketHandle* connection, - const SSLConfig& used_ssl_config, - const ProxyInfo& used_proxy_info, - const BoundNetLog& net_log, - bool was_npn_negotiated) { - HttpPipelinedConnection* pipeline = factory_->CreateNewPipeline( - connection, this, used_ssl_config, used_proxy_info, net_log, - was_npn_negotiated); - pipelines_.insert(pipeline); - return pipeline->CreateNewStream(); -} - -HttpPipelinedStream* HttpPipelinedHost::CreateStreamOnExistingPipeline() { - HttpPipelinedConnection* available_pipeline = NULL; - for (std::set<HttpPipelinedConnection*>::iterator it = pipelines_.begin(); - it != pipelines_.end(); ++it) { - if ((*it)->usable() && - (*it)->active() && - (*it)->depth() < max_pipeline_depth() && - (!available_pipeline || (*it)->depth() < available_pipeline->depth())) { - available_pipeline = *it; - } - } - if (!available_pipeline) { - return NULL; - } - return available_pipeline->CreateNewStream(); -} - -bool HttpPipelinedHost::IsExistingPipelineAvailable() { - for (std::set<HttpPipelinedConnection*>::iterator it = pipelines_.begin(); - it != pipelines_.end(); ++it) { - if ((*it)->usable() && - (*it)->active() && - (*it)->depth() < max_pipeline_depth()) { - return true; - } - } - return false; -} - -void HttpPipelinedHost::OnPipelineEmpty(HttpPipelinedConnection* pipeline) { - CHECK(ContainsKey(pipelines_, pipeline)); - pipelines_.erase(pipeline); - delete pipeline; - if (pipelines_.empty()) { - delegate_->OnHostIdle(this); - // WARNING: We'll probably be deleted here. - } -} - -void HttpPipelinedHost::OnPipelineHasCapacity( - HttpPipelinedConnection* pipeline) { - if (pipeline->usable() && pipeline->depth() < max_pipeline_depth()) { - delegate_->OnHostHasAdditionalCapacity(this); - } - if (!pipeline->depth()) { - OnPipelineEmpty(pipeline); - // WARNING: We might be deleted here. - } -} - -} // namespace net diff --git a/net/http/http_pipelined_host.h b/net/http/http_pipelined_host.h index ae3128d..c368313 100644 --- a/net/http/http_pipelined_host.h +++ b/net/http/http_pipelined_host.h @@ -6,11 +6,6 @@ #define NET_HTTP_HTTP_PIPELINED_HOST_H_ #pragma once -#include <set> -#include <string> - -#include "base/basictypes.h" -#include "base/memory/scoped_ptr.h" #include "net/base/host_port_pair.h" #include "net/base/net_export.h" #include "net/http/http_pipelined_connection.h" @@ -26,9 +21,16 @@ struct SSLConfig; // Manages all of the pipelining state for specific host with active pipelined // HTTP requests. Manages connection jobs, constructs pipelined streams, and // assigns requests to the least loaded pipelined connection. -class NET_EXPORT_PRIVATE HttpPipelinedHost - : public HttpPipelinedConnection::Delegate { +class NET_EXPORT_PRIVATE HttpPipelinedHost { public: + enum Capability { + UNKNOWN, + INCAPABLE, + CAPABLE, + PROBABLY_CAPABLE, // We are using pipelining, but haven't processed enough + // requests to record this host as known to be capable. + }; + class Delegate { public: // Called when a pipelined host has no outstanding requests on any of its @@ -38,54 +40,44 @@ class NET_EXPORT_PRIVATE HttpPipelinedHost // Called when a pipelined host has newly available pipeline capacity, like // when a request completes. virtual void OnHostHasAdditionalCapacity(HttpPipelinedHost* host) = 0; + + // Called when a host determines if pipelining can be used. + virtual void OnHostDeterminedCapability(HttpPipelinedHost* host, + Capability capability) = 0; + }; + + class Factory { + public: + virtual ~Factory() {} + + // Returns a new HttpPipelinedHost. + virtual HttpPipelinedHost* CreateNewHost( + Delegate* delegate, const HostPortPair& origin, + HttpPipelinedConnection::Factory* factory, + Capability capability) = 0; }; - HttpPipelinedHost(Delegate* delegate, const HostPortPair& origin, - HttpPipelinedConnection::Factory* factory); - virtual ~HttpPipelinedHost(); + virtual ~HttpPipelinedHost() {} // Constructs a new pipeline on |connection| and returns a new // HttpPipelinedStream that uses it. - HttpPipelinedStream* CreateStreamOnNewPipeline( + virtual HttpPipelinedStream* CreateStreamOnNewPipeline( ClientSocketHandle* connection, const SSLConfig& used_ssl_config, const ProxyInfo& used_proxy_info, const BoundNetLog& net_log, - bool was_npn_negotiated); + bool was_npn_negotiated) = 0; // Tries to find an existing pipeline with capacity for a new request. If // successful, returns a new stream on that pipeline. Otherwise, returns NULL. - HttpPipelinedStream* CreateStreamOnExistingPipeline(); + virtual HttpPipelinedStream* CreateStreamOnExistingPipeline() = 0; // Returns true if we have a pipelined connection that can accept new // requests. - bool IsExistingPipelineAvailable(); - - // Callbacks for HttpPipelinedConnection. - - // Called when a pipelined connection completes a request. Adds a pending - // request to the pipeline if the pipeline is still usable. - virtual void OnPipelineHasCapacity( - HttpPipelinedConnection* pipeline) OVERRIDE; - - const HostPortPair& origin() const { return origin_; } - - private: - // Called when a pipeline is empty and there are no pending requests. Closes - // the connection. - void OnPipelineEmpty(HttpPipelinedConnection* pipeline); - - // Adds the next pending request to the pipeline if it's still usuable. - void AddRequestToPipeline(HttpPipelinedConnection* connection); - - int max_pipeline_depth() const { return 3; } - - Delegate* delegate_; - const HostPortPair origin_; - std::set<HttpPipelinedConnection*> pipelines_; - scoped_ptr<HttpPipelinedConnection::Factory> factory_; + virtual bool IsExistingPipelineAvailable() const = 0; - DISALLOW_COPY_AND_ASSIGN(HttpPipelinedHost); + // Returns the host and port associated with this class. + virtual const HostPortPair& origin() const = 0; }; } // namespace net diff --git a/net/http/http_pipelined_host_impl.cc b/net/http/http_pipelined_host_impl.cc new file mode 100644 index 0000000..25014763 --- /dev/null +++ b/net/http/http_pipelined_host_impl.cc @@ -0,0 +1,183 @@ +// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "net/http/http_pipelined_host_impl.h" + +#include "base/stl_util.h" +#include "net/http/http_pipelined_connection_impl.h" +#include "net/http/http_pipelined_stream.h" + +namespace net { + +// TODO(simonjam): Run experiments to see what value minimizes evictions without +// costing too much performance. Until then, this is just a bad guess. +static const int kNumKnownSuccessesThreshold = 3; + +class HttpPipelinedConnectionImplFactory : + public HttpPipelinedConnection::Factory { + public: + HttpPipelinedConnection* CreateNewPipeline( + ClientSocketHandle* connection, + HttpPipelinedConnection::Delegate* delegate, + const SSLConfig& used_ssl_config, + const ProxyInfo& used_proxy_info, + const BoundNetLog& net_log, + bool was_npn_negotiated) OVERRIDE { + return new HttpPipelinedConnectionImpl(connection, delegate, + used_ssl_config, used_proxy_info, + net_log, was_npn_negotiated); + } +}; + +HttpPipelinedHostImpl::HttpPipelinedHostImpl( + HttpPipelinedHost::Delegate* delegate, + const HostPortPair& origin, + HttpPipelinedConnection::Factory* factory, + Capability capability) + : delegate_(delegate), + origin_(origin), + factory_(factory), + capability_(capability) { + if (!factory) { + factory_.reset(new HttpPipelinedConnectionImplFactory()); + } +} + +HttpPipelinedHostImpl::~HttpPipelinedHostImpl() { + CHECK(pipelines_.empty()); +} + +HttpPipelinedStream* HttpPipelinedHostImpl::CreateStreamOnNewPipeline( + ClientSocketHandle* connection, + const SSLConfig& used_ssl_config, + const ProxyInfo& used_proxy_info, + const BoundNetLog& net_log, + bool was_npn_negotiated) { + if (capability_ == INCAPABLE) { + return NULL; + } + HttpPipelinedConnection* pipeline = factory_->CreateNewPipeline( + connection, this, used_ssl_config, used_proxy_info, net_log, + was_npn_negotiated); + PipelineInfo info; + pipelines_.insert(std::make_pair(pipeline, info)); + return pipeline->CreateNewStream(); +} + +HttpPipelinedStream* HttpPipelinedHostImpl::CreateStreamOnExistingPipeline() { + HttpPipelinedConnection* available_pipeline = NULL; + for (PipelineInfoMap::iterator it = pipelines_.begin(); + it != pipelines_.end(); ++it) { + if (it->first->usable() && + it->first->active() && + it->first->depth() < GetPipelineCapacity() && + (!available_pipeline || + it->first->depth() < available_pipeline->depth())) { + available_pipeline = it->first; + } + } + if (!available_pipeline) { + return NULL; + } + return available_pipeline->CreateNewStream(); +} + +bool HttpPipelinedHostImpl::IsExistingPipelineAvailable() const { + for (PipelineInfoMap::const_iterator it = pipelines_.begin(); + it != pipelines_.end(); ++it) { + if (it->first->usable() && + it->first->active() && + it->first->depth() < GetPipelineCapacity()) { + return true; + } + } + return false; +} + +const HostPortPair& HttpPipelinedHostImpl::origin() const { + return origin_; +} + +void HttpPipelinedHostImpl::OnPipelineEmpty(HttpPipelinedConnection* pipeline) { + CHECK(ContainsKey(pipelines_, pipeline)); + pipelines_.erase(pipeline); + delete pipeline; + if (pipelines_.empty()) { + delegate_->OnHostIdle(this); + // WARNING: We'll probably be deleted here. + } +} + +void HttpPipelinedHostImpl::OnPipelineHasCapacity( + HttpPipelinedConnection* pipeline) { + CHECK(ContainsKey(pipelines_, pipeline)); + if (pipeline->usable() && + capability_ != INCAPABLE && + pipeline->depth() < GetPipelineCapacity()) { + delegate_->OnHostHasAdditionalCapacity(this); + } + if (!pipeline->depth()) { + OnPipelineEmpty(pipeline); + // WARNING: We might be deleted here. + } +} + +void HttpPipelinedHostImpl::OnPipelineFeedback( + HttpPipelinedConnection* pipeline, + HttpPipelinedConnection::Feedback feedback) { + CHECK(ContainsKey(pipelines_, pipeline)); + switch (feedback) { + case HttpPipelinedConnection::OK: + ++pipelines_[pipeline].num_successes; + if (capability_ == UNKNOWN) { + capability_ = PROBABLY_CAPABLE; + for (PipelineInfoMap::iterator it = pipelines_.begin(); + it != pipelines_.end(); ++it) { + OnPipelineHasCapacity(it->first); + } + } else if (capability_ == PROBABLY_CAPABLE && + pipelines_[pipeline].num_successes >= + kNumKnownSuccessesThreshold) { + capability_ = CAPABLE; + delegate_->OnHostDeterminedCapability(this, CAPABLE); + } + break; + + case HttpPipelinedConnection::PIPELINE_SOCKET_ERROR: + case HttpPipelinedConnection::OLD_HTTP_VERSION: + capability_ = INCAPABLE; + delegate_->OnHostDeterminedCapability(this, INCAPABLE); + break; + + case HttpPipelinedConnection::MUST_CLOSE_CONNECTION: + break; + } +} + +int HttpPipelinedHostImpl::GetPipelineCapacity() const { + int capacity = 0; + switch (capability_) { + case CAPABLE: + case PROBABLY_CAPABLE: + capacity = max_pipeline_depth(); + break; + + case INCAPABLE: + CHECK(false); + + case UNKNOWN: + capacity = 1; + break; + + default: + CHECK(false) << "Unkown pipeline capability: " << capability_; + } + return capacity; +} + +HttpPipelinedHostImpl::PipelineInfo::PipelineInfo() + : num_successes(0) { +} + +} // namespace net diff --git a/net/http/http_pipelined_host_impl.h b/net/http/http_pipelined_host_impl.h new file mode 100644 index 0000000..af78005 --- /dev/null +++ b/net/http/http_pipelined_host_impl.h @@ -0,0 +1,99 @@ +// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef NET_HTTP_HTTP_PIPELINED_HOST_IMPL_H_ +#define NET_HTTP_HTTP_PIPELINED_HOST_IMPL_H_ +#pragma once + +#include <map> +#include <string> + +#include "base/basictypes.h" +#include "base/memory/scoped_ptr.h" +#include "net/base/host_port_pair.h" +#include "net/base/net_export.h" +#include "net/http/http_pipelined_connection.h" +#include "net/http/http_pipelined_host.h" + +namespace net { + +class BoundNetLog; +class ClientSocketHandle; +class HttpPipelinedStream; +class ProxyInfo; +struct SSLConfig; + +// Manages all of the pipelining state for specific host with active pipelined +// HTTP requests. Manages connection jobs, constructs pipelined streams, and +// assigns requests to the least loaded pipelined connection. +class NET_EXPORT_PRIVATE HttpPipelinedHostImpl + : public HttpPipelinedHost, + public HttpPipelinedConnection::Delegate { + public: + HttpPipelinedHostImpl(HttpPipelinedHost::Delegate* delegate, + const HostPortPair& origin, + HttpPipelinedConnection::Factory* factory, + Capability capability); + virtual ~HttpPipelinedHostImpl(); + + // HttpPipelinedHost interface + virtual HttpPipelinedStream* CreateStreamOnNewPipeline( + ClientSocketHandle* connection, + const SSLConfig& used_ssl_config, + const ProxyInfo& used_proxy_info, + const BoundNetLog& net_log, + bool was_npn_negotiated) OVERRIDE; + + virtual HttpPipelinedStream* CreateStreamOnExistingPipeline() OVERRIDE; + + virtual bool IsExistingPipelineAvailable() const OVERRIDE; + + // HttpPipelinedConnection::Delegate interface + + // Called when a pipelined connection completes a request. Adds a pending + // request to the pipeline if the pipeline is still usable. + virtual void OnPipelineHasCapacity( + HttpPipelinedConnection* pipeline) OVERRIDE; + + virtual void OnPipelineFeedback( + HttpPipelinedConnection* pipeline, + HttpPipelinedConnection::Feedback feedback) OVERRIDE; + + virtual const HostPortPair& origin() const OVERRIDE; + + // Returns the maximum number of in-flight pipelined requests we'll allow on a + // single connection. + NET_EXPORT_PRIVATE static int max_pipeline_depth() { return 3; } + + private: + struct PipelineInfo { + PipelineInfo(); + + int num_successes; + }; + typedef std::map<HttpPipelinedConnection*, PipelineInfo> PipelineInfoMap; + + // Called when a pipeline is empty and there are no pending requests. Closes + // the connection. + void OnPipelineEmpty(HttpPipelinedConnection* pipeline); + + // Adds the next pending request to the pipeline if it's still usuable. + void AddRequestToPipeline(HttpPipelinedConnection* connection); + + // Returns the current pipeline capacity based on |capability_|. This should + // not be called if |capability_| is INCAPABLE. + int GetPipelineCapacity() const; + + HttpPipelinedHost::Delegate* delegate_; + const HostPortPair origin_; + PipelineInfoMap pipelines_; + scoped_ptr<HttpPipelinedConnection::Factory> factory_; + Capability capability_; + + DISALLOW_COPY_AND_ASSIGN(HttpPipelinedHostImpl); +}; + +} // namespace net + +#endif // NET_HTTP_HTTP_PIPELINED_HOST_IMPL_H_ diff --git a/net/http/http_pipelined_host_impl_unittest.cc b/net/http/http_pipelined_host_impl_unittest.cc new file mode 100644 index 0000000..ed7d722 --- /dev/null +++ b/net/http/http_pipelined_host_impl_unittest.cc @@ -0,0 +1,301 @@ +// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "net/http/http_pipelined_host_impl.h" + +#include "base/memory/scoped_ptr.h" +#include "net/base/ssl_config_service.h" +#include "net/http/http_pipelined_connection.h" +#include "net/proxy/proxy_info.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +using testing::_; +using testing::NiceMock; +using testing::Ref; +using testing::Return; +using testing::ReturnNull; + +namespace net { + +namespace { + +ClientSocketHandle* kDummyConnection = + reinterpret_cast<ClientSocketHandle*>(84); +HttpPipelinedStream* kDummyStream = + reinterpret_cast<HttpPipelinedStream*>(42); + +class MockHostDelegate : public HttpPipelinedHost::Delegate { + public: + MOCK_METHOD1(OnHostIdle, void(HttpPipelinedHost* host)); + MOCK_METHOD1(OnHostHasAdditionalCapacity, void(HttpPipelinedHost* host)); + MOCK_METHOD2(OnHostDeterminedCapability, + void(HttpPipelinedHost* host, + HttpPipelinedHost::Capability capability)); +}; + +class MockPipelineFactory : public HttpPipelinedConnection::Factory { + public: + MOCK_METHOD6(CreateNewPipeline, HttpPipelinedConnection*( + ClientSocketHandle* connection, + HttpPipelinedConnection::Delegate* delegate, + const SSLConfig& used_ssl_config, + const ProxyInfo& used_proxy_info, + const BoundNetLog& net_log, + bool was_npn_negotiated)); +}; + +class MockPipeline : public HttpPipelinedConnection { + public: + MockPipeline(int depth, bool usable, bool active) + : depth_(depth), + usable_(usable), + active_(active) { + } + + void SetState(int depth, bool usable, bool active) { + depth_ = depth; + usable_ = usable; + active_ = active; + } + + virtual int depth() const OVERRIDE { return depth_; } + virtual bool usable() const OVERRIDE { return usable_; } + virtual bool active() const OVERRIDE { return active_; } + + MOCK_METHOD0(CreateNewStream, HttpPipelinedStream*()); + MOCK_METHOD1(OnStreamDeleted, void(int pipeline_id)); + MOCK_CONST_METHOD0(used_ssl_config, const SSLConfig&()); + MOCK_CONST_METHOD0(used_proxy_info, const ProxyInfo&()); + MOCK_CONST_METHOD0(source, const NetLog::Source&()); + MOCK_CONST_METHOD0(was_npn_negotiated, bool()); + + private: + int depth_; + bool usable_; + bool active_; +}; + +class HttpPipelinedHostImplTest : public testing::Test { + public: + HttpPipelinedHostImplTest() + : origin_("host", 123), + factory_(new MockPipelineFactory), // Owned by host_. + host_(new HttpPipelinedHostImpl(&delegate_, origin_, factory_, + HttpPipelinedHost::CAPABLE)) { + } + + void SetCapability(HttpPipelinedHost::Capability capability) { + factory_ = new MockPipelineFactory; + host_.reset(new HttpPipelinedHostImpl( + &delegate_, origin_, factory_, capability)); + } + + MockPipeline* AddTestPipeline(int depth, bool usable, bool active) { + MockPipeline* pipeline = new MockPipeline(depth, usable, active); + EXPECT_CALL(*factory_, CreateNewPipeline(kDummyConnection, host_.get(), + Ref(ssl_config_), Ref(proxy_info_), + Ref(net_log_), true)) + .Times(1) + .WillOnce(Return(pipeline)); + EXPECT_CALL(*pipeline, CreateNewStream()) + .Times(1) + .WillOnce(Return(kDummyStream)); + EXPECT_EQ(kDummyStream, host_->CreateStreamOnNewPipeline( + kDummyConnection, ssl_config_, proxy_info_, net_log_, true)); + return pipeline; + } + + void ClearTestPipeline(MockPipeline* pipeline) { + pipeline->SetState(0, true, true); + host_->OnPipelineHasCapacity(pipeline); + } + + NiceMock<MockHostDelegate> delegate_; + HostPortPair origin_; + MockPipelineFactory* factory_; + scoped_ptr<HttpPipelinedHostImpl> host_; + + SSLConfig ssl_config_; + ProxyInfo proxy_info_; + BoundNetLog net_log_; +}; + +TEST_F(HttpPipelinedHostImplTest, Delegate) { + EXPECT_TRUE(origin_.Equals(host_->origin())); +} + +TEST_F(HttpPipelinedHostImplTest, OnUnusablePipelineHasCapacity) { + MockPipeline* pipeline = AddTestPipeline(0, false, true); + + EXPECT_CALL(delegate_, OnHostHasAdditionalCapacity(host_.get())) + .Times(0); + EXPECT_CALL(delegate_, OnHostIdle(host_.get())) + .Times(1); + host_->OnPipelineHasCapacity(pipeline); +} + +TEST_F(HttpPipelinedHostImplTest, OnUsablePipelineHasCapacity) { + MockPipeline* pipeline = AddTestPipeline(1, true, true); + + EXPECT_CALL(delegate_, OnHostHasAdditionalCapacity(host_.get())) + .Times(1); + EXPECT_CALL(delegate_, OnHostIdle(host_.get())) + .Times(0); + + host_->OnPipelineHasCapacity(pipeline); + + EXPECT_CALL(delegate_, OnHostHasAdditionalCapacity(host_.get())) + .Times(1); + EXPECT_CALL(delegate_, OnHostIdle(host_.get())) + .Times(1); + ClearTestPipeline(pipeline); +} + +TEST_F(HttpPipelinedHostImplTest, IgnoresUnusablePipeline) { + MockPipeline* pipeline = AddTestPipeline(1, false, true); + + EXPECT_FALSE(host_->IsExistingPipelineAvailable()); + EXPECT_EQ(NULL, host_->CreateStreamOnExistingPipeline()); + + ClearTestPipeline(pipeline); +} + +TEST_F(HttpPipelinedHostImplTest, IgnoresInactivePipeline) { + MockPipeline* pipeline = AddTestPipeline(1, true, false); + + EXPECT_FALSE(host_->IsExistingPipelineAvailable()); + EXPECT_EQ(NULL, host_->CreateStreamOnExistingPipeline()); + + ClearTestPipeline(pipeline); +} + +TEST_F(HttpPipelinedHostImplTest, IgnoresFullPipeline) { + MockPipeline* pipeline = AddTestPipeline( + HttpPipelinedHostImpl::max_pipeline_depth(), true, true); + + EXPECT_FALSE(host_->IsExistingPipelineAvailable()); + EXPECT_EQ(NULL, host_->CreateStreamOnExistingPipeline()); + + ClearTestPipeline(pipeline); +} + +TEST_F(HttpPipelinedHostImplTest, PicksLeastLoadedPipeline) { + MockPipeline* full_pipeline = AddTestPipeline( + HttpPipelinedHostImpl::max_pipeline_depth(), true, true); + MockPipeline* usable_pipeline = AddTestPipeline( + HttpPipelinedHostImpl::max_pipeline_depth() - 1, true, true); + MockPipeline* empty_pipeline = AddTestPipeline(0, true, true); + + EXPECT_TRUE(host_->IsExistingPipelineAvailable()); + EXPECT_CALL(*empty_pipeline, CreateNewStream()) + .Times(1) + .WillOnce(ReturnNull()); + EXPECT_EQ(NULL, host_->CreateStreamOnExistingPipeline()); + + ClearTestPipeline(full_pipeline); + ClearTestPipeline(usable_pipeline); + ClearTestPipeline(empty_pipeline); +} + +TEST_F(HttpPipelinedHostImplTest, OpensUpOnPipelineSuccess) { + SetCapability(HttpPipelinedHost::UNKNOWN); + MockPipeline* pipeline = AddTestPipeline(1, true, true); + + EXPECT_EQ(NULL, host_->CreateStreamOnExistingPipeline()); + EXPECT_CALL(delegate_, OnHostHasAdditionalCapacity(host_.get())) + .Times(1); + host_->OnPipelineFeedback(pipeline, HttpPipelinedConnection::OK); + + EXPECT_CALL(*pipeline, CreateNewStream()) + .Times(1) + .WillOnce(Return(kDummyStream)); + EXPECT_EQ(kDummyStream, host_->CreateStreamOnExistingPipeline()); + + EXPECT_CALL(delegate_, OnHostHasAdditionalCapacity(host_.get())) + .Times(1); + ClearTestPipeline(pipeline); +} + +TEST_F(HttpPipelinedHostImplTest, OpensAllPipelinesOnPipelineSuccess) { + SetCapability(HttpPipelinedHost::UNKNOWN); + MockPipeline* pipeline1 = AddTestPipeline(1, false, true); + MockPipeline* pipeline2 = AddTestPipeline(1, true, true); + + EXPECT_EQ(NULL, host_->CreateStreamOnExistingPipeline()); + EXPECT_CALL(delegate_, OnHostHasAdditionalCapacity(host_.get())) + .Times(1); + host_->OnPipelineFeedback(pipeline1, HttpPipelinedConnection::OK); + + EXPECT_CALL(*pipeline2, CreateNewStream()) + .Times(1) + .WillOnce(Return(kDummyStream)); + EXPECT_EQ(kDummyStream, host_->CreateStreamOnExistingPipeline()); + + EXPECT_CALL(delegate_, OnHostHasAdditionalCapacity(host_.get())) + .Times(2); + ClearTestPipeline(pipeline1); + ClearTestPipeline(pipeline2); +} + +TEST_F(HttpPipelinedHostImplTest, ShutsDownOnOldVersion) { + SetCapability(HttpPipelinedHost::UNKNOWN); + MockPipeline* pipeline = AddTestPipeline(1, true, true); + + EXPECT_EQ(NULL, host_->CreateStreamOnExistingPipeline()); + EXPECT_CALL(delegate_, OnHostHasAdditionalCapacity(host_.get())) + .Times(0); + EXPECT_CALL(delegate_, + OnHostDeterminedCapability(host_.get(), + HttpPipelinedHost::INCAPABLE)) + .Times(1); + host_->OnPipelineFeedback(pipeline, + HttpPipelinedConnection::OLD_HTTP_VERSION); + + ClearTestPipeline(pipeline); + EXPECT_EQ(NULL, host_->CreateStreamOnNewPipeline( + kDummyConnection, ssl_config_, proxy_info_, net_log_, true)); +} + +TEST_F(HttpPipelinedHostImplTest, ConnectionCloseHasNoEffect) { + SetCapability(HttpPipelinedHost::UNKNOWN); + MockPipeline* pipeline = AddTestPipeline(1, true, true); + + EXPECT_CALL(delegate_, OnHostHasAdditionalCapacity(host_.get())) + .Times(0); + EXPECT_CALL(delegate_, OnHostDeterminedCapability(host_.get(), _)) + .Times(0); + host_->OnPipelineFeedback(pipeline, + HttpPipelinedConnection::MUST_CLOSE_CONNECTION); + EXPECT_EQ(NULL, host_->CreateStreamOnExistingPipeline()); + + EXPECT_CALL(delegate_, OnHostHasAdditionalCapacity(host_.get())) + .Times(1); + ClearTestPipeline(pipeline); +} + +TEST_F(HttpPipelinedHostImplTest, SuccessesLeadToCapable) { + SetCapability(HttpPipelinedHost::UNKNOWN); + MockPipeline* pipeline = AddTestPipeline(1, true, true); + + EXPECT_CALL(delegate_, OnHostHasAdditionalCapacity(host_.get())) + .Times(1); + EXPECT_CALL(delegate_, + OnHostDeterminedCapability(host_.get(), + HttpPipelinedHost::CAPABLE)) + .Times(1); + host_->OnPipelineFeedback(pipeline, HttpPipelinedConnection::OK); + + pipeline->SetState(3, true, true); + host_->OnPipelineFeedback(pipeline, HttpPipelinedConnection::OK); + host_->OnPipelineFeedback(pipeline, HttpPipelinedConnection::OK); + + EXPECT_CALL(delegate_, OnHostHasAdditionalCapacity(host_.get())) + .Times(1); + ClearTestPipeline(pipeline); +} + +} // anonymous namespace + +} // namespace net diff --git a/net/http/http_pipelined_host_pool.cc b/net/http/http_pipelined_host_pool.cc index bd238d8..fa705a0 100644 --- a/net/http/http_pipelined_host_pool.cc +++ b/net/http/http_pipelined_host_pool.cc @@ -6,19 +6,46 @@ #include "base/logging.h" #include "base/stl_util.h" -#include "net/http/http_pipelined_host.h" -#include "net/http/http_stream_factory_impl.h" +#include "net/http/http_pipelined_host_impl.h" namespace net { -HttpPipelinedHostPool::HttpPipelinedHostPool(HttpStreamFactoryImpl* factory) - : factory_(factory) { +// TODO(simonjam): Run experiments with different values of this to see what +// value is good at avoiding evictions without eating too much memory. Until +// then, this is just a bad guess. +static const int kNumHostsToRemember = 200; + +class HttpPipelinedHostImplFactory : public HttpPipelinedHost::Factory { + public: + virtual HttpPipelinedHost* CreateNewHost( + HttpPipelinedHost::Delegate* delegate, const HostPortPair& origin, + HttpPipelinedConnection::Factory* factory, + HttpPipelinedHost::Capability capability) OVERRIDE { + return new HttpPipelinedHostImpl(delegate, origin, factory, capability); + } +}; + +HttpPipelinedHostPool::HttpPipelinedHostPool( + Delegate* delegate, + HttpPipelinedHost::Factory* factory) + : delegate_(delegate), + factory_(factory), + known_capability_map_(kNumHostsToRemember) { + if (!factory) { + factory_.reset(new HttpPipelinedHostImplFactory); + } } HttpPipelinedHostPool::~HttpPipelinedHostPool() { CHECK(host_map_.empty()); } +bool HttpPipelinedHostPool::IsHostEligibleForPipelining( + const HostPortPair& origin) { + HttpPipelinedHost::Capability capability = GetHostCapability(origin); + return capability != HttpPipelinedHost::INCAPABLE; +} + HttpPipelinedStream* HttpPipelinedHostPool::CreateStreamOnNewPipeline( const HostPortPair& origin, ClientSocketHandle* connection, @@ -27,6 +54,9 @@ HttpPipelinedStream* HttpPipelinedHostPool::CreateStreamOnNewPipeline( const BoundNetLog& net_log, bool was_npn_negotiated) { HttpPipelinedHost* host = GetPipelinedHost(origin, true); + if (!host) { + return NULL; + } return host->CreateStreamOnNewPipeline(connection, used_ssl_config, used_proxy_info, net_log, was_npn_negotiated); @@ -52,14 +82,21 @@ bool HttpPipelinedHostPool::IsExistingPipelineAvailableForOrigin( HttpPipelinedHost* HttpPipelinedHostPool::GetPipelinedHost( const HostPortPair& origin, bool create_if_not_found) { - HostMap::iterator it = host_map_.find(origin); - if (it != host_map_.end()) { - CHECK(it->second); - return it->second; + HostMap::iterator host_it = host_map_.find(origin); + if (host_it != host_map_.end()) { + CHECK(host_it->second); + return host_it->second; } else if (!create_if_not_found) { return NULL; } - HttpPipelinedHost* host = new HttpPipelinedHost(this, origin, NULL); + + HttpPipelinedHost::Capability capability = GetHostCapability(origin); + if (capability == HttpPipelinedHost::INCAPABLE) { + return NULL; + } + + HttpPipelinedHost* host = factory_->CreateNewHost( + this, origin, NULL, capability); host_map_[origin] = host; return host; } @@ -67,14 +104,33 @@ HttpPipelinedHost* HttpPipelinedHostPool::GetPipelinedHost( void HttpPipelinedHostPool::OnHostIdle(HttpPipelinedHost* host) { const HostPortPair& origin = host->origin(); CHECK(ContainsKey(host_map_, origin)); - // TODO(simonjam): We should remember the pipeline state for each host. host_map_.erase(origin); delete host; } void HttpPipelinedHostPool::OnHostHasAdditionalCapacity( HttpPipelinedHost* host) { - factory_->OnHttpPipelinedHostHasAdditionalCapacity(host->origin()); + delegate_->OnHttpPipelinedHostHasAdditionalCapacity(host->origin()); +} + +void HttpPipelinedHostPool::OnHostDeterminedCapability( + HttpPipelinedHost* host, + HttpPipelinedHost::Capability capability) { + CapabilityMap::iterator known_it = known_capability_map_.Get(host->origin()); + if (known_it == known_capability_map_.end() || + known_it->second != HttpPipelinedHost::INCAPABLE) { + known_capability_map_.Put(host->origin(), capability); + } +} + +HttpPipelinedHost::Capability HttpPipelinedHostPool::GetHostCapability( + const HostPortPair& origin) { + HttpPipelinedHost::Capability capability = HttpPipelinedHost::UNKNOWN; + CapabilityMap::const_iterator it = known_capability_map_.Get(origin); + if (it != known_capability_map_.end()) { + capability = it->second; + } + return capability; } } // namespace net diff --git a/net/http/http_pipelined_host_pool.h b/net/http/http_pipelined_host_pool.h index 5e7c146..3be0cb5 100644 --- a/net/http/http_pipelined_host_pool.h +++ b/net/http/http_pipelined_host_pool.h @@ -9,21 +9,37 @@ #include <map> #include "base/basictypes.h" +#include "base/gtest_prod_util.h" +#include "base/memory/mru_cache.h" +#include "base/memory/scoped_ptr.h" #include "net/http/http_pipelined_host.h" namespace net { class HostPortPair; class HttpPipelinedStream; -class HttpStreamFactoryImpl; // Manages all of the pipelining state for specific host with active pipelined // HTTP requests. Manages connection jobs, constructs pipelined streams, and // assigns requests to the least loaded pipelined connection. -class HttpPipelinedHostPool : public HttpPipelinedHost::Delegate { +class NET_EXPORT_PRIVATE HttpPipelinedHostPool + : public HttpPipelinedHost::Delegate { public: - explicit HttpPipelinedHostPool(HttpStreamFactoryImpl* factory); - ~HttpPipelinedHostPool(); + class Delegate { + public: + // Called when a HttpPipelinedHost has new capacity. Attempts to allocate + // any pending pipeline-capable requests to pipelines. + virtual void OnHttpPipelinedHostHasAdditionalCapacity( + const HostPortPair& origin) = 0; + }; + + HttpPipelinedHostPool(Delegate* delegate, + HttpPipelinedHost::Factory* factory); + virtual ~HttpPipelinedHostPool(); + + // Returns true if pipelining might work for |origin|. Generally, this returns + // true, unless |origin| is known to have failed pipelining recently. + bool IsHostEligibleForPipelining(const HostPortPair& origin); // Constructs a new pipeline on |connection| and returns a new // HttpPipelinedStream that uses it. @@ -49,14 +65,24 @@ class HttpPipelinedHostPool : public HttpPipelinedHost::Delegate { virtual void OnHostHasAdditionalCapacity(HttpPipelinedHost* host) OVERRIDE; + virtual void OnHostDeterminedCapability( + HttpPipelinedHost* host, + HttpPipelinedHost::Capability capability) OVERRIDE; + private: + typedef base::MRUCache<HostPortPair, + HttpPipelinedHost::Capability> CapabilityMap; typedef std::map<const HostPortPair, HttpPipelinedHost*> HostMap; HttpPipelinedHost* GetPipelinedHost(const HostPortPair& origin, bool create_if_not_found); - HttpStreamFactoryImpl* factory_; + HttpPipelinedHost::Capability GetHostCapability(const HostPortPair& origin); + + Delegate* delegate_; + scoped_ptr<HttpPipelinedHost::Factory> factory_; HostMap host_map_; + CapabilityMap known_capability_map_; DISALLOW_COPY_AND_ASSIGN(HttpPipelinedHostPool); }; diff --git a/net/http/http_pipelined_host_pool_unittest.cc b/net/http/http_pipelined_host_pool_unittest.cc new file mode 100644 index 0000000..928e4e9 --- /dev/null +++ b/net/http/http_pipelined_host_pool_unittest.cc @@ -0,0 +1,185 @@ +// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "net/http/http_pipelined_host_pool.h" + +#include "base/memory/scoped_ptr.h" +#include "net/base/ssl_config_service.h" +#include "net/http/http_pipelined_host.h" +#include "net/proxy/proxy_info.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +using testing::_; +using testing::Ref; +using testing::Return; +using testing::ReturnNull; + +namespace net { + +namespace { + +ClientSocketHandle* kDummyConnection = + reinterpret_cast<ClientSocketHandle*>(188); +HttpPipelinedStream* kDummyStream = + reinterpret_cast<HttpPipelinedStream*>(99); + +class MockPoolDelegate : public HttpPipelinedHostPool::Delegate { + public: + MOCK_METHOD1(OnHttpPipelinedHostHasAdditionalCapacity, + void(const HostPortPair& origin)); +}; + +class MockHostFactory : public HttpPipelinedHost::Factory { + public: + MOCK_METHOD4(CreateNewHost, HttpPipelinedHost*( + HttpPipelinedHost::Delegate* delegate, const HostPortPair& origin, + HttpPipelinedConnection::Factory* factory, + HttpPipelinedHost::Capability capability)); +}; + +class MockHost : public HttpPipelinedHost { + public: + MockHost(const HostPortPair& origin) + : origin_(origin) { + } + + MOCK_METHOD5(CreateStreamOnNewPipeline, HttpPipelinedStream*( + ClientSocketHandle* connection, + const SSLConfig& used_ssl_config, + const ProxyInfo& used_proxy_info, + const BoundNetLog& net_log, + bool was_npn_negotiated)); + MOCK_METHOD0(CreateStreamOnExistingPipeline, HttpPipelinedStream*()); + MOCK_CONST_METHOD0(IsExistingPipelineAvailable, bool()); + + virtual const HostPortPair& origin() const OVERRIDE { return origin_; } + + private: + HostPortPair origin_; +}; + +class HttpPipelinedHostPoolTest : public testing::Test { + public: + HttpPipelinedHostPoolTest() + : origin_("host", 123), + factory_(new MockHostFactory), // Owned by pool_. + host_(new MockHost(origin_)), // Owned by pool_. + pool_(new HttpPipelinedHostPool(&delegate_, factory_)), + was_npn_negotiated_(false) { + } + + void CreateDummyStream() { + EXPECT_CALL(*host_, CreateStreamOnNewPipeline(kDummyConnection, + Ref(ssl_config_), + Ref(proxy_info_), + Ref(net_log_), + was_npn_negotiated_)) + .Times(1) + .WillOnce(Return(kDummyStream)); + EXPECT_EQ(kDummyStream, + pool_->CreateStreamOnNewPipeline(origin_, kDummyConnection, + ssl_config_, proxy_info_, + net_log_, was_npn_negotiated_)); + } + + HostPortPair origin_; + MockPoolDelegate delegate_; + MockHostFactory* factory_; + MockHost* host_; + scoped_ptr<HttpPipelinedHostPool> pool_; + + const SSLConfig ssl_config_; + const ProxyInfo proxy_info_; + const BoundNetLog net_log_; + bool was_npn_negotiated_; +}; + +TEST_F(HttpPipelinedHostPoolTest, DefaultUnknown) { + EXPECT_TRUE(pool_->IsHostEligibleForPipelining(origin_)); + EXPECT_CALL(*factory_, CreateNewHost(pool_.get(), Ref(origin_), _, + HttpPipelinedHost::UNKNOWN)) + .Times(1) + .WillOnce(Return(host_)); + + CreateDummyStream(); + pool_->OnHostIdle(host_); +} + +TEST_F(HttpPipelinedHostPoolTest, RemembersIncapable) { + EXPECT_CALL(*factory_, CreateNewHost(pool_.get(), Ref(origin_), _, + HttpPipelinedHost::UNKNOWN)) + .Times(1) + .WillOnce(Return(host_)); + + CreateDummyStream(); + pool_->OnHostDeterminedCapability(host_, HttpPipelinedHost::INCAPABLE); + pool_->OnHostIdle(host_); + EXPECT_FALSE(pool_->IsHostEligibleForPipelining(origin_)); + EXPECT_CALL(*factory_, CreateNewHost(pool_.get(), Ref(origin_), _, + HttpPipelinedHost::INCAPABLE)) + .Times(0); + EXPECT_EQ(NULL, + pool_->CreateStreamOnNewPipeline(origin_, kDummyConnection, + ssl_config_, proxy_info_, net_log_, + was_npn_negotiated_)); +} + +TEST_F(HttpPipelinedHostPoolTest, RemembersCapable) { + EXPECT_CALL(*factory_, CreateNewHost(pool_.get(), Ref(origin_), _, + HttpPipelinedHost::UNKNOWN)) + .Times(1) + .WillOnce(Return(host_)); + + CreateDummyStream(); + pool_->OnHostDeterminedCapability(host_, HttpPipelinedHost::CAPABLE); + pool_->OnHostIdle(host_); + EXPECT_TRUE(pool_->IsHostEligibleForPipelining(origin_)); + + host_ = new MockHost(origin_); + EXPECT_CALL(*factory_, CreateNewHost(pool_.get(), Ref(origin_), _, + HttpPipelinedHost::CAPABLE)) + .Times(1) + .WillOnce(Return(host_)); + CreateDummyStream(); + pool_->OnHostIdle(host_); +} + +TEST_F(HttpPipelinedHostPoolTest, IncapableIsSticky) { + EXPECT_CALL(*factory_, CreateNewHost(pool_.get(), Ref(origin_), _, + HttpPipelinedHost::UNKNOWN)) + .Times(1) + .WillOnce(Return(host_)); + + CreateDummyStream(); + pool_->OnHostDeterminedCapability(host_, HttpPipelinedHost::CAPABLE); + pool_->OnHostDeterminedCapability(host_, HttpPipelinedHost::INCAPABLE); + pool_->OnHostDeterminedCapability(host_, HttpPipelinedHost::CAPABLE); + pool_->OnHostIdle(host_); + EXPECT_FALSE(pool_->IsHostEligibleForPipelining(origin_)); +} + +TEST_F(HttpPipelinedHostPoolTest, RemainsUnknownWithoutFeedback) { + EXPECT_CALL(*factory_, CreateNewHost(pool_.get(), Ref(origin_), _, + HttpPipelinedHost::UNKNOWN)) + .Times(1) + .WillOnce(Return(host_)); + + CreateDummyStream(); + pool_->OnHostIdle(host_); + EXPECT_TRUE(pool_->IsHostEligibleForPipelining(origin_)); + + host_ = new MockHost(origin_); + EXPECT_CALL(*factory_, CreateNewHost(pool_.get(), Ref(origin_), _, + HttpPipelinedHost::UNKNOWN)) + .Times(1) + .WillOnce(Return(host_)); + + CreateDummyStream(); + pool_->OnHostIdle(host_); +} + +} // anonymous namespace + +} // namespace net diff --git a/net/http/http_pipelined_host_unittest.cc b/net/http/http_pipelined_host_unittest.cc deleted file mode 100644 index de78e9e..0000000 --- a/net/http/http_pipelined_host_unittest.cc +++ /dev/null @@ -1,201 +0,0 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "net/http/http_pipelined_host.h" - -#include "net/base/ssl_config_service.h" -#include "net/http/http_pipelined_connection.h" -#include "net/proxy/proxy_info.h" -#include "testing/gmock/include/gmock/gmock.h" -#include "testing/gtest/include/gtest/gtest.h" - -using testing::_; -using testing::NiceMock; -using testing::Ref; -using testing::Return; -using testing::ReturnNull; - -static const int kMaxCapacity = 3; - -namespace net { - -static ClientSocketHandle* kDummyConnection = - reinterpret_cast<ClientSocketHandle*>(84); -static HttpPipelinedStream* kDummyStream = - reinterpret_cast<HttpPipelinedStream*>(42); - -class MockHostDelegate : public HttpPipelinedHost::Delegate { - public: - MOCK_METHOD1(OnHostIdle, void(HttpPipelinedHost* host)); - MOCK_METHOD1(OnHostHasAdditionalCapacity, void(HttpPipelinedHost* host)); -}; - -class MockPipelineFactory : public HttpPipelinedConnection::Factory { - public: - MOCK_METHOD6(CreateNewPipeline, HttpPipelinedConnection*( - ClientSocketHandle* connection, - HttpPipelinedConnection::Delegate* delegate, - const SSLConfig& used_ssl_config, - const ProxyInfo& used_proxy_info, - const BoundNetLog& net_log, - bool was_npn_negotiated)); -}; - -class MockPipeline : public HttpPipelinedConnection { - public: - MockPipeline(int depth, bool usable, bool active) - : depth_(depth), - usable_(usable), - active_(active) { - } - - void SetState(int depth, bool usable, bool active) { - depth_ = depth; - usable_ = usable; - active_ = active; - } - - virtual int depth() const OVERRIDE { return depth_; } - virtual bool usable() const OVERRIDE { return usable_; } - virtual bool active() const OVERRIDE { return active_; } - - MOCK_METHOD0(CreateNewStream, HttpPipelinedStream*()); - MOCK_METHOD1(OnStreamDeleted, void(int pipeline_id)); - MOCK_CONST_METHOD0(used_ssl_config, const SSLConfig&()); - MOCK_CONST_METHOD0(used_proxy_info, const ProxyInfo&()); - MOCK_CONST_METHOD0(source, const NetLog::Source&()); - MOCK_CONST_METHOD0(was_npn_negotiated, bool()); - - private: - int depth_; - bool usable_; - bool active_; -}; - -class HttpPipelinedHostTest : public testing::Test { - public: - HttpPipelinedHostTest() - : origin_("host", 123), - factory_(new MockPipelineFactory), // Owned by host_. - host_(&delegate_, origin_, factory_) { - } - - MockPipeline* AddTestPipeline(int depth, bool usable, bool active) { - MockPipeline* pipeline = new MockPipeline(depth, usable, active); - EXPECT_CALL(*factory_, CreateNewPipeline(kDummyConnection, &host_, - Ref(ssl_config_), Ref(proxy_info_), - Ref(net_log_), true)) - .Times(1) - .WillOnce(Return(pipeline)); - EXPECT_CALL(*pipeline, CreateNewStream()) - .Times(1) - .WillOnce(Return(kDummyStream)); - EXPECT_EQ(kDummyStream, host_.CreateStreamOnNewPipeline( - kDummyConnection, ssl_config_, proxy_info_, net_log_, true)); - return pipeline; - } - - void ClearTestPipeline(MockPipeline* pipeline) { - pipeline->SetState(0, true, true); - host_.OnPipelineHasCapacity(pipeline); - } - - NiceMock<MockHostDelegate> delegate_; - HostPortPair origin_; - MockPipelineFactory* factory_; - HttpPipelinedHost host_; - - SSLConfig ssl_config_; - ProxyInfo proxy_info_; - BoundNetLog net_log_; -}; - -TEST_F(HttpPipelinedHostTest, Delegate) { - EXPECT_TRUE(origin_.Equals(host_.origin())); -} - -TEST_F(HttpPipelinedHostTest, OnHostIdle) { - MockPipeline* pipeline = AddTestPipeline(0, false, true); - - EXPECT_CALL(delegate_, OnHostHasAdditionalCapacity(&host_)) - .Times(0); - EXPECT_CALL(delegate_, OnHostIdle(&host_)) - .Times(1); - host_.OnPipelineHasCapacity(pipeline); -} - -TEST_F(HttpPipelinedHostTest, OnHostHasAdditionalCapacity) { - MockPipeline* pipeline = AddTestPipeline(1, true, true); - - EXPECT_CALL(delegate_, OnHostHasAdditionalCapacity(&host_)) - .Times(2); - EXPECT_CALL(delegate_, OnHostIdle(&host_)) - .Times(0); - - host_.OnPipelineHasCapacity(pipeline); - - EXPECT_CALL(delegate_, OnHostIdle(&host_)) - .Times(1); - ClearTestPipeline(pipeline); -} - -TEST_F(HttpPipelinedHostTest, IgnoresUnusablePipeline) { - MockPipeline* pipeline = AddTestPipeline(1, false, true); - - EXPECT_FALSE(host_.IsExistingPipelineAvailable()); - EXPECT_EQ(NULL, host_.CreateStreamOnExistingPipeline()); - - ClearTestPipeline(pipeline); -} - -TEST_F(HttpPipelinedHostTest, IgnoresInactivePipeline) { - MockPipeline* pipeline = AddTestPipeline(1, true, false); - - EXPECT_FALSE(host_.IsExistingPipelineAvailable()); - EXPECT_EQ(NULL, host_.CreateStreamOnExistingPipeline()); - - ClearTestPipeline(pipeline); -} - -TEST_F(HttpPipelinedHostTest, IgnoresFullPipeline) { - MockPipeline* pipeline = AddTestPipeline(kMaxCapacity, true, true); - - EXPECT_FALSE(host_.IsExistingPipelineAvailable()); - EXPECT_EQ(NULL, host_.CreateStreamOnExistingPipeline()); - - ClearTestPipeline(pipeline); -} - -TEST_F(HttpPipelinedHostTest, PicksLeastLoadedPipeline) { - MockPipeline* full_pipeline = AddTestPipeline(kMaxCapacity, true, true); - MockPipeline* usable_pipeline = AddTestPipeline(kMaxCapacity - 1, true, true); - MockPipeline* empty_pipeline = AddTestPipeline(0, true, true); - - EXPECT_TRUE(host_.IsExistingPipelineAvailable()); - EXPECT_CALL(*empty_pipeline, CreateNewStream()) - .Times(1) - .WillOnce(ReturnNull()); - EXPECT_EQ(NULL, host_.CreateStreamOnExistingPipeline()); - - ClearTestPipeline(full_pipeline); - ClearTestPipeline(usable_pipeline); - ClearTestPipeline(empty_pipeline); -} - -TEST_F(HttpPipelinedHostTest, EmptyPipelineIsRemoved) { - MockPipeline* empty_pipeline = AddTestPipeline(0, true, true); - - EXPECT_TRUE(host_.IsExistingPipelineAvailable()); - EXPECT_CALL(*empty_pipeline, CreateNewStream()) - .Times(1) - .WillOnce(Return(kDummyStream)); - EXPECT_EQ(kDummyStream, host_.CreateStreamOnExistingPipeline()); - - ClearTestPipeline(empty_pipeline); - - EXPECT_FALSE(host_.IsExistingPipelineAvailable()); - EXPECT_EQ(NULL, host_.CreateStreamOnExistingPipeline()); -} - -} // namespace net diff --git a/net/http/http_pipelined_network_transaction_unittest.cc b/net/http/http_pipelined_network_transaction_unittest.cc index c342b2c..70e8d657 100644 --- a/net/http/http_pipelined_network_transaction_unittest.cc +++ b/net/http/http_pipelined_network_transaction_unittest.cc @@ -97,7 +97,7 @@ class HttpPipelinedNetworkTransactionTest : public testing::Test { EXPECT_EQ(OK, transaction.Read(buffer.get(), expected.size(), &callback_)); } - void CompleteTwoRequests() { + void CompleteTwoRequests(int data_index, int stop_at_step) { scoped_ptr<HttpNetworkTransaction> one_transaction( new HttpNetworkTransaction(session_.get())); TestOldCompletionCallback one_callback; @@ -117,9 +117,10 @@ class HttpPipelinedNetworkTransactionTest : public testing::Test { EXPECT_EQ(ERR_IO_PENDING, one_transaction->Read(buffer.get(), 8, &one_read_callback)); - data_vector_[0]->RunFor(2); + data_vector_[data_index]->SetStop(stop_at_step); + data_vector_[data_index]->Run(); EXPECT_EQ(8, one_read_callback.WaitForResult()); - data_vector_[0]->SetStop(10); + data_vector_[data_index]->SetStop(10); std::string actual(buffer->data(), 8); EXPECT_THAT(actual, StrEq("one.html")); EXPECT_EQ(OK, one_transaction->Read(buffer.get(), 8, &one_read_callback)); @@ -128,6 +129,44 @@ class HttpPipelinedNetworkTransactionTest : public testing::Test { ExpectResponse("two.html", two_transaction); } + void CompleteFourRequests() { + scoped_ptr<HttpNetworkTransaction> one_transaction( + new HttpNetworkTransaction(session_.get())); + TestOldCompletionCallback one_callback; + EXPECT_EQ(ERR_IO_PENDING, + one_transaction->Start(GetRequestInfo("one.html"), &one_callback, + BoundNetLog())); + EXPECT_EQ(OK, one_callback.WaitForResult()); + + HttpNetworkTransaction two_transaction(session_.get()); + TestOldCompletionCallback two_callback; + EXPECT_EQ(ERR_IO_PENDING, + two_transaction.Start(GetRequestInfo("two.html"), &two_callback, + BoundNetLog())); + + HttpNetworkTransaction three_transaction(session_.get()); + TestOldCompletionCallback three_callback; + EXPECT_EQ(ERR_IO_PENDING, + three_transaction.Start(GetRequestInfo("three.html"), + &three_callback, BoundNetLog())); + + HttpNetworkTransaction four_transaction(session_.get()); + TestOldCompletionCallback four_callback; + EXPECT_EQ(ERR_IO_PENDING, + four_transaction.Start(GetRequestInfo("four.html"), + &four_callback, BoundNetLog())); + + ExpectResponse("one.html", *one_transaction.get()); + EXPECT_EQ(OK, two_callback.WaitForResult()); + ExpectResponse("two.html", two_transaction); + EXPECT_EQ(OK, three_callback.WaitForResult()); + ExpectResponse("three.html", three_transaction); + + one_transaction.reset(); + EXPECT_EQ(OK, four_callback.WaitForResult()); + ExpectResponse("four.html", four_transaction); + } + DeterministicMockClientSocketFactory factory_; ClientSocketPoolHistograms histograms_; MockTransportClientSocketPool pool_; @@ -188,7 +227,7 @@ TEST_F(HttpPipelinedNetworkTransactionTest, ReusePipeline) { }; AddExpectedConnection(reads, arraysize(reads), writes, arraysize(writes)); - CompleteTwoRequests(); + CompleteTwoRequests(0, 5); } TEST_F(HttpPipelinedNetworkTransactionTest, ReusesOnSpaceAvailable) { @@ -226,41 +265,7 @@ TEST_F(HttpPipelinedNetworkTransactionTest, ReusesOnSpaceAvailable) { }; AddExpectedConnection(reads, arraysize(reads), writes, arraysize(writes)); - scoped_ptr<HttpNetworkTransaction> one_transaction( - new HttpNetworkTransaction(session_.get())); - TestOldCompletionCallback one_callback; - EXPECT_EQ(ERR_IO_PENDING, - one_transaction->Start(GetRequestInfo("one.html"), &one_callback, - BoundNetLog())); - EXPECT_EQ(OK, one_callback.WaitForResult()); - - HttpNetworkTransaction two_transaction(session_.get()); - TestOldCompletionCallback two_callback; - EXPECT_EQ(ERR_IO_PENDING, - two_transaction.Start(GetRequestInfo("two.html"), &two_callback, - BoundNetLog())); - - HttpNetworkTransaction three_transaction(session_.get()); - TestOldCompletionCallback three_callback; - EXPECT_EQ(ERR_IO_PENDING, - three_transaction.Start(GetRequestInfo("three.html"), - &three_callback, BoundNetLog())); - - HttpNetworkTransaction four_transaction(session_.get()); - TestOldCompletionCallback four_callback; - EXPECT_EQ(ERR_IO_PENDING, - four_transaction.Start(GetRequestInfo("four.html"), &four_callback, - BoundNetLog())); - - ExpectResponse("one.html", *one_transaction.get()); - EXPECT_EQ(OK, two_callback.WaitForResult()); - ExpectResponse("two.html", two_transaction); - EXPECT_EQ(OK, three_callback.WaitForResult()); - ExpectResponse("three.html", three_transaction); - - one_transaction.reset(); - EXPECT_EQ(OK, four_callback.WaitForResult()); - ExpectResponse("four.html", four_transaction); + CompleteFourRequests(); ClientSocketPoolManager::set_max_sockets_per_group(old_max_sockets); } @@ -272,14 +277,11 @@ TEST_F(HttpPipelinedNetworkTransactionTest, UnknownSizeEvictsToNewPipeline) { MockWrite(false, 0, "GET /one.html HTTP/1.1\r\n" "Host: localhost\r\n" "Connection: keep-alive\r\n\r\n"), - MockWrite(false, 2, "GET /two.html HTTP/1.1\r\n" - "Host: localhost\r\n" - "Connection: keep-alive\r\n\r\n"), }; MockRead reads[] = { MockRead(false, 1, "HTTP/1.1 200 OK\r\n\r\n"), - MockRead(true, 3, "one.html"), - MockRead(false, OK, 4), + MockRead(true, 2, "one.html"), + MockRead(false, OK, 3), }; AddExpectedConnection(reads, arraysize(reads), writes, arraysize(writes)); @@ -295,7 +297,7 @@ TEST_F(HttpPipelinedNetworkTransactionTest, UnknownSizeEvictsToNewPipeline) { }; AddExpectedConnection(reads2, arraysize(reads2), writes2, arraysize(writes2)); - CompleteTwoRequests(); + CompleteTwoRequests(0, 3); } TEST_F(HttpPipelinedNetworkTransactionTest, ConnectionCloseEvictToNewPipeline) { @@ -329,7 +331,7 @@ TEST_F(HttpPipelinedNetworkTransactionTest, ConnectionCloseEvictToNewPipeline) { }; AddExpectedConnection(reads2, arraysize(reads2), writes2, arraysize(writes2)); - CompleteTwoRequests(); + CompleteTwoRequests(0, 5); } TEST_F(HttpPipelinedNetworkTransactionTest, ErrorEvictsToNewPipeline) { @@ -508,6 +510,136 @@ TEST_F(HttpPipelinedNetworkTransactionTest, BasicHttpAuthentication) { ExpectResponse("one.html", transaction); } +TEST_F(HttpPipelinedNetworkTransactionTest, OldVersionDisablesPipelining) { + Initialize(); + + MockWrite writes[] = { + MockWrite(false, 0, "GET /pipelined.html HTTP/1.1\r\n" + "Host: localhost\r\n" + "Connection: keep-alive\r\n\r\n"), + }; + MockRead reads[] = { + MockRead(false, 1, "HTTP/1.0 200 OK\r\n"), + MockRead(false, 2, "Content-Length: 14\r\n\r\n"), + MockRead(false, 3, "pipelined.html"), + }; + AddExpectedConnection(reads, arraysize(reads), writes, arraysize(writes)); + + MockWrite writes2[] = { + MockWrite(false, 0, "GET /one.html HTTP/1.1\r\n" + "Host: localhost\r\n" + "Connection: keep-alive\r\n\r\n"), + }; + MockRead reads2[] = { + MockRead(false, 1, "HTTP/1.1 200 OK\r\n"), + MockRead(false, 2, "Content-Length: 8\r\n\r\n"), + MockRead(true, 3, "one.html"), + MockRead(false, OK, 4), + }; + AddExpectedConnection(reads2, arraysize(reads2), writes2, arraysize(writes2)); + + MockWrite writes3[] = { + MockWrite(false, 0, "GET /two.html HTTP/1.1\r\n" + "Host: localhost\r\n" + "Connection: keep-alive\r\n\r\n"), + }; + MockRead reads3[] = { + MockRead(false, 1, "HTTP/1.1 200 OK\r\n"), + MockRead(false, 2, "Content-Length: 8\r\n\r\n"), + MockRead(false, 3, "two.html"), + MockRead(false, OK, 4), + }; + AddExpectedConnection(reads3, arraysize(reads3), writes3, arraysize(writes3)); + + HttpNetworkTransaction one_transaction(session_.get()); + TestOldCompletionCallback one_callback; + EXPECT_EQ(ERR_IO_PENDING, + one_transaction.Start(GetRequestInfo("pipelined.html"), + &one_callback, BoundNetLog())); + EXPECT_EQ(OK, one_callback.WaitForResult()); + ExpectResponse("pipelined.html", one_transaction); + + CompleteTwoRequests(1, 4); +} + +TEST_F(HttpPipelinedNetworkTransactionTest, PipelinesImmediatelyIfKnownGood) { + // The first request gets us an HTTP/1.1. The next 3 test pipelining. When the + // 3rd request completes, we know pipelining is safe. After the first 4 + // complete, the 5th and 6th should then be immediately sent pipelined on a + // new HttpPipelinedConnection. + int old_max_sockets = ClientSocketPoolManager::max_sockets_per_group(); + ClientSocketPoolManager::set_max_sockets_per_group(1); + Initialize(); + + MockWrite writes[] = { + MockWrite(false, 0, "GET /one.html HTTP/1.1\r\n" + "Host: localhost\r\n" + "Connection: keep-alive\r\n\r\n"), + MockWrite(false, 4, "GET /two.html HTTP/1.1\r\n" + "Host: localhost\r\n" + "Connection: keep-alive\r\n\r\n"), + MockWrite(false, 7, "GET /three.html HTTP/1.1\r\n" + "Host: localhost\r\n" + "Connection: keep-alive\r\n\r\n"), + MockWrite(false, 12, "GET /four.html HTTP/1.1\r\n" + "Host: localhost\r\n" + "Connection: keep-alive\r\n\r\n"), + MockWrite(false, 16, "GET /second-pipeline-one.html HTTP/1.1\r\n" + "Host: localhost\r\n" + "Connection: keep-alive\r\n\r\n"), + MockWrite(false, 17, "GET /second-pipeline-two.html HTTP/1.1\r\n" + "Host: localhost\r\n" + "Connection: keep-alive\r\n\r\n"), + }; + MockRead reads[] = { + MockRead(false, 1, "HTTP/1.1 200 OK\r\n"), + MockRead(false, 2, "Content-Length: 8\r\n\r\n"), + MockRead(false, 3, "one.html"), + MockRead(false, 5, "HTTP/1.1 200 OK\r\n"), + MockRead(false, 6, "Content-Length: 8\r\n\r\n"), + MockRead(false, 8, "two.html"), + MockRead(false, 9, "HTTP/1.1 200 OK\r\n"), + MockRead(false, 10, "Content-Length: 10\r\n\r\n"), + MockRead(false, 11, "three.html"), + MockRead(false, 13, "HTTP/1.1 200 OK\r\n"), + MockRead(false, 14, "Content-Length: 9\r\n\r\n"), + MockRead(false, 15, "four.html"), + MockRead(true, 18, "HTTP/1.1 200 OK\r\n"), + MockRead(true, 19, "Content-Length: 24\r\n\r\n"), + MockRead(false, 20, "second-pipeline-one.html"), + MockRead(false, 21, "HTTP/1.1 200 OK\r\n"), + MockRead(false, 22, "Content-Length: 24\r\n\r\n"), + MockRead(false, 23, "second-pipeline-two.html"), + }; + AddExpectedConnection(reads, arraysize(reads), writes, arraysize(writes)); + + CompleteFourRequests(); + + HttpNetworkTransaction second_one_transaction(session_.get()); + TestOldCompletionCallback second_one_callback; + EXPECT_EQ(ERR_IO_PENDING, + second_one_transaction.Start( + GetRequestInfo("second-pipeline-one.html"), + &second_one_callback, BoundNetLog())); + MessageLoop::current()->RunAllPending(); + + HttpNetworkTransaction second_two_transaction(session_.get()); + TestOldCompletionCallback second_two_callback; + EXPECT_EQ(ERR_IO_PENDING, + second_two_transaction.Start( + GetRequestInfo("second-pipeline-two.html"), + &second_two_callback, BoundNetLog())); + + data_vector_[0]->RunFor(3); + EXPECT_EQ(OK, second_one_callback.WaitForResult()); + data_vector_[0]->StopAfter(100); + ExpectResponse("second-pipeline-one.html", second_one_transaction); + EXPECT_EQ(OK, second_two_callback.WaitForResult()); + ExpectResponse("second-pipeline-two.html", second_two_transaction); + + ClientSocketPoolManager::set_max_sockets_per_group(old_max_sockets); +} + } // anonymous namespace } // namespace net diff --git a/net/http/http_stream_factory_impl.cc b/net/http/http_stream_factory_impl.cc index 9593dc4..dd7776f 100644 --- a/net/http/http_stream_factory_impl.cc +++ b/net/http/http_stream_factory_impl.cc @@ -37,7 +37,7 @@ GURL UpgradeUrlToHttps(const GURL& original_url, int port) { HttpStreamFactoryImpl::HttpStreamFactoryImpl(HttpNetworkSession* session) : session_(session), - http_pipelined_host_pool_(this) {} + http_pipelined_host_pool_(this, NULL) {} HttpStreamFactoryImpl::~HttpStreamFactoryImpl() { DCHECK(request_map_.empty()); diff --git a/net/http/http_stream_factory_impl.h b/net/http/http_stream_factory_impl.h index ce1a04eb..8f13f3f 100644 --- a/net/http/http_stream_factory_impl.h +++ b/net/http/http_stream_factory_impl.h @@ -21,12 +21,14 @@ class HttpNetworkSession; class HttpPipelinedHost; class SpdySession; -class NET_EXPORT_PRIVATE HttpStreamFactoryImpl : public HttpStreamFactory { +class NET_EXPORT_PRIVATE HttpStreamFactoryImpl : + public HttpStreamFactory, + public HttpPipelinedHostPool::Delegate { public: explicit HttpStreamFactoryImpl(HttpNetworkSession* session); virtual ~HttpStreamFactoryImpl(); - // HttpStreamFactory Interface + // HttpStreamFactory interface virtual HttpStreamRequest* RequestStream( const HttpRequestInfo& info, const SSLConfig& server_ssl_config, @@ -42,10 +44,9 @@ class NET_EXPORT_PRIVATE HttpStreamFactoryImpl : public HttpStreamFactory { virtual void AddTLSIntolerantServer(const HostPortPair& server) OVERRIDE; virtual bool IsTLSIntolerantServer(const HostPortPair& server) const OVERRIDE; - // Called when a HttpPipelinedHost has new capacity. Attempts to allocate any - // pending pipeline-capable requests to pipelines. + // HttpPipelinedHostPool::Delegate interface virtual void OnHttpPipelinedHostHasAdditionalCapacity( - const HostPortPair& origin); + const HostPortPair& origin) OVERRIDE; private: class Request; diff --git a/net/http/http_stream_factory_impl_job.cc b/net/http/http_stream_factory_impl_job.cc index a6ca05e..e1bffb374 100644 --- a/net/http/http_stream_factory_impl_job.cc +++ b/net/http/http_stream_factory_impl_job.cc @@ -1137,7 +1137,7 @@ bool HttpStreamFactoryImpl::Job::IsOrphaned() const { return !IsPreconnecting() && !request_; } -bool HttpStreamFactoryImpl::Job::IsRequestEligibleForPipelining() const { +bool HttpStreamFactoryImpl::Job::IsRequestEligibleForPipelining() { if (!HttpStreamFactory::http_pipelining_enabled()) { return false; } @@ -1147,7 +1147,11 @@ bool HttpStreamFactoryImpl::Job::IsRequestEligibleForPipelining() const { if (using_ssl_) { return false; } - return request_info_.method == "GET" || request_info_.method == "HEAD"; + if (request_info_.method != "GET" && request_info_.method != "HEAD") { + return false; + } + return stream_factory_->http_pipelined_host_pool_.IsHostEligibleForPipelining( + origin_); } } // namespace net diff --git a/net/http/http_stream_factory_impl_job.h b/net/http/http_stream_factory_impl_job.h index b14897c..7c95135 100644 --- a/net/http/http_stream_factory_impl_job.h +++ b/net/http/http_stream_factory_impl_job.h @@ -192,7 +192,7 @@ class HttpStreamFactoryImpl::Job { // Should we force SPDY to run without SSL for this stream request. bool ShouldForceSpdyWithoutSSL() const; - bool IsRequestEligibleForPipelining() const; + bool IsRequestEligibleForPipelining(); // Record histograms of latency until Connect() completes. static void LogHttpConnectedMetrics(const ClientSocketHandle& handle); diff --git a/net/net.gyp b/net/net.gyp index 2694b27..777f0f6 100644 --- a/net/net.gyp +++ b/net/net.gyp @@ -427,8 +427,9 @@ 'http/http_pipelined_connection.h', 'http/http_pipelined_connection_impl.cc', 'http/http_pipelined_connection_impl.h', - 'http/http_pipelined_host.cc', 'http/http_pipelined_host.h', + 'http/http_pipelined_host_impl.cc', + 'http/http_pipelined_host_impl.h', 'http/http_pipelined_host_pool.cc', 'http/http_pipelined_host_pool.h', 'http/http_pipelined_stream.cc', @@ -1089,7 +1090,8 @@ 'http/http_network_layer_unittest.cc', 'http/http_network_transaction_unittest.cc', 'http/http_pipelined_connection_impl_unittest.cc', - 'http/http_pipelined_host_unittest.cc', + 'http/http_pipelined_host_impl_unittest.cc', + 'http/http_pipelined_host_pool_unittest.cc', 'http/http_pipelined_network_transaction_unittest.cc', 'http/http_proxy_client_socket_pool_unittest.cc', 'http/http_request_headers_unittest.cc', |