From f2893a20c5ce5094b3838778de157943254b3586 Mon Sep 17 00:00:00 2001 From: David Su Date: Fri, 5 Jun 2020 17:30:17 -0700 Subject: WifiManager#getCurrentNetwork: return null if not L3 connected According to the API contract of Network, Wifi should not expose a Network object before it is L3 connected. Previously, getCurrentNetwork returned a non-null Network object after L2ConnectedState. Changed to only return a Network object in ConnectedState/RoamingState. Return null in all other states. Note that getCurrentNetwork was marked as @Nullable and did return null when Wifi was disconnected, so callers should be able to handle this change in behavior. Bug: 150188453 Test: connect to network using SUW Test: connect/disconnect to network using Settings Test: atest ClientModeImplTest Change-Id: Ib65664b7925176bc34287d37bcd1ba1085c351a4 Merged-In: Ib65664b7925176bc34287d37bcd1ba1085c351a4 (dirty cherry-pick from master) --- .../com/android/server/wifi/ClientModeImpl.java | 67 +++++++++++++--------- .../com/android/server/wifi/WifiServiceImpl.java | 2 +- .../android/server/wifi/ClientModeImplTest.java | 22 +++++-- 3 files changed, 59 insertions(+), 32 deletions(-) diff --git a/service/java/com/android/server/wifi/ClientModeImpl.java b/service/java/com/android/server/wifi/ClientModeImpl.java index e10d5367e..063f9a4c5 100644 --- a/service/java/com/android/server/wifi/ClientModeImpl.java +++ b/service/java/com/android/server/wifi/ClientModeImpl.java @@ -107,7 +107,6 @@ import android.util.Log; import android.util.Pair; import android.util.SparseArray; -import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.AsyncChannel; import com.android.internal.util.MessageUtils; @@ -445,9 +444,7 @@ public class ClientModeImpl extends StateMachine { private WifiNetworkFactory mNetworkFactory; private UntrustedWifiNetworkFactory mUntrustedNetworkFactory; - @GuardedBy("mNetworkAgentLock") private WifiNetworkAgent mNetworkAgent; - private final Object mNetworkAgentLock = new Object(); private byte[] mRssiRanges; @@ -630,6 +627,9 @@ public class ClientModeImpl extends StateMachine { /* Start connection to FILS AP*/ static final int CMD_START_FILS_CONNECTION = BASE + 262; + + private static final int CMD_GET_CURRENT_NETWORK = BASE + 263; + // For message logging. private static final Class[] sMessageClasses = { AsyncChannel.class, ClientModeImpl.class }; @@ -1785,19 +1785,29 @@ public class ClientModeImpl extends StateMachine { } /** - * Get Network object of current wifi network - * @return Network object of current wifi network + * Should only be used internally. + * External callers should use {@link #syncGetCurrentNetwork(AsyncChannel)}. */ - public Network getCurrentNetwork() { - synchronized (mNetworkAgentLock) { - if (mNetworkAgent != null) { - return mNetworkAgent.getNetwork(); - } else { - return null; - } + private Network getCurrentNetwork() { + if (mNetworkAgent != null) { + return mNetworkAgent.getNetwork(); + } else { + return null; } } + /** + * Get Network object of currently connected wifi network, or null if not connected. + * @return Network object of current wifi network + */ + public Network syncGetCurrentNetwork(AsyncChannel channel) { + Message resultMsg = channel.sendMessageSynchronously(CMD_GET_CURRENT_NETWORK); + if (messageIsNull(resultMsg)) return null; + Network network = (Network) resultMsg.obj; + resultMsg.recycle(); + return network; + } + /** * Enable TDLS for a specific MAC address */ @@ -2757,11 +2767,9 @@ public class ClientModeImpl extends StateMachine { mIsAutoRoaming = false; sendNetworkChangeBroadcast(DetailedState.DISCONNECTED); - synchronized (mNetworkAgentLock) { - if (mNetworkAgent != null) { - mNetworkAgent.unregister(); - mNetworkAgent = null; - } + if (mNetworkAgent != null) { + mNetworkAgent.unregister(); + mNetworkAgent = null; } /* Clear network properties */ @@ -3492,7 +3500,8 @@ public class ClientModeImpl extends StateMachine { replyToMessage(message, message.what, Long.valueOf(featureSet)); break; case CMD_GET_LINK_LAYER_STATS: - // Not supported hence reply with error message + case CMD_GET_CURRENT_NETWORK: + // Not supported hence reply with null message.obj replyToMessage(message, message.what, null); break; case WifiP2pServiceImpl.P2P_CONNECTION_CHANGED: @@ -4752,16 +4761,14 @@ public class ClientModeImpl extends StateMachine { .setPartialConnectivityAcceptable(config.noInternetAccessExpected) .build(); final NetworkCapabilities nc = getCapabilities(getCurrentWifiConfiguration()); - synchronized (mNetworkAgentLock) { - // This should never happen. - if (mNetworkAgent != null) { - Log.wtf(TAG, "mNetworkAgent is not null: " + mNetworkAgent); - mNetworkAgent.unregister(); - } - mNetworkAgent = new WifiNetworkAgent(mContext, getHandler().getLooper(), - "WifiNetworkAgent", nc, mLinkProperties, 60, naConfig, - mNetworkFactory.getProvider()); + // This should never happen. + if (mNetworkAgent != null) { + Log.wtf(TAG, "mNetworkAgent is not null: " + mNetworkAgent); + mNetworkAgent.unregister(); } + mNetworkAgent = new WifiNetworkAgent(mContext, getHandler().getLooper(), + "WifiNetworkAgent", nc, mLinkProperties, 60, naConfig, + mNetworkFactory.getProvider()); mWifiScoreReport.setNetworkAgent(mNetworkAgent); // We must clear the config BSSID, as the wifi chipset may decide to roam @@ -5317,6 +5324,9 @@ public class ClientModeImpl extends StateMachine { transitionTo(mDisconnectedState); } break; + case CMD_GET_CURRENT_NETWORK: + replyToMessage(message, message.what, getCurrentNetwork()); + break; default: handleStatus = NOT_HANDLED; break; @@ -5541,6 +5551,9 @@ public class ClientModeImpl extends StateMachine { mWifiMetrics.incrementIpRenewalFailure(); handleStatus = NOT_HANDLED; break; + case CMD_GET_CURRENT_NETWORK: + replyToMessage(message, message.what, getCurrentNetwork()); + break; default: handleStatus = NOT_HANDLED; break; diff --git a/service/java/com/android/server/wifi/WifiServiceImpl.java b/service/java/com/android/server/wifi/WifiServiceImpl.java index 52fb61fdb..070422775 100644 --- a/service/java/com/android/server/wifi/WifiServiceImpl.java +++ b/service/java/com/android/server/wifi/WifiServiceImpl.java @@ -3497,7 +3497,7 @@ public class WifiServiceImpl extends BaseWifiService { if (mVerboseLoggingEnabled) { mLog.info("getCurrentNetwork uid=%").c(Binder.getCallingUid()).flush(); } - return mClientModeImpl.getCurrentNetwork(); + return mClientModeImpl.syncGetCurrentNetwork(mClientModeImplChannel); } public static String toHexString(String s) { diff --git a/tests/wifitests/src/com/android/server/wifi/ClientModeImplTest.java b/tests/wifitests/src/com/android/server/wifi/ClientModeImplTest.java index 20766898d..58f5b1ccc 100644 --- a/tests/wifitests/src/com/android/server/wifi/ClientModeImplTest.java +++ b/tests/wifitests/src/com/android/server/wifi/ClientModeImplTest.java @@ -421,6 +421,7 @@ public class ClientModeImplTest extends WifiBaseTest { @Mock ThroughputPredictor mThroughputPredictor; @Mock ScanRequestProxy mScanRequestProxy; @Mock DeviceConfigFacade mDeviceConfigFacade; + @Mock Network mNetwork; final ArgumentCaptor mConfigUpdateListenerCaptor = ArgumentCaptor.forClass(WifiConfigManager.OnNetworkUpdateListener.class); @@ -533,10 +534,8 @@ public class ClientModeImplTest extends WifiBaseTest { return null; }).when(mIpClient).shutdown(); when(mConnectivityManager.registerNetworkAgent(any(), any(), any(), any(), anyInt(), any(), - anyInt())).thenReturn(mock(Network.class)); - List subList = new ArrayList<>() {{ - add(mock(SubscriptionInfo.class)); - }}; + anyInt())).thenReturn(mNetwork); + List subList = Arrays.asList(mock(SubscriptionInfo.class)); when(mSubscriptionManager.getActiveSubscriptionInfoList()).thenReturn(subList); when(mSubscriptionManager.getActiveSubscriptionIdList()) .thenReturn(new int[]{DATA_SUBID}); @@ -5266,4 +5265,19 @@ public class ClientModeImplTest extends WifiBaseTest { verifyNoMoreInteractions(mNetworkAgentHandler); } + + @Test + public void testSyncGetCurrentNetwork() throws Exception { + // syncGetCurrentNetwork() returns null when disconnected + mLooper.startAutoDispatch(); + assertNull(mCmi.syncGetCurrentNetwork(mCmiAsyncChannel)); + mLooper.stopAutoDispatch(); + + connect(); + + // syncGetCurrentNetwork() returns non-null Network when connected + mLooper.startAutoDispatch(); + assertEquals(mNetwork, mCmi.syncGetCurrentNetwork(mCmiAsyncChannel)); + mLooper.stopAutoDispatch(); + } } -- cgit v1.2.3