Reset PiP params on PiP component change

In PipTaskListener, the internal PictureInPictureParams is updated via
the copyOnlySet method. Therefore, if an app enters PiP with empty
aspect ratio, it would accidentally inherit the aspect ratio from the
last Activity in pip.

There is a similar issue: when enter PiP from Maps after exit Meet PiP,
the menu controls would be accidentally inherited.

Fix this issue by reset the PictureInPictureParams value once we detect
a PiP component change, i.e. a different app enters PiP. And in
PhonePipMenuController, cleanup the actions accordingly.

Flag: com.android.wm.shell.enable_pip2
Bug: 381181514
Video: http://recall/-/aaaaaabFQoRHlzixHdtY/eb99XBhbvNmwWawpeIqPM0
Test: atest WMShellUnitTests:PipTaskListenerTest \
            WMShellUnitTests:PipBoundsStateTest
Test: manually following the reproduce steps in bug, see Video
Change-Id: I6796afda4a13b5110bd5fc5af076c6bf719f1c4d

5 files changed, 119 insertions, 13 deletions

diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/common/pip/PipBoundsState.java b/libs/WindowManager/Shell/src/com/android/wm/shell/common/pip/PipBoundsState.java
index 214e6ad455a1..aeef211ae3f3 100644
--- a/libs/WindowManager/Shell/src/com/android/wm/shell/common/pip/PipBoundsState.java
+++ b/libs/WindowManager/Shell/src/com/android/wm/shell/common/pip/PipBoundsState.java
@@ -143,6 +143,9 @@ public class PipBoundsState {
      */
     public final Rect mCachedLauncherShelfHeightKeepClearArea = new Rect();
 
+    private final List<OnPipComponentChangedListener> mOnPipComponentChangedListeners =
+            new ArrayList<>();
+
     // the size of the current bounds relative to the max size spec
     private float mBoundsScale;
 
@@ -156,9 +159,7 @@ public class PipBoundsState {
         // Update the relative proportion of the bounds compared to max possible size. Max size
         // spec takes the aspect ratio of the bounds into account, so both width and height
         // scale by the same factor.
-        addPipExclusionBoundsChangeCallback((bounds) -> {
-            updateBoundsScale();
-        });
+        addPipExclusionBoundsChangeCallback((bounds) -> updateBoundsScale());
     }
 
     /** Reloads the resources. */
@@ -341,11 +342,14 @@ public class PipBoundsState {
     /** Set the last {@link ComponentName} to enter PIP mode. */
     public void setLastPipComponentName(@Nullable ComponentName lastPipComponentName) {
         final boolean changed = !Objects.equals(mLastPipComponentName, lastPipComponentName);
+        if (!changed) return;
+        clearReentryState();
+        setHasUserResizedPip(false);
+        setHasUserMovedPip(false);
+        final ComponentName oldComponentName = mLastPipComponentName;
         mLastPipComponentName = lastPipComponentName;
-        if (changed) {
-            clearReentryState();
-            setHasUserResizedPip(false);
-            setHasUserMovedPip(false);
+        for (OnPipComponentChangedListener listener : mOnPipComponentChangedListeners) {
+            listener.onPipComponentChanged(oldComponentName, mLastPipComponentName);
         }
     }
 
@@ -616,6 +620,21 @@ public class PipBoundsState {
         }
     }
 
+    /** Adds callback to listen on component change. */
+    public void addOnPipComponentChangedListener(@NonNull OnPipComponentChangedListener listener) {
+        if (!mOnPipComponentChangedListeners.contains(listener)) {
+            mOnPipComponentChangedListeners.add(listener);
+        }
+    }
+
+    /** Removes callback to listen on component change. */
+    public void removeOnPipComponentChangedListener(
+            @NonNull OnPipComponentChangedListener listener) {
+        if (mOnPipComponentChangedListeners.contains(listener)) {
+            mOnPipComponentChangedListeners.remove(listener);
+        }
+    }
+
     public LauncherState getLauncherState() {
         return mLauncherState;
     }
@@ -695,7 +714,7 @@ public class PipBoundsState {
      * Represents the state of pip to potentially restore upon reentry.
      */
     @VisibleForTesting
-    public static final class PipReentryState {
+    static final class PipReentryState {
         private static final String TAG = PipReentryState.class.getSimpleName();
 
         private final float mSnapFraction;
@@ -722,6 +741,22 @@ public class PipBoundsState {
         }
     }
 
+    /**
+     * Listener interface for PiP component change, i.e. the app in pip mode changes
+     * TODO: Move this out of PipBoundsState once pip1 is deprecated.
+     */
+    public interface OnPipComponentChangedListener {
+        /**
+         * Callback when the component in pip mode changes.
+         * @param oldPipComponent previous component in pip mode,
+         *                        {@code null} if this is the very first time PiP appears.
+         * @param newPipComponent new component that enters pip mode.
+         */
+        void onPipComponentChanged(
+                @Nullable ComponentName oldPipComponent,
+                @NonNull ComponentName newPipComponent);
+    }
+
     /** Dumps internal state. */
     public void dump(PrintWriter pw, String prefix) {
         final String innerPrefix = prefix + "  ";
diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/pip2/phone/PhonePipMenuController.java b/libs/WindowManager/Shell/src/com/android/wm/shell/pip2/phone/PhonePipMenuController.java
index 65099c2dfb9d..671eae3d84ef 100644
--- a/libs/WindowManager/Shell/src/com/android/wm/shell/pip2/phone/PhonePipMenuController.java
+++ b/libs/WindowManager/Shell/src/com/android/wm/shell/pip2/phone/PhonePipMenuController.java
@@ -153,7 +153,12 @@ public class PhonePipMenuController implements PipMenuController,
         mPipUiEventLogger = pipUiEventLogger;
 
         mPipTransitionState.addPipTransitionStateChangedListener(this);
-
+        // Clear actions after exit PiP. Otherwise, next PiP could accidentally inherit the
+        // actions provided by the previous app in PiP mode.
+        mPipBoundsState.addOnPipComponentChangedListener(((oldPipComponent, newPipComponent) -> {
+            if (mAppActions != null) mAppActions.clear();
+            mCloseAction = null;
+        }));
         mPipTaskListener.addParamsChangedListener(new PipTaskListener.PipParamsChangedCallback() {
             @Override
             public void onActionsChanged(List<RemoteAction> actions, RemoteAction closeAction) {
diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/pip2/phone/PipTaskListener.java b/libs/WindowManager/Shell/src/com/android/wm/shell/pip2/phone/PipTaskListener.java
index d6634845ee21..294ef48c01d0 100644
--- a/libs/WindowManager/Shell/src/com/android/wm/shell/pip2/phone/PipTaskListener.java
+++ b/libs/WindowManager/Shell/src/com/android/wm/shell/pip2/phone/PipTaskListener.java
@@ -61,7 +61,7 @@ public class PipTaskListener implements ShellTaskOrganizer.TaskListener,
     private final PipBoundsState mPipBoundsState;
     private final PipBoundsAlgorithm mPipBoundsAlgorithm;
     private final ShellExecutor mMainExecutor;
-    private final PictureInPictureParams mPictureInPictureParams =
+    private PictureInPictureParams mPictureInPictureParams =
             new PictureInPictureParams.Builder().build();
 
     private boolean mWaitingForAspectRatioChange = false;
@@ -92,6 +92,11 @@ public class PipTaskListener implements ShellTaskOrganizer.TaskListener,
         }
         mPipResizeAnimatorSupplier = PipResizeAnimator::new;
         mPipScheduler.setPipParamsSupplier(this::getPictureInPictureParams);
+        // Reset {@link #mPictureInPictureParams} after exiting PiP. For instance, next Activity
+        // with null aspect ratio would accidentally inherit the aspect ratio from a previous
+        // PiP Activity.
+        mPipBoundsState.addOnPipComponentChangedListener(((oldPipComponent, newPipComponent) ->
+                mPictureInPictureParams = new PictureInPictureParams.Builder().build()));
     }
 
     void setPictureInPictureParams(@Nullable PictureInPictureParams params) {
@@ -138,9 +143,8 @@ public class PipTaskListener implements ShellTaskOrganizer.TaskListener,
         if (mPictureInPictureParams.hasSetAspectRatio()
                 && mPipBoundsAlgorithm.isValidPictureInPictureAspectRatio(newAspectRatio)
                 && PipUtils.aspectRatioChanged(newAspectRatio, mPipBoundsState.getAspectRatio())) {
-            mPipTransitionState.setOnIdlePipTransitionStateRunnable(() -> {
-                onAspectRatioChanged(newAspectRatio);
-            });
+            mPipTransitionState.setOnIdlePipTransitionStateRunnable(
+                    () -> onAspectRatioChanged(newAspectRatio));
         }
     }
 
diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/common/pip/PipBoundsStateTest.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/common/pip/PipBoundsStateTest.java
index 01b76edd9b25..1066276becc7 100644
--- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/common/pip/PipBoundsStateTest.java
+++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/common/pip/PipBoundsStateTest.java
@@ -19,6 +19,8 @@ package com.android.wm.shell.common.pip;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
@@ -128,6 +130,31 @@ public class PipBoundsStateTest extends ShellTestCase {
     }
 
     @Test
+    public void setLastPipComponentName_notChanged_doesNotCallbackComponentChangedListener() {
+        mPipBoundsState.setLastPipComponentName(mTestComponentName1);
+        PipBoundsState.OnPipComponentChangedListener mockListener =
+                mock(PipBoundsState.OnPipComponentChangedListener.class);
+
+        mPipBoundsState.addOnPipComponentChangedListener(mockListener);
+        mPipBoundsState.setLastPipComponentName(mTestComponentName1);
+
+        verify(mockListener, never()).onPipComponentChanged(any(), any());
+    }
+
+    @Test
+    public void setLastPipComponentName_changed_callbackComponentChangedListener() {
+        mPipBoundsState.setLastPipComponentName(mTestComponentName1);
+        PipBoundsState.OnPipComponentChangedListener mockListener =
+                mock(PipBoundsState.OnPipComponentChangedListener.class);
+
+        mPipBoundsState.addOnPipComponentChangedListener(mockListener);
+        mPipBoundsState.setLastPipComponentName(mTestComponentName2);
+
+        verify(mockListener).onPipComponentChanged(
+                eq(mTestComponentName1), eq(mTestComponentName2));
+    }
+
+    @Test
     public void testSetLastPipComponentName_notChanged_doesNotClearReentryState() {
         mPipBoundsState.setLastPipComponentName(mTestComponentName1);
         mPipBoundsState.saveReentryState(DEFAULT_SNAP_FRACTION);
diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip2/phone/PipTaskListenerTest.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip2/phone/PipTaskListenerTest.java
index 333569a7206e..5029371c3419 100644
--- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip2/phone/PipTaskListenerTest.java
+++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/pip2/phone/PipTaskListenerTest.java
@@ -24,6 +24,7 @@ import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.anyFloat;
 import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 import static org.mockito.kotlin.MatchersKt.eq;
 import static org.mockito.kotlin.VerificationKt.clearInvocations;
@@ -35,7 +36,9 @@ import android.app.ActivityManager;
 import android.app.PendingIntent;
 import android.app.PictureInPictureParams;
 import android.app.RemoteAction;
+import android.content.ComponentName;
 import android.content.Context;
+import android.content.res.Resources;
 import android.graphics.Rect;
 import android.graphics.drawable.Icon;
 import android.os.Bundle;
@@ -48,8 +51,10 @@ import androidx.test.filters.SmallTest;
 
 import com.android.wm.shell.ShellTaskOrganizer;
 import com.android.wm.shell.common.ShellExecutor;
+import com.android.wm.shell.common.pip.PhoneSizeSpecSource;
 import com.android.wm.shell.common.pip.PipBoundsAlgorithm;
 import com.android.wm.shell.common.pip.PipBoundsState;
+import com.android.wm.shell.common.pip.PipDisplayLayoutState;
 import com.android.wm.shell.pip2.animation.PipResizeAnimator;
 
 import org.junit.Before;
@@ -107,6 +112,16 @@ public class PipTaskListenerTest {
     }
 
     @Test
+    public void constructor_addOnPipComponentChangedListener() {
+        mPipTaskListener = new PipTaskListener(mMockContext, mMockShellTaskOrganizer,
+                mMockPipTransitionState, mMockPipScheduler, mMockPipBoundsState,
+                mMockPipBoundsAlgorithm, mMockShellExecutor);
+
+        verify(mMockPipBoundsState).addOnPipComponentChangedListener(
+                any(PipBoundsState.OnPipComponentChangedListener.class));
+    }
+
+    @Test
     public void setPictureInPictureParams_updatePictureInPictureParams() {
         mPipTaskListener = new PipTaskListener(mMockContext, mMockShellTaskOrganizer,
                 mMockPipTransitionState, mMockPipScheduler, mMockPipBoundsState,
@@ -359,6 +374,26 @@ public class PipTaskListenerTest {
         verify(mMockPipResizeAnimator, times(0)).start();
     }
 
+    @Test
+    public void onPipComponentChanged_clearPictureInPictureParams() {
+        when(mMockContext.getResources()).thenReturn(mock(Resources.class));
+        PipBoundsState pipBoundsState = new PipBoundsState(mMockContext,
+                mock(PhoneSizeSpecSource.class), mock(PipDisplayLayoutState.class));
+        pipBoundsState.setLastPipComponentName(new ComponentName("org.test", "test1"));
+
+        mPipTaskListener = new PipTaskListener(mMockContext, mMockShellTaskOrganizer,
+                mMockPipTransitionState, mMockPipScheduler, pipBoundsState,
+                mMockPipBoundsAlgorithm, mMockShellExecutor);
+        Rational aspectRatio = new Rational(4, 3);
+        String action1 = "action1";
+        mPipTaskListener.setPictureInPictureParams(getPictureInPictureParams(
+                aspectRatio, action1));
+
+        pipBoundsState.setLastPipComponentName(new ComponentName("org.test", "test2"));
+
+        assertTrue(mPipTaskListener.getPictureInPictureParams().empty());
+    }
+
     private PictureInPictureParams getPictureInPictureParams(Rational aspectRatio,
             String... actions) {
         final PictureInPictureParams.Builder builder = new PictureInPictureParams.Builder();