LineageOS/android_packages_modules_Connectivity
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge "Fix legacy APIs when VPN switches to suspended underlying network."

Browse Source
This commit is contained in:
Lorenzo Colitti
2021-01-28 07:07:37 +00:00
committed by Gerrit Code Review
parent 32f8bec5ae 4484070163
commit 61a13bb86d
2 changed files with 30 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
services/core/java/com/android/server/ConnectivityService.java
View File

@@ -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();

29
tests/net/java/com/android/server/ConnectivityServiceTest.java
View File

@@ -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