diff options
author | jkarlin <jkarlin@chromium.org> | 2016-01-15 11:47:05 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2016-01-15 19:47:52 +0000 |
commit | cfb1a4c18c0170b6ec0324509ad15e372d8208bb (patch) | |
tree | e5c7e83955ba37afe05106e128afa5c81da70503 | |
parent | ab450c5ede0635194331286088d0f488f4086ba5 (diff) | |
download | chromium_src-cfb1a4c18c0170b6ec0324509ad15e372d8208bb.zip chromium_src-cfb1a4c18c0170b6ec0324509ad15e372d8208bb.tar.gz chromium_src-cfb1a4c18c0170b6ec0324509ad15e372d8208bb.tar.bz2 |
[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}
8 files changed, 180 insertions, 34 deletions
diff --git a/net/android/java/src/org/chromium/net/NetworkChangeNotifier.java b/net/android/java/src/org/chromium/net/NetworkChangeNotifier.java index 2ed71e1..ef573af 100644 --- a/net/android/java/src/org/chromium/net/NetworkChangeNotifier.java +++ b/net/android/java/src/org/chromium/net/NetworkChangeNotifier.java @@ -7,6 +7,7 @@ package org.chromium.net; 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 class NetworkChangeNotifier { 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 class NetworkChangeNotifier { 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 class NetworkChangeNotifier { 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); } diff --git a/net/android/java/src/org/chromium/net/NetworkChangeNotifierAutoDetect.java b/net/android/java/src/org/chromium/net/NetworkChangeNotifierAutoDetect.java index 333c85c..24ab74d 100644 --- a/net/android/java/src/org/chromium/net/NetworkChangeNotifierAutoDetect.java +++ b/net/android/java/src/org/chromium/net/NetworkChangeNotifierAutoDetect.java @@ -348,6 +348,7 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { 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 class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { mConnectionType = getCurrentConnectionType(networkState); mWifiSSID = getCurrentWifiSSID(networkState); mMaxBandwidthMbps = getCurrentMaxBandwidthInMbps(networkState); + mMaxBandwidthConnectionType = mConnectionType; mIntentFilter = new NetworkConnectivityIntentFilter(mWifiManagerDelegate.getHasWifiPermission()); mRegistrationPolicy = policy; @@ -687,8 +689,12 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { 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); } diff --git a/net/android/javatests/src/org/chromium/net/NetworkChangeNotifierTest.java b/net/android/javatests/src/org/chromium/net/NetworkChangeNotifierTest.java index ae667b5..91f40df 100644 --- a/net/android/javatests/src/org/chromium/net/NetworkChangeNotifierTest.java +++ b/net/android/javatests/src/org/chromium/net/NetworkChangeNotifierTest.java @@ -58,6 +58,30 @@ public class NetworkChangeNotifierTest extends InstrumentationTestCase { } /** + * 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. */ private static class MockConnectivityManagerDelegate extends ConnectivityManagerDelegate { @@ -210,6 +234,7 @@ public class NetworkChangeNotifierTest extends InstrumentationTestCase { } // Network.Network(int netId) pointer. + private TestNetworkChangeNotifier mNotifier; private Constructor<Network> mNetworkConstructor; private NetworkChangeNotifierAutoDetect mReceiver; private MockConnectivityManagerDelegate mConnectivityDelegate; @@ -227,7 +252,8 @@ public class NetworkChangeNotifierTest extends InstrumentationTestCase { */ 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 { @@ -468,6 +494,49 @@ public class NetworkChangeNotifierTest extends InstrumentationTestCase { } /** + * 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. */ diff --git a/net/android/network_change_notifier_android_unittest.cc b/net/android/network_change_notifier_android_unittest.cc index bd5df46..e07a89e 100644 --- a/net/android/network_change_notifier_android_unittest.cc +++ b/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,9 +289,37 @@ 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_); delegate_.AddObserver(&other_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()); diff --git a/net/android/network_change_notifier_delegate_android.cc b/net/android/network_change_notifier_delegate_android.cc index 71aa166..ff07297 100644 --- a/net/android/network_change_notifier_delegate_android.cc +++ b/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 diff --git a/net/android/network_change_notifier_delegate_android.h b/net/android/network_change_notifier_delegate_android.h index bd200d4..a9f18d5 100644 --- a/net/android/network_change_notifier_delegate_android.h +++ b/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_; diff --git a/third_party/WebKit/LayoutTests/netinfo/connection-types-expected.txt b/third_party/WebKit/LayoutTests/netinfo/connection-types-expected.txt index 30b810f..8a7c3b6 100644 --- a/third_party/WebKit/LayoutTests/netinfo/connection-types-expected.txt +++ b/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 diff --git a/third_party/WebKit/LayoutTests/netinfo/connection-types.html b/third_party/WebKit/LayoutTests/netinfo/connection-types.html index 8c9f7c5..c6039ed 100644 --- a/third_party/WebKit/LayoutTests/netinfo/connection-types.html +++ b/third_party/WebKit/LayoutTests/netinfo/connection-types.html @@ -18,7 +18,9 @@ var types = [ ['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 |