[NetworkChangeNotifier] Notify max bandwidth changed on Android when …

…connection type changes but the max bandwidth doesn't.

Most of the CL is me going nutso with tests.

BUG=576315

Review URL: https://codereview.chromium.org/1580103002

Cr-Commit-Position: refs/heads/master@{#369816}

net/android/java/src/org/chromium/net/NetworkChangeNotifier.java

@@ -7,6 +7,7 @@
 import android.content.Context;
 
 import org.chromium.base.ObserverList;
+import org.chromium.base.VisibleForTesting;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.base.annotations.NativeClassQualifiedName;
@@ -39,10 +40,12 @@ public interface ConnectionTypeObserver {
     private NetworkChangeNotifierAutoDetect mAutoDetector;
     private int mCurrentConnectionType = ConnectionType.CONNECTION_UNKNOWN;
     private double mCurrentMaxBandwidth = Double.POSITIVE_INFINITY;
+    private int mMaxBandwidthConnectionType = mCurrentConnectionType;
 
     private static NetworkChangeNotifier sInstance;
 
-    private NetworkChangeNotifier(Context context) {
+    @VisibleForTesting
+    protected NetworkChangeNotifier(Context context) {
         mContext = context.getApplicationContext();
         mNativeChangeNotifiers = new ArrayList<Long>();
         mConnectionTypeObservers = new ObserverList<ConnectionTypeObserver>();
@@ -63,8 +66,8 @@ public static boolean isInitialized() {
         return sInstance != null;
     }
 
-    static void resetInstanceForTests(Context context) {
-        sInstance = new NetworkChangeNotifier(context);
+    static void resetInstanceForTests(NetworkChangeNotifier notifier) {
+        sInstance = notifier;
     }
 
     @CalledByNative
@@ -272,14 +275,25 @@ public static void fakeDefaultNetwork(int netId, int connectionType) {
         getInstance().notifyObserversOfConnectionTypeChange(connectionType, netId);
     }
 
+    // For testing, pretend the max bandwidth has changed.
+    @CalledByNative
+    public static void fakeMaxBandwidthChanged(double maxBandwidthMbps) {
+        setAutoDetectConnectivityState(false);
+        getInstance().notifyObserversOfMaxBandwidthChange(maxBandwidthMbps);
+    }
+
     private void updateCurrentConnectionType(int newConnectionType) {
         mCurrentConnectionType = newConnectionType;
         notifyObserversOfConnectionTypeChange(newConnectionType);
     }
 
     private void updateCurrentMaxBandwidth(double maxBandwidthMbps) {
-        if (maxBandwidthMbps == mCurrentMaxBandwidth) return;
+        if (maxBandwidthMbps == mCurrentMaxBandwidth
+                && mCurrentConnectionType == mMaxBandwidthConnectionType) {
+            return;
+        }
         mCurrentMaxBandwidth = maxBandwidthMbps;
+        mMaxBandwidthConnectionType = mCurrentConnectionType;
         notifyObserversOfMaxBandwidthChange(maxBandwidthMbps);
     }
 

net/android/java/src/org/chromium/net/NetworkChangeNotifierAutoDetect.java

@@ -348,6 +348,7 @@ protected void init(NetworkChangeNotifierAutoDetect notifier) {
     private int mConnectionType;
     private String mWifiSSID;
     private double mMaxBandwidthMbps;
+    private int mMaxBandwidthConnectionType;
 
     /**
      * Observer interface by which observer is notified of network changes.
@@ -420,6 +421,7 @@ public NetworkChangeNotifierAutoDetect(
         mConnectionType = getCurrentConnectionType(networkState);
         mWifiSSID = getCurrentWifiSSID(networkState);
         mMaxBandwidthMbps = getCurrentMaxBandwidthInMbps(networkState);
+        mMaxBandwidthConnectionType = mConnectionType;
         mIntentFilter =
                 new NetworkConnectivityIntentFilter(mWifiManagerDelegate.getHasWifiPermission());
         mRegistrationPolicy = policy;
@@ -687,8 +689,12 @@ private void connectionTypeChanged(NetworkState networkState) {
 
     private void maxBandwidthChanged(NetworkState networkState) {
         double newMaxBandwidthMbps = getCurrentMaxBandwidthInMbps(networkState);
-        if (newMaxBandwidthMbps == mMaxBandwidthMbps) return;
+        if (newMaxBandwidthMbps == mMaxBandwidthMbps
+                && mConnectionType == mMaxBandwidthConnectionType) {
+            return;
+        }
         mMaxBandwidthMbps = newMaxBandwidthMbps;
+        mMaxBandwidthConnectionType = mConnectionType;
         mObserver.onMaxBandwidthChanged(newMaxBandwidthMbps);
     }
 

net/android/javatests/src/org/chromium/net/NetworkChangeNotifierTest.java

@@ -57,6 +57,30 @@ public void resetHasReceivedNotification() {
         }
     }
 
+    /**
+      * Listens for native notifications of max bandwidth change.
+      */
+    private static class TestNetworkChangeNotifier extends NetworkChangeNotifier {
+        private TestNetworkChangeNotifier(Context context) {
+            super(context);
+        }
+
+        @Override
+        void notifyObserversOfMaxBandwidthChange(double maxBandwidthMbps) {
+            mReceivedMaxBandwidthNotification = true;
+        }
+
+        public boolean hasReceivedMaxBandwidthNotification() {
+            return mReceivedMaxBandwidthNotification;
+        }
+
+        public void resetHasReceivedMaxBandwidthNotification() {
+            mReceivedMaxBandwidthNotification = false;
+        }
+
+        private boolean mReceivedMaxBandwidthNotification = false;
+    }
+
     /**
      * Mocks out calls to the ConnectivityManager.
      */
@@ -210,6 +234,7 @@ public void assertLastChange(ChangeType type, int netId) throws Exception {
     }
 
     // Network.Network(int netId) pointer.
+    private TestNetworkChangeNotifier mNotifier;
     private Constructor<Network> mNetworkConstructor;
     private NetworkChangeNotifierAutoDetect mReceiver;
     private MockConnectivityManagerDelegate mConnectivityDelegate;
@@ -227,7 +252,8 @@ private static enum WatchForChanges {
      */
     private void createTestNotifier(WatchForChanges watchForChanges) {
         Context context = getInstrumentation().getTargetContext();
-        NetworkChangeNotifier.resetInstanceForTests(context);
+        mNotifier = new TestNetworkChangeNotifier(context);
+        NetworkChangeNotifier.resetInstanceForTests(mNotifier);
         if (watchForChanges == WatchForChanges.ALWAYS) {
             NetworkChangeNotifier.registerToReceiveNotificationsAlways();
         } else {
@@ -467,6 +493,49 @@ public void testNetworkChangeNotifierJavaObservers() throws InterruptedException
         assertTrue(observer.hasReceivedNotification());
     }
 
+    /**
+     * Tests that when Chrome gets an intent indicating a change in max bandwidth, it sends a
+     * notification to Java observers.
+     */
+    @UiThreadTest
+    @MediumTest
+    @Feature({"Android-AppBase"})
+    public void testNetworkChangeNotifierMaxBandwidthNotifications() throws InterruptedException {
+        // Initialize the NetworkChangeNotifier with a connection.
+        mConnectivityDelegate.setActiveNetworkExists(true);
+        mConnectivityDelegate.setNetworkType(ConnectivityManager.TYPE_WIFI);
+        mWifiDelegate.setLinkSpeedInMbps(1);
+        Intent connectivityIntent = new Intent(ConnectivityManager.CONNECTIVITY_ACTION);
+        mReceiver.onReceive(getInstrumentation().getTargetContext(), connectivityIntent);
+        assertTrue(mNotifier.hasReceivedMaxBandwidthNotification());
+        mNotifier.resetHasReceivedMaxBandwidthNotification();
+
+        // We shouldn't be re-notified if the connection hasn't actually changed.
+        NetworkChangeNotifierTestObserver observer = new NetworkChangeNotifierTestObserver();
+        NetworkChangeNotifier.addConnectionTypeObserver(observer);
+        mReceiver.onReceive(getInstrumentation().getTargetContext(), connectivityIntent);
+        assertFalse(mNotifier.hasReceivedMaxBandwidthNotification());
+
+        // We should be notified if the bandwidth changed but not the connection type.
+        mWifiDelegate.setLinkSpeedInMbps(2);
+        mReceiver.onReceive(getInstrumentation().getTargetContext(), connectivityIntent);
+        assertTrue(mNotifier.hasReceivedMaxBandwidthNotification());
+        mNotifier.resetHasReceivedMaxBandwidthNotification();
+
+        // We should be notified if bandwidth and connection type changed.
+        mConnectivityDelegate.setNetworkType(ConnectivityManager.TYPE_ETHERNET);
+        mReceiver.onReceive(getInstrumentation().getTargetContext(), connectivityIntent);
+        assertTrue(mNotifier.hasReceivedMaxBandwidthNotification());
+        mNotifier.resetHasReceivedMaxBandwidthNotification();
+
+        // We should be notified if the connection type changed, but not the bandwidth.
+        // Note that TYPE_ETHERNET and TYPE_BLUETOOTH have the same +INFINITY max bandwidth.
+        // This test will fail if that changes.
+        mConnectivityDelegate.setNetworkType(ConnectivityManager.TYPE_BLUETOOTH);
+        mReceiver.onReceive(getInstrumentation().getTargetContext(), connectivityIntent);
+        assertTrue(mNotifier.hasReceivedMaxBandwidthNotification());
+    }
+
     /**
      * Tests that when setting {@code registerToReceiveNotificationsAlways()},
      * a NetworkChangeNotifierAutoDetect object is successfully created.

net/android/network_change_notifier_android_unittest.cc

@@ -71,7 +71,7 @@ class NetworkChangeNotifierObserver
  public:
   NetworkChangeNotifierObserver() : notifications_count_(0) {}
 
-  // NetworkChangeNotifier::Observer:
+  // NetworkChangeNotifier::ConnectionTypeObserver:
   void OnConnectionTypeChanged(
       NetworkChangeNotifier::ConnectionType connection_type) override {
     notifications_count_++;
@@ -85,6 +85,22 @@ class NetworkChangeNotifierObserver
   int notifications_count_;
 };
 
+class NetworkChangeNotifierMaxBandwidthObserver
+    : public NetworkChangeNotifier::MaxBandwidthObserver {
+ public:
+  // NetworkChangeNotifier::MaxBandwidthObserver:
+  void OnMaxBandwidthChanged(
+      double max_bandwidth_mbps,
+      NetworkChangeNotifier::ConnectionType type) override {
+    notifications_count_++;
+  }
+
+  int notifications_count() const { return notifications_count_; }
+
+ private:
+  int notifications_count_ = 0;
+};
+
 class DNSChangeObserver : public NetworkChangeNotifier::DNSObserver {
  public:
   DNSChangeObserver()
@@ -206,6 +222,11 @@ class BaseNetworkChangeNotifierAndroidTest : public testing::Test {
     base::MessageLoop::current()->RunUntilIdle();
   }
 
+  void FakeMaxBandwidthChange(double max_bandwidth_mbps) {
+    delegate_.FakeMaxBandwidthChanged(max_bandwidth_mbps);
+    base::MessageLoop::current()->RunUntilIdle();
+  }
+
   void FakeNetworkChange(ChangeType change,
                          NetworkChangeNotifier::NetworkHandle network,
                          ConnectionType type) {
@@ -268,8 +289,36 @@ TEST_F(BaseNetworkChangeNotifierAndroidTest,
             other_delegate->GetCurrentConnectionType());
 }
 
-class NetworkChangeNotifierDelegateAndroidTest
+class NetworkChangeNotifierAndroidTest
     : public BaseNetworkChangeNotifierAndroidTest {
+ protected:
+  void SetUp() override {
+    IPAddressNumber dns_number;
+    ASSERT_TRUE(ParseIPLiteralToNumber("8.8.8.8", &dns_number));
+    dns_config_.nameservers.push_back(
+        IPEndPoint(dns_number, dns_protocol::kDefaultPort));
+    notifier_.reset(new NetworkChangeNotifierAndroid(&delegate_, &dns_config_));
+    NetworkChangeNotifier::AddConnectionTypeObserver(
+        &connection_type_observer_);
+    NetworkChangeNotifier::AddConnectionTypeObserver(
+        &other_connection_type_observer_);
+    NetworkChangeNotifier::AddMaxBandwidthObserver(&max_bandwidth_observer_);
+  }
+
+  void ForceNetworkHandlesSupportedForTesting() {
+    notifier_->ForceNetworkHandlesSupportedForTesting();
+  }
+
+  NetworkChangeNotifierObserver connection_type_observer_;
+  NetworkChangeNotifierMaxBandwidthObserver max_bandwidth_observer_;
+  NetworkChangeNotifierObserver other_connection_type_observer_;
+  NetworkChangeNotifier::DisableForTest disable_for_test_;
+  DnsConfig dns_config_;
+  scoped_ptr<NetworkChangeNotifierAndroid> notifier_;
+};
+
+class NetworkChangeNotifierDelegateAndroidTest
+    : public NetworkChangeNotifierAndroidTest {
  protected:
   NetworkChangeNotifierDelegateAndroidTest() {
     delegate_.AddObserver(&delegate_observer_);
@@ -302,31 +351,6 @@ TEST_F(NetworkChangeNotifierDelegateAndroidTest, DelegateObserverNotified) {
             other_delegate_observer_.type_notifications_count());
 }
 
-class NetworkChangeNotifierAndroidTest
-    : public BaseNetworkChangeNotifierAndroidTest {
- protected:
-  void SetUp() override {
-    IPAddressNumber dns_number;
-    ASSERT_TRUE(ParseIPLiteralToNumber("8.8.8.8", &dns_number));
-    dns_config_.nameservers.push_back(
-        IPEndPoint(dns_number, dns_protocol::kDefaultPort));
-    notifier_.reset(new NetworkChangeNotifierAndroid(&delegate_, &dns_config_));
-    NetworkChangeNotifier::AddConnectionTypeObserver(
-        &connection_type_observer_);
-    NetworkChangeNotifier::AddConnectionTypeObserver(
-        &other_connection_type_observer_);
-  }
-
-  void ForceNetworkHandlesSupportedForTesting() {
-    notifier_->ForceNetworkHandlesSupportedForTesting();
-  }
-
-  NetworkChangeNotifierObserver connection_type_observer_;
-  NetworkChangeNotifierObserver other_connection_type_observer_;
-  NetworkChangeNotifier::DisableForTest disable_for_test_;
-  DnsConfig dns_config_;
-  scoped_ptr<NetworkChangeNotifierAndroid> notifier_;
-};
 
 // When a NetworkChangeNotifierAndroid is observing a
 // NetworkChangeNotifierDelegateAndroid for network state changes, and the
@@ -370,6 +394,22 @@ TEST_F(NetworkChangeNotifierAndroidTest, MaxBandwidth) {
   EXPECT_EQ(0.0, max_bandwidth_mbps);
 }
 
+TEST_F(NetworkChangeNotifierDelegateAndroidTest, MaxBandwidthCallbackNotifier) {
+  // The bandwidth notification should always be forwarded, even if the value
+  // doesn't change (because the type might have changed).
+  FakeMaxBandwidthChange(100.0);
+  EXPECT_EQ(1, delegate_observer_.bandwidth_notifications_count());
+  EXPECT_EQ(1, max_bandwidth_observer_.notifications_count());
+
+  FakeMaxBandwidthChange(100.0);
+  EXPECT_EQ(2, delegate_observer_.bandwidth_notifications_count());
+  EXPECT_EQ(2, max_bandwidth_observer_.notifications_count());
+
+  FakeMaxBandwidthChange(101.0);
+  EXPECT_EQ(3, delegate_observer_.bandwidth_notifications_count());
+  EXPECT_EQ(3, max_bandwidth_observer_.notifications_count());
+}
+
 TEST_F(NetworkChangeNotifierDelegateAndroidTest,
        MaxBandwidthNotifiedOnConnectionChange) {
   EXPECT_EQ(0, delegate_observer_.bandwidth_notifications_count());

net/android/network_change_notifier_delegate_android.cc

@@ -356,4 +356,10 @@ void NetworkChangeNotifierDelegateAndroid::FakeDefaultNetwork(
   Java_NetworkChangeNotifier_fakeDefaultNetwork(env, network, type);
 }
 
+void NetworkChangeNotifierDelegateAndroid::FakeMaxBandwidthChanged(
+    double max_bandwidth_mbps) {
+  JNIEnv* env = base::android::AttachCurrentThread();
+  Java_NetworkChangeNotifier_fakeMaxBandwidthChanged(env, max_bandwidth_mbps);
+}
+
 }  // namespace net

net/android/network_change_notifier_delegate_android.h

@@ -136,6 +136,7 @@ class NET_EXPORT_PRIVATE NetworkChangeNotifierDelegateAndroid {
   void FakeNetworkDisconnected(NetworkHandle network);
   void FakeUpdateActiveNetworkList(NetworkList networks);
   void FakeDefaultNetwork(NetworkHandle network, ConnectionType type);
+  void FakeMaxBandwidthChanged(double max_bandwidth_mbps);
 
   base::ThreadChecker thread_checker_;
   scoped_refptr<base::ObserverListThreadSafe<Observer>> observers_;

third_party/WebKit/LayoutTests/netinfo/connection-types-expected.txt

@@ -35,6 +35,14 @@ PASS connection.type is types[count][0]
 PASS connection.downlinkMax is types[count][1]
 PASS connection.type is types[count][0]
 PASS connection.downlinkMax is types[count][1]
+PASS connection.type is types[count][0]
+PASS connection.downlinkMax is types[count][1]
+PASS connection.type is types[count][0]
+PASS connection.downlinkMax is types[count][1]
+PASS connection.type is types[count][0]
+PASS connection.downlinkMax is types[count][1]
+PASS connection.type is types[count][0]
+PASS connection.downlinkMax is types[count][1]
 PASS successfullyParsed is true
 
 TEST COMPLETE

third_party/WebKit/LayoutTests/netinfo/connection-types.html

@@ -18,7 +18,9 @@
   ['wimax', 5.0],
   ['other', 6.0],
   ['none', 7.0],
-  ['unknown', 8.0]
+  ['unknown', 8.0],
+  ['wimax', 8.0],
+  ['wimax', 9.0]
 ];
 
 // ontypechange is deprecated but this test is here to