Massively simplify the NetworkChangeNotifier infrastructure:

  * Use a process-wide object (singleton pattern)
  * Create/destroy this object on the main thread, make it outlive all consumers
  * Make observer-related functions threadsafe

As a result, the notifier can now be used by any thread (eliminating things like NetworkChangeObserverProxy and NetworkChangeNotifierProxy, and expanding its usefulness); its creation and inner workings are much simplified (eliminating implementation-specific classes); and it is simpler to access (eliminating things like NetworkChangeNotifierThread and a LOT of passing pointers around).

BUG=none
TEST=Unittests; network changes still trigger notifications
Review URL: http://codereview.chromium.org/2802015

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

7 files changed, 15 insertions, 215 deletions

diff --git a/chrome/service/cloud_print/cloud_print_proxy_backend.cc b/chrome/service/cloud_print/cloud_print_proxy_backend.cc
index 683584d..c30d588 100644
--- a/chrome/service/cloud_print/cloud_print_proxy_backend.cc
+++ b/chrome/service/cloud_print/cloud_print_proxy_backend.cc
@@ -15,7 +15,6 @@
 #include "chrome/common/net/notifier/listener/mediator_thread_impl.h"
 #include "chrome/common/net/notifier/listener/talk_mediator_impl.h"
 #include "chrome/service/gaia/service_gaia_authenticator.h"
-#include "chrome/service/net/service_network_change_notifier_thread.h"
 #include "chrome/service/service_process.h"
 
 #include "googleurl/src/gurl.h"
