PawletOS/app_Settings
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Wifi details: minor ordering fixes.

Browse Source 
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
This commit is contained in:
Lorenzo Colitti
2017-06-04 23:15:40 +09:00
parent 2561df8083
commit 088215d877
2 changed files with 48 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
src/com/android/settings/wifi/details/WifiDetailPreferenceController.java
View File

@@ -269,16 +269,19 @@ public class WifiDetailPreferenceController extends PreferenceController impleme
mForgetButton = (Button) mButtonsPref.findViewById(R.id.forget_button); mForgetButton = (Button) mButtonsPref.findViewById(R.id.forget_button);
mForgetButton.setText(R.string.forget); mForgetButton.setText(R.string.forget);
mForgetButton.setOnClickListener(view -> forgetNetwork()); mForgetButton.setOnClickListener(view -> forgetNetwork());
updateInfo();
} }
@Override @Override
public void onResume() { 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, mConnectivityManagerWrapper.registerNetworkCallback(mNetworkRequest, mNetworkCallback,
mHandler); mHandler);
// updateInfo() will be called during registration because NETWORK_STATE_CHANGED_ACTION is
// a sticky broadcast.
mContext.registerReceiver(mReceiver, mFilter);
} }
@Override @Override
@@ -293,9 +296,8 @@ public class WifiDetailPreferenceController extends PreferenceController impleme
} }
private void updateInfo() { private void updateInfo() {
mNetwork = mWifiManager.getCurrentNetwork(); // No need to fetch LinkProperties and NetworkCapabilities, they are updated by the
mLinkProperties = mConnectivityManager.getLinkProperties(mNetwork); // callbacks. mNetwork doesn't change except in onResume.
mNetworkCapabilities = mConnectivityManager.getNetworkCapabilities(mNetwork);
mNetworkInfo = mConnectivityManager.getNetworkInfo(mNetwork); mNetworkInfo = mConnectivityManager.getNetworkInfo(mNetwork);
mWifiInfo = mWifiManager.getConnectionInfo(); mWifiInfo = mWifiManager.getConnectionInfo();
if (mNetwork == null || mNetworkInfo == null || mWifiInfo == null) { if (mNetwork == null || mNetworkInfo == null || mWifiInfo == null) {

79
tests/robotests/src/com/android/settings/wifi/details/WifiDetailPreferenceControllerTest.java
View File

@@ -265,6 +265,11 @@ public class WifiDetailPreferenceControllerTest {
.thenReturn(mockIpv6AddressesPref); .thenReturn(mockIpv6AddressesPref);
} }
private void displayAndResume() {
mController.displayPreference(mockScreen);
mController.onResume();
}
@Test @Test
public void isAvailable_shouldAlwaysReturnTrue() { public void isAvailable_shouldAlwaysReturnTrue() {
mController.displayPreference(mockScreen); mController.displayPreference(mockScreen);
@@ -274,28 +279,28 @@ public class WifiDetailPreferenceControllerTest {
@Test @Test
public void securityPreference_stringShouldBeSet() { public void securityPreference_stringShouldBeSet() {
mController.displayPreference(mockScreen); displayAndResume();
verify(mockSecurityPref).setDetailText(SECURITY); verify(mockSecurityPref).setDetailText(SECURITY);
} }
@Test @Test
public void latestWifiInfo_shouldBeFetchedInDisplayPreference() { public void latestWifiInfo_shouldBeFetchedInDisplayPreference() {
mController.displayPreference(mockScreen); displayAndResume();
verify(mockWifiManager, times(1)).getConnectionInfo(); verify(mockWifiManager, times(1)).getConnectionInfo();
} }
@Test @Test
public void latestNetworkInfo_shouldBeFetchedInDisplayPreference() { public void latestNetworkInfo_shouldBeFetchedInDisplayPreference() {
mController.displayPreference(mockScreen); displayAndResume();
verify(mockConnectivityManager, times(1)).getNetworkInfo(any(Network.class)); verify(mockConnectivityManager, times(1)).getNetworkInfo(any(Network.class));
} }
@Test @Test
public void networkCallback_shouldBeRegisteredOnResume() { public void networkCallback_shouldBeRegisteredOnResume() {
mController.onResume(); displayAndResume();
verify(mockConnectivityManagerWrapper, times(1)).registerNetworkCallback( verify(mockConnectivityManagerWrapper, times(1)).registerNetworkCallback(
any(NetworkRequest.class), mCallbackCaptor.capture(), any(Handler.class)); any(NetworkRequest.class), mCallbackCaptor.capture(), any(Handler.class));
@@ -303,7 +308,7 @@ public class WifiDetailPreferenceControllerTest {
@Test @Test
public void networkCallback_shouldBeUnregisteredOnPause() { public void networkCallback_shouldBeUnregisteredOnPause() {
mController.onResume(); displayAndResume();
mController.onPause(); mController.onPause();
verify(mockConnectivityManager, times(1)).unregisterNetworkCallback( verify(mockConnectivityManager, times(1)).unregisterNetworkCallback(
@@ -315,7 +320,7 @@ public class WifiDetailPreferenceControllerTest {
Drawable expectedIcon = Drawable expectedIcon =
NetworkBadging.getWifiIcon(LEVEL, NetworkBadging.BADGING_NONE, mContext.getTheme()); NetworkBadging.getWifiIcon(LEVEL, NetworkBadging.BADGING_NONE, mContext.getTheme());
mController.displayPreference(mockScreen); displayAndResume();
verify(mockConnectionDetailPref).setIcon(expectedIcon); verify(mockConnectionDetailPref).setIcon(expectedIcon);
} }
@@ -325,14 +330,14 @@ public class WifiDetailPreferenceControllerTest {
String summary = "summary"; String summary = "summary";
when(mockAccessPoint.getSettingsSummary()).thenReturn(summary); when(mockAccessPoint.getSettingsSummary()).thenReturn(summary);
mController.displayPreference(mockScreen); displayAndResume();
verify(mockConnectionDetailPref).setTitle(summary); verify(mockConnectionDetailPref).setTitle(summary);
} }
@Test @Test
public void signalStrengthPref_shouldHaveIconSet() { public void signalStrengthPref_shouldHaveIconSet() {
mController.displayPreference(mockScreen); displayAndResume();
verify(mockSignalStrengthPref).setIcon(any(Drawable.class)); verify(mockSignalStrengthPref).setIcon(any(Drawable.class));
} }
@@ -342,7 +347,7 @@ public class WifiDetailPreferenceControllerTest {
String expectedStrength = String expectedStrength =
mContext.getResources().getStringArray(R.array.wifi_signal)[LEVEL]; mContext.getResources().getStringArray(R.array.wifi_signal)[LEVEL];
mController.displayPreference(mockScreen); displayAndResume();
verify(mockSignalStrengthPref).setDetailText(expectedStrength); verify(mockSignalStrengthPref).setDetailText(expectedStrength);
} }
@@ -351,7 +356,7 @@ public class WifiDetailPreferenceControllerTest {
public void linkSpeedPref_shouldHaveDetailTextSet() { public void linkSpeedPref_shouldHaveDetailTextSet() {
String expectedLinkSpeed = mContext.getString(R.string.link_speed, LINK_SPEED); String expectedLinkSpeed = mContext.getString(R.string.link_speed, LINK_SPEED);
mController.displayPreference(mockScreen); displayAndResume();
verify(mockLinkSpeedPref).setDetailText(expectedLinkSpeed); verify(mockLinkSpeedPref).setDetailText(expectedLinkSpeed);
} }
@@ -360,14 +365,14 @@ public class WifiDetailPreferenceControllerTest {
public void linkSpeedPref_shouldNotShowIfNotSet() { public void linkSpeedPref_shouldNotShowIfNotSet() {
when(mockWifiInfo.getLinkSpeed()).thenReturn(-1); when(mockWifiInfo.getLinkSpeed()).thenReturn(-1);
mController.displayPreference(mockScreen); displayAndResume();
verify(mockLinkSpeedPref).setVisible(false); verify(mockLinkSpeedPref).setVisible(false);
} }
@Test @Test
public void macAddressPref_shouldHaveDetailTextSet() { public void macAddressPref_shouldHaveDetailTextSet() {
mController.displayPreference(mockScreen); displayAndResume();
verify(mockMacAddressPref).setDetailText(MAC_ADDRESS); verify(mockMacAddressPref).setDetailText(MAC_ADDRESS);
} }
@@ -376,7 +381,7 @@ public class WifiDetailPreferenceControllerTest {
public void ipAddressPref_shouldHaveDetailTextSet() { public void ipAddressPref_shouldHaveDetailTextSet() {
mLinkProperties.addLinkAddress(Constants.IPV4_ADDR); mLinkProperties.addLinkAddress(Constants.IPV4_ADDR);
mController.displayPreference(mockScreen); displayAndResume();
verify(mockIpAddressPref).setDetailText(Constants.IPV4_ADDR.getAddress().getHostAddress()); verify(mockIpAddressPref).setDetailText(Constants.IPV4_ADDR.getAddress().getHostAddress());
} }
@@ -387,7 +392,7 @@ public class WifiDetailPreferenceControllerTest {
mLinkProperties.addRoute(Constants.IPV4_DEFAULT); mLinkProperties.addRoute(Constants.IPV4_DEFAULT);
mLinkProperties.addRoute(Constants.IPV4_SUBNET); mLinkProperties.addRoute(Constants.IPV4_SUBNET);
mController.displayPreference(mockScreen); displayAndResume();
verify(mockSubnetPref).setDetailText("255.255.255.128"); verify(mockSubnetPref).setDetailText("255.255.255.128");
verify(mockGatewayPref).setDetailText("192.0.2.127"); verify(mockGatewayPref).setDetailText("192.0.2.127");
@@ -398,7 +403,7 @@ public class WifiDetailPreferenceControllerTest {
mLinkProperties.addDnsServer(InetAddress.getByAddress(new byte[]{8,8,4,4})); mLinkProperties.addDnsServer(InetAddress.getByAddress(new byte[]{8,8,4,4}));
mLinkProperties.addDnsServer(InetAddress.getByAddress(new byte[]{8,8,8,8})); 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"); verify(mockDnsPref).setDetailText("8.8.4.4,8.8.8.8");
} }
@@ -409,7 +414,7 @@ public class WifiDetailPreferenceControllerTest {
// nor connecting and WifiStateMachine has not reached L2ConnectedState. // nor connecting and WifiStateMachine has not reached L2ConnectedState.
when(mockWifiManager.getCurrentNetwork()).thenReturn(null); when(mockWifiManager.getCurrentNetwork()).thenReturn(null);
mController.displayPreference(mockScreen); displayAndResume();
verify(mockActivity).finish(); verify(mockActivity).finish();
} }
@@ -420,7 +425,7 @@ public class WifiDetailPreferenceControllerTest {
reset(mockIpv6Category, mockIpAddressPref, mockSubnetPref, mockGatewayPref, reset(mockIpv6Category, mockIpAddressPref, mockSubnetPref, mockGatewayPref,
mockDnsPref); mockDnsPref);
mController.displayPreference(mockScreen); displayAndResume();
verify(mockIpv6Category).setVisible(false); verify(mockIpv6Category).setVisible(false);
verify(mockIpAddressPref).setVisible(false); verify(mockIpAddressPref).setVisible(false);
@@ -465,8 +470,7 @@ public class WifiDetailPreferenceControllerTest {
@Test @Test
public void onLinkPropertiesChanged_updatesFields() { public void onLinkPropertiesChanged_updatesFields() {
mController.displayPreference(mockScreen); displayAndResume();
mController.onResume();
InOrder inOrder = inOrder(mockIpAddressPref, mockGatewayPref, mockSubnetPref, InOrder inOrder = inOrder(mockIpAddressPref, mockGatewayPref, mockSubnetPref,
mockDnsPref, mockIpv6Category, mockIpv6AddressesPref); mockDnsPref, mockIpv6Category, mockIpv6AddressesPref);
@@ -528,8 +532,7 @@ public class WifiDetailPreferenceControllerTest {
when(mockAccessPoint.getSettingsSummary()).thenReturn(summary); when(mockAccessPoint.getSettingsSummary()).thenReturn(summary);
InOrder inOrder = inOrder(mockConnectionDetailPref); InOrder inOrder = inOrder(mockConnectionDetailPref);
mController.displayPreference(mockScreen); displayAndResume();
mController.onResume();
inOrder.verify(mockConnectionDetailPref).setTitle(summary); inOrder.verify(mockConnectionDetailPref).setTitle(summary);
// Check that an irrelevant capability update does not update the access point summary, as // Check that an irrelevant capability update does not update the access point summary, as
@@ -562,7 +565,7 @@ public class WifiDetailPreferenceControllerTest {
when(mockAccessPoint.getConfig()).thenReturn(null); when(mockAccessPoint.getConfig()).thenReturn(null);
mController = newWifiDetailPreferenceController(); mController = newWifiDetailPreferenceController();
mController.displayPreference(mockScreen); displayAndResume();
verify(mockForgetButton).setVisibility(View.INVISIBLE); verify(mockForgetButton).setVisibility(View.INVISIBLE);
} }
@@ -572,14 +575,14 @@ public class WifiDetailPreferenceControllerTest {
when(mockWifiInfo.isEphemeral()).thenReturn(true); when(mockWifiInfo.isEphemeral()).thenReturn(true);
when(mockAccessPoint.getConfig()).thenReturn(null); when(mockAccessPoint.getConfig()).thenReturn(null);
mController.displayPreference(mockScreen); displayAndResume();
verify(mockForgetButton).setVisibility(View.VISIBLE); verify(mockForgetButton).setVisibility(View.VISIBLE);
} }
@Test @Test
public void canForgetNetwork_saved() { public void canForgetNetwork_saved() {
mController.displayPreference(mockScreen); displayAndResume();
verify(mockForgetButton).setVisibility(View.VISIBLE); verify(mockForgetButton).setVisibility(View.VISIBLE);
} }
@@ -590,7 +593,7 @@ public class WifiDetailPreferenceControllerTest {
when(mockWifiInfo.isEphemeral()).thenReturn(true); when(mockWifiInfo.isEphemeral()).thenReturn(true);
when(mockWifiInfo.getSSID()).thenReturn(ssid); when(mockWifiInfo.getSSID()).thenReturn(ssid);
mController.displayPreference(mockScreen); displayAndResume();
mForgetClickListener.getValue().onClick(null); mForgetClickListener.getValue().onClick(null);
verify(mockWifiManager).disableEphemeralNetwork(ssid); verify(mockWifiManager).disableEphemeralNetwork(ssid);
@@ -612,8 +615,7 @@ public class WifiDetailPreferenceControllerTest {
@Test @Test
public void networkStateChangedIntent_shouldRefetchInfo() { public void networkStateChangedIntent_shouldRefetchInfo() {
mController.displayPreference(mockScreen); displayAndResume();
mController.onResume();
verify(mockConnectivityManager, times(1)).getNetworkInfo(any(Network.class)); verify(mockConnectivityManager, times(1)).getNetworkInfo(any(Network.class));
verify(mockWifiManager, times(1)).getConnectionInfo(); verify(mockWifiManager, times(1)).getConnectionInfo();
@@ -626,8 +628,7 @@ public class WifiDetailPreferenceControllerTest {
@Test @Test
public void rssiChangedIntent_shouldRefetchInfo() { public void rssiChangedIntent_shouldRefetchInfo() {
mController.displayPreference(mockScreen); displayAndResume();
mController.onResume();
verify(mockConnectivityManager, times(1)).getNetworkInfo(any(Network.class)); verify(mockConnectivityManager, times(1)).getNetworkInfo(any(Network.class));
verify(mockWifiManager, times(1)).getConnectionInfo(); verify(mockWifiManager, times(1)).getConnectionInfo();
@@ -640,7 +641,7 @@ public class WifiDetailPreferenceControllerTest {
@Test @Test
public void networkDisconnectedState_shouldFinishActivity() { public void networkDisconnectedState_shouldFinishActivity() {
mController.onResume(); displayAndResume();
when(mockConnectivityManager.getNetworkInfo(any(Network.class))).thenReturn(null); when(mockConnectivityManager.getNetworkInfo(any(Network.class))).thenReturn(null);
mContext.sendBroadcast(new Intent(WifiManager.NETWORK_STATE_CHANGED_ACTION)); mContext.sendBroadcast(new Intent(WifiManager.NETWORK_STATE_CHANGED_ACTION));
@@ -650,8 +651,7 @@ public class WifiDetailPreferenceControllerTest {
@Test @Test
public void networkOnLost_shouldFinishActivity() { public void networkOnLost_shouldFinishActivity() {
mController.displayPreference(mockScreen); displayAndResume();
mController.onResume();
mCallbackCaptor.getValue().onLost(mockNetwork); mCallbackCaptor.getValue().onLost(mockNetwork);
@@ -664,7 +664,7 @@ public class WifiDetailPreferenceControllerTest {
mLinkProperties.addLinkAddress(Constants.IPV6_GLOBAL1); mLinkProperties.addLinkAddress(Constants.IPV6_GLOBAL1);
mLinkProperties.addLinkAddress(Constants.IPV6_GLOBAL2); mLinkProperties.addLinkAddress(Constants.IPV6_GLOBAL2);
mController.displayPreference(mockScreen); displayAndResume();
List <Preference> addrs = mIpv6AddressCaptor.getAllValues(); List <Preference> addrs = mIpv6AddressCaptor.getAllValues();
@@ -680,7 +680,7 @@ public class WifiDetailPreferenceControllerTest {
public void ipv6AddressPref_shouldNotBeSelectable() { public void ipv6AddressPref_shouldNotBeSelectable() {
mLinkProperties.addLinkAddress(Constants.IPV6_GLOBAL2); mLinkProperties.addLinkAddress(Constants.IPV6_GLOBAL2);
mController.displayPreference(mockScreen); displayAndResume();
assertThat(mockIpv6AddressesPref.isSelectable()).isFalse(); assertThat(mockIpv6AddressesPref.isSelectable()).isFalse();
} }
@@ -689,8 +689,7 @@ public class WifiDetailPreferenceControllerTest {
public void captivePortal_shouldShowSignInButton() { public void captivePortal_shouldShowSignInButton() {
InOrder inOrder = inOrder(mockSignInButton); InOrder inOrder = inOrder(mockSignInButton);
mController.displayPreference(mockScreen); displayAndResume();
mController.onResume();
inOrder.verify(mockSignInButton).setVisibility(View.INVISIBLE); inOrder.verify(mockSignInButton).setVisibility(View.INVISIBLE);
@@ -709,7 +708,7 @@ public class WifiDetailPreferenceControllerTest {
@Test @Test
public void testSignInButton_shouldStartCaptivePortalApp() { public void testSignInButton_shouldStartCaptivePortalApp() {
mController.displayPreference(mockScreen); displayAndResume();
ArgumentCaptor<OnClickListener> captor = ArgumentCaptor.forClass(OnClickListener.class); ArgumentCaptor<OnClickListener> captor = ArgumentCaptor.forClass(OnClickListener.class);
verify(mockSignInButton).setOnClickListener(captor.capture()); verify(mockSignInButton).setOnClickListener(captor.capture());
@@ -722,7 +721,7 @@ public class WifiDetailPreferenceControllerTest {
when(mockSignInButton.getVisibility()).thenReturn(View.VISIBLE); when(mockSignInButton.getVisibility()).thenReturn(View.VISIBLE);
when(mockForgetButton.getVisibility()).thenReturn(View.INVISIBLE); when(mockForgetButton.getVisibility()).thenReturn(View.INVISIBLE);
mController.displayPreference(mockScreen); displayAndResume();
verify(mockButtonsPref).setVisible(true); verify(mockButtonsPref).setVisible(true);
} }
@@ -732,7 +731,7 @@ public class WifiDetailPreferenceControllerTest {
when(mockSignInButton.getVisibility()).thenReturn(View.INVISIBLE); when(mockSignInButton.getVisibility()).thenReturn(View.INVISIBLE);
when(mockForgetButton.getVisibility()).thenReturn(View.VISIBLE); when(mockForgetButton.getVisibility()).thenReturn(View.VISIBLE);
mController.displayPreference(mockScreen); displayAndResume();
verify(mockButtonsPref).setVisible(true); verify(mockButtonsPref).setVisible(true);
} }
@@ -742,7 +741,7 @@ public class WifiDetailPreferenceControllerTest {
when(mockSignInButton.getVisibility()).thenReturn(View.INVISIBLE); when(mockSignInButton.getVisibility()).thenReturn(View.INVISIBLE);
when(mockForgetButton.getVisibility()).thenReturn(View.INVISIBLE); when(mockForgetButton.getVisibility()).thenReturn(View.INVISIBLE);
mController.displayPreference(mockScreen); displayAndResume();
verify(mockButtonsPref).setVisible(false); verify(mockButtonsPref).setVisible(false);
} }