Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 79e2dd2a authored by Lorenzo Colitti's avatar Lorenzo Colitti Committed by Automerger Merge Worker
Browse files

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

Merge "Fix legacy APIs when VPN switches to suspended underlying network." am: 4452f55a am: f8bac85e am: 2a0bb42e am: a1accd44 am: da229e3b

Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/1547497

MUST ONLY BE SUBMITTED BY AUTOMERGER

Change-Id: I51169d06d7b6c18e49f36af2b587b1fc960d0f83
parents 16773571 da229e3b

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