@@ -305,9 +304,8 @@ void CloudPrintProxyBackend::Core::DoInitializeWithToken(
   const bool kConnectImmediately = false;
   const bool kInvalidateXmppAuthToken = false;
   talk_mediator_.reset(new notifier::TalkMediatorImpl(
-      new notifier::MediatorThreadImpl(
-          g_service_process->network_change_notifier_thread()),
-      kInitializeSsl, kConnectImmediately, kInvalidateXmppAuthToken));
+      new notifier::MediatorThreadImpl(), kInitializeSsl, kConnectImmediately,
+      kInvalidateXmppAuthToken));
   talk_mediator_->AddSubscribedServiceUrl(kCloudPrintTalkServiceUrl);
   talk_mediator_->SetDelegate(this);
   talk_mediator_->SetAuthToken(email, cloud_print_xmpp_token,
diff --git a/chrome/service/net/service_network_change_notifier_thread.cc b/chrome/service/net/service_network_change_notifier_thread.cc
deleted file mode 100644
index 3ccd146..0000000
--- a/chrome/service/net/service_network_change_notifier_thread.cc
+++ /dev/null
@@ -1,46 +0,0 @@
-// 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 "chrome/service/net/service_network_change_notifier_thread.h"
-
-#include "base/logging.h"
-#include "base/message_loop.h"
-#include "net/base/network_change_notifier.h"
-
-ServiceNetworkChangeNotifierThread::ServiceNetworkChangeNotifierThread(
-    MessageLoop* io_thread_message_loop)
-        : io_thread_message_loop_(io_thread_message_loop) {
-  DCHECK(io_thread_message_loop_);
-}
-
-ServiceNetworkChangeNotifierThread::~ServiceNetworkChangeNotifierThread() {
-  io_thread_message_loop_->DeleteSoon(FROM_HERE,
-                                      network_change_notifier_.release());
-}
-
-void ServiceNetworkChangeNotifierThread::Initialize() {
-  io_thread_message_loop_->PostTask(
-      FROM_HERE,
-      NewRunnableMethod(
-          this,
-          &ServiceNetworkChangeNotifierThread::CreateNetworkChangeNotifier));
-}
-
-MessageLoop* ServiceNetworkChangeNotifierThread::GetMessageLoop() const {
-  DCHECK(io_thread_message_loop_);
-  return io_thread_message_loop_;
-}
-
-net::NetworkChangeNotifier*
-ServiceNetworkChangeNotifierThread::GetNetworkChangeNotifier() const {
-  DCHECK(MessageLoop::current() == io_thread_message_loop_);
-  return network_change_notifier_.get();
-}
-
-void ServiceNetworkChangeNotifierThread::CreateNetworkChangeNotifier() {
-  DCHECK(MessageLoop::current() == io_thread_message_loop_);
-  network_change_notifier_.reset(
-      net::NetworkChangeNotifier::CreateDefaultNetworkChangeNotifier());
-}
-
diff --git a/chrome/service/net/service_network_change_notifier_thread.h b/chrome/service/net/service_network_change_notifier_thread.h
deleted file mode 100644
index 14975a6..0000000
--- a/chrome/service/net/service_network_change_notifier_thread.h
+++ /dev/null
@@ -1,50 +0,0 @@
-// 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.
-
-#ifndef CHROME_SERVICE_NET_SERVICE_NETWORK_CHANGE_NOTIFIER_THREAD_H_
-#define CHROME_SERVICE_NET_SERVICE_NETWORK_CHANGE_NOTIFIER_THREAD_H_
-
-#include "base/basictypes.h"
-#include "base/ref_counted.h"
-#include "base/scoped_ptr.h"
-#include "chrome/common/net/network_change_notifier_thread.h"
-
-class MessageLoop;
-
-namespace net {
-class NetworkChangeNotifier;
-}  // namespace net
-
-// This is a simple implementation of NetworkChangeNotifierThread.
-// Note that Initialize MUST be called before this class can be used.
-class ServiceNetworkChangeNotifierThread
-    : public chrome_common_net::NetworkChangeNotifierThread,
-      public base::RefCountedThreadSafe<ServiceNetworkChangeNotifierThread> {
- public:
-  // Does not take ownership of |io_thread_message_loop|. This instance must
-  // live no longer than |io_thread_message_loop|.
-  // TODO(sanjeevr): Change NetworkChangeNotifierThread to use MessageLoopProxy
-  explicit ServiceNetworkChangeNotifierThread(
-      MessageLoop* io_thread_message_loop);
-  virtual ~ServiceNetworkChangeNotifierThread();
-
-  // Initialize MUST be called before this class can be used.
-  void Initialize();
-
-  // chrome_common_net::NetworkChangeNotifierThread implementation.
-
-  virtual MessageLoop* GetMessageLoop() const;
-
-  virtual net::NetworkChangeNotifier* GetNetworkChangeNotifier() const;
-
- private:
-  MessageLoop* const io_thread_message_loop_;
-  scoped_ptr<net::NetworkChangeNotifier> network_change_notifier_;
-
-  void CreateNetworkChangeNotifier();
-  DISALLOW_COPY_AND_ASSIGN(ServiceNetworkChangeNotifierThread);
-};
-
-#endif  // CHROME_SERVICE_NET_SERVICE_NETWORK_CHANGE_NOTIFIER_THREAD_H_
-
diff --git a/chrome/service/net/service_network_change_notifier_thread_unittest.cc b/chrome/service/net/service_network_change_notifier_thread_unittest.cc
deleted file mode 100644
index 917d51a..0000000
--- a/chrome/service/net/service_network_change_notifier_thread_unittest.cc
+++ /dev/null
@@ -1,100 +0,0 @@
-// 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 "chrome/service/net/service_network_change_notifier_thread.h"
-
-#include "base/basictypes.h"
-#include "base/message_loop.h"
-#include "base/scoped_ptr.h"
-#include "base/task.h"
-#include "base/thread.h"
-#include "chrome/common/net/thread_blocker.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace {
-
-class ServiceNetworkChangeNotifierThreadTest : public testing::Test {
- protected:
-  ServiceNetworkChangeNotifierThreadTest()
-     : io_thread_("ServiceNetworkChangeNotifierThreadTest_IO") {
-  }
-
-  virtual ~ServiceNetworkChangeNotifierThreadTest() {}
-
-  virtual void SetUp() {
-    // We need to set the message loop type explicitly because
-    // IOThread doesn't do it for us and the Linux
-    // NetworkChangeNotifier expects it.
-    base::Thread::Options options;
-    options.message_loop_type = MessageLoop::TYPE_IO;
-    EXPECT_TRUE(io_thread_.StartWithOptions(options));
-    thread_blocker_.reset(new chrome_common_net::ThreadBlocker(&io_thread_));
-    thread_blocker_->Block();
-  }
-
-  virtual void TearDown() {
-    // Nothing should be posted on |io_thread_| at this point.
-    thread_blocker_->Unblock();
-    thread_blocker_.reset();
-    io_thread_.Stop();
-  }
-
-  base::Thread io_thread_;
-  scoped_ptr<chrome_common_net::ThreadBlocker> thread_blocker_;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(ServiceNetworkChangeNotifierThreadTest);
-};
-
-void CheckNonNullNetworkChangeNotifier(
-    ServiceNetworkChangeNotifierThread* network_change_notifier_thread) {
-  EXPECT_EQ(network_change_notifier_thread->GetMessageLoop(),
-            MessageLoop::current());
-  EXPECT_TRUE(
-      network_change_notifier_thread->GetNetworkChangeNotifier() != NULL);
-}
-
-void CheckNullNetworkChangeNotifier(
-    ServiceNetworkChangeNotifierThread* network_change_notifier_thread) {
-  EXPECT_EQ(network_change_notifier_thread->GetMessageLoop(),
-            MessageLoop::current());
-  EXPECT_TRUE(
-      network_change_notifier_thread->GetNetworkChangeNotifier() == NULL);
-}
-
-TEST_F(ServiceNetworkChangeNotifierThreadTest, Basic) {
-  scoped_refptr<ServiceNetworkChangeNotifierThread> change_notifier_thread =
-      new ServiceNetworkChangeNotifierThread(io_thread_.message_loop());
-  EXPECT_EQ(io_thread_.message_loop(),
-            change_notifier_thread->GetMessageLoop());
-  change_notifier_thread->Initialize();
-  io_thread_.message_loop()->PostTask(
-      FROM_HERE,
-      NewRunnableFunction(&CheckNonNullNetworkChangeNotifier,
-                          change_notifier_thread.get()));
-  // Pump the thread to make sure the task we just posted is run
-  // before this test ends.
-  thread_blocker_->Unblock();
-  thread_blocker_->Block();
-}
-
-TEST_F(ServiceNetworkChangeNotifierThreadTest, Uninitialized) {
-  scoped_refptr<ServiceNetworkChangeNotifierThread> change_notifier_thread =
-      new ServiceNetworkChangeNotifierThread(io_thread_.message_loop());
-  EXPECT_EQ(io_thread_.message_loop(),
-            change_notifier_thread->GetMessageLoop());
-  // We have not called Initialize. Expect the GetNetworkChangeNotifier() call
-  // to return NULL.
-  io_thread_.message_loop()->PostTask(
-      FROM_HERE,
-      NewRunnableFunction(&CheckNullNetworkChangeNotifier,
-                          change_notifier_thread.get()));
-  // Pump the thread to make sure the task we just posted is run
-  // before this test ends.
-  thread_blocker_->Unblock();
-  thread_blocker_->Block();
-}
-
-}  // namespace
-
diff --git a/chrome/service/net/service_url_request_context.cc b/chrome/service/net/service_url_request_context.cc
index 3ca57cd..ee416fe 100644
--- a/chrome/service/net/service_url_request_context.cc
+++ b/chrome/service/net/service_url_request_context.cc
@@ -21,7 +21,7 @@ ServiceURLRequestContextGetter::ServiceURLRequestContextGetter()
 }
 
 ServiceURLRequestContext::ServiceURLRequestContext() {
-  host_resolver_ = net::CreateSystemHostResolver(NULL);
+  host_resolver_ = net::CreateSystemHostResolver();
   DCHECK(g_service_process);
   // TODO(sanjeevr): Change CreateSystemProxyConfigService to accept a
   // MessageLoopProxy* instead of MessageLoop*.
@@ -30,13 +30,13 @@ ServiceURLRequestContext::ServiceURLRequestContext() {
       net::ProxyService::CreateSystemProxyConfigService(
           g_service_process->io_thread()->message_loop(),
           g_service_process->file_thread()->message_loop());
-  proxy_service_ = net::ProxyService::Create(proxy_config_service, false, this,
-                                             NULL, NULL, NULL);
+  proxy_service_ =
+      net::ProxyService::Create(proxy_config_service, false, this, NULL, NULL);
   ftp_transaction_factory_ = new net::FtpNetworkLayer(host_resolver_);
   ssl_config_service_ = new net::SSLConfigServiceDefaults;
   http_auth_handler_factory_ = net::HttpAuthHandlerFactory::CreateDefault();
   http_transaction_factory_ = new net::HttpCache(
-      net::HttpNetworkLayer::CreateFactory(NULL, host_resolver_,
+      net::HttpNetworkLayer::CreateFactory(host_resolver_,
                                            proxy_service_,
                                            ssl_config_service_,
                                            http_auth_handler_factory_,
diff --git a/chrome/service/service_process.cc b/chrome/service/service_process.cc
index 6e9baf2..09399f0 100644
--- a/chrome/service/service_process.cc
+++ b/chrome/service/service_process.cc
@@ -6,7 +6,7 @@
 
 #include "base/stl_util-inl.h"
 #include "chrome/service/cloud_print/cloud_print_proxy.h"
-#include "chrome/service/net/service_network_change_notifier_thread.h"
+#include "net/base/network_change_notifier.h"
 
 ServiceProcess* g_service_process = NULL;
 
@@ -16,6 +16,7 @@ ServiceProcess::ServiceProcess() {
 }
 
 bool ServiceProcess::Initialize() {
+  network_change_notifier_.reset(net::NetworkChangeNotifier::Create());
   base::Thread::Options options;
   options.message_loop_type = MessageLoop::TYPE_IO;
   io_thread_.reset(new base::Thread("ServiceProcess_IO"));
@@ -26,17 +27,16 @@ bool ServiceProcess::Initialize() {
     Teardown();
     return false;
   }
-  network_change_notifier_thread_ =
-      new ServiceNetworkChangeNotifierThread(io_thread_->message_loop());
-  network_change_notifier_thread_->Initialize();
   return true;
 }
 
 bool ServiceProcess::Teardown() {
-  network_change_notifier_thread_ = NULL;
   io_thread_.reset();
   file_thread_.reset();
   STLDeleteElements(&cloud_print_proxy_list_);
+  // The NetworkChangeNotifier must be destroyed after all other threads that
+  // might use it have been shut down.
+  network_change_notifier_.reset();
   return true;
 }
 
diff --git a/chrome/service/service_process.h b/chrome/service/service_process.h
index 287ecb2..10757fe 100644
--- a/chrome/service/service_process.h
+++ b/chrome/service/service_process.h
@@ -12,7 +12,9 @@
 
 class CloudPrintProxy;
 class JsonPrefStore;
-class ServiceNetworkChangeNotifierThread;
+namespace net {
+class NetworkChangeNotifier;
+}
 
 // The ServiceProcess does not inherit from ChildProcess because this
 // process can live independently of the browser process.
@@ -46,16 +48,12 @@ class ServiceProcess {
     return file_thread_.get();
   }
   CloudPrintProxy* CreateCloudPrintProxy(JsonPrefStore* service_prefs);
-  ServiceNetworkChangeNotifierThread* network_change_notifier_thread() const {
-    return network_change_notifier_thread_.get();
-  }
 
  private:
+  scoped_ptr<net::NetworkChangeNotifier> network_change_notifier_;
   scoped_ptr<base::Thread> io_thread_;
   scoped_ptr<base::Thread> file_thread_;
   std::vector<CloudPrintProxy*> cloud_print_proxy_list_;
-  scoped_refptr<ServiceNetworkChangeNotifierThread>
-      network_change_notifier_thread_;
 
   DISALLOW_COPY_AND_ASSIGN(ServiceProcess);
 };