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

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

Merge "Unbreak extraInfo values returned to apps." am: 78cf2f60 am: 4fe7f5bf

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

MUST ONLY BE SUBMITTED BY AUTOMERGER

Change-Id: I99ea36c50a38dfc54b69ec5ad46d11d50e458925
parents 0ce87eac 4fe7f5bf
Loading
Loading
Loading
Loading
+31 −12
Original line number Diff line number Diff line
@@ -1514,11 +1514,11 @@ public class ConnectivityService extends IConnectivityManager.Stub
        // but only exists if an app asks about them or requests them. Ensure the requesting app
        // gets the type it asks for.
        filtered.setType(type);
        final DetailedState state = isNetworkWithCapabilitiesBlocked(nc, uid, ignoreBlocked)
                ? DetailedState.BLOCKED
                : filtered.getDetailedState();
        filtered.setDetailedState(getLegacyLockdownState(state),
                "" /* reason */, null /* extraInfo */);
        if (isNetworkWithCapabilitiesBlocked(nc, uid, ignoreBlocked)) {
            filtered.setDetailedState(DetailedState.BLOCKED, null /* reason */,
                    null /* extraInfo */);
        }
        filterForLegacyLockdown(filtered);
        return filtered;
    }

@@ -1594,8 +1594,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
        final DetailedState state = isNetworkWithCapabilitiesBlocked(nc, uid, false)
                ? DetailedState.BLOCKED
                : DetailedState.DISCONNECTED;
        info.setDetailedState(getLegacyLockdownState(state),
                "" /* reason */, null /* extraInfo */);
        info.setDetailedState(state, null /* reason */, null /* extraInfo */);
        filterForLegacyLockdown(info);
        return info;
    }

@@ -2389,9 +2389,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
        mContext.enforceCallingOrSelfPermission(KeepaliveTracker.PERMISSION, "ConnectivityService");
    }

    // Public because it's used by mLockdownTracker.
    public void sendConnectedBroadcast(NetworkInfo info) {
        PermissionUtils.enforceNetworkStackPermission(mContext);
    private void sendConnectedBroadcast(NetworkInfo info) {
        sendGeneralBroadcast(info, CONNECTIVITY_ACTION);
    }

@@ -5036,8 +5034,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
        // The legacy lockdown VPN always uses the default network.
        // If the VPN's underlying network is no longer the current default network, it means that
        // the default network has just switched, and the VPN is about to disconnect.
        // Report that the VPN is not connected, so when the state of NetworkInfo objects
        // overwritten by getLegacyLockdownState will be set to CONNECTING and not CONNECTED.
        // Report that the VPN is not connected, so the state of NetworkInfo objects overwritten
        // by filterForLegacyLockdown will be set to CONNECTING and not CONNECTED.
        final NetworkAgentInfo defaultNetwork = getDefaultNetwork();
        if (defaultNetwork == null || !defaultNetwork.network.equals(underlying[0])) {
            return null;
@@ -5046,6 +5044,9 @@ public class ConnectivityService extends IConnectivityManager.Stub
        return nai;
    };

    // TODO: move all callers to filterForLegacyLockdown and delete this method.
    // This likely requires making sendLegacyNetworkBroadcast take a NetworkInfo object instead of
    // just a DetailedState object.
    private DetailedState getLegacyLockdownState(DetailedState origState) {
        if (origState != DetailedState.CONNECTED) {
            return origState;
@@ -5055,6 +5056,23 @@ public class ConnectivityService extends IConnectivityManager.Stub
                : DetailedState.CONNECTED;
    }

    private void filterForLegacyLockdown(NetworkInfo ni) {
        if (!mLockdownEnabled || !ni.isConnected()) return;
        // The legacy lockdown VPN replaces the state of every network in CONNECTED state with the
        // state of its VPN. This is to ensure that when an underlying network connects, apps will
        // not see a CONNECTIVITY_ACTION broadcast for a network in state CONNECTED until the VPN
        // comes up, at which point there is a new CONNECTIVITY_ACTION broadcast for the underlying
        // network, this time with a state of CONNECTED.
        //
        // Now that the legacy lockdown code lives in ConnectivityService, and no longer has access
        // to the internal state of the Vpn object, always replace the state with CONNECTING. This
        // is not too far off the truth, since an always-on VPN, when not connected, is always
        // trying to reconnect.
        if (getLegacyLockdownNai() == null) {
            ni.setDetailedState(DetailedState.CONNECTING, "", null);
        }
    }

    @Override
    public void setProvisioningNotificationVisible(boolean visible, int networkType,
            String action) {
@@ -7938,6 +7956,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
        // and is still connected.
        NetworkInfo info = new NetworkInfo(nai.networkInfo);
        info.setType(type);
        filterForLegacyLockdown(info);
        if (state != DetailedState.DISCONNECTED) {
            info.setDetailedState(state, null, info.getExtraInfo());
            sendConnectedBroadcast(info);
+66 −4
Original line number Diff line number Diff line
@@ -1740,11 +1740,29 @@ public class ConnectivityServiceTest {
        return expected;
    }
    private boolean extraInfoInBroadcastHasExpectedNullness(NetworkInfo ni) {
        final DetailedState state = ni.getDetailedState();
        if (state == DetailedState.CONNECTED && ni.getExtraInfo() == null) return false;
        // Expect a null extraInfo if the network is CONNECTING, because a CONNECTIVITY_ACTION
        // broadcast with a state of CONNECTING only happens due to legacy VPN lockdown, which also
        // nulls out extraInfo.
        if (state == DetailedState.CONNECTING && ni.getExtraInfo() != null) return false;
        // Can't make any assertions about DISCONNECTED broadcasts. When a network actually
        // disconnects, disconnectAndDestroyNetwork sets its state to DISCONNECTED and its extraInfo
        // to null. But if the DISCONNECTED broadcast is just simulated by LegacyTypeTracker due to
        // a network switch, extraInfo will likely be populated.
        // This is likely a bug in CS, but likely not one we can fix without impacting apps.
        return true;
    }
    private ExpectedBroadcast expectConnectivityAction(int type, NetworkInfo.DetailedState state) {
        return registerConnectivityBroadcastThat(1, intent ->
                type == intent.getIntExtra(EXTRA_NETWORK_TYPE, -1) && state.equals(
                        ((NetworkInfo) intent.getParcelableExtra(EXTRA_NETWORK_INFO))
                                .getDetailedState()));
        return registerConnectivityBroadcastThat(1, intent -> {
            final int actualType = intent.getIntExtra(EXTRA_NETWORK_TYPE, -1);
            final NetworkInfo ni = intent.getParcelableExtra(EXTRA_NETWORK_INFO);
            return type == actualType
                    && state == ni.getDetailedState()
                    && extraInfoInBroadcastHasExpectedNullness(ni);
        });
    }
    @Test
@@ -7182,12 +7200,14 @@ public class ConnectivityServiceTest {
        assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork());
        assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
        assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
        assertExtraInfoFromCmPresent(mCellNetworkAgent);
        setUidRulesChanged(RULE_REJECT_ALL);
        cellNetworkCallback.expectBlockedStatusCallback(true, mCellNetworkAgent);
        assertNull(mCm.getActiveNetwork());
        assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED);
        assertNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED);
        assertExtraInfoFromCmBlocked(mCellNetworkAgent);
        // ConnectivityService should cache it not to invoke the callback again.
        setUidRulesChanged(RULE_REJECT_METERED);
@@ -7198,12 +7218,14 @@ public class ConnectivityServiceTest {
        assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork());
        assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
        assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
        assertExtraInfoFromCmPresent(mCellNetworkAgent);
        setUidRulesChanged(RULE_REJECT_METERED);
        cellNetworkCallback.expectBlockedStatusCallback(true, mCellNetworkAgent);
        assertNull(mCm.getActiveNetwork());
        assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED);
        assertNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED);
        assertExtraInfoFromCmBlocked(mCellNetworkAgent);
        // Restrict the network based on UID rule and NOT_METERED capability change.
        mCellNetworkAgent.addCapability(NET_CAPABILITY_NOT_METERED);
