Move 'is already always-on' check into Vpn.java

It's with the rest of the logic now and allows checking whether the
lockdown state matches, too, which led to a lot of misunderstandings.

Fix: 29199431
Change-Id: I94a2c38c4837f9c33b5b9c2becb52eeb7e2a2534

2 files changed, 18 insertions, 11 deletions

diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index acf8009e34a6..8620b8da9e1e 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -3417,10 +3417,6 @@ public class ConnectivityService extends IConnectivityManager.Stub
                 Slog.w(TAG, "User " + userId + " has no Vpn configuration");
                 return false;
             }
-            // If the current VPN package is the same as the new one, this is a no-op
-            if (TextUtils.equals(packageName, vpn.getAlwaysOnPackage())) {
-                return true;
-            }
             if (!vpn.setAlwaysOnPackage(packageName, lockdown)) {
                 return false;
             }
diff --git a/services/core/java/com/android/server/connectivity/Vpn.java b/services/core/java/com/android/server/connectivity/Vpn.java
index ebacc71515bb..ede3bda5bea2 100644
--- a/services/core/java/com/android/server/connectivity/Vpn.java
+++ b/services/core/java/com/android/server/connectivity/Vpn.java
@@ -259,28 +259,39 @@ public class Vpn {
      *
      * @param packageName the package to designate as always-on VPN supplier.
      * @param lockdown whether to prevent traffic outside of a VPN, for example while connecting.
+     * @return {@code true} if the package has been set as always-on, {@code false} otherwise.
      */
     public synchronized boolean setAlwaysOnPackage(String packageName, boolean lockdown) {
         enforceControlPermissionOrInternalCaller();
+        if (VpnConfig.LEGACY_VPN.equals(packageName)) {
+            Log.w(TAG, "Not setting legacy VPN \"" + packageName + "\" as always-on.");
+            return false;
+        }
 
-        // Disconnect current VPN.
-        prepareInternal(VpnConfig.LEGACY_VPN);
-
-        // Pre-authorize new always-on VPN package.
         if (packageName != null) {
+            // Pre-authorize new always-on VPN package.
             if (!setPackageAuthorization(packageName, true)) {
                 return false;
             }
-            prepareInternal(packageName);
+            mAlwaysOn = true;
+        } else {
+            packageName = VpnConfig.LEGACY_VPN;
+            mAlwaysOn = false;
         }
 
-        mAlwaysOn = (packageName != null);
         mLockdown = (mAlwaysOn && lockdown);
+        if (!isCurrentPreparedPackage(packageName)) {
+            prepareInternal(packageName);
+        }
         maybeRegisterPackageChangeReceiverLocked(packageName);
         setVpnForcedLocked(mLockdown);
         return true;
     }
 
+    private static boolean isNullOrLegacyVpn(String packageName) {
+        return packageName == null || VpnConfig.LEGACY_VPN.equals(packageName);
+    }
+
     private void unregisterPackageChangeReceiverLocked() {
         // register previous intent filter
         if (mIsPackageIntentReceiverRegistered) {
@@ -293,7 +304,7 @@ public class Vpn {
         // Unregister IntentFilter listening for previous always-on package change
         unregisterPackageChangeReceiverLocked();
 
-        if (packageName != null) {
+        if (!isNullOrLegacyVpn(packageName)) {
             mIsPackageIntentReceiverRegistered = true;
 
             IntentFilter intentFilter = new IntentFilter();