Stop NetworkChangeNotifierMac from calling a virtual function before the constructor finishes.

NetworkChangeNotifierMac was posting a task to a separate thread in its constructor.  It's possible (although highly unlikely, but happens on buildbots) for the separate thread to start up and run the task (which invokes a virtual function) before the constructor completes (and, more importantly, the subtype's constructor initializes its vtable entries which are NULL / pure virtual in the base class).
The solution is to simply use a Delegate instead.  The Delegate should have been fully constructed (and thus, will not have the vtable initialization race) before being passed into NetworkChangeWatcherMac's constructor.  This also solves the stylistic issue of avoiding multiple inheritance, since NetworkConfigWatcherMac was not strictly an interface.

BUG=53138

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

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

1 files changed, 20 insertions, 14 deletions

diff --git a/net/base/network_config_watcher_mac.cc b/net/base/network_config_watcher_mac.cc
index 8767dff..7a77b35 100644
--- a/net/base/network_config_watcher_mac.cc
+++ b/net/base/network_config_watcher_mac.cc
@@ -15,8 +15,23 @@ DISABLE_RUNNABLE_METHOD_REFCOUNT(net::NetworkConfigWatcherMac);
 
 namespace net {
 
-NetworkConfigWatcherMac::NetworkConfigWatcherMac()
-    : notifier_thread_(new base::Thread("NetworkConfigWatcher")) {
+namespace {
+
+// Called back by OS.  Calls OnNetworkConfigChange().
+void DynamicStoreCallback(SCDynamicStoreRef /* store */,
+                          CFArrayRef changed_keys,
+                          void* config_delegate) {
+  NetworkConfigWatcherMac::Delegate* net_config_delegate =
+      static_cast<NetworkConfigWatcherMac::Delegate*>(config_delegate);
+  net_config_delegate->OnNetworkConfigChange(changed_keys);
+}
+
+}  // namespace
+
+NetworkConfigWatcherMac::NetworkConfigWatcherMac(
+    Delegate* delegate)
+    : notifier_thread_(new base::Thread("NetworkConfigWatcher")),
+      delegate_(delegate) {
   // We create this notifier thread because the notification implementation
   // needs a thread with a CFRunLoop, and there's no guarantee that
   // MessageLoop::current() meets that criterion.
@@ -25,7 +40,8 @@ NetworkConfigWatcherMac::NetworkConfigWatcherMac()
   // TODO(willchan): Look to see if there's a better signal for when it's ok to
   // initialize this, rather than just delaying it by a fixed time.
   const int kNotifierThreadInitializationDelayMS = 1000;
-  notifier_thread_->message_loop()->PostDelayedTask(FROM_HERE,
+  notifier_thread_->message_loop()->PostDelayedTask(
+      FROM_HERE,
       NewRunnableMethod(this, &NetworkConfigWatcherMac::Init),
       kNotifierThreadInitializationDelayMS);
 }
@@ -37,16 +53,6 @@ NetworkConfigWatcherMac::~NetworkConfigWatcherMac() {
   DCHECK(run_loop_source_ == NULL);
 }
 
-// static
-void NetworkConfigWatcherMac::DynamicStoreCallback(
-    SCDynamicStoreRef /* store */,
-    CFArrayRef changed_keys,
-    void* config) {
-  NetworkConfigWatcherMac* net_config =
-      static_cast<NetworkConfigWatcherMac*>(config);
-  net_config->OnNetworkConfigChange(changed_keys);
-}
-
 void NetworkConfigWatcherMac::WillDestroyCurrentMessageLoop() {
   DCHECK(notifier_thread_ != NULL);
   // We can't check the notifier_thread_'s message_loop(), as it's now 0.
@@ -78,7 +84,7 @@ void NetworkConfigWatcherMac::Init() {
                      kCFRunLoopCommonModes);
 
   // Set up notifications for interface and IP address changes.
-  SetDynamicStoreNotificationKeys(store.get());
+  delegate_->SetDynamicStoreNotificationKeys(store.get());
 
   MessageLoop::current()->AddDestructionObserver(this);
 }