Fix potential threading issues in NetworkChangeNotifierAndroid.

net/android/network_change_notifier_android.cc

Index: net/android/network_change_notifier_android.cc
diff --git a/net/android/network_change_notifier_android.cc b/net/android/network_change_notifier_android.cc
index f055af971db4a41bf54e6d386da5c2dc4a2d873e..6fd830c33a47dc3b14857946e8a518f8ab6260d2 100644
--- a/net/android/network_change_notifier_android.cc
+++ b/net/android/network_change_notifier_android.cc
@@ -5,7 +5,10 @@
 #include "net/android/network_change_notifier_android.h"
 
 #include "base/android/jni_android.h"
+#include "base/basictypes.h"
 #include "base/logging.h"
+#include "base/synchronization/lock.h"
+#include "base/task_runner.h"
 #include "jni/NetworkChangeNotifier_jni.h"
 
 namespace net {
@@ -31,23 +34,124 @@ bool CheckConnectionType(int connection_type) {
 
 }  // namespace
 
-NetworkChangeNotifierAndroid::~NetworkChangeNotifierAndroid() {
-  JNIEnv* env = base::android::AttachCurrentThread();
-  Java_NetworkChangeNotifier_destroyInstance(env);
-}
+class NetworkChangeNotifierAndroid::DelegateImpl
+    : public NetworkChangeNotifierAndroid::Delegate {
+ public:
+  DelegateImpl(base::TaskRunner* ui_task_runner)
+      : ui_task_runner_(ui_task_runner) {
+    DCHECK(ui_task_runner);
+    SetDeletionFlag(false);
+    SetConnectionType(NetworkChangeNotifier::CONNECTION_UNKNOWN);
+  }
 
-void NetworkChangeNotifierAndroid::NotifyObserversOfConnectionTypeChange(
-    JNIEnv* env,
-    jobject obj,
-    jint new_connection_type) {
-  int connection_type = CheckConnectionType(new_connection_type) ?
-      new_connection_type : CONNECTION_UNKNOWN;
-  SetConnectionType(connection_type);
-  NetworkChangeNotifier::NotifyObserversOfConnectionTypeChange();
-}
+  void Init() {
+    if (IsRunningOnUIThread()) {
+      CreateJavaInstanceOnUIThread();
+    } else {
+      ui_task_runner_->PostTask(
+          FROM_HERE,
+          base::Bind(&DelegateImpl::CreateJavaInstanceOnUIThread,
+                     base::Unretained(this)));
+    }
+  }
+
+  void SetConnectionType(ConnectionType connection_type) {
+    base::AutoLock auto_lock(lock_);
+    connection_type_ = connection_type;
+  }
+
+  void Destroy() {
+    if (IsRunningOnUIThread()) {
+      DestroyOnUIThread();
+    } else {
+      // Note that the deletion flag is only set on a non-UI thread.
+      SetDeletionFlag(true);
+      ui_task_runner_->PostTask(
+          FROM_HERE,
+          base::Bind(&DelegateImpl::DestroyOnUIThread, base::Unretained(this)));
+    }
+  }
+
+  virtual ConnectionType GetConnectionType() const {
+    base::AutoLock auto_lock(lock_);
+    return connection_type_;
+  }
+
+  // Delegate:
+  virtual void NotifyObserversOfConnectionTypeChange(
+      JNIEnv* env, jobject obj, jint new_connection_type) OVERRIDE {
+    DCHECK(IsRunningOnUIThread());
+    ConnectionType connection_type = CheckConnectionType(new_connection_type) ?
+        static_cast<ConnectionType>(new_connection_type) : CONNECTION_UNKNOWN;
+    SetConnectionType(connection_type);
+    base::AutoLock auto_lock(delete_lock_);
+    if (GetDeletionFlag())
+      // NetworkChangeNotifier's destruction has started on a non-UI thread.
+      // Omitting the return statement below could lead to a use after free in
+      // NotifyObservers() below.
+      return;
+    NetworkChangeNotifierAndroid::NotifyObservers();
+  }
+
+  virtual jint GetConnectionType(JNIEnv*, jobject) const OVERRIDE {
+    return GetConnectionType();
+  }
+
+ private:
+  virtual ~DelegateImpl() {}
 
-jint NetworkChangeNotifierAndroid::GetConnectionType(JNIEnv* env, jobject obj) {
-  return GetCurrentConnectionType();
+  void CreateJavaInstanceOnUIThread() {
+    DCHECK(IsRunningOnUIThread());
+    JNIEnv* env = base::android::AttachCurrentThread();
+    java_network_change_notifier_.Reset(
+        Java_NetworkChangeNotifier_createInstance(
+            env,
+            base::android::GetApplicationContext(),
+            reinterpret_cast<jint>(this)));
+  }
+
+  void DestroyOnUIThread() {
+    DCHECK(IsRunningOnUIThread());
+    JNIEnv* env = base::android::AttachCurrentThread();
+    Java_NetworkChangeNotifier_destroyInstance(env);
+    delete this;
+  }
+
+  bool IsRunningOnUIThread() const {
+    return ui_task_runner_->RunsTasksOnCurrentThread();
+  }
+
+  void SetDeletionFlag(bool value) {
+    base::AutoLock auto_lock(delete_lock_);
+    delete_started_on_non_ui_thread_ = value;
+  }
+
+  bool GetDeletionFlag() {
+    base::AutoLock auto_lock(delete_lock_);
+    return delete_started_on_non_ui_thread_;
+  }
+
+  base::TaskRunner* const ui_task_runner_;
+  base::android::ScopedJavaGlobalRef<jobject> java_network_change_notifier_;
+
+  mutable base::Lock lock_;  // Protects the state below.
+  // Written from the UI thread, read from any thread.
+  ConnectionType connection_type_;
+
+  base::Lock delete_lock_;  // Protects the state below.
+  // Tells whether NetworkChangeNotifierAndroid's deletion has started on a
+  // thread other than the UI thread. This is used to handle the case where we
+  // receive a notification on the UI thread from the Java side after
+  // NetworkChangeNotifierAndroid was deleted on a non-UI thread (e.g. network
+  // thread).
+  // Written from a non-UI thread, read from the UI thread.
+  bool delete_started_on_non_ui_thread_;
+
+  DISALLOW_COPY_AND_ASSIGN(DelegateImpl);
+};
+
+NetworkChangeNotifierAndroid::~NetworkChangeNotifierAndroid() {
+  delegate_->Destroy();
 }
 
 // static
@@ -55,19 +159,15 @@ bool NetworkChangeNotifierAndroid::Register(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
 
-NetworkChangeNotifierAndroid::NetworkChangeNotifierAndroid() {
-  SetConnectionType(CONNECTION_UNKNOWN);
-  JNIEnv* env = base::android::AttachCurrentThread();
-  java_network_change_notifier_.Reset(
-      Java_NetworkChangeNotifier_createInstance(
-          env,
-          base::android::GetApplicationContext(),
-          reinterpret_cast<jint>(this)));
+NetworkChangeNotifierAndroid::NetworkChangeNotifierAndroid(
+    base::TaskRunner* ui_task_runner)
+    : delegate_(new DelegateImpl(ui_task_runner)) {
+  delegate_->Init();
 }
 
-void NetworkChangeNotifierAndroid::SetConnectionType(int connection_type) {
-  base::AutoLock auto_lock(lock_);
-  connection_type_ = connection_type;
+// static
+void NetworkChangeNotifierAndroid::NotifyObservers() {
+  NetworkChangeNotifier::NotifyObserversOfConnectionTypeChange();
 }
 
 void NetworkChangeNotifierAndroid::ForceConnectivityState(bool state) {
@@ -77,8 +177,7 @@ void NetworkChangeNotifierAndroid::ForceConnectivityState(bool state) {
 
 NetworkChangeNotifier::ConnectionType
     NetworkChangeNotifierAndroid::GetCurrentConnectionType() const {
-  base::AutoLock auto_lock(lock_);
-  return static_cast<NetworkChangeNotifier::ConnectionType>(connection_type_);
+  return delegate_->GetConnectionType();
 }
 
 }  // namespace net