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

Commit da229e3b 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

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

MUST ONLY BE SUBMITTED BY AUTOMERGER

Change-Id: I9119af32c3676823d8820b0cf513dde645a88be0
parents 2aad5ce7 a1accd44

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