Remove activity from task if not set properly.

We do not start an activity in the case of a lock task mode
violation. However, the activity is still associated with the task
and therefore participates in future interactions, such as
visibility checks.

This changelist addresses this issue by removing the assocation in
the case of failures.

Change-Id: Ibf7edd2bd4532de5e7f355311a808ffd21e542e3
Fixes: 35034729
Fixes: 35035258
Fixes: 34179495
Test: cts-tradefed run cts-dev --module DevicePolicyManager --test com.android.cts.devicepolicy.DeviceOwnerTest#testLockTask_deviceOwnerUser

6 files changed, 81 insertions, 40 deletions

diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index 9a0194219e49..88ceb6fa4109 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -3067,15 +3067,24 @@ public class ActivityManagerService extends IActivityManager.Stub
             startRunningVoiceLocked(r.task.voiceSession, r.info.applicationInfo.uid);
         } else {
             finishRunningVoiceLocked();
-            IVoiceInteractionSession session;
-            if (mLastResumedActivity != null
-                    && ((session = mLastResumedActivity.task.voiceSession) != null
-                    || (session = mLastResumedActivity.voiceSession) != null)) {
-                // We had been in a voice interaction session, but now focused has
-                // move to something different.  Just finish the session, we can't
-                // return to it and retain the proper state and synchronization with
-                // the voice interaction service.
-                finishVoiceTask(session);
+
+            if (mLastResumedActivity != null) {
+                final IVoiceInteractionSession session;
+
+                if (mLastResumedActivity.task != null
+                        && mLastResumedActivity.task.voiceSession != null) {
+                    session = mLastResumedActivity.task.voiceSession;
+                } else {
+                    session = mLastResumedActivity.voiceSession;
+                }
+
+                if (session != null) {
+                    // We had been in a voice interaction session, but now focused has
+                    // move to something different.  Just finish the session, we can't
+                    // return to it and retain the proper state and synchronization with
+                    // the voice interaction service.
+                    finishVoiceTask(session);
+                }
             }
         }
 
diff --git a/services/core/java/com/android/server/am/ActivityRecord.java b/services/core/java/com/android/server/am/ActivityRecord.java
index 672a6ce25e4a..fa7dd28eb856 100644
--- a/services/core/java/com/android/server/am/ActivityRecord.java
+++ b/services/core/java/com/android/server/am/ActivityRecord.java
@@ -802,10 +802,7 @@ final class ActivityRecord implements AppWindowContainerListener {
 
         // Remove the activity from the old task and add it to the new task
         prevTask.removeActivity(this);
-        // TODO(b/34179495): This should really be set to null in removeActivity() call above,
-        // but really bad things that I can't track down right now happen when I do that.
-        // So, setting it here now and will change later when there is time for investigation.
-        task = null;
+
         newTask.addActivityAtIndex(position, this);
     }
 
diff --git a/services/core/java/com/android/server/am/ActivityStack.java b/services/core/java/com/android/server/am/ActivityStack.java
index 09ac9b889cfe..827a41e776d8 100644
--- a/services/core/java/com/android/server/am/ActivityStack.java
+++ b/services/core/java/com/android/server/am/ActivityStack.java
@@ -3827,7 +3827,9 @@ class ActivityStack<T extends StackWindowController> extends ConfigurationContai
         r.app = null;
         r.removeWindowContainer();
         final TaskRecord task = r.task;
-        if (task != null && task.removeActivity(r)) {
+        final boolean lastActivity = task != null ? task.removeActivity(r) : false;
+
+        if (lastActivity) {
             if (DEBUG_STACK) Slog.i(TAG_STACK,
                     "removeActivityFromHistoryLocked: last activity removed from " + this);
             if (mStackSupervisor.isFocusedStack(this) && task == topTask() &&
diff --git a/services/core/java/com/android/server/am/ActivityStackSupervisor.java b/services/core/java/com/android/server/am/ActivityStackSupervisor.java
index 35145a11a8c0..d3ad0572d556 100644
--- a/services/core/java/com/android/server/am/ActivityStackSupervisor.java
+++ b/services/core/java/com/android/server/am/ActivityStackSupervisor.java
@@ -177,7 +177,6 @@ import java.io.IOException;
 import java.io.PrintWriter;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
 import java.util.Set;
@@ -4904,7 +4903,7 @@ public class ActivityStackSupervisor extends ConfigurationContainer implements D
                 setResizingDuringAnimation(task);
             }
 
-            mService.mActivityStarter.postStartActivityUncheckedProcessing(task.getTopActivity(),
+            mService.mActivityStarter.postStartActivityProcessing(task.getTopActivity(),
                     ActivityManager.START_TASK_TO_FRONT,
                     sourceRecord != null ? sourceRecord.task.getStackId() : INVALID_STACK_ID,
                     sourceRecord, task.getStack());
diff --git a/services/core/java/com/android/server/am/ActivityStarter.java b/services/core/java/com/android/server/am/ActivityStarter.java
index e13d92409b00..857241afa0a8 100644
--- a/services/core/java/com/android/server/am/ActivityStarter.java
+++ b/services/core/java/com/android/server/am/ActivityStarter.java
@@ -17,6 +17,7 @@
 package com.android.server.am;
 
 import static android.app.Activity.RESULT_CANCELED;
+import static android.app.ActivityManager.START_CANCELED;
 import static android.app.ActivityManager.START_CLASS_NOT_FOUND;
 import static android.app.ActivityManager.START_DELIVERED_TO_TOP;
 import static android.app.ActivityManager.START_FLAG_ONLY_IF_NEEDED;
@@ -88,12 +89,10 @@ import android.app.ActivityOptions;
 import android.app.AppGlobals;
 import android.app.IActivityContainer;
 import android.app.IApplicationThread;
-import android.app.KeyguardManager;
 import android.app.PendingIntent;
 import android.app.ProfilerInfo;
 import android.app.WaitResult;
 import android.content.ComponentName;
-import android.content.Context;
 import android.content.IIntentSender;
 import android.content.Intent;
 import android.content.IntentSender;
@@ -101,7 +100,6 @@ import android.content.pm.ActivityInfo;
 import android.content.pm.ApplicationInfo;
 import android.content.pm.AuxiliaryResolveInfo;
 import android.content.pm.PackageManager;
-import android.content.pm.PackageManagerInternal;
 import android.content.pm.ResolveInfo;
 import android.content.pm.UserInfo;
 import android.content.res.Configuration;
@@ -509,21 +507,15 @@ class ActivityStarter {
 
         doPendingActivityLaunchesLocked(false);
 
-        try {
-            mService.mWindowManager.deferSurfaceLayout();
-            err = startActivityUnchecked(r, sourceRecord, voiceSession, voiceInteractor, startFlags,
-                    true, options, inTask);
-        } finally {
-            mService.mWindowManager.continueSurfaceLayout();
-        }
-        postStartActivityUncheckedProcessing(r, err, stack.mStackId, mSourceRecord, mTargetStack);
-        return err;
+        return startActivity(r, sourceRecord, voiceSession, voiceInteractor, startFlags, true,
+                options, inTask);
     }
 
-    /** Creates a launch intent for the given auxiliary resolution data. */
+    /**
+     * Creates a launch intent for the given auxiliary resolution data.
+     */
     private @NonNull Intent createLaunchIntent(@NonNull AuxiliaryResolveInfo auxiliaryResponse,
-            Intent originalIntent, String callingPackage,
-            String resolvedType, int userId) {
+            Intent originalIntent, String callingPackage, String resolvedType, int userId) {
         if (auxiliaryResponse.needsPhaseTwo) {
             // request phase two resolution
             mService.getPackageManagerInternalLocked().requestInstantAppResolutionPhaseTwo(
@@ -535,7 +527,7 @@ class ActivityStarter {
                 auxiliaryResponse.token, auxiliaryResponse.needsPhaseTwo);
     }
 
-    void postStartActivityUncheckedProcessing(
+    void postStartActivityProcessing(
             ActivityRecord r, int result, int prevFocusedStackId, ActivityRecord sourceRecord,
             ActivityStack targetStack) {
 
@@ -937,6 +929,31 @@ class ActivityStarter {
         }
     }
 
+    private int startActivity(final ActivityRecord r, ActivityRecord sourceRecord,
+            IVoiceInteractionSession voiceSession, IVoiceInteractor voiceInteractor,
+            int startFlags, boolean doResume, ActivityOptions options, TaskRecord inTask) {
+        int result = START_CANCELED;
+        try {
+            mService.mWindowManager.deferSurfaceLayout();
+            result = startActivityUnchecked(r, sourceRecord, voiceSession, voiceInteractor,
+                    startFlags, doResume, options, inTask);
+        } finally {
+            // If we are not able to proceed, disassociate the activity from the task. Leaving an
+            // activity in an incomplete state can lead to issues, such as performing operations
+            // without a window container.
+            if (result != START_SUCCESS && mStartActivity.task != null) {
+                mStartActivity.task.removeActivity(mStartActivity);
+            }
+            mService.mWindowManager.continueSurfaceLayout();
+        }
+
+        postStartActivityProcessing(r, result, mSupervisor.mFocusedStack.mStackId,  mSourceRecord,
+                mTargetStack);
+
+        return result;
+    }
+
+    // Note: This method should only be called from {@link startActivity}.
     private int startActivityUnchecked(final ActivityRecord r, ActivityRecord sourceRecord,
             IVoiceInteractionSession voiceSession, IVoiceInteractor voiceInteractor,
             int startFlags, boolean doResume, ActivityOptions options, TaskRecord inTask) {
@@ -982,11 +999,22 @@ class ActivityStarter {
             if ((mLaunchFlags & FLAG_ACTIVITY_CLEAR_TOP) != 0
                     || isDocumentLaunchesIntoExisting(mLaunchFlags)
                     || mLaunchSingleInstance || mLaunchSingleTask) {
+                final TaskRecord task = mReusedActivity.task;
+
                 // In this situation we want to remove all activities from the task up to the one
                 // being started. In most cases this means we are resetting the task to its initial
                 // state.
-                final ActivityRecord top = mReusedActivity.task.performClearTaskForReuseLocked(
-                        mStartActivity, mLaunchFlags);
+                final ActivityRecord top = task.performClearTaskForReuseLocked(mStartActivity,
+                        mLaunchFlags);
+
+                // The above code can remove {@code mReusedActivity} from the task, leading to the
+                // the {@code ActivityRecord} removing its reference to the {@code TaskRecord}. The
+                // task reference is needed in the call below to
+                // {@link setTargetStackAndMoveToFrontIfNeeded}.
+                if (mReusedActivity.task == null) {
+                    mReusedActivity.task = task;
+                }
+
                 if (top != null) {
                     if (top.frontOfTask) {
                         // Activity aliases may mean we use different intents for the top activity,
@@ -1846,11 +1874,8 @@ class ActivityStarter {
             final PendingActivityLaunch pal = mPendingActivityLaunches.remove(0);
             final boolean resume = doResume && mPendingActivityLaunches.isEmpty();
             try {
-                final int result = startActivityUnchecked(
-                        pal.r, pal.sourceRecord, null, null, pal.startFlags, resume, null, null);
-                postStartActivityUncheckedProcessing(
-                        pal.r, result, mSupervisor.mFocusedStack.mStackId, mSourceRecord,
-                        mTargetStack);
+                startActivity(pal.r, pal.sourceRecord, null, null, pal.startFlags, resume, null,
+                        null);
             } catch (Exception e) {
                 Slog.e(TAG, "Exception during pending activity launch pal=" + pal, e);
                 pal.sendErrorResult(e.getMessage());
diff --git a/services/core/java/com/android/server/am/TaskRecord.java b/services/core/java/com/android/server/am/TaskRecord.java
index f8645d65fd92..241042452fb1 100644
--- a/services/core/java/com/android/server/am/TaskRecord.java
+++ b/services/core/java/com/android/server/am/TaskRecord.java
@@ -1081,8 +1081,17 @@ final class TaskRecord extends ConfigurationContainer implements TaskWindowConta
         r.onOverrideConfigurationSent();
     }
 
-    /** @return true if this was the last activity in the task */
+    /**
+     * @return true if this was the last activity in the task
+     */
     boolean removeActivity(ActivityRecord r) {
+        if (r.task != this) {
+            throw new IllegalArgumentException(
+                    "Activity=" + r + " does not belong to task=" + this);
+        }
+
+        r.task = null;
+
         if (mActivities.remove(r) && r.fullscreen) {
             // Was previously in list.
             numFullscreen--;