Skip to content

Commit

Permalink
Stop refcounting ProxyService.
Browse files Browse the repository at this point in the history
BUG=none
TEST=none

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@83222 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
willchan@chromium.org committed Apr 27, 2011
1 parent 418e953 commit 6104ea5
Show file tree
Hide file tree
Showing 37 changed files with 320 additions and 334 deletions.
1 change: 1 addition & 0 deletions chrome/browser/automation/automation_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <string>

#include "base/memory/scoped_ptr.h"
#include "base/synchronization/waitable_event.h"
#include "base/time.h"
#include "base/values.h"
#include "chrome/browser/automation/automation_provider.h"
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/chromeos/login/login_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
#include "googleurl/src/gurl.h"
#include "net/base/cookie_store.h"
#include "net/proxy/proxy_config_service.h"
#include "net/proxy/proxy_service.h"
#include "net/url_request/url_request_context.h"
#include "net/url_request/url_request_context_getter.h"
#include "views/widget/widget_gtk.h"
Expand Down
13 changes: 7 additions & 6 deletions chrome/browser/io_thread.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
#include "net/base/host_resolver_impl.h"
#include "net/base/mapped_host_resolver.h"
#include "net/base/net_util.h"
#include "net/proxy/proxy_config_service.h"
#include "net/ftp/ftp_network_layer.h"
#include "net/http/http_auth_filter.h"
#include "net/http/http_auth_handler_factory.h"
Expand All @@ -49,7 +48,9 @@
#if defined(USE_NSS)
#include "net/ocsp/nss_ocsp.h"
#endif // defined(USE_NSS)
#include "net/proxy/proxy_config_service.h"
#include "net/proxy/proxy_script_fetcher_impl.h"
#include "net/proxy/proxy_service.h"
#include "webkit/glue/webkit_glue.h"

