Stop refcounting HostResolver.

BUG=46049
TEST=none

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@61256 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/io_thread.cc

@@ -169,9 +169,10 @@ void IOThread::Init() {
   network_change_observer_.reset(
       new LoggingNetworkChangeObserver(globals_->net_log.get()));
 
-  globals_->host_resolver = CreateGlobalHostResolver(globals_->net_log.get());
+  globals_->host_resolver.reset(
+      CreateGlobalHostResolver(globals_->net_log.get()));
   globals_->http_auth_handler_factory.reset(CreateDefaultAuthHandlerFactory(
-      globals_->host_resolver));
+      globals_->host_resolver.get()));
 }
 
 void IOThread::CleanUp() {
@@ -279,7 +280,7 @@ void IOThread::InitNetworkPredictorOnIOThread(
   chrome_browser_net::EnablePredictor(prefetching_enabled);
 
   predictor_ = new chrome_browser_net::Predictor(
-      globals_->host_resolver,
+      globals_->host_resolver.get(),
       max_dns_queue_delay,
       max_concurrent,
       preconnect_enabled);

chrome/browser/io_thread.h

@@ -32,9 +32,7 @@ class IOThread : public BrowserProcessSubThread {
  public:
   struct Globals {
     scoped_ptr<ChromeNetLog> net_log;
-    // TODO(willchan): Stop reference counting HostResolver.  It's owned by
-    // IOThread now.
-    scoped_refptr<net::HostResolver> host_resolver;
+    scoped_ptr<net::HostResolver> host_resolver;
     scoped_ptr<net::HttpAuthHandlerFactory> http_auth_handler_factory;
     scoped_ptr<net::URLSecurityManager> url_security_manager;
     ChromeNetworkDelegate network_delegate;

chrome/browser/net/chrome_url_request_context.cc

@@ -254,7 +254,7 @@ ChromeURLRequestContext* FactoryForOriginal::Create() {
   IOThread::Globals* io_thread_globals = io_thread()->globals();
 
   // Global host resolver for the context.
-  context->set_host_resolver(io_thread_globals->host_resolver);
+  context->set_host_resolver(io_thread_globals->host_resolver.get());
   context->set_http_auth_handler_factory(
       io_thread_globals->http_auth_handler_factory.get());
 

chrome/browser/net/connection_tester.cc

@@ -12,6 +12,7 @@
 #include "chrome/browser/importer/firefox_proxy_settings.h"
 #include "chrome/common/chrome_switches.h"
 #include "net/base/cookie_monster.h"
+#include "net/base/host_resolver.h"
 #include "net/base/host_resolver_impl.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
@@ -71,6 +72,7 @@ class ExperimentURLRequestContext : public URLRequestContext {
     delete ftp_transaction_factory_;
     delete http_transaction_factory_;
     delete http_auth_handler_factory_;
+    delete host_resolver_;
   }
 
  private:
@@ -79,10 +81,10 @@ class ExperimentURLRequestContext : public URLRequestContext {
   // error code.
   int CreateHostResolver(
       ConnectionTester::HostResolverExperiment experiment,
-      scoped_refptr<net::HostResolver>* host_resolver) {
+      net::HostResolver** host_resolver) {
     // Create a vanilla HostResolver that disables caching.
     const size_t kMaxJobs = 50u;
-    scoped_refptr<net::HostResolverImpl> impl =
+    net::HostResolverImpl* impl =
         new net::HostResolverImpl(NULL, NULL, kMaxJobs, NULL);
 
     *host_resolver = impl;

chrome/browser/net/predictor.h

@@ -270,8 +270,8 @@ class Predictor : public base::RefCountedThreadSafe<Predictor> {
   // reduction mode, and discard all queued (but not yet assigned) resolutions.
   const base::TimeDelta max_dns_queue_delay_;
 
-  // The host resovler we warm DNS entries for.
-  scoped_refptr<net::HostResolver> host_resolver_;
+  // The host resolver we warm DNS entries for.
+  net::HostResolver* const host_resolver_;
 
   // Are we currently using preconnection, rather than just DNS resolution, for
   // subresources and omni-box search URLs.

chrome/browser/net/predictor_unittest.cc

@@ -99,7 +99,7 @@ class PredictorTest : public testing::Test {
   ChromeThread io_thread_;
 
  protected:
-  scoped_refptr<net::MockCachingHostResolver> host_resolver_;
+  scoped_ptr<net::MockCachingHostResolver> host_resolver_;
 
   // Shorthand to access TimeDelta of PredictorInit::kMaxQueueingDelayMs.
   // (It would be a static constant... except style rules preclude that :-/ ).
@@ -109,10 +109,11 @@ class PredictorTest : public testing::Test {
 //------------------------------------------------------------------------------
 
 TEST_F(PredictorTest, StartupShutdownTest) {
-  scoped_refptr<Predictor> testing_master = new Predictor(host_resolver_,
-      default_max_queueing_delay_,
-      PredictorInit::kMaxPrefetchConcurrentLookups,
-      false);
+  scoped_refptr<Predictor> testing_master =
+      new Predictor(host_resolver_.get(),
+                    default_max_queueing_delay_,
+                    PredictorInit::kMaxPrefetchConcurrentLookups,
+                    false);
   testing_master->Shutdown();
 }
 
@@ -122,10 +123,11 @@ TEST_F(PredictorTest, ShutdownWhenResolutionIsPendingTest) {
       new net::WaitingHostResolverProc(NULL);
   host_resolver_->Reset(resolver_proc);
 
-  scoped_refptr<Predictor> testing_master = new Predictor(host_resolver_,
-      default_max_queueing_delay_,
-      PredictorInit::kMaxPrefetchConcurrentLookups,
-      false);
+  scoped_refptr<Predictor> testing_master =
+      new Predictor(host_resolver_.get(),
+                    default_max_queueing_delay_,
+                    PredictorInit::kMaxPrefetchConcurrentLookups,
+                    false);
 
   GURL localhost("http://localhost:80");
   UrlList names;
@@ -147,10 +149,11 @@ TEST_F(PredictorTest, ShutdownWhenResolutionIsPendingTest) {
 }
 
 TEST_F(PredictorTest, SingleLookupTest) {
-  scoped_refptr<Predictor> testing_master = new Predictor(host_resolver_,
-      default_max_queueing_delay_,
-      PredictorInit::kMaxPrefetchConcurrentLookups,
-      false);
+  scoped_refptr<Predictor> testing_master =
+      new Predictor(host_resolver_.get(),
+                    default_max_queueing_delay_,
+                    PredictorInit::kMaxPrefetchConcurrentLookups,
+                    false);
 
   GURL goog("http://www.google.com:80");
 
@@ -178,10 +181,11 @@ TEST_F(PredictorTest, SingleLookupTest) {
 TEST_F(PredictorTest, ConcurrentLookupTest) {
   host_resolver_->rules()->AddSimulatedFailure("*.notfound");
 
-  scoped_refptr<Predictor> testing_master = new Predictor(host_resolver_,
-      default_max_queueing_delay_,
-      PredictorInit::kMaxPrefetchConcurrentLookups,
-      false);
+  scoped_refptr<Predictor> testing_master =
+      new Predictor(host_resolver_.get(),
+                    default_max_queueing_delay_,
+                    PredictorInit::kMaxPrefetchConcurrentLookups,
+                    false);
 
   GURL goog("http://www.google.com:80"),
       goog2("http://gmail.google.com.com:80"),
@@ -228,11 +232,11 @@ TEST_F(PredictorTest, ConcurrentLookupTest) {
 TEST_F(PredictorTest, MassiveConcurrentLookupTest) {
   host_resolver_->rules()->AddSimulatedFailure("*.notfound");
 
-  scoped_refptr<Predictor> testing_master = new Predictor(
-      host_resolver_,
-      default_max_queueing_delay_,
-      PredictorInit::kMaxPrefetchConcurrentLookups,
-      false);
+  scoped_refptr<Predictor> testing_master =
+      new Predictor(host_resolver_.get(),
+                    default_max_queueing_delay_,
+                    PredictorInit::kMaxPrefetchConcurrentLookups,
+                    false);
 
   UrlList names;
   for (int i = 0; i < 100; i++)
@@ -348,10 +352,11 @@ static bool GetDataFromSerialization(const GURL& motivation,
 
 // Make sure nil referral lists really have no entries, and no latency listed.
 TEST_F(PredictorTest, ReferrerSerializationNilTest) {
-  scoped_refptr<Predictor> predictor = new Predictor(host_resolver_,
-      default_max_queueing_delay_,
-      PredictorInit::kMaxPrefetchConcurrentLookups,
-      false);
+  scoped_refptr<Predictor> predictor =
+      new Predictor(host_resolver_.get(),
+                    default_max_queueing_delay_,
+                    PredictorInit::kMaxPrefetchConcurrentLookups,
+                    false);
   scoped_ptr<ListValue> referral_list(NewEmptySerializationList());
   predictor->SerializeReferrers(referral_list.get());
   EXPECT_EQ(1U, referral_list->GetSize());
@@ -366,10 +371,11 @@ TEST_F(PredictorTest, ReferrerSerializationNilTest) {
 // deserialized into the database, and can be extracted back out via
 // serialization without being changed.
 TEST_F(PredictorTest, ReferrerSerializationSingleReferrerTest) {
-  scoped_refptr<Predictor> predictor = new Predictor(host_resolver_,
-      default_max_queueing_delay_,
-      PredictorInit::kMaxPrefetchConcurrentLookups,
-      false);
+  scoped_refptr<Predictor> predictor =
+      new Predictor(host_resolver_.get(),
+                    default_max_queueing_delay_,
+                    PredictorInit::kMaxPrefetchConcurrentLookups,
+                    false);
   const GURL motivation_url("http://www.google.com:91");
   const GURL subresource_url("http://icons.google.com:90");
   const double kUseRate = 23.4;
@@ -393,10 +399,11 @@ TEST_F(PredictorTest, ReferrerSerializationSingleReferrerTest) {
 
 // Make sure the Trim() functionality works as expected.
 TEST_F(PredictorTest, ReferrerSerializationTrimTest) {
-  scoped_refptr<Predictor> predictor = new Predictor(host_resolver_,
-      default_max_queueing_delay_,
-      PredictorInit::kMaxPrefetchConcurrentLookups,
-      false);
+  scoped_refptr<Predictor> predictor =
+      new Predictor(host_resolver_.get(),
+                    default_max_queueing_delay_,
+                    PredictorInit::kMaxPrefetchConcurrentLookups,
+                    false);
   GURL motivation_url("http://www.google.com:110");
 
   GURL icon_subresource_url("http://icons.google.com:111");

jingle/notifier/communicator/xmpp_connection_generator.cc

@@ -37,7 +37,7 @@
 namespace notifier {
 
 XmppConnectionGenerator::XmppConnectionGenerator(
-    const scoped_refptr<net::HostResolver>& host_resolver,
+    net::HostResolver* host_resolver,
     const ConnectionOptions* options,
     bool try_ssltcp_first,
     const ServerInformation* server_list,

jingle/notifier/communicator/xmpp_connection_generator.h

@@ -40,7 +40,7 @@ class XmppConnectionGenerator : public sigslot::has_slots<> {
   // server_list is the list of connections to attempt in priority order.
   // server_count is the number of items in the server list.
   XmppConnectionGenerator(
-      const scoped_refptr<net::HostResolver>& host_resolver,
+      net::HostResolver* host_resolver,
       const ConnectionOptions* options,
       bool try_ssltcp_first,
       const ServerInformation* server_list,

jingle/notifier/listener/mediator_thread_impl.cc

@@ -126,9 +126,9 @@ void MediatorThreadImpl::DoLogin(
 
   // TODO(akalin): Use an existing HostResolver from somewhere (maybe
   // the IOThread one).
-  host_resolver_ =
+  host_resolver_.reset(
       net::CreateSystemHostResolver(net::HostResolver::kDefaultParallelism,
-                                    NULL);
+                                    NULL));
 
   notifier::ServerInformation server_list[2];
   int server_list_count = 0;
@@ -171,7 +171,7 @@ void MediatorThreadImpl::DoDisconnect() {
   DCHECK_EQ(MessageLoop::current(), worker_message_loop());
   LOG(INFO) << "P2P: Thread logging out of talk network.";
   login_.reset();
-  host_resolver_ = NULL;
+  host_resolver_.reset();
   base_task_.reset();
 }
 

jingle/notifier/listener/mediator_thread_impl.h

@@ -24,7 +24,6 @@
 #include <vector>
 
 #include "base/basictypes.h"
-#include "base/ref_counted.h"
 #include "base/scoped_ptr.h"
 #include "base/thread.h"
 #include "jingle/notifier/base/notifier_options.h"
@@ -115,7 +114,7 @@ class MediatorThreadImpl
   const NotifierOptions notifier_options_;
 
   base::Thread worker_thread_;
-  scoped_refptr<net::HostResolver> host_resolver_;
+  scoped_ptr<net::HostResolver> host_resolver_;
 
   scoped_ptr<notifier::Login> login_;
 

net/base/host_resolver.h

@@ -8,7 +8,7 @@
 
 #include <string>
 
-#include "base/ref_counted.h"
+#include "base/scoped_ptr.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/address_family.h"
 #include "net/base/completion_callback.h"
@@ -31,7 +31,7 @@ class NetLog;
 // request at a time is to create a SingleRequestHostResolver wrapper around
 // HostResolver (which will automatically cancel the single request when it
 // goes out of scope).
-class HostResolver : public base::RefCounted<HostResolver> {
+class HostResolver {
  public:
   // The parameters for doing a Resolve(). A hostname and port are required,
   // the rest are optional (and have reasonable defaults).
@@ -124,6 +124,11 @@ class HostResolver : public base::RefCounted<HostResolver> {
   // concurrency.
   static const size_t kDefaultParallelism = 0;
 
+  // If any completion callbacks are pending when the resolver is destroyed,
+  // the host resolutions are cancelled, and the completion callbacks will not
+  // be called.
+  virtual ~HostResolver();
+
   // Resolves the given hostname (or IP address literal), filling out the
   // |addresses| object upon success.  The |info.port| parameter will be set as
   // the sin(6)_port field of the sockaddr_in{6} struct.  Returns OK if
@@ -174,15 +179,8 @@ class HostResolver : public base::RefCounted<HostResolver> {
   virtual void Shutdown() {}
 
  protected:
-  friend class base::RefCounted<HostResolver>;
-
   HostResolver();
 
-  // If any completion callbacks are pending when the resolver is destroyed,
-  // the host resolutions are cancelled, and the completion callbacks will not
-  // be called.
-  virtual ~HostResolver();
-
  private:
   DISALLOW_COPY_AND_ASSIGN(HostResolver);
 };
@@ -192,6 +190,7 @@ class HostResolver : public base::RefCounted<HostResolver> {
 // single hostname at a time and cancels this request when going out of scope.
 class SingleRequestHostResolver {
  public:
+  // |resolver| must remain valid for the lifetime of |this|.
   explicit SingleRequestHostResolver(HostResolver* resolver);
 
   // If a completion callback is pending when the resolver is destroyed, the
@@ -216,7 +215,7 @@ class SingleRequestHostResolver {
   void OnResolveCompletion(int result);
 
   // The actual host resolver that will handle the request.
-  scoped_refptr<HostResolver> resolver_;
+  HostResolver* const resolver_;
 
   // The current request (if any).
   HostResolver::RequestHandle cur_request_;

net/base/host_resolver_impl.h

@@ -82,6 +82,11 @@ class HostResolverImpl : public HostResolver,
                    size_t max_jobs,
                    NetLog* net_log);
 
+  // If any completion callbacks are pending when the resolver is destroyed,
+  // the host resolutions are cancelled, and the completion callbacks will not
+  // be called.
+  virtual ~HostResolverImpl();
+
   // HostResolver methods:
   virtual int Resolve(const RequestInfo& info,
                       AddressList* addresses,
@@ -132,11 +137,6 @@ class HostResolverImpl : public HostResolver,
   typedef std::map<Key, scoped_refptr<Job> > JobMap;
   typedef std::vector<HostResolver::Observer*> ObserversList;
 
-  // If any completion callbacks are pending when the resolver is destroyed,
-  // the host resolutions are cancelled, and the completion callbacks will not
-  // be called.
-  virtual ~HostResolverImpl();
-
   // Returns the HostResolverProc to use for this instance.
   HostResolverProc* effective_resolver_proc() const {
     return resolver_proc_ ?