@@ -7212,6 +7234,7 @@ public class ConnectivityServiceTest {
        assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork());
        assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
        assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
        assertExtraInfoFromCmPresent(mCellNetworkAgent);
        mCellNetworkAgent.removeCapability(NET_CAPABILITY_NOT_METERED);
        cellNetworkCallback.expectCapabilitiesWithout(NET_CAPABILITY_NOT_METERED,
@@ -7220,12 +7243,14 @@ public class ConnectivityServiceTest {
        assertNull(mCm.getActiveNetwork());
        assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED);
        assertNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED);
        assertExtraInfoFromCmBlocked(mCellNetworkAgent);
        setUidRulesChanged(RULE_ALLOW_METERED);
        cellNetworkCallback.expectBlockedStatusCallback(false, mCellNetworkAgent);
        assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork());
        assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
        assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
        assertExtraInfoFromCmPresent(mCellNetworkAgent);
        setUidRulesChanged(RULE_NONE);
        cellNetworkCallback.assertNoCallback();
@@ -7236,6 +7261,7 @@ public class ConnectivityServiceTest {
        assertNull(mCm.getActiveNetwork());
        assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED);
        assertNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED);
        assertExtraInfoFromCmBlocked(mCellNetworkAgent);
        setRestrictBackgroundChanged(true);
        cellNetworkCallback.assertNoCallback();
@@ -7243,12 +7269,14 @@ public class ConnectivityServiceTest {
        cellNetworkCallback.expectBlockedStatusCallback(false, mCellNetworkAgent);
        assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
        assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
        assertExtraInfoFromCmPresent(mCellNetworkAgent);
        setRestrictBackgroundChanged(false);
        cellNetworkCallback.assertNoCallback();
        assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork());
        assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
        assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
        assertExtraInfoFromCmPresent(mCellNetworkAgent);
        mCm.unregisterNetworkCallback(cellNetworkCallback);
    }
