commit ed0624e681a521782ff6fb8cf9c40dafb93c82d5
author Hyangseok Chae <neo.chae@lge.com> Thu Nov 07 10:15:46 2019 +0900
committer chaviw <chaviw@google.com> Wed Dec 18 17:05:13 2019 -0800
tree b07286f369fc72eb4ab544ed5d87b6fd58790dc2
parent df1552202170dd4d336c649d6852c041937ff003

Ignore PIP mode request for destroyed activity

PIP request can be postponed if keyguard is on.
If the activity is destroyed before keyguard is dismissed,
Then PIP operation causes NullPointerException.

In such case we should ignore PIP mode request.

Test:
Manual test -
 1.Configure Keyguard (Pattern, Password) for secure lock
 2.Receive a WhatsApp video call
 3.Press Back Key (WhatsApp enters into PIP mode)
 4.Caller ends a video call
 5.When user (receiver) unlocks the screen, system reboots
Test: atest WmTests:ActivityTaskManagerServiceTests#testEnterPipModeWhenRecordParentChangesToNull

Bug: 144045134

Change-Id: I8f67e57b2bfaa187829622b050c6ce387d749e8d
(cherry-picked from ae2c5e52efbd59b6bccd5b677b4047a292fe86fc)

services/core/java/com/android/server/wm/ActivityTaskManagerService.java

diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java
index f0bc412..e02456e 100644
--- a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java
+++ b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java
@@ -4176,6 +4176,10 @@
 
                 final Runnable enterPipRunnable = () -> {
                     synchronized (mGlobalLock) {
+                        if (r.getParent() == null) {
+                            Slog.e(TAG, "Skip enterPictureInPictureMode, destroyed " + r);
+                            return;
+                        }
                         // Only update the saved args from the args that are set
                         r.pictureInPictureArgs.copyOnlySet(params);
                         final float aspectRatio = r.pictureInPictureArgs.getAspectRatio();

services/tests/wmtests/src/com/android/server/wm/ActivityTaskManagerServiceTests.java

diff --git a/services/tests/wmtests/src/com/android/server/wm/ActivityTaskManagerServiceTests.java b/services/tests/wmtests/src/com/android/server/wm/ActivityTaskManagerServiceTests.java
index 591ed51..f1de6e9 100644
--- a/services/tests/wmtests/src/com/android/server/wm/ActivityTaskManagerServiceTests.java
+++ b/services/tests/wmtests/src/com/android/server/wm/ActivityTaskManagerServiceTests.java
@@ -19,15 +19,21 @@
 import static android.app.WindowConfiguration.ACTIVITY_TYPE_STANDARD;
 import static android.app.WindowConfiguration.WINDOWING_MODE_FREEFORM;
 
+import static com.android.dx.mockito.inline.extended.ExtendedMockito.any;
 import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn;
+import static com.android.dx.mockito.inline.extended.ExtendedMockito.mock;
+import static com.android.dx.mockito.inline.extended.ExtendedMockito.mockitoSession;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.when;
 
 import android.app.Activity;
 import android.app.ActivityManager;
+import android.app.PictureInPictureParams;
 import android.content.res.Configuration;
 import android.graphics.Rect;
+import android.os.IBinder;
 import android.view.IDisplayWindowListener;
 import android.view.WindowContainerTransaction;
 
@@ -36,6 +42,7 @@
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.MockitoSession;
 
 import java.util.ArrayList;
 
@@ -148,5 +155,43 @@
         assertEquals(0, changed.size());
         assertEquals(1, removed.size());
     }
+
+    /*
+        a test to verify b/144045134 - ignore PIP mode request for destroyed activity.
+        mocks r.getParent() to return null to cause NPE inside enterPipRunnable#run() in
+        ActivityTaskMangerservice#enterPictureInPictureMode(), which rebooted the device.
+        It doesn't fully simulate the issue's reproduce steps, but this should suffice.
+     */
+    @Test
+    public void testEnterPipModeWhenRecordParentChangesToNull() {
+        MockitoSession mockSession = mockitoSession()
+                .initMocks(this)
+                .mockStatic(ActivityRecord.class)
+                .startMocking();
+
+        ActivityRecord record = mock(ActivityRecord.class);
+        IBinder token = mock(IBinder.class);
+        PictureInPictureParams params = mock(PictureInPictureParams.class);
+        record.pictureInPictureArgs = params;
+
+        //mock operations in private method ensureValidPictureInPictureActivityParamsLocked()
+        when(ActivityRecord.forTokenLocked(token)).thenReturn(record);
+        doReturn(true).when(record).supportsPictureInPicture();
+        doReturn(false).when(params).hasSetAspectRatio();
+
+        //mock other operations
+        doReturn(true).when(record)
+                .checkEnterPictureInPictureState("enterPictureInPictureMode", false);
+        doReturn(false).when(mService).isInPictureInPictureMode(any());
+        doReturn(false).when(mService).isKeyguardLocked();
+
+        //to simulate NPE
+        doReturn(null).when(record).getParent();
+
+        mService.enterPictureInPictureMode(token, params);
+        //if record's null parent is not handled gracefully, test will fail with NPE
+
+        mockSession.finishMocking();
+    }
 }