Fix race condition in NetworkChangeNotifier on Android.

net/android/network_change_notifier_android.h

Index: net/android/network_change_notifier_android.h
diff --git a/net/android/network_change_notifier_android.h b/net/android/network_change_notifier_android.h
index 3c71dc5995bb31fdfacf88b46fc8e37ce2cb8106..dacf84b3f22a4bdd7eb3837de73f2be881280a5e 100644
--- a/net/android/network_change_notifier_android.h
+++ b/net/android/network_change_notifier_android.h
@@ -6,6 +6,7 @@
 #define NET_ANDROID_NETWORK_CHANGE_NOTIFIER_ANDROID_H_
 
 #include "base/android/scoped_java_ref.h"
+#include "base/atomicops.h"
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "net/base/network_change_notifier.h"
@@ -15,14 +16,19 @@ namespace android {
 
 class NetworkChangeNotifier : public net::NetworkChangeNotifier {
  public:
-  NetworkChangeNotifier();
   virtual ~NetworkChangeNotifier();
 
-  void NotifyObservers(JNIEnv* env, jobject obj);
+  // Called from Java on the UI thread.
+  void NotifyObservers(JNIEnv* env, jobject obj, int new_connection_type);
+  jint GetConnectionType(JNIEnv* env, jobject obj);
 
   static bool Register(JNIEnv* env);
 
  private:
+  friend class NetworkChangeNotifierFactory;
+
+  NetworkChangeNotifier();
+
   void CreateJavaObject(JNIEnv* env);
 
   // NetworkChangeNotifier:
@@ -30,6 +36,8 @@ class NetworkChangeNotifier : public net::NetworkChangeNotifier {
       GetCurrentConnectionType() const OVERRIDE;
 
   base::android::ScopedJavaGlobalRef<jobject> java_network_change_notifier_;
+  // Wrote from the UI thread, read from any thread.
+  base::subtle::AtomicWord connection_type_;

[szym, 2012/09/14 18:25:23]

In an offline conversation with pauljensen@ we pondered if you could wrap all
the "subtlety" in a class, like ThreadsafeWord { Read(), Write(...) } Probably
best to keep it in this source file for now.

That said, I wonder if this memory barrier is actually buying you anything.
There are two categories of GetConnectionType callers: OnConnectionTypeChanged,
where the value is passed via callback, and NCN::IsOffline. This CL does not
affect the first category, and I'm not sure if makes a difference for the second
either.

[Philippe, 2012/09/19 17:35:14]

I used a Lock with a TODO instead after Ryan and Paul's replies.
I think the barrier/mutex is needed regardless of the clients. The issue was
that there was no guarantee that the value returned by
GetCurrentConnectionType() was up-to-date (i.e. the one just set from the UI
thread) on multi-core ARM. That might not happen in practice but it seemed
pretty dangerous. I might be misunderstanding something though.

 
   DISALLOW_COPY_AND_ASSIGN(NetworkChangeNotifier);
 };