Merge changes from topic 'show-deleted-vpn'

* changes:
  VpnSettings PreferenceList tests
  VpnSettings: slightly more robust callback context
  VpnSettings: show connected VPN even if deleted

src/com/android/settings/vpn2/ConfigDialogFragment.java

@@ -55,6 +55,7 @@ public class ConfigDialogFragment extends InstrumentedDialogFragment implements
 
     private final IConnectivityManager mService = IConnectivityManager.Stub.asInterface(
             ServiceManager.getService(Context.CONNECTIVITY_SERVICE));
+    private Context mContext;
 
     private boolean mUnlocking = false;
 
@@ -78,6 +79,12 @@ public class ConfigDialogFragment extends InstrumentedDialogFragment implements
         frag.show(parent.getFragmentManager(), TAG_CONFIG_DIALOG);
     }
 
+    @Override
+    public void onAttach(final Context context) {
+        super.onAttach(context);
+        mContext = context;
+    }
+
     @Override
     public void onResume() {
         super.onResume();
@@ -86,7 +93,7 @@ public class ConfigDialogFragment extends InstrumentedDialogFragment implements
         if (!KeyStore.getInstance().isUnlocked()) {
             if (!mUnlocking) {
                 // Let us unlock KeyStore. See you later!
-                Credentials.getInstance().unlock(getActivity());
+                Credentials.getInstance().unlock(mContext);
             } else {
                 // We already tried, but it is still not working!
                 dismiss();
@@ -142,9 +149,9 @@ public class ConfigDialogFragment extends InstrumentedDialogFragment implements
             // Possibly throw up a dialog to explain lockdown VPN.
             final boolean shouldLockdown = dialog.isVpnAlwaysOn();
             final boolean shouldConnect = shouldLockdown || !dialog.isEditing();
-            final boolean wasAlwaysOn = VpnUtils.isAlwaysOnOrLegacyLockdownActive(getContext());
+            final boolean wasAlwaysOn = VpnUtils.isAlwaysOnOrLegacyLockdownActive(mContext);
             try {
-                final boolean replace = VpnUtils.isVpnActive(getContext());
+                final boolean replace = VpnUtils.isVpnActive(mContext);
                 if (shouldConnect && !isConnected(profile) &&
                         ConfirmLockdownFragment.shouldShow(replace, wasAlwaysOn, shouldLockdown)) {
                     final Bundle opts = new Bundle();
@@ -185,19 +192,19 @@ public class ConfigDialogFragment extends InstrumentedDialogFragment implements
         if (isVpnAlwaysOn) {
             // Show toast if vpn profile is not valid
             if (!profile.isValidLockdownProfile()) {
-                Toast.makeText(getContext(), R.string.vpn_lockdown_config_error,
+                Toast.makeText(mContext, R.string.vpn_lockdown_config_error,
                         Toast.LENGTH_LONG).show();
                 return;
             }
 
-            final ConnectivityManager conn = ConnectivityManager.from(getActivity());
+            final ConnectivityManager conn = ConnectivityManager.from(mContext);
             conn.setAlwaysOnVpnPackageForUser(UserHandle.myUserId(), null,
                     /* lockdownEnabled */ false);
-            VpnUtils.setLockdownVpn(getContext(), profile.key);
+            VpnUtils.setLockdownVpn(mContext, profile.key);
         } else {
             // update only if lockdown vpn has been changed
             if (VpnUtils.isVpnLockdown(profile.key)) {
-                VpnUtils.clearLockdownVpn(getContext());
+                VpnUtils.clearLockdownVpn(mContext);
             }
         }
     }
@@ -219,11 +226,11 @@ public class ConfigDialogFragment extends InstrumentedDialogFragment implements
         // Now try to start the VPN - this is not necessary if the profile is set as lockdown,
         // because just saving the profile in this mode will start a connection.
         if (!VpnUtils.isVpnLockdown(profile.key)) {
-            VpnUtils.clearLockdownVpn(getContext());
+            VpnUtils.clearLockdownVpn(mContext);
             try {
                 mService.startLegacyVpn(profile);
             } catch (IllegalStateException e) {
-                Toast.makeText(getActivity(), R.string.vpn_no_network, Toast.LENGTH_LONG).show();
+                Toast.makeText(mContext, R.string.vpn_no_network, Toast.LENGTH_LONG).show();
             } catch (RemoteException e) {
                 Log.e(TAG, "Failed to connect", e);
             }
@@ -241,7 +248,7 @@ public class ConfigDialogFragment extends InstrumentedDialogFragment implements
             if (!isConnected(profile)) {
                 return true;
             }
-            VpnUtils.clearLockdownVpn(getContext());
+            VpnUtils.clearLockdownVpn(mContext);
             return mService.prepareVpn(null, VpnConfig.LEGACY_VPN, UserHandle.myUserId());
         } catch (RemoteException e) {
             Log.e(TAG, "Failed to disconnect", e);

src/com/android/settings/vpn2/VpnSettings.java

@@ -49,6 +49,8 @@ import android.view.Menu;
 import android.view.MenuInflater;
 import android.view.MenuItem;
 
+import com.android.internal.annotations.GuardedBy;
+import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.logging.nano.MetricsProto.MetricsEvent;
 import com.android.internal.net.LegacyVpnInfo;
 import com.android.internal.net.VpnConfig;
@@ -62,6 +64,7 @@ import com.android.settingslib.RestrictedLockUtils;
 import com.google.android.collect.Lists;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -96,8 +99,9 @@ public class VpnSettings extends RestrictedSettingsFragment implements
     private Map<String, LegacyVpnPreference> mLegacyVpnPreferences = new ArrayMap<>();
     private Map<AppVpnInfo, AppPreference> mAppPreferences = new ArrayMap<>();
 
-    private HandlerThread mUpdaterThread;
+    @GuardedBy("this")
     private Handler mUpdater;
+    private HandlerThread mUpdaterThread;
     private LegacyVpnInfo mConnectedLegacyVpn;
 
     private boolean mUnavailable;
@@ -181,11 +185,9 @@ public class VpnSettings extends RestrictedSettingsFragment implements
         mConnectivityManager.registerNetworkCallback(VPN_REQUEST, mNetworkCallback);
 
         // Trigger a refresh
-        if (mUpdater == null) {
+        mUpdaterThread = new HandlerThread("Refresh VPN list in background");
-            mUpdaterThread = new HandlerThread("Refresh VPN list in background");
+        mUpdaterThread.start();
-            mUpdaterThread.start();
+        mUpdater = new Handler(mUpdaterThread.getLooper(), this);
-            mUpdater = new Handler(mUpdaterThread.getLooper(), this);
-        }
         mUpdater.sendEmptyMessage(RESCAN_MESSAGE);
     }
 
@@ -199,7 +201,7 @@ public class VpnSettings extends RestrictedSettingsFragment implements
         // Stop monitoring
         mConnectivityManager.unregisterNetworkCallback(mNetworkCallback);
 
-        if (mUpdater != null) {
+        synchronized (this) {
             mUpdater.removeCallbacksAndMessages(null);
             mUpdater = null;
             mUpdaterThread.quit();
@@ -211,8 +213,6 @@ public class VpnSettings extends RestrictedSettingsFragment implements
 
     @Override @WorkerThread
     public boolean handleMessage(Message message) {
-        mUpdater.removeMessages(RESCAN_MESSAGE);
-
         // Run heavy RPCs before switching to UI thread
         final List<VpnProfile> vpnProfiles = loadVpnProfiles(mKeyStore);
         final List<AppVpnInfo> vpnApps = getVpnApps(getActivity(), /* includeProfiles */ true);
@@ -224,63 +224,128 @@ public class VpnSettings extends RestrictedSettingsFragment implements
         final String lockdownVpnKey = VpnUtils.getLockdownVpn();
 
         // Refresh list of VPNs
-        getActivity().runOnUiThread(new Runnable() {
+        getActivity().runOnUiThread(new UpdatePreferences(this)
-            @Override
+                .legacyVpns(vpnProfiles, connectedLegacyVpns, lockdownVpnKey)
-            public void run() {
+                .appVpns(vpnApps, connectedAppVpns, alwaysOnAppVpnInfos));
-                // Can't do anything useful if the context has gone away
-                if (!isAdded()) {
-                    return;
-                }
 
-                // Find new VPNs by subtracting existing ones from the full set
+        synchronized (this) {
-                final Set<Preference> updates = new ArraySet<>();
+            if (mUpdater != null) {
-
+                mUpdater.removeMessages(RESCAN_MESSAGE);
-                for (VpnProfile profile : vpnProfiles) {
+                mUpdater.sendEmptyMessageDelayed(RESCAN_MESSAGE, RESCAN_INTERVAL_MS);
-                    LegacyVpnPreference p = findOrCreatePreference(profile);
-                    if (connectedLegacyVpns.containsKey(profile.key)) {
-                        p.setState(connectedLegacyVpns.get(profile.key).state);
-                    } else {
-                        p.setState(LegacyVpnPreference.STATE_NONE);
-                    }
-                    p.setAlwaysOn(lockdownVpnKey != null && lockdownVpnKey.equals(profile.key));
-                    updates.add(p);
-                }
-                for (AppVpnInfo app : vpnApps) {
-                    AppPreference p = findOrCreatePreference(app);
-                    if (connectedAppVpns.contains(app)) {
-                        p.setState(AppPreference.STATE_CONNECTED);
-                    } else {
-                        p.setState(AppPreference.STATE_DISCONNECTED);
-                    }
-                    p.setAlwaysOn(alwaysOnAppVpnInfos.contains(app));
-                    updates.add(p);
-                }
-
-                // Trim out deleted VPN preferences
-                mLegacyVpnPreferences.values().retainAll(updates);
-                mAppPreferences.values().retainAll(updates);
-
-                final PreferenceGroup vpnGroup = getPreferenceScreen();
-                for (int i = vpnGroup.getPreferenceCount() - 1; i >= 0; i--) {
-                    Preference p = vpnGroup.getPreference(i);
-                    if (updates.contains(p)) {
-                        updates.remove(p);
-                    } else {
-                        vpnGroup.removePreference(p);
-                    }
-                }
-
-                // Show any new preferences on the screen
-                for (Preference pref : updates) {
-                    vpnGroup.addPreference(pref);
-                }
             }
-        });
+        }
-
-        mUpdater.sendEmptyMessageDelayed(RESCAN_MESSAGE, RESCAN_INTERVAL_MS);
         return true;
     }
 
+    @VisibleForTesting
+    static class UpdatePreferences implements Runnable {
+        private List<VpnProfile> vpnProfiles = Collections.<VpnProfile>emptyList();
+        private List<AppVpnInfo> vpnApps = Collections.<AppVpnInfo>emptyList();
+
+        private Map<String, LegacyVpnInfo> connectedLegacyVpns =
+                Collections.<String, LegacyVpnInfo>emptyMap();
+        private Set<AppVpnInfo> connectedAppVpns = Collections.<AppVpnInfo>emptySet();
+
+        private Set<AppVpnInfo> alwaysOnAppVpnInfos = Collections.<AppVpnInfo>emptySet();
+        private String lockdownVpnKey = null;
+
+        private final VpnSettings mSettings;
+
+        public UpdatePreferences(VpnSettings settings) {
+            mSettings = settings;
+        }
+
+        public final UpdatePreferences legacyVpns(List<VpnProfile> vpnProfiles,
+                Map<String, LegacyVpnInfo> connectedLegacyVpns, String lockdownVpnKey) {
+            this.vpnProfiles = vpnProfiles;
+            this.connectedLegacyVpns = connectedLegacyVpns;
+            this.lockdownVpnKey = lockdownVpnKey;
+            return this;
+        }
+
+        public final UpdatePreferences appVpns(List<AppVpnInfo> vpnApps,
+                Set<AppVpnInfo> connectedAppVpns, Set<AppVpnInfo> alwaysOnAppVpnInfos) {
+            this.vpnApps = vpnApps;
+            this.connectedAppVpns = connectedAppVpns;
+            this.alwaysOnAppVpnInfos = alwaysOnAppVpnInfos;
+            return this;
+        }
+
+        @Override @UiThread
+        public void run() {
+            if (!mSettings.canAddPreferences()) {
+                return;
+            }
+
+            // Find new VPNs by subtracting existing ones from the full set
+            final Set<Preference> updates = new ArraySet<>();
+
+            // Add legacy VPNs
+            for (VpnProfile profile : vpnProfiles) {
+                LegacyVpnPreference p = mSettings.findOrCreatePreference(profile, true);
+                if (connectedLegacyVpns.containsKey(profile.key)) {
+                    p.setState(connectedLegacyVpns.get(profile.key).state);
+                } else {
+                    p.setState(LegacyVpnPreference.STATE_NONE);
+                }
+                p.setAlwaysOn(lockdownVpnKey != null && lockdownVpnKey.equals(profile.key));
+                updates.add(p);
+            }
+
+            // Show connected VPNs even if the original entry in keystore is gone
+            for (LegacyVpnInfo vpn : connectedLegacyVpns.values()) {
+                final VpnProfile stubProfile = new VpnProfile(vpn.key);
+                LegacyVpnPreference p = mSettings.findOrCreatePreference(stubProfile, false);
+                p.setState(vpn.state);
+                p.setAlwaysOn(lockdownVpnKey != null && lockdownVpnKey.equals(vpn.key));
+                updates.add(p);
+            }
+
+            // Add VpnService VPNs
+            for (AppVpnInfo app : vpnApps) {
+                AppPreference p = mSettings.findOrCreatePreference(app);
+                if (connectedAppVpns.contains(app)) {
+                    p.setState(AppPreference.STATE_CONNECTED);
+                } else {
+                    p.setState(AppPreference.STATE_DISCONNECTED);
+                }
+                p.setAlwaysOn(alwaysOnAppVpnInfos.contains(app));
+                updates.add(p);
+            }
+
+            // Trim out deleted VPN preferences
+            mSettings.setShownPreferences(updates);
+        }
+    }
+
+    @VisibleForTesting
+    public boolean canAddPreferences() {
+        return isAdded();
+    }
+
+    @VisibleForTesting @UiThread
+    public void setShownPreferences(final Collection<Preference> updates) {
+        mLegacyVpnPreferences.values().retainAll(updates);
+        mAppPreferences.values().retainAll(updates);
+
+        // Change {@param updates} in-place to only contain new preferences that were not already
+        // added to the preference screen.
+        final PreferenceGroup vpnGroup = getPreferenceScreen();
+        for (int i = vpnGroup.getPreferenceCount() - 1; i >= 0; i--) {
+            Preference p = vpnGroup.getPreference(i);
+            if (updates.contains(p)) {
+                updates.remove(p);
+            } else {
+                vpnGroup.removePreference(p);
+            }
+        }
+
+        // Show any new preferences on the screen
+        for (Preference pref : updates) {
+            vpnGroup.addPreference(pref);
+        }
+    }
+
     @Override
     public boolean onPreferenceClick(Preference preference) {
         if (preference instanceof LegacyVpnPreference) {
@@ -360,22 +425,26 @@ public class VpnSettings extends RestrictedSettingsFragment implements
         }
     };
 
-    @UiThread
+    @VisibleForTesting @UiThread
-    private LegacyVpnPreference findOrCreatePreference(VpnProfile profile) {
+    public LegacyVpnPreference findOrCreatePreference(VpnProfile profile, boolean update) {
         LegacyVpnPreference pref = mLegacyVpnPreferences.get(profile.key);
-        if (pref == null) {
+        boolean created = false;
+        if (pref == null ) {
             pref = new LegacyVpnPreference(getPrefContext());
             pref.setOnGearClickListener(mGearListener);
             pref.setOnPreferenceClickListener(this);
             mLegacyVpnPreferences.put(profile.key, pref);
+            created = true;
+        }
+        if (created || update) {
+            // This can change call-to-call because the profile can update and keep the same key.
+            pref.setProfile(profile);
         }
-        // This may change as the profile can update and keep the same key.
-        pref.setProfile(profile);
         return pref;
     }
 
-    @UiThread
+    @VisibleForTesting @UiThread
-    private AppPreference findOrCreatePreference(AppVpnInfo app) {
+    public AppPreference findOrCreatePreference(AppVpnInfo app) {
         AppPreference pref = mAppPreferences.get(app);
         if (pref == null) {
             pref = new AppPreference(getPrefContext(), app.userId, app.packageName);

tests/unit/src/com/android/settings/vpn2/PreferenceListTest.java

@@ -0,0 +1,159 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.settings.vpn2;
+
+import static org.mockito.AdditionalMatchers.not;
+import static org.mockito.Matchers.*;
+import static org.mockito.Mockito.*;
+
+import android.content.Context;
+import android.content.Context;
+import android.test.AndroidTestCase;
+import android.test.suitebuilder.annotation.SmallTest;
+import android.text.TextUtils;
+
+import com.android.internal.net.LegacyVpnInfo;
+import com.android.internal.net.VpnProfile;
+import com.android.settings.R;
+import com.android.settings.vpn2.VpnSettings;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.mockito.ArgumentCaptor;
+import org.mockito.ArgumentMatcher;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+
+public class PreferenceListTest extends AndroidTestCase {
+    private static final String TAG = "PreferenceListTest";
+
+    @Mock VpnSettings mSettings;
+
+    final Map<String, LegacyVpnPreference> mLegacyMocks = new HashMap<>();
+    final Map<AppVpnInfo, AppPreference> mAppMocks = new HashMap<>();
+
+    @Override
+    public void setUp() throws Exception {
+        MockitoAnnotations.initMocks(this);
+
+        mLegacyMocks.clear();
+        mAppMocks.clear();
+
+        doAnswer(invocation -> {
+            final String key = ((VpnProfile)(invocation.getArguments()[0])).key;
+            if (!mLegacyMocks.containsKey(key)) {
+                mLegacyMocks.put(key, mock(LegacyVpnPreference.class));
+            }
+            return mLegacyMocks.get(key);
+        }).when(mSettings).findOrCreatePreference(any(VpnProfile.class), anyBoolean());
+
+        doAnswer(invocation -> {
+            final AppVpnInfo key = (AppVpnInfo)(invocation.getArguments()[0]);
+            if (!mAppMocks.containsKey(key)) {
+                mAppMocks.put(key, mock(AppPreference.class));
+            }
+            return mAppMocks.get(key);
+        }).when(mSettings).findOrCreatePreference(any(AppVpnInfo.class));
+
+        doNothing().when(mSettings).setShownPreferences(any());
+        doReturn(true).when(mSettings).canAddPreferences();
+    }
+
+    @SmallTest
+    public void testNothingShownByDefault() {
+        final VpnSettings.UpdatePreferences updater = new VpnSettings.UpdatePreferences(mSettings);
+        updater.run();
+
+        verify(mSettings, never()).findOrCreatePreference(any(VpnProfile.class), anyBoolean());
+        assertEquals(0, mLegacyMocks.size());
+        assertEquals(0, mAppMocks.size());
+    }
+
+    @SmallTest
+    public void testDisconnectedLegacyVpnShown() {
+        final VpnProfile vpnProfile = new VpnProfile("test-disconnected");
+
+        final VpnSettings.UpdatePreferences updater = new VpnSettings.UpdatePreferences(mSettings);
+        updater.legacyVpns(
+                /* vpnProfiles */ Collections.<VpnProfile>singletonList(vpnProfile),
+                /* connectedLegacyVpns */ Collections.<String, LegacyVpnInfo>emptyMap(),
+                /* lockdownVpnKey */ null);
+        updater.run();
+
+        verify(mSettings, times(1)).findOrCreatePreference(any(VpnProfile.class), eq(true));
+        assertEquals(1, mLegacyMocks.size());
+        assertEquals(0, mAppMocks.size());
+    }
+
+    @SmallTest
+    public void testConnectedLegacyVpnShownIfDeleted() {
+        final LegacyVpnInfo connectedLegacyVpn =new LegacyVpnInfo();
+        connectedLegacyVpn.key = "test-connected";
+
+        final VpnSettings.UpdatePreferences updater = new VpnSettings.UpdatePreferences(mSettings);
+        updater.legacyVpns(
+                /* vpnProfiles */ Collections.<VpnProfile>emptyList(),
+                /* connectedLegacyVpns */ new HashMap<String, LegacyVpnInfo>() {{
+                    put(connectedLegacyVpn.key, connectedLegacyVpn);
+                }},
+                /* lockdownVpnKey */ null);
+        updater.run();
+
+        verify(mSettings, times(1)).findOrCreatePreference(any(VpnProfile.class), eq(false));
+        assertEquals(1, mLegacyMocks.size());
+        assertEquals(0, mAppMocks.size());
+    }
+
+    @SmallTest
+    public void testConnectedLegacyVpnShownExactlyOnce() {
+        final VpnProfile vpnProfile = new VpnProfile("test-no-duplicates");
+        final LegacyVpnInfo connectedLegacyVpn = new LegacyVpnInfo();
+        connectedLegacyVpn.key = new String(vpnProfile.key);
+
+        final VpnSettings.UpdatePreferences updater = new VpnSettings.UpdatePreferences(mSettings);
+        updater.legacyVpns(
+                /* vpnProfiles */ Collections.<VpnProfile>singletonList(vpnProfile),
+                /* connectedLegacyVpns */ new HashMap<String, LegacyVpnInfo>() {{
+                    put(connectedLegacyVpn.key, connectedLegacyVpn);
+                }},
+                /* lockdownVpnKey */ null);
+        updater.run();
+
+        final ArgumentMatcher<VpnProfile> equalsFake = new ArgumentMatcher<VpnProfile>() {
+            @Override
+            public boolean matches(final Object arg) {
+                if (arg == vpnProfile) return true;
+                if (arg == null) return false;
+                return TextUtils.equals(((VpnProfile) arg).key, vpnProfile.key);
+            }
+        };
+
+        // The VPN profile should have been used to create a preference and set up at laest once
+        // with update=true to fill in all the fields.
+        verify(mSettings, atLeast(1)).findOrCreatePreference(argThat(equalsFake), eq(true));
+
+        // ...But no other VPN profile key should ever have been passed in.
+        verify(mSettings, never()).findOrCreatePreference(not(argThat(equalsFake)), anyBoolean());
+
+        // And so we should still have exactly 1 preference created.
+        assertEquals(1, mLegacyMocks.size());
+        assertEquals(0, mAppMocks.size());
+    }
+}