Preventing cases where an app can be stopped while entering PiP.

- Ensure that we clear the timeouts on the old stack and reschedule
  them on the new stack when moving an activity to a new stack,
  otherwise the pause timeout from the old stack will cause onStop()
  to be called.
- When adding an activity to the stopping list for processing, prevent
  scheduling an idle immediately in case an activity tries to enter
  picture-in-picture when handling onUserLeaveHint().  In that case,
  schedule an idle after the default idle delay instead.
- In addition, when resuming a resumeWhilePausing activity, prevent the
  activity idle to trigger pausing activities to be immediately put into
  a stopped state.  This was a race between the handling of pause/resume
  that would cause a pip activity to get onStop() even if it called enter
  pip on pause. Instead, when processing an idle from the activity or
  from an immediate idle scheduled by the system, we defer processing
  pausing activities until a later idle (that it reschedules).

Test: android.server.cts.ActivityManagerPinnedStackTests
Test: #testEnterPipWithResumeWhilePausingActivityNoStop

Change-Id: I375369a800b7fadaa57d6e00e0564bc3ee338979
Signed-off-by: Winson Chung <winsonc@google.com>

diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index 732cd10..171e90b 100644 (file)
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -6730,7 +6730,8 @@ public class ActivityManagerService extends IActivityManager.Stub
             ActivityStack stack = ActivityRecord.getStackLocked(token);
             if (stack != null) {
                 ActivityRecord r =
-                        mStackSupervisor.activityIdleInternalLocked(token, false, config);
+                        mStackSupervisor.activityIdleInternalLocked(token, false /* fromTimeout */,
+                                false /* processPausingActivities */, config);
                 if (stopProfiling) {
                     if ((mProfileProc == r.app) && (mProfileFd != null)) {
                         try {
@@ -7610,7 +7611,8 @@ public class ActivityManagerService extends IActivityManager.Stub
 
                 // Activity supports picture-in-picture, now check that we can enter PiP at this
                 // point, if it is
-                if (!r.checkEnterPictureInPictureState("enterPictureInPictureMode")) {
+                if (!r.checkEnterPictureInPictureState("enterPictureInPictureMode",
+                        false /* noThrow */)) {
                     return false;
                 }
 

diff --git a/services/core/java/com/android/server/am/ActivityRecord.java b/services/core/java/com/android/server/am/ActivityRecord.java
index baf7772..cd28da7 100644 (file)
--- a/services/core/java/com/android/server/am/ActivityRecord.java
+++ b/services/core/java/com/android/server/am/ActivityRecord.java
@@ -944,9 +944,9 @@ final class ActivityRecord implements AppWindowContainerListener {
 
     /**
      * @return whether this activity is currently allowed to enter PIP, throwing an exception if
-     *         the activity is not currently visible.
+     *         the activity is not currently visible and {@param noThrow} is not set.
      */
-    boolean checkEnterPictureInPictureState(String caller) {
+    boolean checkEnterPictureInPictureState(String caller, boolean noThrow) {
         boolean isKeyguardLocked = service.isKeyguardLocked();
         boolean hasPinnedStack = mStackSupervisor.getStack(PINNED_STACK_ID) != null;
         switch (state) {
@@ -969,9 +969,13 @@ final class ActivityRecord implements AppWindowContainerListener {
                             && checkEnterPictureInPictureOnHideAppOpsState();
                 }
             default:
-                throw new IllegalStateException(caller
-                        + ": Current activity is not visible (state=" + state.name() + ") "
-                        + "r=" + this);
+                if (noThrow) {
+                    return false;
+                } else {
+                    throw new IllegalStateException(caller
+                            + ": Current activity is not visible (state=" + state.name() + ") "
+                            + "r=" + this);
+                }
         }
     }
 
@@ -1669,7 +1673,8 @@ final class ActivityRecord implements AppWindowContainerListener {
                 if (!idle) {
                     // Instead of doing the full stop routine here, let's just hide any activities
                     // we now can, and let them stop when the normal idle happens.
-                    mStackSupervisor.processStoppingActivitiesLocked(false);
+                    mStackSupervisor.processStoppingActivitiesLocked(null /* idleActivity */,
+                            false /* remove */, true /* processPausingActivities */);
                 } else {
                     // If this activity was already idle, then we now need to make sure we perform
                     // the full stop of any activities that are waiting to do so. This is because
@@ -2139,7 +2144,7 @@ final class ActivityRecord implements AppWindowContainerListener {
             // if the app is relaunched when it's stopped, and we're not resuming,
             // put it back into stopped state.
             if (stopped) {
-                getStack().addToStopping(this, true /* immediate */);
+                getStack().addToStopping(this, true /* scheduleIdle */, false /* idleDelayed */);
             }
         }
 

diff --git a/services/core/java/com/android/server/am/ActivityStack.java b/services/core/java/com/android/server/am/ActivityStack.java
index 9ce7ae3..df78a3d 100644 (file)
--- a/services/core/java/com/android/server/am/ActivityStack.java
+++ b/services/core/java/com/android/server/am/ActivityStack.java
@@ -1139,6 +1139,18 @@ final class ActivityStack extends ConfigurationContainer implements StackWindowL
     }
 
     /**
+     * Schedule a pause timeout in case the app doesn't respond. We don't give it much time because
+     * this directly impacts the responsiveness seen by the user.
+     */
+    private void schedulePauseTimeout(ActivityRecord r) {
+        final Message msg = mHandler.obtainMessage(PAUSE_TIMEOUT_MSG);
+        msg.obj = r;
+        r.pauseTime = SystemClock.uptimeMillis();
+        mHandler.sendMessageDelayed(msg, PAUSE_TIMEOUT);
+        if (DEBUG_PAUSE) Slog.v(TAG_PAUSE, "Waiting for pause to complete...");
+    }
+
+    /**
      * Start pausing the currently resumed activity.  It is an error to call this if there
      * is already an activity being paused or there is no resumed activity.
      *
@@ -1244,14 +1256,7 @@ final class ActivityStack extends ConfigurationContainer implements StackWindowL
                 return false;
 
             } else {
-                // Schedule a pause timeout in case the app doesn't respond.
-                // We don't give it much time because this directly impacts the
-                // responsiveness seen by the user.
-                Message msg = mHandler.obtainMessage(PAUSE_TIMEOUT_MSG);
-                msg.obj = prev;
-                prev.pauseTime = SystemClock.uptimeMillis();
-                mHandler.sendMessageDelayed(msg, PAUSE_TIMEOUT);
-                if (DEBUG_PAUSE) Slog.v(TAG_PAUSE, "Waiting for pause to complete...");
+                schedulePauseTimeout(prev);
                 return true;
             }
 
@@ -1332,7 +1337,7 @@ final class ActivityStack extends ConfigurationContainer implements StackWindowL
                         || mService.isSleepingOrShuttingDownLocked()) {
                     // If we were visible then resumeTopActivities will release resources before
                     // stopping.
-                    addToStopping(prev, true /* immediate */);
+                    addToStopping(prev, true /* scheduleIdle */, false /* idleDelayed */);
                 }
             } else {
                 if (DEBUG_PAUSE) Slog.v(TAG_PAUSE, "App died during pause, not stopping: " + prev);
@@ -1398,7 +1403,7 @@ final class ActivityStack extends ConfigurationContainer implements StackWindowL
         mStackSupervisor.ensureActivitiesVisibleLocked(resuming, 0, !PRESERVE_WINDOWS);
     }
 
-    void addToStopping(ActivityRecord r, boolean immediate) {
+    void addToStopping(ActivityRecord r, boolean scheduleIdle, boolean idleDelayed) {
         if (!mStackSupervisor.mStoppingActivities.contains(r)) {
             mStackSupervisor.mStoppingActivities.add(r);
         }
@@ -1409,11 +1414,14 @@ final class ActivityStack extends ConfigurationContainer implements StackWindowL
         // be cleared immediately.
         boolean forceIdle = mStackSupervisor.mStoppingActivities.size() > MAX_STOPPING_TO_FORCE
                 || (r.frontOfTask && mTaskHistory.size() <= 1);
-
-        if (immediate || forceIdle) {
+        if (scheduleIdle || forceIdle) {
             if (DEBUG_PAUSE) Slog.v(TAG_PAUSE, "Scheduling idle now: forceIdle="
-                    + forceIdle + "immediate=" + immediate);
-            mStackSupervisor.scheduleIdleLocked();
+                    + forceIdle + "immediate=" + !idleDelayed);
+            if (!idleDelayed) {
+                mStackSupervisor.scheduleIdleLocked();
+            } else {
+                mStackSupervisor.scheduleIdleTimeoutLocked(r);
+            }
         } else {
             mStackSupervisor.checkReadyForSleepLocked();
         }
@@ -1993,7 +2001,14 @@ final class ActivityStack extends ConfigurationContainer implements StackWindowL
                     if (visibleBehind == r) {
                         releaseBackgroundResources(r);
                     } else {
-                        addToStopping(r, true /* immediate */);
+                        // If this activity is in a state where it can currently enter
+                        // picture-in-picture, then don't immediately schedule the idle now in case
+                        // the activity tries to enterPictureInPictureMode() later. Otherwise,
+                        // we will try and stop the activity next time idle is processed.
+                        final boolean canEnterPictureInPicture = r.checkEnterPictureInPictureState(
+                                "makeInvisible", true /* noThrow */);
+                        addToStopping(r, true /* scheduleIdle */,
+                                canEnterPictureInPicture /* idleDelayed */);
                     }
                     break;
 
@@ -3555,7 +3570,7 @@ final class ActivityStack extends ConfigurationContainer implements StackWindowL
         if (mode == FINISH_AFTER_VISIBLE && (r.visible || r.nowVisible)
                 && next != null && !next.nowVisible) {
             if (!mStackSupervisor.mStoppingActivities.contains(r)) {
-                addToStopping(r, false /* immediate */);
+                addToStopping(r, false /* scheduleIdle */, false /* idleDelayed */);
             }
             if (DEBUG_STATES) Slog.v(TAG_STATES,
                     "Moving to STOPPING: "+ r + " (finish requested)");
@@ -3808,7 +3823,7 @@ final class ActivityStack extends ConfigurationContainer implements StackWindowL
         mWindowManager.notifyAppRelaunchesCleared(r.appToken);
     }
 
-    private void removeTimeoutsForActivityLocked(ActivityRecord r) {
+    void removeTimeoutsForActivityLocked(ActivityRecord r) {
         mStackSupervisor.removeTimeoutsForActivityLocked(r);
         mHandler.removeMessages(PAUSE_TIMEOUT_MSG, r);
         mHandler.removeMessages(STOP_TIMEOUT_MSG, r);
@@ -5073,6 +5088,7 @@ final class ActivityStack extends ConfigurationContainer implements StackWindowL
         // If the activity was previously pausing, then ensure we transfer that as well
         if (setPause) {
             mPausingActivity = r;
+            schedulePauseTimeout(r);
         }
         // Move the stack in which we are placing the activity to the front. The call will also
         // make sure the activity focus is set.
@@ -5114,6 +5130,7 @@ final class ActivityStack extends ConfigurationContainer implements StackWindowL
         }
         if (wasPaused) {
             prevStack.mPausingActivity = null;
+            prevStack.removeTimeoutsForActivityLocked(r);
         }
     }
 

diff --git a/services/core/java/com/android/server/am/ActivityStackSupervisor.java b/services/core/java/com/android/server/am/ActivityStackSupervisor.java
index b2b3e61..83a786f 100644 (file)
--- a/services/core/java/com/android/server/am/ActivityStackSupervisor.java
+++ b/services/core/java/com/android/server/am/ActivityStackSupervisor.java
@@ -1705,7 +1705,7 @@ public class ActivityStackSupervisor extends ConfigurationContainer implements D
 
     // Checked.
     final ActivityRecord activityIdleInternalLocked(final IBinder token, boolean fromTimeout,
-            Configuration config) {
+            boolean processPausingActivities, Configuration config) {
         if (DEBUG_ALL) Slog.v(TAG, "Activity idle: " + token);
 
         ArrayList<ActivityRecord> finishes = null;
@@ -1761,7 +1761,8 @@ public class ActivityStackSupervisor extends ConfigurationContainer implements D
         }
 
         // Atomically retrieve all of the other things to do.
-        final ArrayList<ActivityRecord> stops = processStoppingActivitiesLocked(true);
+        final ArrayList<ActivityRecord> stops = processStoppingActivitiesLocked(r,
+                true /* remove */, processPausingActivities);
         NS = stops != null ? stops.size() : 0;
         if ((NF = mFinishingActivities.size()) > 0) {
             finishes = new ArrayList<>(mFinishingActivities);
@@ -2689,6 +2690,7 @@ public class ActivityStackSupervisor extends ConfigurationContainer implements D
         // Reset the paused activity on the previous stack
         if (wasPaused) {
             prevStack.mPausingActivity = null;
+            prevStack.removeTimeoutsForActivityLocked(r);
         }
 
         // If the task had focus before (or we're requested to move focus),
@@ -3367,7 +3369,8 @@ public class ActivityStackSupervisor extends ConfigurationContainer implements D
         return mService.mUserController.isCurrentProfileLocked(userId);
     }
 
-    final ArrayList<ActivityRecord> processStoppingActivitiesLocked(boolean remove) {
+    final ArrayList<ActivityRecord> processStoppingActivitiesLocked(ActivityRecord idleActivity,
+            boolean remove, boolean processPausingActivities) {
         ArrayList<ActivityRecord> stops = null;
 
         final boolean nowVisible = allResumedActivitiesVisible();
@@ -3392,6 +3395,14 @@ public class ActivityStackSupervisor extends ConfigurationContainer implements D
                 }
             }
             if ((!waitingVisible || mService.isSleepingOrShuttingDownLocked()) && remove) {
+                if (!processPausingActivities && s.state == PAUSING) {
+                    // Defer processing pausing activities in this iteration and reschedule
+                    // a delayed idle to reprocess it again
+                    removeTimeoutsForActivityLocked(idleActivity);
+                    scheduleIdleTimeoutLocked(idleActivity);
+                    continue;
+                }
+
                 if (DEBUG_STATES) Slog.v(TAG, "Ready to stop: " + s);
                 if (stops == null) {
                     stops = new ArrayList<>();
@@ -4113,9 +4124,10 @@ public class ActivityStackSupervisor extends ConfigurationContainer implements D
             super(looper);
         }
 
-        void activityIdleInternal(ActivityRecord r) {
+        void activityIdleInternal(ActivityRecord r, boolean processPausingActivities) {
             synchronized (mService) {
-                activityIdleInternalLocked(r != null ? r.appToken : null, true, null);
+                activityIdleInternalLocked(r != null ? r.appToken : null, true /* fromTimeout */,
+                        processPausingActivities, null);
             }
         }
 
@@ -4150,11 +4162,13 @@ public class ActivityStackSupervisor extends ConfigurationContainer implements D
                     }
                     // We don't at this point know if the activity is fullscreen,
                     // so we need to be conservative and assume it isn't.
-                    activityIdleInternal((ActivityRecord)msg.obj);
+                    activityIdleInternal((ActivityRecord) msg.obj,
+                            true /* processPausingActivities */);
                 } break;
                 case IDLE_NOW_MSG: {
                     if (DEBUG_IDLE) Slog.d(TAG_IDLE, "handleMessage: IDLE_NOW_MSG: r=" + msg.obj);
-                    activityIdleInternal((ActivityRecord)msg.obj);
+                    activityIdleInternal((ActivityRecord) msg.obj,
+                            false /* processPausingActivities */);
                 } break;
                 case RESUME_TOP_ACTIVITY_MSG: {
                     synchronized (mService) {