namespace {
Expand Down Expand Up @@ -428,8 +429,8 @@ void IOThread::Init() {
globals_->http_auth_handler_factory.reset(CreateDefaultAuthHandlerFactory(
globals_->host_resolver.get()));
// For the ProxyScriptFetcher, we use a direct ProxyService.
globals_->proxy_script_fetcher_proxy_service =
net::ProxyService::CreateDirectWithNetLog(net_log_);
globals_->proxy_script_fetcher_proxy_service.reset(
net::ProxyService::CreateDirectWithNetLog(net_log_));
net::HttpNetworkSession::Params session_params;
session_params.host_resolver = globals_->host_resolver.get();
session_params.cert_verifier = globals_->cert_verifier.get();
Expand Down Expand Up @@ -625,16 +626,16 @@ void IOThread::ClearHostCache() {

void IOThread::InitSystemRequestContext() {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
DCHECK(!globals_->system_proxy_service);
DCHECK(!globals_->system_proxy_service.get());
DCHECK(system_proxy_config_service_.get());

const CommandLine& command_line = *CommandLine::ForCurrentProcess();
globals_->system_proxy_service =
globals_->system_proxy_service.reset(
ProxyServiceFactory::CreateProxyService(
net_log_,
globals_->proxy_script_fetcher_context,
system_proxy_config_service_.release(),
command_line);
command_line));
net::HttpNetworkSession::Params system_params;
system_params.host_resolver = globals_->host_resolver.get();
system_params.cert_verifier = globals_->cert_verifier.get();
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/io_thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,16 +59,16 @@ class IOThread : public BrowserProcessSubThread {
scoped_ptr<net::DnsRRResolver> dnsrr_resolver;
scoped_refptr<net::SSLConfigService> ssl_config_service;
scoped_ptr<net::HttpAuthHandlerFactory> http_auth_handler_factory;
scoped_refptr<net::ProxyService> proxy_script_fetcher_proxy_service;
scoped_ptr<net::ProxyService> proxy_script_fetcher_proxy_service;
scoped_ptr<net::HttpTransactionFactory>
proxy_script_fetcher_http_transaction_factory;
scoped_ptr<net::FtpTransactionFactory>
proxy_script_fetcher_ftp_transaction_factory;
scoped_ptr<net::URLSecurityManager> url_security_manager;
scoped_refptr<net::URLRequestContext> proxy_script_fetcher_context;
scoped_ptr<net::ProxyService> system_proxy_service;
scoped_ptr<net::HttpTransactionFactory> system_http_transaction_factory;
scoped_ptr<net::FtpTransactionFactory> system_ftp_transaction_factory;
scoped_refptr<net::ProxyService> system_proxy_service;
// NOTE(willchan): This request context is unusable until a system
// SSLConfigService is provided that doesn't rely on
// Profiles. Do NOT use this yet.
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/memory_purger.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "content/browser/renderer_host/resource_dispatcher_host.h"
#include "content/common/notification_service.h"
#include "net/proxy/proxy_resolver.h"
#include "net/proxy/proxy_service.h"
#include "net/url_request/url_request_context.h"
#include "net/url_request/url_request_context_getter.h"
#include "third_party/tcmalloc/chromium/src/google/malloc_extension.h"
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/net/chrome_url_request_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include "base/message_loop.h"
#include "base/message_loop_proxy.h"
#include "base/synchronization/waitable_event.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/io_thread.h"
#include "chrome/browser/profiles/profile.h"
Expand Down
9 changes: 6 additions & 3 deletions chrome/browser/net/chrome_url_request_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,19 @@
class ChromeURLDataManagerBackend;
class ChromeURLRequestContextFactory;
class IOThread;
class PrefService;
class Profile;
class ProfileIOData;
namespace base {
class WaitableEvent;
}
namespace net {
class DnsCertProvenanceChecker;
class NetworkDelegate;
}
class PrefService;
namespace prerender {
class PrerenderManager;
}
class Profile;
class ProfileIOData;

// Subclass of net::URLRequestContext which can be used to store extra
// information for requests.
Expand Down
51 changes: 25 additions & 26 deletions chrome/browser/net/connection_tester.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,10 @@
#include "net/http/http_network_session.h"
#include "net/proxy/proxy_config_service_fixed.h"
#include "net/proxy/proxy_script_fetcher_impl.h"
#include "net/proxy/proxy_service.h"
#include "net/url_request/url_request.h"
#include "net/url_request/url_request_context.h"
#include "net/url_request/url_request_context_storage.h"

namespace {

Expand All @@ -42,62 +44,58 @@ class ExperimentURLRequestContext : public net::URLRequestContext {
public:
explicit ExperimentURLRequestContext(
net::URLRequestContext* proxy_request_context)
: proxy_request_context_(proxy_request_context) {}
: proxy_request_context_(proxy_request_context),
ALLOW_THIS_IN_INITIALIZER_LIST(storage_(this)) {}

int Init(const ConnectionTester::Experiment& experiment) {
int rv;

// Create a custom HostResolver for this experiment.
net::HostResolver* host_resolver_tmp = NULL;
scoped_ptr<net::HostResolver> host_resolver_tmp;
rv = CreateHostResolver(experiment.host_resolver_experiment,
&host_resolver_tmp);
if (rv != net::OK)
return rv; // Failure.
set_host_resolver(host_resolver_tmp);
storage_.set_host_resolver(host_resolver_tmp.release());

// Create a custom ProxyService for this this experiment.
scoped_refptr<net::ProxyService> proxy_service_tmp = NULL;
scoped_ptr<net::ProxyService> proxy_service_tmp;
rv = CreateProxyService(experiment.proxy_settings_experiment,
&proxy_service_tmp);
if (rv != net::OK)
return rv; // Failure.
set_proxy_service(proxy_service_tmp);
storage_.set_proxy_service(proxy_service_tmp.release());

// The rest of the dependencies are standard, and don't depend on the
// experiment being run.
set_cert_verifier(new net::CertVerifier);
set_dnsrr_resolver(new net::DnsRRResolver);
set_ftp_transaction_factory(new net::FtpNetworkLayer(host_resolver_tmp));
set_ssl_config_service(new net::SSLConfigServiceDefaults);
set_http_auth_handler_factory(net::HttpAuthHandlerFactory::CreateDefault(
host_resolver_tmp));
storage_.set_cert_verifier(new net::CertVerifier);
storage_.set_dnsrr_resolver(new net::DnsRRResolver);
storage_.set_ftp_transaction_factory(
new net::FtpNetworkLayer(host_resolver()));
storage_.set_ssl_config_service(new net::SSLConfigServiceDefaults);
storage_.set_http_auth_handler_factory(
net::HttpAuthHandlerFactory::CreateDefault(host_resolver()));

net::HttpNetworkSession::Params session_params;
session_params.host_resolver = host_resolver_tmp;
session_params.host_resolver = host_resolver();
session_params.dnsrr_resolver = dnsrr_resolver();
session_params.cert_verifier = cert_verifier();
session_params.proxy_service = proxy_service_tmp;
session_params.proxy_service = proxy_service();
session_params.http_auth_handler_factory = http_auth_handler_factory();
session_params.ssl_config_service = ssl_config_service();
scoped_refptr<net::HttpNetworkSession> network_session(
new net::HttpNetworkSession(session_params));
set_http_transaction_factory(new net::HttpCache(
storage_.set_http_transaction_factory(new net::HttpCache(
network_session,
net::HttpCache::DefaultBackend::InMemory(0)));
// In-memory cookie store.
set_cookie_store(new net::CookieMonster(NULL, NULL));
storage_.set_cookie_store(new net::CookieMonster(NULL, NULL));

return net::OK;
}

protected:
virtual ~ExperimentURLRequestContext() {
delete ftp_transaction_factory();
delete http_transaction_factory();
delete http_auth_handler_factory();
delete dnsrr_resolver();
delete cert_verifier();
delete host_resolver();
}

private:
Expand All @@ -106,13 +104,13 @@ class ExperimentURLRequestContext : public net::URLRequestContext {
// error code.
int CreateHostResolver(
ConnectionTester::HostResolverExperiment experiment,
net::HostResolver** host_resolver) {
scoped_ptr<net::HostResolver>* host_resolver) {
// Create a vanilla HostResolver that disables caching.
const size_t kMaxJobs = 50u;
net::HostResolverImpl* impl =
new net::HostResolverImpl(NULL, NULL, kMaxJobs, NULL);

*host_resolver = impl;
host_resolver->reset(impl);

// Modify it slightly based on the experiment being run.
switch (experiment) {
Expand Down Expand Up @@ -170,7 +168,7 @@ class ExperimentURLRequestContext : public net::URLRequestContext {
// error code.
int CreateProxyService(
ConnectionTester::ProxySettingsExperiment experiment,
scoped_refptr<net::ProxyService>* proxy_service) {
scoped_ptr<net::ProxyService>* proxy_service) {
// Create an appropriate proxy config service.
scoped_ptr<net::ProxyConfigService> config_service;
int rv = CreateProxyConfigService(experiment, &config_service);
Expand All @@ -184,12 +182,12 @@ class ExperimentURLRequestContext : public net::URLRequestContext {
return net::ERR_NOT_IMPLEMENTED;
}

*proxy_service = net::ProxyService::CreateUsingV8ProxyResolver(
proxy_service->reset(net::ProxyService::CreateUsingV8ProxyResolver(
config_service.release(),
0u,
new net::ProxyScriptFetcherImpl(proxy_request_context_),
host_resolver(),
NULL);
NULL));

return net::OK;
}
Expand Down Expand Up @@ -235,6 +233,7 @@ class ExperimentURLRequestContext : public net::URLRequestContext {
}

const scoped_refptr<net::URLRequestContext> proxy_request_context_;
net::URLRequestContextStorage storage_;
};

} // namespace
Expand Down
9 changes: 5 additions & 4 deletions chrome/browser/net/connection_tester_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "net/http/http_network_layer.h"
#include "net/http/http_network_session.h"
#include "net/proxy/proxy_config_service_fixed.h"
#include "net/proxy/proxy_service.h"
#include "net/test/test_server.h"
#include "net/url_request/url_request_context.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -105,7 +106,7 @@ class ConnectionTesterTest : public PlatformTest {
net::MockHostResolver host_resolver_;
net::CertVerifier cert_verifier_;
net::DnsRRResolver dnsrr_resolver_;
scoped_refptr<net::ProxyService> proxy_service_;
scoped_ptr<net::ProxyService> proxy_service_;
scoped_refptr<net::SSLConfigService> ssl_config_service_;
scoped_ptr<net::HttpTransactionFactory> http_transaction_factory_;
net::HttpAuthHandlerRegistryFactory http_auth_handler_factory_;
Expand All @@ -118,16 +119,16 @@ class ConnectionTesterTest : public PlatformTest {
proxy_script_fetcher_context_->set_dnsrr_resolver(&dnsrr_resolver_);
proxy_script_fetcher_context_->set_http_auth_handler_factory(
&http_auth_handler_factory_);
proxy_service_ = net::ProxyService::CreateDirect();
proxy_script_fetcher_context_->set_proxy_service(proxy_service_);
proxy_service_.reset(net::ProxyService::CreateDirect());
proxy_script_fetcher_context_->set_proxy_service(proxy_service_.get());
ssl_config_service_ = net::SSLConfigService::CreateSystemSSLConfigService();
net::HttpNetworkSession::Params session_params;
session_params.host_resolver = &host_resolver_;
session_params.cert_verifier = &cert_verifier_;
session_params.dnsrr_resolver = &dnsrr_resolver_;
session_params.http_auth_handler_factory = &http_auth_handler_factory_;
session_params.ssl_config_service = ssl_config_service_;
session_params.proxy_service = proxy_service_;
session_params.proxy_service = proxy_service_.get();
scoped_refptr<net::HttpNetworkSession> network_session(
new net::HttpNetworkSession(session_params));
http_transaction_factory_.reset(
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/net/proxy_service_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "net/base/net_log.h"
#include "net/proxy/proxy_config_service.h"
#include "net/proxy/proxy_script_fetcher_impl.h"
#include "net/proxy/proxy_service.h"
#include "net/url_request/url_request_context.h"

#if defined(OS_CHROMEOS)
Expand Down
9 changes: 5 additions & 4 deletions chrome/browser/net/resolve_proxy_msg_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,7 @@ void ResolveProxyMsgHelper::StartPendingRequest() {
OnResolveProxyCompleted(result);
}

bool ResolveProxyMsgHelper::GetProxyService(
scoped_refptr<net::ProxyService>* out) const {
bool ResolveProxyMsgHelper::GetProxyService(net::ProxyService** out) const {
// Unit-tests specify their own proxy service to use.
if (proxy_service_override_) {
*out = proxy_service_override_;
Expand All @@ -102,8 +101,10 @@ bool ResolveProxyMsgHelper::GetProxyService(
}

ResolveProxyMsgHelper::~ResolveProxyMsgHelper() {
// Clear all pending requests.
if (!pending_requests_.empty()) {
// Clear all pending requests if the ProxyService is still alive (if we have a
// default request context or override).
if (!pending_requests_.empty() &&
(Profile::GetDefaultRequestContext() || proxy_service_override_)) {
PendingRequest req = pending_requests_.front();
proxy_service_->CancelPacRequest(req.pac_req);
}
Expand Down
6 changes: 3 additions & 3 deletions chrome/browser/net/resolve_proxy_msg_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class ResolveProxyMsgHelper : public BrowserMessageFilter {

// Get the proxy service instance to use. On success returns true and
// sets |*out|. Otherwise returns false.
bool GetProxyService(scoped_refptr<net::ProxyService>* out) const;
bool GetProxyService(net::ProxyService** out) const;

// A PendingRequest is a resolve request that is in progress, or queued.
struct PendingRequest {
Expand All @@ -69,7 +69,7 @@ class ResolveProxyMsgHelper : public BrowserMessageFilter {
};

// Members for the current outstanding proxy request.
scoped_refptr<net::ProxyService> proxy_service_;
net::ProxyService* proxy_service_;
net::CompletionCallbackImpl<ResolveProxyMsgHelper> callback_;
net::ProxyInfo proxy_info_;

Expand All @@ -79,7 +79,7 @@ class ResolveProxyMsgHelper : public BrowserMessageFilter {

// Specified by unit-tests, to use this proxy service in place of the
// global one.
scoped_refptr<net::ProxyService> proxy_service_override_;
net::ProxyService* proxy_service_override_;
};

#endif // CHROME_BROWSER_NET_RESOLVE_PROXY_MSG_HELPER_H_
3 changes: 2 additions & 1 deletion chrome/browser/net/resolve_proxy_msg_helper_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "net/base/net_errors.h"
#include "net/proxy/mock_proxy_resolver.h"
#include "net/proxy/proxy_config_service.h"
#include "net/proxy/proxy_service.h"
#include "testing/gtest/include/gtest/gtest.h"

// This ProxyConfigService always returns "http://pac" as the PAC url to use.
Expand Down Expand Up @@ -61,7 +62,7 @@ class ResolveProxyMsgHelperTest : public testing::Test,
}

net::MockAsyncProxyResolver* resolver_;
scoped_refptr<net::ProxyService> service_;
scoped_ptr<net::ProxyService> service_;
scoped_refptr<ResolveProxyMsgHelper> helper_;
scoped_ptr<PendingResult> pending_result_;

Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/profiles/profile_io_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -302,12 +302,12 @@ void ProfileIOData::LazyInitialize() const {
CreateDnsCertProvenanceChecker(io_thread_globals->dnsrr_resolver.get(),
main_request_context_));

proxy_service_ =
proxy_service_.reset(
ProxyServiceFactory::CreateProxyService(
io_thread->net_log(),
io_thread_globals->proxy_script_fetcher_context.get(),
profile_params_->proxy_config_service.release(),
command_line);
command_line));

// Take ownership over these parameters.
database_tracker_ = profile_params_->database_tracker;
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/profiles/profile_io_data.h
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ class ProfileIOData : public base::RefCountedThreadSafe<ProfileIOData> {
// Pointed to by URLRequestContext.
mutable scoped_ptr<net::NetworkDelegate> network_delegate_;
mutable scoped_ptr<net::DnsCertProvenanceChecker> dns_cert_checker_;
mutable scoped_refptr<net::ProxyService> proxy_service_;
mutable scoped_ptr<net::ProxyService> proxy_service_;
mutable scoped_ptr<net::CookiePolicy> cookie_policy_;

// Pointed to by ResourceContext.
Expand Down
Loading

0 comments on commit 6104ea5

Please sign in to comment.