Fix potential threading issues in NetworkChangeNotifierAndroid.

NetworkChangeNotifierAndroid's constructor can be called on any thread. It
calls some Java code (still on any thread) initializing a singleton.
This singleton is also accessed from the UI thread on the application side.

While in practice it didn't seem to be a problem it seems safer to perform all
the JNI calls on a single thread (UI thread) to avoid accessing mutable state
from multiple threads.

Another potential issue with the current implementation is that
NetworkChangeNotifierAndroid could be deleted on the network thread while it
was receiving at the same time a notification on the UI thread. That could
have led to a user after free.

This CL addresses these two issues by introducing a Delegate which outlives
NetworkChangeNotifierAndroid and calls JNI methods on the UI thread.


Review URL: https://chromiumcodereview.appspot.com/10979055

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

9 files changed, 443 insertions, 143 deletions

diff --git a/net/android/java/src/org/chromium/net/NetworkChangeNotifier.java b/net/android/java/src/org/chromium/net/NetworkChangeNotifier.java
index a8c21a6..a2deed7 100644
--- a/net/android/java/src/org/chromium/net/NetworkChangeNotifier.java
+++ b/net/android/java/src/org/chromium/net/NetworkChangeNotifier.java
@@ -157,7 +157,7 @@ public class NetworkChangeNotifier {
      */
     void notifyObserversOfConnectionTypeChange(int newConnectionType) {
         if (mNativeChangeNotifier != 0) {
-            nativeNotifyObserversOfConnectionTypeChange(mNativeChangeNotifier, newConnectionType);
+            nativeNotifyConnectionTypeChanged(mNativeChangeNotifier, newConnectionType);
         }
 
         for (ConnectionTypeObserver observer : mConnectionTypeObservers) {
@@ -188,11 +188,10 @@ public class NetworkChangeNotifier {
         return mConnectionTypeObservers.remove(observer);
     }
 
-    @NativeClassQualifiedName("NetworkChangeNotifierAndroid")
-    private native void nativeNotifyObserversOfConnectionTypeChange(
-            int nativePtr, int newConnectionType);
+    @NativeClassQualifiedName("NetworkChangeNotifierDelegateAndroid")
+    private native void nativeNotifyConnectionTypeChanged(int nativePtr, int newConnectionType);
 
-    @NativeClassQualifiedName("NetworkChangeNotifierAndroid")
+    @NativeClassQualifiedName("NetworkChangeNotifierDelegateAndroid")
     private native int nativeGetConnectionType(int nativePtr);
 
     // For testing only.
diff --git a/net/android/network_change_notifier_android.cc b/net/android/network_change_notifier_android.cc
index f055af9..18b3fb1 100644
--- a/net/android/network_change_notifier_android.cc
+++ b/net/android/network_change_notifier_android.cc
@@ -2,83 +2,97 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "net/android/network_change_notifier_android.h"
+////////////////////////////////////////////////////////////////////////////////
+// Threading considerations:
+//
+// This class is designed to meet various threading guarantees starting from the
+// ones imposed by NetworkChangeNotifier:
+// - The notifier can be constructed on any thread.
+// - GetCurrentConnectionType() can be called on any thread.
+//
+// The fact that this implementation of NetworkChangeNotifier is backed by a
+// Java side singleton class (see NetworkChangeNotifier.java) adds another
+// threading constraint:
+// - The calls to the Java side (stateful) object must be performed from a
+//   single thread. This object happens to be a singleton which is used on the
+//   application side on the main thread. Therefore all the method calls from
+//   the native NetworkChangeNotifierAndroid class to its Java counterpart are
+//   performed on the main thread.
+//
+// This leads to a design involving the following native classes:
+// 1) NetworkChangeNotifierFactoryAndroid ('factory')
+// 2) NetworkChangeNotifierDelegateAndroid ('delegate')
+// 3) NetworkChangeNotifierAndroid ('notifier')
+//
+// The factory constructs and owns the delegate. The factory is constructed and
+// destroyed on the main thread which makes it construct and destroy the
+// delegate on the main thread too. This guarantees that the calls to the Java
+// side are performed on the main thread.
+// Note that after the factory's construction, the factory's creation method can
+// be called from any thread since the delegate's construction (performing the
+// JNI calls) already happened on the main thread (when the factory was
+// constructed).
+//
+////////////////////////////////////////////////////////////////////////////////
+// Propagation of network change notifications:
+//
+// When the factory is requested to create a new instance of the notifier, the
+// factory passes the delegate to the notifier (without transferring ownership).
+// Note that there is a one-to-one mapping between the factory and the
+// delegate as explained above. But the factory naturally creates multiple
+// instances of the notifier. That means that there is a one-to-many mapping
+// between delegate and notifier (i.e. a single delegate can be shared by
+// multiple notifiers).
+// At construction the notifier (which is also an observer) subscribes to
+// notifications fired by the delegate. These notifications, received by the
+// delegate (and forwarded to the notifier(s)), are sent by the Java side
+// notifier (see NetworkChangeNotifier.java) and are initiated by the Android
+// platform.
+// Notifications from the Java side always arrive on the main thread. The
+// delegate then forwards these notifications to the threads of each observer
+// (network change notifier). The network change notifier than processes the
+// state change, and notifies each of its observers on their threads.
+//
+// This can also be seen as:
+// Android platform -> NetworkChangeNotifier (Java) ->
+// NetworkChangeNotifierDelegateAndroid -> NetworkChangeNotifierAndroid.
 
-#include "base/android/jni_android.h"
-#include "base/logging.h"
-#include "jni/NetworkChangeNotifier_jni.h"
+#include "net/android/network_change_notifier_android.h"
 
 namespace net {
 
-namespace {
-
-// Returns whether the provided connection type is known.
-bool CheckConnectionType(int connection_type) {
-  switch (connection_type) {
-    case NetworkChangeNotifier::CONNECTION_UNKNOWN:
-    case NetworkChangeNotifier::CONNECTION_ETHERNET:
-    case NetworkChangeNotifier::CONNECTION_WIFI:
-    case NetworkChangeNotifier::CONNECTION_2G:
-    case NetworkChangeNotifier::CONNECTION_3G:
-    case NetworkChangeNotifier::CONNECTION_4G:
-    case NetworkChangeNotifier::CONNECTION_NONE:
-      return true;
-    default:
-      NOTREACHED() << "Unknown connection type received: " << connection_type;
-      return false;
-  }
-}
-
-}  // namespace
-
 NetworkChangeNotifierAndroid::~NetworkChangeNotifierAndroid() {
-  JNIEnv* env = base::android::AttachCurrentThread();
-  Java_NetworkChangeNotifier_destroyInstance(env);
+  delegate_->RemoveObserver(this);
 }
 
-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();
+NetworkChangeNotifier::ConnectionType
+NetworkChangeNotifierAndroid::GetCurrentConnectionType() const {
+  base::AutoLock auto_lock(connection_type_lock_);
+  return connection_type_;
 }
 
-jint NetworkChangeNotifierAndroid::GetConnectionType(JNIEnv* env, jobject obj) {
-  return GetCurrentConnectionType();
+void NetworkChangeNotifierAndroid::OnConnectionTypeChanged(
+    ConnectionType new_connection_type) {
+  SetConnectionType(new_connection_type);
+  NetworkChangeNotifier::NotifyObserversOfConnectionTypeChange();
 }
 
 // static
 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)));
+  return NetworkChangeNotifierDelegateAndroid::Register(env);
 }
 
-void NetworkChangeNotifierAndroid::SetConnectionType(int connection_type) {
-  base::AutoLock auto_lock(lock_);
-  connection_type_ = connection_type;
+NetworkChangeNotifierAndroid::NetworkChangeNotifierAndroid(
+    NetworkChangeNotifierDelegateAndroid* delegate)
+    : delegate_(delegate) {
+  SetConnectionType(NetworkChangeNotifier::CONNECTION_UNKNOWN);
+  delegate_->AddObserver(this);
 }
 
-void NetworkChangeNotifierAndroid::ForceConnectivityState(bool state) {
-  JNIEnv* env = base::android::AttachCurrentThread();
-  Java_NetworkChangeNotifier_forceConnectivityState(env, state);
-}
-
-NetworkChangeNotifier::ConnectionType
-    NetworkChangeNotifierAndroid::GetCurrentConnectionType() const {
-  base::AutoLock auto_lock(lock_);
-  return static_cast<NetworkChangeNotifier::ConnectionType>(connection_type_);
+void NetworkChangeNotifierAndroid::SetConnectionType(
+    ConnectionType new_connection_type) {
+  base::AutoLock auto_lock(connection_type_lock_);
+  connection_type_ = new_connection_type;
 }
 
 }  // namespace net
diff --git a/net/android/network_change_notifier_android.h b/net/android/network_change_notifier_android.h
index e4d8469..fe5c8da 100644
--- a/net/android/network_change_notifier_android.h
+++ b/net/android/network_change_notifier_android.h
@@ -5,24 +5,52 @@
 #ifndef NET_ANDROID_NETWORK_CHANGE_NOTIFIER_ANDROID_H_
 #define NET_ANDROID_NETWORK_CHANGE_NOTIFIER_ANDROID_H_
 
-#include "base/android/scoped_java_ref.h"
+#include "base/android/jni_android.h"
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/synchronization/lock.h"
+#include "net/android/network_change_notifier_delegate_android.h"
 #include "net/base/network_change_notifier.h"
 
 namespace net {
 
 class NetworkChangeNotifierAndroidTest;
-
-class NET_EXPORT NetworkChangeNotifierAndroid : public NetworkChangeNotifier {
+class NetworkChangeNotifierFactoryAndroid;
+
+// NetworkChangeNotifierAndroid observes network events from the Android
+// notification system and forwards them to observers.
+//
+// The implementation is complicated by the differing lifetime and thread
+// affinity requirements of Android notifications and of NetworkChangeNotifier.
+//
+// High-level overview:
+// NetworkChangeNotifier.java - Receives notifications from Android system, and
+// notifies native code via JNI (on the main application thread).
+// NetworkChangeNotifierDelegateAndroid ('Delegate') - Listens for notifications
+//   sent via JNI on the main application thread, and forwards them to observers
+//   on their threads. Owned by Factory, lives exclusively on main application
+//   thread.
+// NetworkChangeNotifierFactoryAndroid ('Factory') - Creates the Delegate on the
+//   main thread to receive JNI events, and vends Notifiers. Lives exclusively
+//   on main application thread, and outlives all other classes.
+// NetworkChangeNotifierAndroid ('Notifier') - Receives event notifications from
+//   the Delegate. Processes and forwards these events to the
+//   NetworkChangeNotifier observers on their threads. May live on any thread
+//   and be called by any thread.
+//
+// For more details, see the implementation file.
+class NetworkChangeNotifierAndroid
+    : public NetworkChangeNotifier,
+      public NetworkChangeNotifierDelegateAndroid::Observer {
  public:
   virtual ~NetworkChangeNotifierAndroid();
 
-  // Called from Java on the UI thread.
-  void NotifyObserversOfConnectionTypeChange(
-      JNIEnv* env, jobject obj, jint new_connection_type);
-  jint GetConnectionType(JNIEnv* env, jobject obj);
+  // NetworkChangeNotifier:
+  virtual ConnectionType GetCurrentConnectionType() const OVERRIDE;
+
+  // NetworkChangeNotifierDelegateAndroid::Observer:
+  virtual void OnConnectionTypeChanged(
+      ConnectionType new_connection_type) OVERRIDE;
 
   static bool Register(JNIEnv* env);
 
@@ -30,22 +58,14 @@ class NET_EXPORT NetworkChangeNotifierAndroid : public NetworkChangeNotifier {
   friend class NetworkChangeNotifierAndroidTest;
   friend class NetworkChangeNotifierFactoryAndroid;
 
-  NetworkChangeNotifierAndroid();
+  explicit NetworkChangeNotifierAndroid(
+      NetworkChangeNotifierDelegateAndroid* delegate);
 
-  void SetConnectionType(int connection_type);
-
-  void ForceConnectivityState(bool state);
-
-  // NetworkChangeNotifier:
-  virtual ConnectionType GetCurrentConnectionType() const OVERRIDE;
+  void SetConnectionType(ConnectionType new_connection_type);
 
-  base::android::ScopedJavaGlobalRef<jobject> java_network_change_notifier_;
-  // TODO(pliard): http://crbug.com/150867. Use an atomic integer for the
-  // connection type without the lock once a non-subtle atomic integer is
-  // available under base/. That might never happen though.
-  mutable base::Lock lock_;  // Protects the state below.
-  // Written from the UI thread, read from any thread.
-  int connection_type_;
+  NetworkChangeNotifierDelegateAndroid* const delegate_;
+  mutable base::Lock connection_type_lock_;  // Protects the state below.
+  ConnectionType connection_type_;
 
   DISALLOW_COPY_AND_ASSIGN(NetworkChangeNotifierAndroid);
 };
diff --git a/net/android/network_change_notifier_android_unittest.cc b/net/android/network_change_notifier_android_unittest.cc
index 9b86470..b66d647 100644
--- a/net/android/network_change_notifier_android_unittest.cc
+++ b/net/android/network_change_notifier_android_unittest.cc
@@ -2,10 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "net/android/network_change_notifier_android.h"
+// See network_change_notifier_android.h for design explanations.
 
+#include "base/basictypes.h"
+#include "base/bind.h"
+#include "base/callback.h"
+#include "base/compiler_specific.h"
 #include "base/message_loop.h"
-#include "net/android/network_change_notifier_factory_android.h"
+#include "net/android/network_change_notifier_android.h"
+#include "net/android/network_change_notifier_delegate_android.h"
 #include "net/base/network_change_notifier.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -13,22 +18,26 @@ namespace net {
 
 namespace {
 
-class TestConnectionTypeObserver :
-    public NetworkChangeNotifier::ConnectionTypeObserver {
+// Template used to generate both the NetworkChangeNotifierDelegateAndroid and
+// NetworkChangeNotifier::ConnectionTypeObserver implementations which have the
+// same interface.
+template <typename BaseObserver>
+class ObserverImpl : public BaseObserver {
  public:
-  TestConnectionTypeObserver() :
-      times_connection_type_has_been_changed_(0),
-      current_connection_(NetworkChangeNotifier::CONNECTION_UNKNOWN) {
+  ObserverImpl()
+      : times_connection_type_changed_(0),
+        current_connection_(NetworkChangeNotifier::CONNECTION_UNKNOWN) {
   }
 
-  void OnConnectionTypeChanged(
-      NetworkChangeNotifier::ConnectionType type) {
-    times_connection_type_has_been_changed_++;
+  // BaseObserver:
+  virtual void OnConnectionTypeChanged(
+      NetworkChangeNotifier::ConnectionType type) OVERRIDE {
+    times_connection_type_changed_++;
     current_connection_ = type;
   }
 
-  int times_connection_type_has_been_changed() const {
-    return times_connection_type_has_been_changed_;
+  int times_connection_type_changed() const {
+    return times_connection_type_changed_;
   }
 
   NetworkChangeNotifier::ConnectionType current_connection() const {
@@ -36,69 +45,143 @@ class TestConnectionTypeObserver :
   }
 
  private:
-  int times_connection_type_has_been_changed_;
+  int times_connection_type_changed_;
   NetworkChangeNotifier::ConnectionType current_connection_;
 
-  DISALLOW_COPY_AND_ASSIGN(TestConnectionTypeObserver);
+  DISALLOW_COPY_AND_ASSIGN(ObserverImpl);
 };
 
 }  // namespace
 
-class NetworkChangeNotifierAndroidTest : public testing::Test {
- public:
-  NetworkChangeNotifierAndroidTest() : connection_type_observer_(NULL) {
+class BaseNetworkChangeNotifierAndroidTest : public testing::Test {
+ protected:
+  typedef NetworkChangeNotifier::ConnectionType ConnectionType;
+
+  virtual ~BaseNetworkChangeNotifierAndroidTest() {}
+
+  void RunTest(
+      const base::Callback<int(void)>& times_connection_type_changed_callback,
+      const base::Callback<ConnectionType(void)>&  connection_type_getter) {
+    EXPECT_EQ(0, times_connection_type_changed_callback.Run());
+    EXPECT_EQ(NetworkChangeNotifier::CONNECTION_UNKNOWN,
+              connection_type_getter.Run());
+
+    ForceConnectivityState(NetworkChangeNotifierDelegateAndroid::OFFLINE);
+    EXPECT_EQ(1, times_connection_type_changed_callback.Run());
+    EXPECT_EQ(NetworkChangeNotifier::CONNECTION_NONE,
+              connection_type_getter.Run());
+
+    ForceConnectivityState(NetworkChangeNotifierDelegateAndroid::OFFLINE);
+    EXPECT_EQ(1, times_connection_type_changed_callback.Run());
+    EXPECT_EQ(NetworkChangeNotifier::CONNECTION_NONE,
+              connection_type_getter.Run());
+
+    ForceConnectivityState(NetworkChangeNotifierDelegateAndroid::ONLINE);
+    EXPECT_EQ(2, times_connection_type_changed_callback.Run());
+    EXPECT_EQ(NetworkChangeNotifier::CONNECTION_UNKNOWN,
+              connection_type_getter.Run());
   }
 
-  void ForceConnectivityState(bool state) {
-    notifier_->ForceConnectivityState(state);
+  void ForceConnectivityState(
+      NetworkChangeNotifierDelegateAndroid::ConnectivityState state) {
+    delegate_.ForceConnectivityState(state);
+    // Note that this is needed because ObserverListThreadSafe uses PostTask().
+    MessageLoop::current()->RunUntilIdle();
   }
 
-  const TestConnectionTypeObserver* observer() const {
-    return connection_type_observer_.get();
-  }
+  NetworkChangeNotifierDelegateAndroid delegate_;
+};
 
+class NetworkChangeNotifierDelegateAndroidTest
+    : public BaseNetworkChangeNotifierAndroidTest {
  protected:
-  virtual void SetUp() {
-    notifier_.reset(new NetworkChangeNotifierAndroid());
-    connection_type_observer_.reset(new TestConnectionTypeObserver());
-    NetworkChangeNotifier::AddConnectionTypeObserver(
-        connection_type_observer_.get());
-  }
+  typedef ObserverImpl<
+      NetworkChangeNotifierDelegateAndroid::Observer> TestDelegateObserver;
 
- private:
-  NetworkChangeNotifier::DisableForTest disable_for_test_;
-  scoped_ptr<NetworkChangeNotifierAndroid> notifier_;
-  scoped_ptr<TestConnectionTypeObserver> connection_type_observer_;
-};
+  NetworkChangeNotifierDelegateAndroidTest() {
+    delegate_.AddObserver(&delegate_observer_);
+    delegate_.AddObserver(&other_delegate_observer_);
+  }
 
+  virtual ~NetworkChangeNotifierDelegateAndroidTest() {
+    delegate_.RemoveObserver(&delegate_observer_);
+    delegate_.RemoveObserver(&other_delegate_observer_);
+  }
 
-TEST_F(NetworkChangeNotifierAndroidTest, ObserverNotified) {
-  // This test exercises JNI calls between the native-side
-  // NetworkChangeNotifierAndroid and java-side NetworkChangeNotifier.
-  EXPECT_EQ(0, observer()->times_connection_type_has_been_changed());
-  EXPECT_EQ(NetworkChangeNotifier::CONNECTION_UNKNOWN,
-      observer()->current_connection());
+  TestDelegateObserver delegate_observer_;
+  TestDelegateObserver other_delegate_observer_;
+};
 
-  ForceConnectivityState(false);
-  MessageLoop::current()->RunUntilIdle();
+// Tests that the NetworkChangeNotifierDelegateAndroid's observers are notified.
+// A testing-only observer is used here for testing. In production the
+// delegate's observers are instances of NetworkChangeNotifierAndroid.
+TEST_F(NetworkChangeNotifierDelegateAndroidTest, DelegateObserverNotified) {
+  // Test the logic with a single observer.
+  RunTest(
+      base::Bind(
+          &TestDelegateObserver::times_connection_type_changed,
+          base::Unretained(&delegate_observer_)),
+      base::Bind(
+          &TestDelegateObserver::current_connection,
+          base::Unretained(&delegate_observer_)));
+  // Check that *all* the observers are notified. Both observers should have the
+  // same state.
+  EXPECT_EQ(delegate_observer_.times_connection_type_changed(),
+            other_delegate_observer_.times_connection_type_changed());
+  EXPECT_EQ(delegate_observer_.current_connection(),
+            other_delegate_observer_.current_connection());
+}
 
-  EXPECT_EQ(1, observer()->times_connection_type_has_been_changed());
-  EXPECT_EQ(NetworkChangeNotifier::CONNECTION_NONE,
-      observer()->current_connection());
+class NetworkChangeNotifierAndroidTest
+    : public BaseNetworkChangeNotifierAndroidTest {
+ protected:
+  typedef ObserverImpl<
+      NetworkChangeNotifier::ConnectionTypeObserver> TestConnectionTypeObserver;
 
-  ForceConnectivityState(false);
-  MessageLoop::current()->RunUntilIdle();
+  NetworkChangeNotifierAndroidTest() : notifier_(&delegate_) {
+    NetworkChangeNotifier::AddConnectionTypeObserver(
+        &connection_type_observer_);
+    NetworkChangeNotifier::AddConnectionTypeObserver(
+        &other_connection_type_observer_);
+  }
 
-  EXPECT_EQ(1, observer()->times_connection_type_has_been_changed());
-  EXPECT_EQ(NetworkChangeNotifier::CONNECTION_NONE,
-      observer()->current_connection());
+  TestConnectionTypeObserver connection_type_observer_;
+  TestConnectionTypeObserver other_connection_type_observer_;
+  NetworkChangeNotifier::DisableForTest disable_for_test_;
+  NetworkChangeNotifierAndroid notifier_;
+};
 
-  ForceConnectivityState(true);
-  MessageLoop::current()->RunUntilIdle();
+// When a NetworkChangeNotifierAndroid is observing a
+// NetworkChangeNotifierDelegateAndroid for network state changes, and the
+// NetworkChangeNotifierDelegateAndroid's connectivity state changes, the
+// NetworkChangeNotifierAndroid should reflect that state.
+TEST_F(NetworkChangeNotifierAndroidTest,
+       NotificationsSentToNetworkChangeNotifierAndroid) {
+  RunTest(
+      base::Bind(
+          &TestConnectionTypeObserver::times_connection_type_changed,
+          base::Unretained(&connection_type_observer_)),
+      base::Bind(
+          &NetworkChangeNotifierAndroid::GetCurrentConnectionType,
+          base::Unretained(&notifier_)));
+}
 
-  EXPECT_EQ(2, observer()->times_connection_type_has_been_changed());
-  EXPECT_EQ(NetworkChangeNotifier::CONNECTION_UNKNOWN,
-      observer()->current_connection());
+// When a NetworkChangeNotifierAndroid's connection state changes, it should
+// notify all of its observers.
+TEST_F(NetworkChangeNotifierAndroidTest,
+       NotificationsSentToClientsOfNetworkChangeNotifier) {
+  RunTest(
+      base::Bind(
+          &TestConnectionTypeObserver::times_connection_type_changed,
+          base::Unretained(&connection_type_observer_)),
+      base::Bind(
+          &TestConnectionTypeObserver::current_connection,
+          base::Unretained(&connection_type_observer_)));
+  // Check that *all* the observers are notified.
+  EXPECT_EQ(connection_type_observer_.times_connection_type_changed(),
+            other_connection_type_observer_.times_connection_type_changed());
+  EXPECT_EQ(connection_type_observer_.current_connection(),
+            other_connection_type_observer_.current_connection());
 }
 
 }  // namespace net
diff --git a/net/android/network_change_notifier_delegate_android.cc b/net/android/network_change_notifier_delegate_android.cc
new file mode 100644
index 0000000..91b0cb0
--- /dev/null
+++ b/net/android/network_change_notifier_delegate_android.cc
@@ -0,0 +1,88 @@
+// Copyright (c) 2012 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 "net/android/network_change_notifier_delegate_android.h"
+
+#include "base/logging.h"
+#include "jni/NetworkChangeNotifier_jni.h"
+
+namespace net {
+
+namespace {
+
+// Returns whether the provided connection type is known.
+bool CheckConnectionType(int connection_type) {
+  switch (connection_type) {
+    case NetworkChangeNotifier::CONNECTION_UNKNOWN:
+    case NetworkChangeNotifier::CONNECTION_ETHERNET:
+    case NetworkChangeNotifier::CONNECTION_WIFI:
+    case NetworkChangeNotifier::CONNECTION_2G:
+    case NetworkChangeNotifier::CONNECTION_3G:
+    case NetworkChangeNotifier::CONNECTION_4G:
+    case NetworkChangeNotifier::CONNECTION_NONE:
+      return true;
+    default:
+      NOTREACHED() << "Unknown connection type received: " << connection_type;
+      return false;
+  }
+}
+
+}  // namespace
+
+NetworkChangeNotifierDelegateAndroid::NetworkChangeNotifierDelegateAndroid()
+    : observers_(new ObserverListThreadSafe<Observer>()) {
+  java_network_change_notifier_.Reset(
+      Java_NetworkChangeNotifier_createInstance(
+          base::android::AttachCurrentThread(),
+          base::android::GetApplicationContext(),
+          reinterpret_cast<jint>(this)));
+}
+
+NetworkChangeNotifierDelegateAndroid::~NetworkChangeNotifierDelegateAndroid() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  observers_->AssertEmpty();
+  JNIEnv* env = base::android::AttachCurrentThread();
+  Java_NetworkChangeNotifier_destroyInstance(env);
+}
+
+void NetworkChangeNotifierDelegateAndroid::NotifyConnectionTypeChanged(
+    JNIEnv* env,
+    jobject obj,
+    jint new_connection_type) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  connection_type_ = CheckConnectionType(new_connection_type) ?
+      static_cast<ConnectionType>(new_connection_type) :
+      NetworkChangeNotifier::CONNECTION_UNKNOWN;
+  observers_->Notify(&Observer::OnConnectionTypeChanged, connection_type_);
+}
+
+jint NetworkChangeNotifierDelegateAndroid::GetConnectionType(JNIEnv*,
+                                                             jobject) const {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  return connection_type_;
+}
+
+void NetworkChangeNotifierDelegateAndroid::AddObserver(
+    Observer* observer) {
+  observers_->AddObserver(observer);
+}
+
+void NetworkChangeNotifierDelegateAndroid::RemoveObserver(
+    Observer* observer) {
+  observers_->RemoveObserver(observer);
+}
+
+void NetworkChangeNotifierDelegateAndroid::ForceConnectivityState(
+    ConnectivityState state) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  JNIEnv* env = base::android::AttachCurrentThread();
+  Java_NetworkChangeNotifier_forceConnectivityState(env, state == ONLINE);
+}
+
+// static
+bool NetworkChangeNotifierDelegateAndroid::Register(JNIEnv* env) {
+  return RegisterNativesImpl(env);
+}
+
+}  // namespace net
diff --git a/net/android/network_change_notifier_delegate_android.h b/net/android/network_change_notifier_delegate_android.h
new file mode 100644
index 0000000..f1d8127
--- /dev/null
+++ b/net/android/network_change_notifier_delegate_android.h
@@ -0,0 +1,80 @@
+// Copyright (c) 2012 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 NET_ANDROID_NETWORK_CHANGE_NOTIFIER_DELEGATE_ANDROID_H_
+#define NET_ANDROID_NETWORK_CHANGE_NOTIFIER_DELEGATE_ANDROID_H_
+
+#include "base/android/jni_android.h"
+#include "base/basictypes.h"
+#include "base/memory/ref_counted.h"
+#include "base/observer_list_threadsafe.h"
+#include "base/threading/thread_checker.h"
+#include "net/base/network_change_notifier.h"
+
+namespace net {
+
+// Delegate used to thread-safely notify NetworkChangeNotifierAndroid whenever a
+// network connection change notification is signaled by the Java side (on the
+// JNI thread).
+// All the methods exposed below must be called exclusively on the JNI thread
+// unless otherwise stated (e.g. AddObserver()/RemoveObserver()).
+class NetworkChangeNotifierDelegateAndroid {
+ public:
+  enum ConnectivityState {
+    OFFLINE,
+    ONLINE,
+  };
+
+  // Observer interface implemented by NetworkChangeNotifierAndroid which
+  // subscribes to network change notifications fired by the delegate (and
+  // initiated by the Java side).
+  class Observer {
+   public:
+    virtual ~Observer() {}
+
+    // Updates the current connection type.
+    virtual void OnConnectionTypeChanged(
+        NetworkChangeNotifier::ConnectionType new_connection_type) = 0;
+  };
+
+  NetworkChangeNotifierDelegateAndroid();
+  ~NetworkChangeNotifierDelegateAndroid();
+
+  // Called from NetworkChangeNotifierAndroid.java on the JNI thread whenever
+  // the connection type changes. This updates the current connection type seen
+  // by this class and forwards the notification to the observers that
+  // subscribed through AddObserver().
+  void NotifyConnectionTypeChanged(JNIEnv* env,
+                                   jobject obj,
+                                   jint new_connection_type);
+  jint GetConnectionType(JNIEnv* env, jobject obj) const;
+
+  // These methods can be called on any thread. Note that the provided observer
+  // will be notified on the thread AddObserver() is called on.
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
+  // Exposed for testing.
+  void ForceConnectivityState(ConnectivityState state);
+
+  // Initializes JNI bindings.
+  static bool Register(JNIEnv* env);
+
+ private:
+  friend class NetworkChangeNotifierDelegateAndroidTest;
+
+  typedef NetworkChangeNotifier::ConnectionType ConnectionType;
+
+  base::ThreadChecker thread_checker_;
+  scoped_refptr<ObserverListThreadSafe<Observer> > observers_;
+  scoped_refptr<base::SingleThreadTaskRunner> jni_task_runner_;
+  base::android::ScopedJavaGlobalRef<jobject> java_network_change_notifier_;
+  ConnectionType connection_type_;
+
+  DISALLOW_COPY_AND_ASSIGN(NetworkChangeNotifierDelegateAndroid);
+};
+
+}  // namespace net
+
+#endif  // NET_ANDROID_NETWORK_CHANGE_NOTIFIER_DELEGATE_H_
diff --git a/net/android/network_change_notifier_factory_android.cc b/net/android/network_change_notifier_factory_android.cc
index a28a69e..64b9ca2 100644
--- a/net/android/network_change_notifier_factory_android.cc
+++ b/net/android/network_change_notifier_factory_android.cc
@@ -5,6 +5,7 @@
 #include "net/android/network_change_notifier_factory_android.h"
 
 #include "net/android/network_change_notifier_android.h"
+#include "net/android/network_change_notifier_delegate_android.h"
 
 namespace net {
 
@@ -13,7 +14,7 @@ NetworkChangeNotifierFactoryAndroid::NetworkChangeNotifierFactoryAndroid() {}
 NetworkChangeNotifierFactoryAndroid::~NetworkChangeNotifierFactoryAndroid() {}
 
 NetworkChangeNotifier* NetworkChangeNotifierFactoryAndroid::CreateInstance() {
-  return new NetworkChangeNotifierAndroid();
+  return new NetworkChangeNotifierAndroid(&delegate_);
 }
 
 }  // namespace net
diff --git a/net/android/network_change_notifier_factory_android.h b/net/android/network_change_notifier_factory_android.h
index 8c5e414..e71ec27 100644
--- a/net/android/network_change_notifier_factory_android.h
+++ b/net/android/network_change_notifier_factory_android.h
@@ -5,24 +5,37 @@
 #ifndef NET_ANDROID_NETWORK_CHANGE_NOTIFIER_FACTORY_ANDROID_H_
 #define NET_ANDROID_NETWORK_CHANGE_NOTIFIER_FACTORY_ANDROID_H_
 
+#include "base/basictypes.h"
 #include "base/compiler_specific.h"
+#include "net/android/network_change_notifier_delegate_android.h"
 #include "net/base/net_export.h"
 #include "net/base/network_change_notifier_factory.h"
 
 namespace net {
 
 class NetworkChangeNotifier;
+class NetworkChangeNotifierDelegateAndroid;
 
 // NetworkChangeNotifierFactory creates Android-specific specialization of
-// NetworkChangeNotifier.
+// NetworkChangeNotifier. See network_change_notifier_android.h for more
+// details.
 class NET_EXPORT NetworkChangeNotifierFactoryAndroid :
     public NetworkChangeNotifierFactory {
  public:
+  // Must be called on the JNI thread.
   NetworkChangeNotifierFactoryAndroid();
+
+  // Must be called on the JNI thread.
   virtual ~NetworkChangeNotifierFactoryAndroid();
 
-  // Overrides of NetworkChangeNotifierFactory.
+  // NetworkChangeNotifierFactory:
   virtual NetworkChangeNotifier* CreateInstance() OVERRIDE;
+
+ private:
+  // Delegate passed to the instances created by this class.
+  NetworkChangeNotifierDelegateAndroid delegate_;
+
+  DISALLOW_COPY_AND_ASSIGN(NetworkChangeNotifierFactoryAndroid);
 };
 
 }  // namespace net
diff --git a/net/net.gyp b/net/net.gyp
index b5ccff3..795f627 100644
--- a/net/net.gyp
+++ b/net/net.gyp
@@ -60,6 +60,8 @@
         'android/net_jni_registrar.h',
         'android/network_change_notifier_android.cc',
         'android/network_change_notifier_android.h',
+        'android/network_change_notifier_delegate_android.cc',
+        'android/network_change_notifier_delegate_android.h',
         'android/network_change_notifier_factory_android.cc',
         'android/network_change_notifier_factory_android.h',
         'android/network_library.cc',