@@ -7307,6 +7335,15 @@ public class ConnectivityServiceTest {
        assertNotNull(ni);
        assertEquals(type, ni.getType());
        assertEquals(ConnectivityManager.getNetworkTypeName(type), state, ni.getDetailedState());
        if (state == DetailedState.CONNECTED || state == DetailedState.SUSPENDED) {
            assertNotNull(ni.getExtraInfo());
        } else {
            // Technically speaking, a network that's in CONNECTING state will generally have a
            // non-null extraInfo. This doesn't actually happen in this test because it never calls
            // a legacy API while a network is connecting. When a network is in CONNECTING state
            // because of legacy lockdown VPN, its extraInfo is always null.
            assertNull(ni.getExtraInfo());
        }
    }
    private void assertActiveNetworkInfo(int type, DetailedState state) {
@@ -7316,6 +7353,26 @@ public class ConnectivityServiceTest {
        checkNetworkInfo(mCm.getNetworkInfo(type), type, state);
    }
    private void assertExtraInfoFromCm(TestNetworkAgentWrapper network, boolean present) {
        final NetworkInfo niForNetwork = mCm.getNetworkInfo(network.getNetwork());
        final NetworkInfo niForType = mCm.getNetworkInfo(network.getLegacyType());
        if (present) {
            assertEquals(network.getExtraInfo(), niForNetwork.getExtraInfo());
            assertEquals(network.getExtraInfo(), niForType.getExtraInfo());
        } else {
            assertNull(niForNetwork.getExtraInfo());
            assertNull(niForType.getExtraInfo());
        }
    }
    private void assertExtraInfoFromCmBlocked(TestNetworkAgentWrapper network) {
        assertExtraInfoFromCm(network, false);
    }
    private void assertExtraInfoFromCmPresent(TestNetworkAgentWrapper network) {
        assertExtraInfoFromCm(network, true);
    }
    // Checks that each of the |agents| receive a blocked status change callback with the specified
    // |blocked| value, in any order. This is needed because when an event affects multiple
    // networks, ConnectivityService does not guarantee the order in which callbacks are fired.
@@ -7630,6 +7687,7 @@ public class ConnectivityServiceTest {
        assertNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED);
        assertNetworkInfo(TYPE_WIFI, DetailedState.BLOCKED);
        assertNetworkInfo(TYPE_VPN, DetailedState.BLOCKED);
        assertExtraInfoFromCmBlocked(mCellNetworkAgent);
        // TODO: it would be nice if we could simply rely on the production code here, and have
        // LockdownVpnTracker start the VPN, have the VPN code register its NetworkAgent with
@@ -7658,6 +7716,7 @@ public class ConnectivityServiceTest {
        assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
        assertNetworkInfo(TYPE_WIFI, DetailedState.DISCONNECTED);
        assertNetworkInfo(TYPE_VPN, DetailedState.CONNECTED);
        assertExtraInfoFromCmPresent(mCellNetworkAgent);
        assertTrue(vpnNc.hasTransport(TRANSPORT_VPN));
        assertTrue(vpnNc.hasTransport(TRANSPORT_CELLULAR));
        assertFalse(vpnNc.hasTransport(TRANSPORT_WIFI));
@@ -7700,6 +7759,7 @@ public class ConnectivityServiceTest {
        assertNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED);
        assertNetworkInfo(TYPE_WIFI, DetailedState.BLOCKED);
        assertNetworkInfo(TYPE_VPN, DetailedState.BLOCKED);
        assertExtraInfoFromCmBlocked(mWiFiNetworkAgent);
        // The VPN comes up again on wifi.
        b1 = expectConnectivityAction(TYPE_VPN, DetailedState.CONNECTED);
@@ -7714,6 +7774,7 @@ public class ConnectivityServiceTest {
        assertNetworkInfo(TYPE_MOBILE, DetailedState.DISCONNECTED);
        assertNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED);
        assertNetworkInfo(TYPE_VPN, DetailedState.CONNECTED);
        assertExtraInfoFromCmPresent(mWiFiNetworkAgent);
        vpnNc = mCm.getNetworkCapabilities(mMockVpn.getNetwork());
        assertTrue(vpnNc.hasTransport(TRANSPORT_VPN));
        assertTrue(vpnNc.hasTransport(TRANSPORT_WIFI));
@@ -7730,6 +7791,7 @@ public class ConnectivityServiceTest {
        assertNetworkInfo(TYPE_MOBILE, DetailedState.DISCONNECTED);
        assertNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED);
        assertNetworkInfo(TYPE_VPN, DetailedState.CONNECTED);
        assertExtraInfoFromCmPresent(mWiFiNetworkAgent);
        b1 = expectConnectivityAction(TYPE_WIFI, DetailedState.DISCONNECTED);
        mWiFiNetworkAgent.disconnect();