Pass the NetworkChangeNotifier to HostResolver.

This requires the following refactors:
  (1) NetworkChangeNotifier moves out of HttpNetworkSession into IOThread.
  (2) HostResolver gets initialized with NetworkChangeNotifier.
  (3) NetworkChangeNotifier needs to get passed into HttpCache and HttpNetworkSession (required updating a lot of files).
  (4) NetworkChangeNotifier is no longer reference counted.  It is owned by IOThread.
  (5) IOThread gains a new struct: Globals.  It can only be used on the io thread.
  (6) ChromeURLRequestContextFactory uses IOThread::Globals to initialize ChromeURLRequest objects with the host resolver and network change notifier.
BUG=26159

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

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

chrome/browser/io_thread.cc

@@ -15,11 +15,13 @@
 #include "net/base/host_cache.h"
 #include "net/base/host_resolver.h"
 #include "net/base/host_resolver_impl.h"
+#include "net/base/network_change_notifier.h"
 #include "net/url_request/url_request.h"
 
 namespace {
 
-net::HostResolver* CreateGlobalHostResolver() {
+net::HostResolver* CreateGlobalHostResolver(
+    net::NetworkChangeNotifier* network_change_notifier) {
   net::HostResolver* global_host_resolver = NULL;
 
   const CommandLine& command_line = *CommandLine::ForCurrentProcess();
@@ -31,7 +33,8 @@ net::HostResolver* CreateGlobalHostResolver() {
         command_line.GetSwitchValueASCII(switches::kFixedHost);
     global_host_resolver = new net::FixedHostResolver(host);
   } else {
-    global_host_resolver = net::CreateSystemHostResolver();
+    global_host_resolver =
+        net::CreateSystemHostResolver(network_change_notifier);
 
     if (command_line.HasSwitch(switches::kDisableIPv6))
       global_host_resolver->SetDefaultAddressFamily(net::ADDRESS_FAMILY_IPV4);
@@ -52,19 +55,20 @@ struct RunnableMethodTraits<IOThread> {
 
 IOThread::IOThread()
     : BrowserProcessSubThread(ChromeThread::IO),
-      host_resolver_(NULL),
+      globals_(NULL),
       prefetch_observer_(NULL),
       dns_master_(NULL) {}
 
 IOThread::~IOThread() {
   // We cannot rely on our base class to stop the thread since we want our
   // CleanUp function to run.
   Stop();
+  DCHECK(!globals_);
 }
 
-net::HostResolver* IOThread::host_resolver() {
+IOThread::Globals* IOThread::globals() {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
-  return host_resolver_;
+  return globals_;
 }
 
 void IOThread::InitDnsMaster(
@@ -95,9 +99,13 @@ void IOThread::ChangedToOnTheRecord() {
 void IOThread::Init() {
   BrowserProcessSubThread::Init();
 
-  DCHECK(!host_resolver_);
-  host_resolver_ = CreateGlobalHostResolver();
-  host_resolver_->AddRef();
+  DCHECK(!globals_);
+  globals_ = new Globals;
+
+  globals_->network_change_notifier.reset(
+      net::NetworkChangeNotifier::CreateDefaultNetworkChangeNotifier());
+  globals_->host_resolver =
+      CreateGlobalHostResolver(globals_->network_change_notifier.get());
 }
 
 void IOThread::CleanUp() {
@@ -116,18 +124,16 @@ void IOThread::CleanUp() {
 
   // Not initialized in Init().  May not be initialized.
   if (prefetch_observer_) {
-    host_resolver_->RemoveObserver(prefetch_observer_);
+    globals_->host_resolver->RemoveObserver(prefetch_observer_);
     delete prefetch_observer_;
     prefetch_observer_ = NULL;
   }
 
   // TODO(eroman): temp hack for http://crbug.com/15513
-  host_resolver_->Shutdown();
+  globals_->host_resolver->Shutdown();
 
-  // TODO(willchan): Stop reference counting HostResolver.  It's owned by
-  // IOThread now.
-  host_resolver_->Release();
-  host_resolver_ = NULL;
+  delete globals_;
+  globals_ = NULL;
 
   // URLFetcher and URLRequest instances must NOT outlive the IO thread.
   //
@@ -157,12 +163,12 @@ void IOThread::InitDnsMasterOnIOThread(
   chrome_browser_net::EnableDnsPrefetch(prefetching_enabled);
 
   dns_master_ = new chrome_browser_net::DnsMaster(
-      host_resolver_, max_queue_delay, max_concurrent);
+      globals_->host_resolver, max_queue_delay, max_concurrent);
   dns_master_->AddRef();
 
   DCHECK(!prefetch_observer_);
   prefetch_observer_ = chrome_browser_net::CreatePrefetchObserver();
-  host_resolver_->AddObserver(prefetch_observer_);
+  globals_->host_resolver->AddObserver(prefetch_observer_);
 
   FinalizeDnsPrefetchInitialization(
       dns_master_, prefetch_observer_, hostnames_to_prefetch, referral_list);
@@ -178,9 +184,9 @@ void IOThread::ChangedToOnTheRecordOnIOThread() {
 
   // Clear the host cache to avoid showing entries from the OTR session
   // in about:net-internals.
-  if (host_resolver_->IsHostResolverImpl()) {
+  if (globals_->host_resolver->IsHostResolverImpl()) {
     net::HostCache* host_cache = static_cast<net::HostResolverImpl*>(
-        host_resolver_)->cache();
+        globals_->host_resolver.get())->cache();
     if (host_cache)
       host_cache->clear();
   }

chrome/browser/io_thread.h

@@ -6,6 +6,8 @@
 #define CHROME_BROWSER_IO_THREAD_H_
 
 #include "base/basictypes.h"
+#include "base/ref_counted.h"
+#include "base/scoped_ptr.h"
 #include "base/task.h"
 #include "chrome/browser/browser_process_sub_thread.h"
 #include "chrome/common/net/dns.h"
@@ -17,13 +19,25 @@ namespace chrome_browser_net {
 class DnsMaster;
 }  // namespace chrome_browser_net
 
+namespace net {
+class NetworkChangeNotifier;
+}  // namespace net
+
 class IOThread : public BrowserProcessSubThread {
  public:
+  struct Globals {
+    scoped_ptr<net::NetworkChangeNotifier> network_change_notifier;
+    // TODO(willchan): Stop reference counting HostResolver.  It's owned by
+    // IOThread now.
+    scoped_refptr<net::HostResolver> host_resolver;
+  };
+
   IOThread();
 
   virtual ~IOThread();
 
-  net::HostResolver* host_resolver();
+  // Can only be called on the IO thread.
+  Globals* globals();
 
   // Initializes the DnsMaster.  |prefetching_enabled| indicates whether or
   // not dns prefetching should be enabled.  This should be called by the UI
@@ -56,10 +70,15 @@ class IOThread : public BrowserProcessSubThread {
   // These member variables are basically global, but their lifetimes are tied
   // to the IOThread.  IOThread owns them all, despite not using scoped_ptr.
   // This is because the destructor of IOThread runs on the wrong thread.  All
-  // member variables should be deleted in CleanUp().  Most of these will be
-  // initialized in Init().
+  // member variables should be deleted in CleanUp().
+
+  // These member variables are initialized in Init() and do not change for the
+  // lifetime of the IO thread.
+
+  Globals* globals_;
 
-  net::HostResolver* host_resolver_;
+  // These member variables are initialized by a task posted to the IO thread,
+  // which gets posted by calling certain member functions of IOThread.
 
   net::HostResolver::Observer* prefetch_observer_;
   chrome_browser_net::DnsMaster* dns_master_;

chrome/browser/net/chrome_url_request_context.cc

@@ -141,7 +141,7 @@ ChromeURLRequestContext* FactoryForOriginal::Create() {
   ApplyProfileParametersToContext(context);
 
   // Global host resolver for the context.
-  context->set_host_resolver(io_thread()->host_resolver());
+  context->set_host_resolver(io_thread()->globals()->host_resolver);
 
   const CommandLine& command_line = *CommandLine::ForCurrentProcess();
 
@@ -152,7 +152,8 @@ ChromeURLRequestContext* FactoryForOriginal::Create() {
                          MessageLoop::current() /*io_loop*/));
 
   net::HttpCache* cache =
-      new net::HttpCache(context->host_resolver(),
+      new net::HttpCache(io_thread()->globals()->network_change_notifier.get(),
+                         context->host_resolver(),
                          context->proxy_service(),
                          context->ssl_config_service(),
                          disk_cache_path_, cache_size_);
@@ -262,7 +263,8 @@ ChromeURLRequestContext* FactoryForOffTheRecord::Create() {
   context->set_proxy_service(original_context->proxy_service());
 
   net::HttpCache* cache =
-      new net::HttpCache(context->host_resolver(), context->proxy_service(),
+      new net::HttpCache(io_thread()->globals()->network_change_notifier.get(),
+                         context->host_resolver(), context->proxy_service(),
                          context->ssl_config_service(), 0);
   context->set_cookie_store(new net::CookieMonster);
   context->set_cookie_policy(
@@ -371,10 +373,12 @@ ChromeURLRequestContext* FactoryForMedia::Create() {
   } else {
     // If original HttpCache doesn't exist, simply construct one with a whole
     // new set of network stack.
-    cache = new net::HttpCache(main_context->host_resolver(),
-                               main_context->proxy_service(),
-                               main_context->ssl_config_service(),
-                               disk_cache_path_, cache_size_);
+    cache = new net::HttpCache(
+        io_thread()->globals()->network_change_notifier.get(),
+        main_context->host_resolver(),
+        main_context->proxy_service(),
+        main_context->ssl_config_service(),
+        disk_cache_path_, cache_size_);
   }
 
   if (CommandLine::ForCurrentProcess()->HasSwitch(

chrome/browser/net/chrome_url_request_context.h

@@ -10,6 +10,7 @@
 #include "net/base/cookie_policy.h"
 #include "chrome/browser/host_content_settings_map.h"
 #include "chrome/browser/host_zoom_map.h"
+#include "chrome/browser/io_thread.h"
 #include "chrome/browser/privacy_blacklist/blacklist.h"
 #include "chrome/browser/net/chrome_cookie_policy.h"
 #include "chrome/browser/net/url_request_context_getter.h"
@@ -28,7 +29,6 @@ class ProxyConfig;
 
 class ChromeURLRequestContext;
 class ChromeURLRequestContextFactory;
-class IOThread;
 
 // Subclass of URLRequestContext which can be used to store extra information
 // for requests.

chrome_frame/test/test_server_test.cc

@@ -60,12 +60,13 @@ class ScopedInternet {
 class URLRequestTestContext : public URLRequestContext {
  public:
   URLRequestTestContext() {
-    host_resolver_ = net::CreateSystemHostResolver();
+    host_resolver_ = net::CreateSystemHostResolver(NULL);
     proxy_service_ = net::ProxyService::CreateNull();
     ssl_config_service_ = new net::SSLConfigServiceDefaults;
     http_transaction_factory_ =
         new net::HttpCache(
-          net::HttpNetworkLayer::CreateFactory(host_resolver_, proxy_service_,
+          net::HttpNetworkLayer::CreateFactory(NULL, host_resolver_,
+                                               proxy_service_,
                                                ssl_config_service_),
           disk_cache::CreateInMemoryCacheBackend(0));
     // In-memory cookie store.

net/base/host_resolver.h

@@ -20,6 +20,7 @@ namespace net {
 class AddressList;
 class HostCache;
 class LoadLog;
+class NetworkChangeNotifier;
 
 // This class represents the task of resolving hostnames (or IP address
 // literal) to an AddressList object.
@@ -219,7 +220,10 @@ class SingleRequestHostResolver {
 // Creates a HostResolver implementation that queries the underlying system.
 // (Except if a unit-test has changed the global HostResolverProc using
 // ScopedHostResolverProc to intercept requests to the system).
-HostResolver* CreateSystemHostResolver();
+// |network_change_notifier| must outlive HostResolver.  It can optionally be
+// NULL, in which case HostResolver will not respond to network changes.
+HostResolver* CreateSystemHostResolver(
+    NetworkChangeNotifier* network_change_notifier);
 
 }  // namespace net
 

net/base/host_resolver_impl.cc

@@ -43,14 +43,15 @@ HostCache* CreateDefaultCache() {
 
 }  // anonymous namespace
 
-HostResolver* CreateSystemHostResolver() {
+HostResolver* CreateSystemHostResolver(
+    NetworkChangeNotifier* network_change_notifier) {
   // Maximum of 50 concurrent threads.
   // TODO(eroman): Adjust this, do some A/B experiments.
   static const size_t kMaxJobs = 50u;
 
   // TODO(willchan): Pass in the NetworkChangeNotifier.
   HostResolverImpl* resolver = new HostResolverImpl(
-      NULL, CreateDefaultCache(), NULL, kMaxJobs);
+      NULL, CreateDefaultCache(), network_change_notifier, kMaxJobs);
 
   return resolver;
 }
@@ -525,7 +526,7 @@ class HostResolverImpl::JobPool {
 HostResolverImpl::HostResolverImpl(
     HostResolverProc* resolver_proc,
     HostCache* cache,
-    const scoped_refptr<NetworkChangeNotifier>& network_change_notifier,
+    NetworkChangeNotifier* network_change_notifier,
     size_t max_jobs)
     : cache_(cache),
       max_jobs_(max_jobs),

net/base/host_resolver_impl.h

@@ -69,12 +69,13 @@ class HostResolverImpl : public HostResolver,
   // thread-safe since it is run from multiple worker threads. If
   // |resolver_proc| is NULL then the default host resolver procedure is
   // used (which is SystemHostResolverProc except if overridden).
-  //
+  // |notifier| must outlive HostResolverImpl.  It can optionally be NULL, in
+  // which case HostResolverImpl will not respond to network changes.
   // |max_jobs| specifies the maximum number of threads that the host resolver
   // will use. Use SetPoolConstraints() to specify finer-grain settings.
   HostResolverImpl(HostResolverProc* resolver_proc,
                    HostCache* cache,
-                   const scoped_refptr<NetworkChangeNotifier>& notifier,
+                   NetworkChangeNotifier* notifier,
                    size_t max_jobs);
 
   // HostResolver methods:
@@ -236,7 +237,7 @@ class HostResolverImpl : public HostResolver,
   // TODO(eroman): temp hack for http://crbug.com/15513
   bool shutdown_;
 
-  const scoped_refptr<NetworkChangeNotifier> network_change_notifier_;
+  NetworkChangeNotifier* const network_change_notifier_;
 
   scoped_refptr<RequestsTrace> requests_trace_;
 

net/base/host_resolver_impl_unittest.cc

@@ -978,11 +978,10 @@ TEST_F(HostResolverImplTest, CancellationObserver) {
 
 // Test that IP address changes flush the cache.
 TEST_F(HostResolverImplTest, FlushCacheOnIPAddressChange) {
-  scoped_refptr<MockNetworkChangeNotifier> mock_network_change_notifier(
-      new MockNetworkChangeNotifier);
+  MockNetworkChangeNotifier mock_network_change_notifier;
   scoped_refptr<HostResolver> host_resolver(
       new HostResolverImpl(NULL, CreateDefaultCache(),
-                           mock_network_change_notifier,
+                           &mock_network_change_notifier,
                            kMaxJobs));
 
   AddressList addrlist;
@@ -1000,7 +999,7 @@ TEST_F(HostResolverImplTest, FlushCacheOnIPAddressChange) {
   ASSERT_EQ(OK, rv);  // Should complete synchronously.
 
   // Flush cache by triggering an IP address change.
-  mock_network_change_notifier->NotifyIPAddressChange();
+  mock_network_change_notifier.NotifyIPAddressChange();
 
   // Resolve "host1" again -- this time it won't be served from cache, so it
   // will complete asynchronously.

net/base/network_change_notifier.cc

@@ -15,8 +15,8 @@
 namespace net {
 
 // static
-scoped_refptr<NetworkChangeNotifier>
-    NetworkChangeNotifier::CreateDefaultNetworkChangeNotifier() {
+NetworkChangeNotifier*
+NetworkChangeNotifier::CreateDefaultNetworkChangeNotifier() {
 #if defined(OS_WIN)
   return new NetworkChangeNotifierWin();
 #elif defined(OS_LINUX)

net/base/network_change_notifier.h

@@ -6,13 +6,13 @@
 #define NET_BASE_NETWORK_CHANGE_NOTIFIER_H_
 
 #include "base/basictypes.h"
-#include "base/ref_counted.h"
+#include "base/non_thread_safe.h"
 
 namespace net {
 
 // NetworkChangeNotifier monitors the system for network changes, and notifies
 // observers on those events.
-class NetworkChangeNotifier : public base::RefCounted<NetworkChangeNotifier> {
+class NetworkChangeNotifier : public NonThreadSafe {
  public:
   class Observer {
    public:
@@ -30,6 +30,7 @@ class NetworkChangeNotifier : public base::RefCounted<NetworkChangeNotifier> {
   };
 
   NetworkChangeNotifier() {}
+  virtual ~NetworkChangeNotifier() {}
 
   // These functions add and remove observers to/from the NetworkChangeNotifier.
   // Each call to AddObserver() must be matched with a corresponding call to
@@ -40,12 +41,7 @@ class NetworkChangeNotifier : public base::RefCounted<NetworkChangeNotifier> {
   virtual void RemoveObserver(Observer* observer) = 0;
 
   // This will create the platform specific default NetworkChangeNotifier.
-  static scoped_refptr<NetworkChangeNotifier>
-      CreateDefaultNetworkChangeNotifier();
-
- protected:
-  friend class base::RefCounted<NetworkChangeNotifier>;
-  virtual ~NetworkChangeNotifier() {}
+  static NetworkChangeNotifier* CreateDefaultNetworkChangeNotifier();
 
  private:
   DISALLOW_COPY_AND_ASSIGN(NetworkChangeNotifier);