Skip to content

Commit

Permalink
net: clean up SSLHostInfo construction.
Browse files Browse the repository at this point in the history
Create an SSLHostInfoFactory interface and plumb it from the HttpCache
to the SSLConnectJob. Also, move the SSLHostInfo reference from the
ssl_config to being passed to the SSLClientSocket.

BUG=none
TEST=net_unittests

Review URL: http://codereview.chromium.org/3747003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@62918 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
agl@chromium.org committed Oct 18, 2010
1 parent 46aeaa8 commit 8281f70
Show file tree
Hide file tree
Showing 50 changed files with 244 additions and 98 deletions.
4 changes: 2 additions & 2 deletions chrome/browser/net/connection_tester.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ class ExperimentURLRequestContext : public URLRequestContext {
host_resolver_);
http_transaction_factory_ = new net::HttpCache(
net::HttpNetworkLayer::CreateFactory(host_resolver_, dnsrr_resolver_,
proxy_service_, ssl_config_service_, http_auth_handler_factory_,
NULL, NULL),
NULL /* ssl_host_info_factory */, proxy_service_,
ssl_config_service_, http_auth_handler_factory_, NULL, NULL),
net::HttpCache::DefaultBackend::InMemory(0));
// In-memory cookie store.
cookie_store_ = new net::CookieMonster(NULL, NULL);
Expand Down
1 change: 1 addition & 0 deletions chrome/service/net/service_url_request_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ ServiceURLRequestContext::ServiceURLRequestContext() {
http_transaction_factory_ = new net::HttpCache(
net::HttpNetworkLayer::CreateFactory(host_resolver_,
dnsrr_resolver_,
NULL /* ssl_host_info_factory */,
proxy_service_,
ssl_config_service_,
http_auth_handler_factory_,
Expand Down
1 change: 1 addition & 0 deletions chrome/test/plugin/plugin_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ class PluginInstallerDownloadTest
http_transaction_factory_ = new net::HttpCache(
net::HttpNetworkLayer::CreateFactory(host_resolver_,
NULL /* dnsrr_resolver */,
NULL /* ssl_host_info_factory */,
proxy_service_,
ssl_config_service_,
http_auth_handler_factory_,
Expand Down
1 change: 1 addition & 0 deletions chrome_frame/metrics_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ class ChromeFrameUploadRequestContext : public URLRequestContext {
http_transaction_factory_ = new net::HttpCache(
net::HttpNetworkLayer::CreateFactory(host_resolver_,
NULL /* dnsrr_resovler */,
NULL /* ssl_host_info */,
proxy_service_,
ssl_config_service_,
http_auth_handler_factory_,
Expand Down
3 changes: 2 additions & 1 deletion chrome_frame/test/test_server_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ class URLRequestTestContext : public URLRequestContext {
host_resolver_);
http_transaction_factory_ = new net::HttpCache(
net::HttpNetworkLayer::CreateFactory(
host_resolver_, NULL /* dnsrr_resolver */, proxy_service_,
host_resolver_, NULL /* dnsrr_resolver */,
NULL /* ssl_host_info_factory */, proxy_service_,
ssl_config_service_, http_auth_handler_factory_, NULL, NULL),
net::HttpCache::DefaultBackend::InMemory(0));
// In-memory cookie store.
Expand Down
3 changes: 2 additions & 1 deletion jingle/notifier/base/chrome_async_socket.cc
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,8 @@ bool ChromeAsyncSocket::StartTls(const std::string& domain_name) {
DCHECK(transport_socket_.get());
transport_socket_.reset(
client_socket_factory_->CreateSSLClientSocket(
transport_socket_.release(), domain_name, ssl_config_));
transport_socket_.release(), domain_name, ssl_config_,
NULL /* ssl_host_info */));
int status = transport_socket_->Connect(&ssl_connect_callback_);
if (status != net::ERR_IO_PENDING) {
MessageLoop* message_loop = MessageLoop::current();
Expand Down
5 changes: 3 additions & 2 deletions jingle/notifier/base/xmpp_client_socket_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,10 @@ net::ClientSocket* XmppClientSocketFactory::CreateTCPClientSocket(
net::SSLClientSocket* XmppClientSocketFactory::CreateSSLClientSocket(
net::ClientSocketHandle* transport_socket,
const std::string& hostname,
const net::SSLConfig& ssl_config) {
const net::SSLConfig& ssl_config,
net::SSLHostInfo* ssl_host_info) {
return client_socket_factory_->CreateSSLClientSocket(
transport_socket, hostname, ssl_config);
transport_socket, hostname, ssl_config, ssl_host_info);
}

} // namespace
6 changes: 5 additions & 1 deletion jingle/notifier/base/xmpp_client_socket_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@

#include "net/socket/client_socket_factory.h"

namespace net {
class SSLHostInfo;
}

namespace notifier {

class XmppClientSocketFactory : public net::ClientSocketFactory {
Expand All @@ -26,7 +30,7 @@ class XmppClientSocketFactory : public net::ClientSocketFactory {
const net::NetLog::Source& source);
virtual net::SSLClientSocket* CreateSSLClientSocket(
net::ClientSocketHandle* transport_socket, const std::string& hostname,
const net::SSLConfig& ssl_config);
const net::SSLConfig& ssl_config, net::SSLHostInfo* ssl_host_info);

private:
net::ClientSocketFactory* const client_socket_factory_;
Expand Down
6 changes: 0 additions & 6 deletions net/base/ssl_config_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,6 @@ struct SSLConfig {
std::string next_protos;

scoped_refptr<X509Certificate> client_cert;

// ssl_host_info contains an optional context that is needed for Snap Start.
// If provided, the SSL socket will assume that the application protocol is
// client-speaks-first. Also needs SSLConfigService::EnableSnapStart to
// have been called.
scoped_refptr<SSLHostInfo> ssl_host_info;
};

// The interface for retrieving the SSL configuration. This interface
Expand Down
13 changes: 13 additions & 0 deletions net/base/ssl_host_info.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Copyright (c) 2010 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/base/ssl_host_info.h"

namespace net {

SSLHostInfo::~SSLHostInfo() {}

SSLHostInfoFactory::~SSLHostInfoFactory() {}

} // namespace net
16 changes: 11 additions & 5 deletions net/base/ssl_host_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@ namespace net {
// This information may be stored on disk so does not include keys or session
// information etc. Primarily it's intended for caching the server's
// certificates.
class SSLHostInfo :
public base::RefCountedThreadSafe<SSLHostInfo> {
class SSLHostInfo {
public:
virtual ~SSLHostInfo();

// Start will commence the lookup. This must be called before any other
// methods. By opportunistically calling this early, it may be possible to
// overlap this object's lookup and reduce latency.
Expand Down Expand Up @@ -45,10 +46,15 @@ class SSLHostInfo :
// this object and the store operation will still complete. This can only be
// called once WaitForDataReady has returned OK or called its callback.
virtual void Set(const std::string& new_data) = 0;
};

class SSLHostInfoFactory {
public:
virtual ~SSLHostInfoFactory();

protected:
friend class base::RefCountedThreadSafe<SSLHostInfo>;
virtual ~SSLHostInfo() { }
// GetForHost returns a fresh, allocated SSLHostInfo for the given hostname
// or NULL on failure.
virtual SSLHostInfo* GetForHost(const std::string& hostname) = 0;
};

} // namespace net
Expand Down
3 changes: 2 additions & 1 deletion net/http/disk_cache_based_ssl_host_info.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ void DiskCacheBasedSSLHostInfo::Start() {

DiskCacheBasedSSLHostInfo::~DiskCacheBasedSSLHostInfo() {
DCHECK(!user_callback_);
DCHECK(!entry_);
if (entry_)
entry_->Close();
callback_->Cancel();
}

Expand Down
23 changes: 22 additions & 1 deletion net/http/http_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@
#include "net/base/io_buffer.h"
#include "net/base/load_flags.h"
#include "net/base/net_errors.h"
#include "net/base/ssl_host_info.h"
#include "net/disk_cache/disk_cache.h"
#include "net/http/disk_cache_based_ssl_host_info.h"
#include "net/http/http_cache_transaction.h"
#include "net/http/http_network_layer.h"
#include "net/http/http_network_session.h"
Expand Down Expand Up @@ -242,6 +244,22 @@ void HttpCache::MetadataWriter::OnIOComplete(int result) {

//-----------------------------------------------------------------------------

class HttpCache::SSLHostInfoFactoryAdaptor : public SSLHostInfoFactory {
public:
SSLHostInfoFactoryAdaptor(HttpCache* http_cache)
: http_cache_(http_cache) {
}

SSLHostInfo* GetForHost(const std::string& hostname) {
return new DiskCacheBasedSSLHostInfo(hostname, http_cache_);
}

private:
HttpCache* const http_cache_;
};

//-----------------------------------------------------------------------------

HttpCache::HttpCache(HostResolver* host_resolver,
DnsRRResolver* dnsrr_resolver,
ProxyService* proxy_service,
Expand All @@ -253,8 +271,11 @@ HttpCache::HttpCache(HostResolver* host_resolver,
: backend_factory_(backend_factory),
building_backend_(false),
mode_(NORMAL),
ssl_host_info_factory_(new SSLHostInfoFactoryAdaptor(
ALLOW_THIS_IN_INITIALIZER_LIST(this))),
network_layer_(HttpNetworkLayer::CreateFactory(host_resolver,
dnsrr_resolver, proxy_service, ssl_config_service,
dnsrr_resolver, ssl_host_info_factory_.get(),
proxy_service, ssl_config_service,
http_auth_handler_factory, network_delegate, net_log)),
ALLOW_THIS_IN_INITIALIZER_LIST(task_factory_(this)),
enable_range_support_(true) {
Expand Down
5 changes: 4 additions & 1 deletion net/http/http_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ class HttpCache : public HttpTransactionFactory,
};

// The disk cache is initialized lazily (by CreateTransaction) in this case.
// The HttpCache takes ownership of the |backend_factory|.
// The HttpCache takes ownership of the |backend_factory|.
HttpCache(HostResolver* host_resolver,
DnsRRResolver* dnsrr_resolver,
ProxyService* proxy_service,
Expand Down Expand Up @@ -200,6 +200,7 @@ class HttpCache : public HttpTransactionFactory,

class BackendCallback;
class MetadataWriter;
class SSLHostInfoFactoryAdaptor;
class Transaction;
class WorkItem;
friend class Transaction;
Expand Down Expand Up @@ -353,6 +354,8 @@ class HttpCache : public HttpTransactionFactory,

Mode mode_;

scoped_ptr<SSLHostInfoFactoryAdaptor> ssl_host_info_factory_;

scoped_ptr<HttpTransactionFactory> network_layer_;
scoped_ptr<disk_cache::Backend> disk_cache_;

Expand Down
10 changes: 0 additions & 10 deletions net/http/http_cache_transaction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -625,16 +625,6 @@ int HttpCache::Transaction::DoSendRequest() {
return rv;

next_state_ = STATE_SEND_REQUEST_COMPLETE;
if (request_->url.SchemeIs("https") &&
SSLConfigService::snap_start_enabled()) {
// TODO(agl): in order to support AlternateProtocol there should probably
// be an object hanging off the HttpNetworkSession which constructs these.
// Note: when this test is removed, don't forget to remove the #include of
// ssl_config_service.h
scoped_refptr<DiskCacheBasedSSLHostInfo> hostinfo =
new DiskCacheBasedSSLHostInfo(request_->url.host(), cache_);
network_trans_->SetSSLHostInfo(hostinfo.get());
}
rv = network_trans_->Start(request_, &io_callback_, net_log_);
return rv;
}
Expand Down
11 changes: 10 additions & 1 deletion net/http/http_network_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ namespace net {
HttpTransactionFactory* HttpNetworkLayer::CreateFactory(
HostResolver* host_resolver,
DnsRRResolver* dnsrr_resolver,
SSLHostInfoFactory* ssl_host_info_factory,
ProxyService* proxy_service,
SSLConfigService* ssl_config_service,
HttpAuthHandlerFactory* http_auth_handler_factory,
Expand All @@ -30,7 +31,8 @@ HttpTransactionFactory* HttpNetworkLayer::CreateFactory(
DCHECK(proxy_service);

return new HttpNetworkLayer(ClientSocketFactory::GetDefaultFactory(),
host_resolver, dnsrr_resolver, proxy_service,
host_resolver, dnsrr_resolver,
ssl_host_info_factory, proxy_service,
ssl_config_service, http_auth_handler_factory,
network_delegate,
net_log);
Expand All @@ -49,6 +51,7 @@ HttpNetworkLayer::HttpNetworkLayer(
ClientSocketFactory* socket_factory,
HostResolver* host_resolver,
DnsRRResolver* dnsrr_resolver,
SSLHostInfoFactory* ssl_host_info_factory,
ProxyService* proxy_service,
SSLConfigService* ssl_config_service,
HttpAuthHandlerFactory* http_auth_handler_factory,
Expand All @@ -57,6 +60,7 @@ HttpNetworkLayer::HttpNetworkLayer(
: socket_factory_(socket_factory),
host_resolver_(host_resolver),
dnsrr_resolver_(dnsrr_resolver),
ssl_host_info_factory_(ssl_host_info_factory),
proxy_service_(proxy_service),
ssl_config_service_(ssl_config_service),
session_(NULL),
Expand All @@ -73,6 +77,7 @@ HttpNetworkLayer::HttpNetworkLayer(
ClientSocketFactory* socket_factory,
HostResolver* host_resolver,
DnsRRResolver* dnsrr_resolver,
SSLHostInfoFactory* ssl_host_info_factory,
ProxyService* proxy_service,
SSLConfigService* ssl_config_service,
SpdySessionPool* spdy_session_pool,
Expand All @@ -82,6 +87,7 @@ HttpNetworkLayer::HttpNetworkLayer(
: socket_factory_(socket_factory),
host_resolver_(host_resolver),
dnsrr_resolver_(dnsrr_resolver),
ssl_host_info_factory_(ssl_host_info_factory),
proxy_service_(proxy_service),
ssl_config_service_(ssl_config_service),
session_(NULL),
Expand All @@ -97,6 +103,7 @@ HttpNetworkLayer::HttpNetworkLayer(
HttpNetworkLayer::HttpNetworkLayer(HttpNetworkSession* session)
: socket_factory_(ClientSocketFactory::GetDefaultFactory()),
dnsrr_resolver_(NULL),
ssl_host_info_factory_(NULL),
ssl_config_service_(NULL),
session_(session),
spdy_session_pool_(NULL),
Expand Down Expand Up @@ -137,6 +144,7 @@ HttpNetworkSession* HttpNetworkLayer::GetSession() {
session_ = new HttpNetworkSession(
host_resolver_,
dnsrr_resolver_,
ssl_host_info_factory_,
proxy_service_,
socket_factory_,
ssl_config_service_,
Expand All @@ -147,6 +155,7 @@ HttpNetworkSession* HttpNetworkLayer::GetSession() {
// These were just temps for lazy-initializing HttpNetworkSession.
host_resolver_ = NULL;
dnsrr_resolver_ = NULL;
ssl_host_info_factory_ = NULL;
proxy_service_ = NULL;
socket_factory_ = NULL;
http_auth_handler_factory_ = NULL;
Expand Down
5 changes: 5 additions & 0 deletions net/http/http_network_layer.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class NetLog;
class ProxyService;
class SpdySessionPool;
class SSLConfigService;
class SSLHostInfoFactory;

class HttpNetworkLayer : public HttpTransactionFactory, public NonThreadSafe {
public:
Expand All @@ -33,6 +34,7 @@ class HttpNetworkLayer : public HttpTransactionFactory, public NonThreadSafe {
HttpNetworkLayer(ClientSocketFactory* socket_factory,
HostResolver* host_resolver,
DnsRRResolver* dnsrr_resolver,
SSLHostInfoFactory* ssl_host_info_factory,
ProxyService* proxy_service,
SSLConfigService* ssl_config_service,
HttpAuthHandlerFactory* http_auth_handler_factory,
Expand All @@ -44,6 +46,7 @@ class HttpNetworkLayer : public HttpTransactionFactory, public NonThreadSafe {
ClientSocketFactory* socket_factory,
HostResolver* host_resolver,
DnsRRResolver* dnsrr_resolver,
SSLHostInfoFactory* ssl_host_info_factory,
ProxyService* proxy_service,
SSLConfigService* ssl_config_service,
SpdySessionPool* spdy_session_pool,
Expand All @@ -59,6 +62,7 @@ class HttpNetworkLayer : public HttpTransactionFactory, public NonThreadSafe {
static HttpTransactionFactory* CreateFactory(
HostResolver* host_resolver,
DnsRRResolver* dnsrr_resolver,
SSLHostInfoFactory* ssl_host_info_factory,
ProxyService* proxy_service,
SSLConfigService* ssl_config_service,
HttpAuthHandlerFactory* http_auth_handler_factory,
Expand Down Expand Up @@ -96,6 +100,7 @@ class HttpNetworkLayer : public HttpTransactionFactory, public NonThreadSafe {
// creating |session_|.
HostResolver* host_resolver_;
DnsRRResolver* dnsrr_resolver_;
SSLHostInfoFactory* ssl_host_info_factory_;
scoped_refptr<ProxyService> proxy_service_;

// The SSL config service being used for the session.
Expand Down
3 changes: 3 additions & 0 deletions net/http/http_network_layer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ TEST_F(HttpNetworkLayerTest, CreateAndDestroy) {
NULL,
&host_resolver,
NULL /* dnsrr_resolver */,
NULL /* ssl_host_info_factory */,
net::ProxyService::CreateDirect(),
new net::SSLConfigServiceDefaults,
NULL,
Expand All @@ -43,6 +44,7 @@ TEST_F(HttpNetworkLayerTest, Suspend) {
NULL,
&host_resolver,
NULL /* dnsrr_resolver */,
NULL /* ssl_host_info_factory */,
net::ProxyService::CreateDirect(),
new net::SSLConfigServiceDefaults,
NULL,
Expand Down Expand Up @@ -90,6 +92,7 @@ TEST_F(HttpNetworkLayerTest, GET) {
&mock_socket_factory,
&host_resolver,
NULL /* dnsrr_resolver */,
NULL /* ssl_host_info_factory */,
net::ProxyService::CreateDirect(),
new net::SSLConfigServiceDefaults,
NULL,
Expand Down
2 changes: 2 additions & 0 deletions net/http/http_network_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ namespace net {
HttpNetworkSession::HttpNetworkSession(
HostResolver* host_resolver,
DnsRRResolver* dnsrr_resolver,
SSLHostInfoFactory* ssl_host_info_factory,
ProxyService* proxy_service,
ClientSocketFactory* client_socket_factory,
SSLConfigService* ssl_config_service,
Expand All @@ -37,6 +38,7 @@ HttpNetworkSession::HttpNetworkSession(
client_socket_factory,
host_resolver,
dnsrr_resolver,
ssl_host_info_factory,
proxy_service,
ssl_config_service),
spdy_session_pool_(spdy_session_pool),
Expand Down
Loading

0 comments on commit 8281f70

Please sign in to comment.