Skip execution of transactions on a destroyed activity

An Activity may not yet create on client side, there is another
launch request with flags to clear task, then a destroy transaction
is scheduled. If client side keeps blocking until destroy timeout,
the token on server side will be removed. When client begins to
handle the first creation, it will report its activity token to
server that causes IllegalArgumentException because there is no
matched ActivityRecord.

Bug: 32375307
Test: atest FrameworksCoreTests:TransactionExecutorTests
Change-Id: I1b7e0c2863b13091c3fd50df602ff31ae02ff38d

6 files changed, 104 insertions, 1 deletions

diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java
index f3c67310c05d..2daa5779bde7 100644
--- a/core/java/android/app/ActivityThread.java
+++ b/core/java/android/app/ActivityThread.java
@@ -35,6 +35,7 @@ import android.app.servertransaction.ActivityLifecycleItem.LifecycleState;
 import android.app.servertransaction.ActivityRelaunchItem;
 import android.app.servertransaction.ActivityResultItem;
 import android.app.servertransaction.ClientTransaction;
+import android.app.servertransaction.ClientTransactionItem;
 import android.app.servertransaction.PendingTransactionActions;
 import android.app.servertransaction.PendingTransactionActions.StopInfo;
 import android.app.servertransaction.TransactionExecutor;
