Wifi details: minor ordering fixes.
1. Set mNetwork before the NetworkCallbacks can be delivered.
Previously, the code would set mNetwork in updateInfo, relying
on the fact that the BroadcastReceiver registered in onResume
is immediately invoked. However, this races with the
callbacks, which are also immediately invoked. If the
callbacks won the race, mNetwork would not be set and they
would be ignored.
2. Call updateInfo in onResume instead of in displayPreference.
This ensures that it's always called exactly once before the
activity starts running, regardless of whether it's being
displayed the first time (i.e., after onStart) or resumed
by switching from another app. displayPreference is only
called in the former case.
3. Don't call getLinkProperties and getNetworkCapabilities in
updateInfo. These calls are superfluous, because this
information is update by the NetworkCallbacks, and they can
cause jank because updateInfo is called on the UI thread.
This requires that the tests be changed so they always call
onResume, since no UI elements are populated until onResume is
called.
Bug: 62209358
Test: make -j64 RunSettingsRoboTests
Change-Id: Iccb1b9ae51188755d890a6df83004dbe9bec565e
diff --git a/src/com/android/settings/wifi/details/WifiDetailPreferenceController.java b/src/com/android/settings/wifi/details/WifiDetailPreferenceController.java
index 0d585c7..f3db5e5 100644
--- a/src/com/android/settings/wifi/details/WifiDetailPreferenceController.java
+++ b/src/com/android/settings/wifi/details/WifiDetailPreferenceController.java
@@ -269,16 +269,19 @@
mForgetButton = (Button) mButtonsPref.findViewById(R.id.forget_button);
mForgetButton.setText(R.string.forget);
mForgetButton.setOnClickListener(view -> forgetNetwork());
- updateInfo();
}
@Override
public void onResume() {
+ // Ensure mNetwork is set before any callbacks above are delivered, since our
+ // NetworkCallback only looks at changes to mNetwork.
+ mNetwork = mWifiManager.getCurrentNetwork();
+ mLinkProperties = mConnectivityManager.getLinkProperties(mNetwork);
+ mNetworkCapabilities = mConnectivityManager.getNetworkCapabilities(mNetwork);
+ updateInfo();
+ mContext.registerReceiver(mReceiver, mFilter);
mConnectivityManagerWrapper.registerNetworkCallback(mNetworkRequest, mNetworkCallback,
mHandler);
- // updateInfo() will be called during registration because NETWORK_STATE_CHANGED_ACTION is
- // a sticky broadcast.
- mContext.registerReceiver(mReceiver, mFilter);
}
@Override
@@ -293,9 +296,8 @@
}
private void updateInfo() {
- mNetwork = mWifiManager.getCurrentNetwork();
- mLinkProperties = mConnectivityManager.getLinkProperties(mNetwork);
- mNetworkCapabilities = mConnectivityManager.getNetworkCapabilities(mNetwork);
+ // No need to fetch LinkProperties and NetworkCapabilities, they are updated by the
+ // callbacks. mNetwork doesn't change except in onResume.
mNetworkInfo = mConnectivityManager.getNetworkInfo(mNetwork);
mWifiInfo = mWifiManager.getConnectionInfo();
if (mNetwork == null || mNetworkInfo == null || mWifiInfo == null) {
diff --git a/tests/robotests/src/com/android/settings/wifi/details/WifiDetailPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/wifi/details/WifiDetailPreferenceControllerTest.java
index 18cfa4e..2aa338c 100644
--- a/tests/robotests/src/com/android/settings/wifi/details/WifiDetailPreferenceControllerTest.java
+++ b/tests/robotests/src/com/android/settings/wifi/details/WifiDetailPreferenceControllerTest.java
@@ -265,6 +265,11 @@
.thenReturn(mockIpv6AddressesPref);
}
+ private void displayAndResume() {
+ mController.displayPreference(mockScreen);
+ mController.onResume();
+ }
+
@Test
public void isAvailable_shouldAlwaysReturnTrue() {
mController.displayPreference(mockScreen);
@@ -274,28 +279,28 @@
@Test
public void securityPreference_stringShouldBeSet() {
- mController.displayPreference(mockScreen);
+ displayAndResume();
verify(mockSecurityPref).setDetailText(SECURITY);
}
@Test
public void latestWifiInfo_shouldBeFetchedInDisplayPreference() {
- mController.displayPreference(mockScreen);
+ displayAndResume();
verify(mockWifiManager, times(1)).getConnectionInfo();
}
@Test
public void latestNetworkInfo_shouldBeFetchedInDisplayPreference() {
- mController.displayPreference(mockScreen);
+ displayAndResume();
verify(mockConnectivityManager, times(1)).getNetworkInfo(any(Network.class));
}
@Test
public void networkCallback_shouldBeRegisteredOnResume() {
- mController.onResume();
+ displayAndResume();
verify(mockConnectivityManagerWrapper, times(1)).registerNetworkCallback(
any(NetworkRequest.class), mCallbackCaptor.capture(), any(Handler.class));
@@ -303,7 +308,7 @@
@Test
public void networkCallback_shouldBeUnregisteredOnPause() {
- mController.onResume();
+ displayAndResume();
mController.onPause();
verify(mockConnectivityManager, times(1)).unregisterNetworkCallback(
@@ -315,7 +320,7 @@
Drawable expectedIcon =
NetworkBadging.getWifiIcon(LEVEL, NetworkBadging.BADGING_NONE, mContext.getTheme());
- mController.displayPreference(mockScreen);
+ displayAndResume();
verify(mockConnectionDetailPref).setIcon(expectedIcon);
}
@@ -325,14 +330,14 @@
String summary = "summary";
when(mockAccessPoint.getSettingsSummary()).thenReturn(summary);
- mController.displayPreference(mockScreen);
+ displayAndResume();
verify(mockConnectionDetailPref).setTitle(summary);
}
@Test
public void signalStrengthPref_shouldHaveIconSet() {
- mController.displayPreference(mockScreen);
+ displayAndResume();
verify(mockSignalStrengthPref).setIcon(any(Drawable.class));
}
@@ -342,7 +347,7 @@
String expectedStrength =
mContext.getResources().getStringArray(R.array.wifi_signal)[LEVEL];
- mController.displayPreference(mockScreen);
+ displayAndResume();
verify(mockSignalStrengthPref).setDetailText(expectedStrength);
}
@@ -351,7 +356,7 @@
public void linkSpeedPref_shouldHaveDetailTextSet() {
String expectedLinkSpeed = mContext.getString(R.string.link_speed, LINK_SPEED);
- mController.displayPreference(mockScreen);
+ displayAndResume();
verify(mockLinkSpeedPref).setDetailText(expectedLinkSpeed);
}
@@ -360,14 +365,14 @@
public void linkSpeedPref_shouldNotShowIfNotSet() {
when(mockWifiInfo.getLinkSpeed()).thenReturn(-1);
- mController.displayPreference(mockScreen);
+ displayAndResume();
verify(mockLinkSpeedPref).setVisible(false);
}
@Test
public void macAddressPref_shouldHaveDetailTextSet() {
- mController.displayPreference(mockScreen);
+ displayAndResume();
verify(mockMacAddressPref).setDetailText(MAC_ADDRESS);
}
@@ -376,7 +381,7 @@
public void ipAddressPref_shouldHaveDetailTextSet() {
mLinkProperties.addLinkAddress(Constants.IPV4_ADDR);
- mController.displayPreference(mockScreen);
+ displayAndResume();
verify(mockIpAddressPref).setDetailText(Constants.IPV4_ADDR.getAddress().getHostAddress());
}
@@ -387,7 +392,7 @@
mLinkProperties.addRoute(Constants.IPV4_DEFAULT);
mLinkProperties.addRoute(Constants.IPV4_SUBNET);
- mController.displayPreference(mockScreen);
+ displayAndResume();
verify(mockSubnetPref).setDetailText("255.255.255.128");
verify(mockGatewayPref).setDetailText("192.0.2.127");
@@ -398,7 +403,7 @@
mLinkProperties.addDnsServer(InetAddress.getByAddress(new byte[]{8,8,4,4}));
mLinkProperties.addDnsServer(InetAddress.getByAddress(new byte[]{8,8,8,8}));
- mController.displayPreference(mockScreen);
+ displayAndResume();
verify(mockDnsPref).setDetailText("8.8.4.4,8.8.8.8");
}
@@ -409,7 +414,7 @@
// nor connecting and WifiStateMachine has not reached L2ConnectedState.
when(mockWifiManager.getCurrentNetwork()).thenReturn(null);
- mController.displayPreference(mockScreen);
+ displayAndResume();
verify(mockActivity).finish();
}
@@ -420,7 +425,7 @@
reset(mockIpv6Category, mockIpAddressPref, mockSubnetPref, mockGatewayPref,
mockDnsPref);
- mController.displayPreference(mockScreen);
+ displayAndResume();
verify(mockIpv6Category).setVisible(false);
verify(mockIpAddressPref).setVisible(false);
@@ -465,8 +470,7 @@
@Test
public void onLinkPropertiesChanged_updatesFields() {
- mController.displayPreference(mockScreen);
- mController.onResume();
+ displayAndResume();
InOrder inOrder = inOrder(mockIpAddressPref, mockGatewayPref, mockSubnetPref,
mockDnsPref, mockIpv6Category, mockIpv6AddressesPref);
@@ -528,8 +532,7 @@
when(mockAccessPoint.getSettingsSummary()).thenReturn(summary);
InOrder inOrder = inOrder(mockConnectionDetailPref);
- mController.displayPreference(mockScreen);
- mController.onResume();
+ displayAndResume();
inOrder.verify(mockConnectionDetailPref).setTitle(summary);
// Check that an irrelevant capability update does not update the access point summary, as
@@ -562,7 +565,7 @@
when(mockAccessPoint.getConfig()).thenReturn(null);
mController = newWifiDetailPreferenceController();
- mController.displayPreference(mockScreen);
+ displayAndResume();
verify(mockForgetButton).setVisibility(View.INVISIBLE);
}
@@ -572,14 +575,14 @@
when(mockWifiInfo.isEphemeral()).thenReturn(true);
when(mockAccessPoint.getConfig()).thenReturn(null);
- mController.displayPreference(mockScreen);
+ displayAndResume();
verify(mockForgetButton).setVisibility(View.VISIBLE);
}
@Test
public void canForgetNetwork_saved() {
- mController.displayPreference(mockScreen);
+ displayAndResume();
verify(mockForgetButton).setVisibility(View.VISIBLE);
}
@@ -590,7 +593,7 @@
when(mockWifiInfo.isEphemeral()).thenReturn(true);
when(mockWifiInfo.getSSID()).thenReturn(ssid);
- mController.displayPreference(mockScreen);
+ displayAndResume();
mForgetClickListener.getValue().onClick(null);
verify(mockWifiManager).disableEphemeralNetwork(ssid);
@@ -612,8 +615,7 @@
@Test
public void networkStateChangedIntent_shouldRefetchInfo() {
- mController.displayPreference(mockScreen);
- mController.onResume();
+ displayAndResume();
verify(mockConnectivityManager, times(1)).getNetworkInfo(any(Network.class));
verify(mockWifiManager, times(1)).getConnectionInfo();
@@ -626,8 +628,7 @@
@Test
public void rssiChangedIntent_shouldRefetchInfo() {
- mController.displayPreference(mockScreen);
- mController.onResume();
+ displayAndResume();
verify(mockConnectivityManager, times(1)).getNetworkInfo(any(Network.class));
verify(mockWifiManager, times(1)).getConnectionInfo();
@@ -640,7 +641,7 @@
@Test
public void networkDisconnectedState_shouldFinishActivity() {
- mController.onResume();
+ displayAndResume();
when(mockConnectivityManager.getNetworkInfo(any(Network.class))).thenReturn(null);
mContext.sendBroadcast(new Intent(WifiManager.NETWORK_STATE_CHANGED_ACTION));
@@ -650,8 +651,7 @@
@Test
public void networkOnLost_shouldFinishActivity() {
- mController.displayPreference(mockScreen);
- mController.onResume();
+ displayAndResume();
mCallbackCaptor.getValue().onLost(mockNetwork);
@@ -664,7 +664,7 @@
mLinkProperties.addLinkAddress(Constants.IPV6_GLOBAL1);
mLinkProperties.addLinkAddress(Constants.IPV6_GLOBAL2);
- mController.displayPreference(mockScreen);
+ displayAndResume();
List <Preference> addrs = mIpv6AddressCaptor.getAllValues();
@@ -680,7 +680,7 @@
public void ipv6AddressPref_shouldNotBeSelectable() {
mLinkProperties.addLinkAddress(Constants.IPV6_GLOBAL2);
- mController.displayPreference(mockScreen);
+ displayAndResume();
assertThat(mockIpv6AddressesPref.isSelectable()).isFalse();
}
@@ -689,8 +689,7 @@
public void captivePortal_shouldShowSignInButton() {
InOrder inOrder = inOrder(mockSignInButton);
- mController.displayPreference(mockScreen);
- mController.onResume();
+ displayAndResume();
inOrder.verify(mockSignInButton).setVisibility(View.INVISIBLE);
@@ -709,7 +708,7 @@
@Test
public void testSignInButton_shouldStartCaptivePortalApp() {
- mController.displayPreference(mockScreen);
+ displayAndResume();
ArgumentCaptor<OnClickListener> captor = ArgumentCaptor.forClass(OnClickListener.class);
verify(mockSignInButton).setOnClickListener(captor.capture());
@@ -722,7 +721,7 @@
when(mockSignInButton.getVisibility()).thenReturn(View.VISIBLE);
when(mockForgetButton.getVisibility()).thenReturn(View.INVISIBLE);
- mController.displayPreference(mockScreen);
+ displayAndResume();
verify(mockButtonsPref).setVisible(true);
}
@@ -732,7 +731,7 @@
when(mockSignInButton.getVisibility()).thenReturn(View.INVISIBLE);
when(mockForgetButton.getVisibility()).thenReturn(View.VISIBLE);
- mController.displayPreference(mockScreen);
+ displayAndResume();
verify(mockButtonsPref).setVisible(true);
}
@@ -742,7 +741,7 @@
when(mockSignInButton.getVisibility()).thenReturn(View.INVISIBLE);
when(mockForgetButton.getVisibility()).thenReturn(View.INVISIBLE);
- mController.displayPreference(mockScreen);
+ displayAndResume();
verify(mockButtonsPref).setVisible(false);
}