Merge "Skip execution of transactions on a destroyed activity"

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);