@@ -176,6 +177,7 @@ import java.net.InetAddress;
 import java.text.DateFormat;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -257,6 +259,8 @@ public final class ActivityThread extends ClientTransactionHandler {
     final H mH = new H();
     final Executor mExecutor = new HandlerExecutor(mH);
     final ArrayMap<IBinder, ActivityClientRecord> mActivities = new ArrayMap<>();
+    final Map<IBinder, ClientTransactionItem> mActivitiesToBeDestroyed =
+            Collections.synchronizedMap(new ArrayMap<IBinder, ClientTransactionItem>());
     // List of new activities (via ActivityRecord.nextIdle) that should
     // be reported when next we idle.
     ActivityClientRecord mNewActivities = null;
@@ -4474,6 +4478,11 @@ public final class ActivityThread extends ClientTransactionHandler {
     }
 
     @Override
+    public Map<IBinder, ClientTransactionItem> getActivitiesToBeDestroyed() {
+        return mActivitiesToBeDestroyed;
+    }
+
+    @Override
     public void handleDestroyActivity(IBinder token, boolean finishing, int configChanges,
             boolean getNonConfigInstance, String reason) {
         ActivityClientRecord r = performDestroyActivity(token, finishing,
diff --git a/core/java/android/app/ClientTransactionHandler.java b/core/java/android/app/ClientTransactionHandler.java
index d9c7cf3ccc74..193f933df782 100644
--- a/core/java/android/app/ClientTransactionHandler.java
+++ b/core/java/android/app/ClientTransactionHandler.java
@@ -16,6 +16,7 @@
 package android.app;
 
 import android.app.servertransaction.ClientTransaction;
+import android.app.servertransaction.ClientTransactionItem;
 import android.app.servertransaction.PendingTransactionActions;
 import android.app.servertransaction.TransactionExecutor;
 import android.content.Intent;
@@ -29,6 +30,7 @@ import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.content.ReferrerIntent;
 
 import java.util.List;
+import java.util.Map;
 
 /**
  * Defines operations that a {@link android.app.servertransaction.ClientTransaction} or its items
@@ -78,6 +80,9 @@ public abstract class ClientTransactionHandler {
     // Execute phase related logic and handlers. Methods here execute actual lifecycle transactions
     // and deliver callbacks.
 
+    /** Get activity and its corresponding transaction item which are going to destroy. */
+    public abstract Map<IBinder, ClientTransactionItem> getActivitiesToBeDestroyed();
+
     /** Destroy the activity. */
     public abstract void handleDestroyActivity(IBinder token, boolean finishing, int configChanges,
             boolean getNonConfigInstance, String reason);
diff --git a/core/java/android/app/servertransaction/ClientTransaction.java b/core/java/android/app/servertransaction/ClientTransaction.java
index 08ad2f055774..2c1e59bfc066 100644
--- a/core/java/android/app/servertransaction/ClientTransaction.java
+++ b/core/java/android/app/servertransaction/ClientTransaction.java
@@ -164,6 +164,31 @@ public class ClientTransaction implements Parcelable, ObjectPoolItem {
         ObjectPool.recycle(this);
     }
 
+    @Override
+    public String toString() {
+        final StringBuilder sb = new StringBuilder(64);
+        sb.append("ClientTransaction{");
+        if (mActivityToken != null) {
+            sb.append(" a:").append(Integer.toHexString(System.identityHashCode(mActivityToken)));
+        }
+        if (mActivityCallbacks != null && !mActivityCallbacks.isEmpty()) {
+            sb.append(" c:");
+            final int size = mActivityCallbacks.size();
+            for (int i = 0; i < size; i++) {
+                sb.append(mActivityCallbacks.get(i).getClass().getSimpleName());
+                if (i < size - 1) {
+                    sb.append(",");
+                }
+            }
+        }
+        if (mLifecycleStateRequest != null) {
+            sb.append(" s:");
+            sb.append(mLifecycleStateRequest.getClass().getSimpleName());
+        }
+        sb.append(" }");
+        return sb.toString();
+    }
+
 
     // Parcelable implementation
 
diff --git a/core/java/android/app/servertransaction/DestroyActivityItem.java b/core/java/android/app/servertransaction/DestroyActivityItem.java
index b443166d151c..5941486c099d 100644
--- a/core/java/android/app/servertransaction/DestroyActivityItem.java
+++ b/core/java/android/app/servertransaction/DestroyActivityItem.java
@@ -33,6 +33,11 @@ public class DestroyActivityItem extends ActivityLifecycleItem {
     private int mConfigChanges;
 
     @Override
+    public void preExecute(ClientTransactionHandler client, IBinder token) {
+        client.getActivitiesToBeDestroyed().put(token, this);
+    }
+
+    @Override
     public void execute(ClientTransactionHandler client, IBinder token,
             PendingTransactionActions pendingActions) {
         Trace.traceBegin(TRACE_TAG_ACTIVITY_MANAGER, "activityDestroy");
diff --git a/core/java/android/app/servertransaction/TransactionExecutor.java b/core/java/android/app/servertransaction/TransactionExecutor.java
index 43a2b4cc43f8..503e18b62cae 100644
--- a/core/java/android/app/servertransaction/TransactionExecutor.java
+++ b/core/java/android/app/servertransaction/TransactionExecutor.java
@@ -35,6 +35,7 @@ import android.util.Slog;
 import com.android.internal.annotations.VisibleForTesting;
 
 import java.util.List;
+import java.util.Map;
 
 /**
  * Class that manages transaction execution in the correct order.
@@ -63,6 +64,24 @@ public class TransactionExecutor {
      */
     public void execute(ClientTransaction transaction) {
         final IBinder token = transaction.getActivityToken();
+        if (token != null) {
+            final Map<IBinder, ClientTransactionItem> activitiesToBeDestroyed =
+                    mTransactionHandler.getActivitiesToBeDestroyed();
+            final ClientTransactionItem destroyItem = activitiesToBeDestroyed.get(token);
+            if (destroyItem != null) {
+                if (transaction.getLifecycleStateRequest() == destroyItem) {
+                    // It is going to execute the transaction that will destroy activity with the
+                    // token, so the corresponding to-be-destroyed record can be removed.
+                    activitiesToBeDestroyed.remove(token);
+                }
+                if (mTransactionHandler.getActivityClient(token) == null) {
+                    // The activity has not been created but has been requested to destroy, so all
+                    // transactions for the token are just like being cancelled.
+                    Slog.w(TAG, "Skip pre-destroyed " + transaction);
+                    return;
+                }
+            }
+        }
         log("Start resolving transaction for client: " + mTransactionHandler + ", token: " + token);
 
         executeCallbacks(transaction);
@@ -76,7 +95,7 @@ public class TransactionExecutor {
     @VisibleForTesting
     public void executeCallbacks(ClientTransaction transaction) {
         final List<ClientTransactionItem> callbacks = transaction.getCallbacks();
-        if (callbacks == null) {
+        if (callbacks == null || callbacks.isEmpty()) {
             // No callbacks to execute, return early.
             return;
         }
diff --git a/core/tests/coretests/src/android/app/servertransaction/TransactionExecutorTests.java b/core/tests/coretests/src/android/app/servertransaction/TransactionExecutorTests.java
index fe58116002f2..3d114f4b7c6c 100644
--- a/core/tests/coretests/src/android/app/servertransaction/TransactionExecutorTests.java
+++ b/core/tests/coretests/src/android/app/servertransaction/TransactionExecutorTests.java
@@ -47,6 +47,7 @@ import android.os.Parcelable;
 import android.platform.test.annotations.Presubmit;
 import android.support.test.filters.SmallTest;
 import android.support.test.runner.AndroidJUnit4;
+import android.util.ArrayMap;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -56,6 +57,7 @@ import org.mockito.InOrder;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 import java.util.stream.Collectors;
 
 /** Test {@link TransactionExecutor} logic. */
@@ -232,6 +234,44 @@ public class TransactionExecutorTests {
     }
 
     @Test
+    public void testDoNotLaunchDestroyedActivity() {
+        final Map<IBinder, ClientTransactionItem> activitiesToBeDestroyed = new ArrayMap<>();
+        when(mTransactionHandler.getActivitiesToBeDestroyed()).thenReturn(activitiesToBeDestroyed);
+        // Assume launch transaction is still in queue, so there is no client record.
+        when(mTransactionHandler.getActivityClient(any())).thenReturn(null);
+
+        // An incoming destroy transaction enters binder thread (preExecute).
+        final IBinder token = mock(IBinder.class);
+        final ClientTransaction destroyTransaction = ClientTransaction.obtain(null /* client */,
+                token /* activityToken */);
+        destroyTransaction.setLifecycleStateRequest(
+                DestroyActivityItem.obtain(false /* finished */, 0 /* configChanges */));
+        destroyTransaction.preExecute(mTransactionHandler);
+        // The activity should be added to to-be-destroyed container.
+        assertEquals(1, mTransactionHandler.getActivitiesToBeDestroyed().size());
+
+        // A previous queued launch transaction runs on main thread (execute).
+        final ClientTransaction launchTransaction = ClientTransaction.obtain(null /* client */,
+                token /* activityToken */);
+        final LaunchActivityItem launchItem = spy(LaunchActivityItem.obtain(
+                null /* intent */, 0 /* ident */, null /* info */, null /* curConfig */,
+                null, /* overrideConfig */ null /* compatInfo */, null /* referrer */ ,
+                null /* voiceInteractor */, 0 /* procState */, null /* state */,
+                null /* persistentState */, null /* pendingResults */,
+                null /* pendingNewIntents */, false /* isForward */, null /* profilerInfo */));
+        launchTransaction.addCallback(launchItem);
+        mExecutor.execute(launchTransaction);
+
+        // The launch transaction should not be executed because its token is in the
+        // to-be-destroyed container.
+        verify(launchItem, times(0)).execute(any(), any(), any());
+
+        // After the destroy transaction has been executed, the token should be removed.
+        mExecutor.execute(destroyTransaction);
+        assertEquals(0, mTransactionHandler.getActivitiesToBeDestroyed().size());
+    }
+
+    @Test
     public void testActivityResultRequiredStateResolution() {
         PostExecItem postExecItem = new PostExecItem(ON_RESUME);