Merge "Fix legacy APIs when VPN switches to suspended underlying network." (4452f55a) · Commits · e / os / android_frameworks_base

services/core/java/com/android/server/ConnectivityService.java

+20 −13

Original line number	Diff line number	Diff line
		@@ -6684,6 +6684,25 @@ public class ConnectivityService extends IConnectivityManager.Stub
		return newNc;
		}

		private void updateNetworkInfoForRoamingAndSuspended(NetworkAgentInfo nai,
		NetworkCapabilities prevNc, NetworkCapabilities newNc) {
		final boolean prevSuspended = !prevNc.hasCapability(NET_CAPABILITY_NOT_SUSPENDED);
		final boolean suspended = !newNc.hasCapability(NET_CAPABILITY_NOT_SUSPENDED);
		final boolean prevRoaming = !prevNc.hasCapability(NET_CAPABILITY_NOT_ROAMING);
		final boolean roaming = !newNc.hasCapability(NET_CAPABILITY_NOT_ROAMING);
		if (prevSuspended != suspended) {
		// TODO (b/73132094) : remove this call once the few users of onSuspended and
		// onResumed have been removed.
		notifyNetworkCallbacks(nai, suspended ? ConnectivityManager.CALLBACK_SUSPENDED
		: ConnectivityManager.CALLBACK_RESUMED);
		}
		if (prevSuspended != suspended \|\| prevRoaming != roaming) {
		// updateNetworkInfo will mix in the suspended info from the capabilities and
		// take appropriate action for the network having possibly changed state.
		updateNetworkInfo(nai, nai.networkInfo);
		}
		}

		/**
		* Update the NetworkCapabilities for {@code nai} to {@code nc}. Specifically:
		*
		@@ -6715,25 +6734,13 @@ public class ConnectivityService extends IConnectivityManager.Stub
		// on this network. We might have been called by rematchNetworkAndRequests when a
		// network changed foreground state.
		processListenRequests(nai);
		final boolean prevSuspended = !prevNc.hasCapability(NET_CAPABILITY_NOT_SUSPENDED);
		final boolean suspended = !newNc.hasCapability(NET_CAPABILITY_NOT_SUSPENDED);
		final boolean prevRoaming = !prevNc.hasCapability(NET_CAPABILITY_NOT_ROAMING);
		final boolean roaming = !newNc.hasCapability(NET_CAPABILITY_NOT_ROAMING);
		if (prevSuspended != suspended \|\| prevRoaming != roaming) {
		// TODO (b/73132094) : remove this call once the few users of onSuspended and
		// onResumed have been removed.
		notifyNetworkCallbacks(nai, suspended ? ConnectivityManager.CALLBACK_SUSPENDED
		: ConnectivityManager.CALLBACK_RESUMED);
		// updateNetworkInfo will mix in the suspended info from the capabilities and
		// take appropriate action for the network having possibly changed state.
		updateNetworkInfo(nai, nai.networkInfo);
		}
		} else {
		// If the requestable capabilities have changed or the score changed, we can't have been
		// called by rematchNetworkAndRequests, so it's safe to start a rematch.
		rematchAllNetworksAndRequests();
		notifyNetworkCallbacks(nai, ConnectivityManager.CALLBACK_CAP_CHANGED);
		}
		updateNetworkInfoForRoamingAndSuspended(nai, prevNc, newNc);

		final boolean oldMetered = prevNc.isMetered();
		final boolean newMetered = newNc.isMetered();

tests/net/java/com/android/server/ConnectivityServiceTest.java

+10 −19

Original line number	Diff line number	Diff line
		@@ -5963,23 +5963,18 @@ public class ConnectivityServiceTest {
		callback.expectCapabilitiesThat(mMockVpn,
		nc -> nc.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)
		&& nc.hasTransport(TRANSPORT_WIFI));

		// BUG: the VPN is no longer suspended, so a RESUMED callback should have been sent.
		// callback.expectCallback(CallbackEntry.RESUMED, mMockVpn);
		callback.expectCallback(CallbackEntry.RESUMED, mMockVpn);
		callback.assertNoCallback();

		assertTrue(mCm.getNetworkCapabilities(mMockVpn.getNetwork())
		.hasCapability(NET_CAPABILITY_NOT_SUSPENDED));
		assertNetworkInfo(TYPE_MOBILE, DetailedState.DISCONNECTED);
		assertNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED);
		assertNetworkInfo(TYPE_VPN, DetailedState.SUSPENDED); // BUG: VPN caps have NOT_SUSPENDED.
		assertNetworkInfo(TYPE_VPN, DetailedState.CONNECTED);
		assertActiveNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED);
		// BUG: the device has connectivity, so this should return true.
		assertGetNetworkInfoOfGetActiveNetworkIsConnected(false);
		assertGetNetworkInfoOfGetActiveNetworkIsConnected(true);

		// Unsuspend cellular and then switch back to it.
		// The same bug happens in the opposite direction: the VPN's capabilities correctly have
		// NOT_SUSPENDED, but the VPN's NetworkInfo is in state SUSPENDED.
		// Unsuspend cellular and then switch back to it. The VPN remains not suspended.
		mCellNetworkAgent.resume();
		callback.assertNoCallback();
		mWiFiNetworkAgent.disconnect();
		@@ -5996,12 +5991,11 @@ public class ConnectivityServiceTest {
		.hasCapability(NET_CAPABILITY_NOT_SUSPENDED));
		assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
		assertNetworkInfo(TYPE_WIFI, DetailedState.DISCONNECTED);
		assertNetworkInfo(TYPE_VPN, DetailedState.SUSPENDED); // BUG: VPN caps have NOT_SUSPENDED.
		assertNetworkInfo(TYPE_VPN, DetailedState.CONNECTED);
		assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
		// BUG: the device has connectivity, so this should return true.
		assertGetNetworkInfoOfGetActiveNetworkIsConnected(false);
		assertGetNetworkInfoOfGetActiveNetworkIsConnected(true);

		// Re-suspending the current network fixes the problem.
		// Suspend cellular and expect no connectivity.
		mCellNetworkAgent.suspend();
		callback.expectCapabilitiesThat(mMockVpn,
		nc -> !nc.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)
		@@ -6017,6 +6011,7 @@ public class ConnectivityServiceTest {
		assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.SUSPENDED);
		assertGetNetworkInfoOfGetActiveNetworkIsConnected(false);

		// Resume cellular and expect that connectivity comes back.
		mCellNetworkAgent.resume();
		callback.expectCapabilitiesThat(mMockVpn,
		nc -> nc.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)
		@@ -6407,10 +6402,7 @@ public class ConnectivityServiceTest {
		&& caps.hasTransport(TRANSPORT_CELLULAR)
		&& !caps.hasCapability(NET_CAPABILITY_NOT_METERED)
		&& !caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED));
		// While the SUSPENDED callback should in theory be sent here, it is not. This is
		// a bug in ConnectivityService, but as the SUSPENDED and RESUMED callbacks have never
		// been public and are deprecated and slated for removal, there is no sense in spending
		// resources fixing this bug now.
		vpnNetworkCallback.expectCallback(CallbackEntry.SUSPENDED, mMockVpn);
		assertDefaultNetworkCapabilities(userId, mCellNetworkAgent, mWiFiNetworkAgent);

		// Use both again.
		@@ -6422,8 +6414,7 @@ public class ConnectivityServiceTest {
		&& caps.hasTransport(TRANSPORT_CELLULAR) && caps.hasTransport(TRANSPORT_WIFI)
		&& !caps.hasCapability(NET_CAPABILITY_NOT_METERED)
		&& caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED));
		// As above, the RESUMED callback not being sent here is a bug, but not a bug that's
		// worth anybody's time to fix.
		vpnNetworkCallback.expectCallback(CallbackEntry.RESUMED, mMockVpn);
		assertDefaultNetworkCapabilities(userId, mCellNetworkAgent, mWiFiNetworkAgent);

		// Disconnect cell. Receive update without even removing the dead network from the