Merge changes from topic "cherrypicker-L19300000956051124:N78200001288956413" into tm-qpr-dev

* changes:
  Skip events on stale Ikev2VpnRunner
  Invert the order of event sending and VpnRunner.exit()
  Stop VPN profiles by exiting VpnRunner instead of prepareInternal

services/core/java/com/android/server/connectivity/Vpn.java

@@ -747,7 +747,7 @@ public class Vpn {
         return true;
     }
 
-    private boolean sendEventToVpnManagerApp(@NonNull String category, int errorClass,
+    private Intent buildVpnManagerEventIntent(@NonNull String category, int errorClass,
             int errorCode, @NonNull final String packageName, @Nullable final String sessionKey,
             @NonNull final VpnProfileState profileState, @Nullable final Network underlyingNetwork,
             @Nullable final NetworkCapabilities nc, @Nullable final LinkProperties lp) {
@@ -766,6 +766,20 @@ public class Vpn {
             intent.putExtra(VpnManager.EXTRA_ERROR_CODE, errorCode);
         }
 
+        return intent;
+    }
+
+    private boolean sendEventToVpnManagerApp(@NonNull String category, int errorClass,
+            int errorCode, @NonNull final String packageName, @Nullable final String sessionKey,
+            @NonNull final VpnProfileState profileState, @Nullable final Network underlyingNetwork,
+            @Nullable final NetworkCapabilities nc, @Nullable final LinkProperties lp) {
+        final Intent intent = buildVpnManagerEventIntent(category, errorClass, errorCode,
+                packageName, sessionKey, profileState, underlyingNetwork, nc, lp);
+        return sendEventToVpnManagerApp(intent, packageName);
+    }
+
+    private boolean sendEventToVpnManagerApp(@NonNull final Intent intent,
+            @NonNull final String packageName) {
         // Allow VpnManager app to temporarily run background services to handle this error.
         // If an app requires anything beyond this grace period, they MUST either declare
         // themselves as a foreground service, or schedule a job/workitem.
@@ -1177,23 +1191,25 @@ public class Vpn {
                 mContext.unbindService(mConnection);
                 cleanupVpnStateLocked();
             } else if (mVpnRunner != null) {
-                if (!VpnConfig.LEGACY_VPN.equals(mPackage)) {
+                // Build intent first because the sessionKey will be reset after performing
-                    mAppOpsManager.finishOp(
+                // VpnRunner.exit(). Also, cache mOwnerUID even if ownerUID will not be changed in
-                            AppOpsManager.OPSTR_ESTABLISH_VPN_MANAGER, mOwnerUID, mPackage, null);
+                // VpnRunner.exit() to prevent design being changed in the future.
-                    // The underlying network, NetworkCapabilities and LinkProperties are not
+                // TODO(b/230548427): Remove SDK check once VPN related stuff are decoupled from
-                    // necessary to send to VPN app since the purpose of this event is to notify
+                //  ConnectivityServiceTest.
-                    // VPN app that VPN is deactivated by the user.
+                final int ownerUid = mOwnerUID;
-                    // TODO(b/230548427): Remove SDK check once VPN related stuff are decoupled from
+                Intent intent = null;
-                    //  ConnectivityServiceTest.
+                if (SdkLevel.isAtLeastT() && isVpnApp(mPackage)) {
-                    if (SdkLevel.isAtLeastT()) {
+                    intent = buildVpnManagerEventIntent(
-                        sendEventToVpnManagerApp(VpnManager.CATEGORY_EVENT_DEACTIVATED_BY_USER,
+                            VpnManager.CATEGORY_EVENT_DEACTIVATED_BY_USER,
-                                -1 /* errorClass */, -1 /* errorCode*/, mPackage,
+                            -1 /* errorClass */, -1 /* errorCode*/, mPackage,
-                                getSessionKeyLocked(), makeVpnProfileStateLocked(),
+                            getSessionKeyLocked(), makeVpnProfileStateLocked(),
-                                null /* underlyingNetwork */, null /* nc */, null /* lp */);
+                            null /* underlyingNetwork */, null /* nc */, null /* lp */);
-                    }
                 }
                 // cleanupVpnStateLocked() is called from mVpnRunner.exit()
                 mVpnRunner.exit();
+                if (intent != null && isVpnApp(mPackage)) {
+                    notifyVpnManagerVpnStopped(mPackage, ownerUid, intent);
+                }
             }
 
             try {
@@ -2887,6 +2903,9 @@ public class Vpn {
                 final LinkProperties lp;
 
                 synchronized (Vpn.this) {
+                    // Ignore stale runner.
+                    if (mVpnRunner != this) return;
+
                     mInterface = interfaceName;
                     mConfig.mtu = maxMtu;
                     mConfig.interfaze = mInterface;
@@ -2988,6 +3007,9 @@ public class Vpn {
 
             try {
                 synchronized (Vpn.this) {
+                    // Ignore stale runner.
+                    if (mVpnRunner != this) return;
+
                     mConfig.underlyingNetworks = new Network[] {network};
                     mNetworkCapabilities =
                             new NetworkCapabilities.Builder(mNetworkCapabilities)
@@ -3077,7 +3099,12 @@ public class Vpn {
 
                 // Clear mInterface to prevent Ikev2VpnRunner being cleared when
                 // interfaceRemoved() is called.
-                mInterface = null;
+                synchronized (Vpn.this) {
+                    // Ignore stale runner.
+                    if (mVpnRunner != this) return;
+
+                    mInterface = null;
+                }
                 // Without MOBIKE, we have no way to seamlessly migrate. Close on old
                 // (non-default) network, and start the new one.
                 resetIkeState();
@@ -3262,6 +3289,9 @@ public class Vpn {
         /** Marks the state as FAILED, and disconnects. */
         private void markFailedAndDisconnect(Exception exception) {
             synchronized (Vpn.this) {
+                // Ignore stale runner.
+                if (mVpnRunner != this) return;
+
                 updateState(DetailedState.FAILED, exception.getMessage());
             }
 
@@ -3300,6 +3330,9 @@ public class Vpn {
             cancelHandleNetworkLostTimeout();
 
             synchronized (Vpn.this) {
+                // Ignore stale runner.
+                if (mVpnRunner != this) return;
+
                 if (exception instanceof IkeProtocolException) {
                     final IkeProtocolException ikeException = (IkeProtocolException) exception;
 
@@ -3420,6 +3453,9 @@ public class Vpn {
             Log.d(TAG, "Resetting state for token: " + mCurrentToken);
 
             synchronized (Vpn.this) {
+                // Ignore stale runner.
+                if (mVpnRunner != this) return;
+
                 // Since this method handles non-fatal errors only, set mInterface to null to
                 // prevent the NetworkManagementEventObserver from killing this VPN based on the
                 // interface going down (which we expect).
@@ -4090,7 +4126,32 @@ public class Vpn {
         // To stop the VPN profile, the caller must be the current prepared package and must be
         // running an Ikev2VpnProfile.
         if (isCurrentIkev2VpnLocked(packageName)) {
-            prepareInternal(VpnConfig.LEGACY_VPN);
+            // Build intent first because the sessionKey will be reset after performing
+            // VpnRunner.exit(). Also, cache mOwnerUID even if ownerUID will not be changed in
+            // VpnRunner.exit() to prevent design being changed in the future.
+            final int ownerUid = mOwnerUID;
+            final Intent intent = buildVpnManagerEventIntent(
+                    VpnManager.CATEGORY_EVENT_DEACTIVATED_BY_USER,
+                    -1 /* errorClass */, -1 /* errorCode*/, packageName,
+                    getSessionKeyLocked(), makeVpnProfileStateLocked(),
+                    null /* underlyingNetwork */, null /* nc */, null /* lp */);
+
+            mVpnRunner.exit();
+            notifyVpnManagerVpnStopped(packageName, ownerUid, intent);
+        }
+    }
+
+    private synchronized void notifyVpnManagerVpnStopped(String packageName, int ownerUID,
+            Intent intent) {
+        mAppOpsManager.finishOp(
+                AppOpsManager.OPSTR_ESTABLISH_VPN_MANAGER, ownerUID, packageName, null);
+        // The underlying network, NetworkCapabilities and LinkProperties are not
+        // necessary to send to VPN app since the purpose of this event is to notify
+        // VPN app that VPN is deactivated by the user.
+        // TODO(b/230548427): Remove SDK check once VPN related stuff are decoupled from
+        //  ConnectivityServiceTest.
+        if (SdkLevel.isAtLeastT()) {
+            sendEventToVpnManagerApp(intent, packageName);
         }
     }