From c4d29f2a1c6e8e9c3cdb3fc2bf8a8151fb24716b Mon Sep 17 00:00:00 2001 From: Jorim Jaggi Date: Thu, 22 Mar 2018 16:30:56 +0100 Subject: [PATCH] Fix NPE when animation doesn't get started If animation start is delayed, there is no guarantee that startAnimation is called on the adapter. Thus, protect RemoteAnimationController against this case, as leash and finishedCallback would be null. Furthermore don't even try to start remote animation that is delayed as it would leave it hanging forever. Test: SurfaceAnimatorTest/RemoteAnimationControllerTest Change-Id: I72c492c4fb6ca8eeae481d2c281e9c1fee95f921 Fixes: 76096628 --- .../com/android/server/wm/AppWindowToken.java | 5 ++- .../server/wm/RemoteAnimationController.java | 12 ++++--- .../android/server/wm/SurfaceAnimator.java | 4 +++ .../wm/RemoteAnimationControllerTest.java | 32 ++++++++++++++++++- .../server/wm/SurfaceAnimatorTest.java | 1 + 5 files changed, 48 insertions(+), 6 deletions(-) diff --git a/services/core/java/com/android/server/wm/AppWindowToken.java b/services/core/java/com/android/server/wm/AppWindowToken.java index a76857e877d80..85436da8dab71 100644 --- a/services/core/java/com/android/server/wm/AppWindowToken.java +++ b/services/core/java/com/android/server/wm/AppWindowToken.java @@ -1698,7 +1698,10 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree stack.getBounds(mTmpRect); mTmpRect.offsetTo(0, 0); } - if (mService.mAppTransition.getRemoteAnimationController() != null) { + + // Delaying animation start isn't compatible with remote animations at all. + if (mService.mAppTransition.getRemoteAnimationController() != null + && !mSurfaceAnimator.isAnimationStartDelayed()) { adapter = mService.mAppTransition.getRemoteAnimationController() .createAnimationAdapter(this, mTmpPoint, mTmpRect); } else { diff --git a/services/core/java/com/android/server/wm/RemoteAnimationController.java b/services/core/java/com/android/server/wm/RemoteAnimationController.java index d7f480b78f859..c5900677e41e9 100644 --- a/services/core/java/com/android/server/wm/RemoteAnimationController.java +++ b/services/core/java/com/android/server/wm/RemoteAnimationController.java @@ -99,6 +99,10 @@ class RemoteAnimationController { mFinishedCallback = new FinishedCallback(this); final RemoteAnimationTarget[] animations = createAnimations(); + if (animations.length == 0) { + onAnimationFinished(); + return; + } mService.mAnimator.addAfterPrepareSurfacesRunnable(() -> { try { mRemoteAnimationAdapter.getRunner().onAnimationStart(animations, @@ -132,6 +136,8 @@ class RemoteAnimationController { mPendingAnimations.get(i).createRemoteAppAnimation(); if (target != null) { targets.add(target); + } else { + mPendingAnimations.remove(i); } } return targets.toArray(new RemoteAnimationTarget[targets.size()]); @@ -225,10 +231,8 @@ class RemoteAnimationController { RemoteAnimationTarget createRemoteAppAnimation() { final Task task = mAppWindowToken.getTask(); final WindowState mainWindow = mAppWindowToken.findMainWindow(); - if (task == null) { - return null; - } - if (mainWindow == null) { + if (task == null || mainWindow == null || mCapturedFinishCallback == null + || mCapturedLeash == null) { return null; } mTarget = new RemoteAnimationTarget(task.mTaskId, getMode(), diff --git a/services/core/java/com/android/server/wm/SurfaceAnimator.java b/services/core/java/com/android/server/wm/SurfaceAnimator.java index e5928b1e66756..ba3d091f4a088 100644 --- a/services/core/java/com/android/server/wm/SurfaceAnimator.java +++ b/services/core/java/com/android/server/wm/SurfaceAnimator.java @@ -234,6 +234,10 @@ class SurfaceAnimator { mService.mAnimationTransferMap.put(mAnimation, this); } + boolean isAnimationStartDelayed() { + return mAnimationStartDelayed; + } + /** * Cancels the animation, and resets the leash. * diff --git a/services/tests/servicestests/src/com/android/server/wm/RemoteAnimationControllerTest.java b/services/tests/servicestests/src/com/android/server/wm/RemoteAnimationControllerTest.java index 26a7313b71d82..64501e49a9a81 100644 --- a/services/tests/servicestests/src/com/android/server/wm/RemoteAnimationControllerTest.java +++ b/services/tests/servicestests/src/com/android/server/wm/RemoteAnimationControllerTest.java @@ -68,6 +68,7 @@ public class RemoteAnimationControllerTest extends WindowTestsBase { super.setUp(); MockitoAnnotations.initMocks(this); mAdapter = new RemoteAnimationAdapter(mMockRunner, 100, 50); + mAdapter.setCallingPid(123); sWm.mH.runWithScissors(() -> { mHandler = new TestHandler(null, mClock); }, 0); @@ -83,7 +84,7 @@ public class RemoteAnimationControllerTest extends WindowTestsBase { new Point(50, 100), new Rect(50, 100, 150, 150)); adapter.startAnimation(mMockLeash, mMockTransaction, mFinishedCallback); mController.goodToGo(); - + sWm.mAnimator.executeAfterPrepareSurfacesRunnables(); final ArgumentCaptor appsCaptor = ArgumentCaptor.forClass(RemoteAnimationTarget[].class); final ArgumentCaptor finishedCaptor = @@ -167,4 +168,33 @@ public class RemoteAnimationControllerTest extends WindowTestsBase { mController.goodToGo(); verifyZeroInteractions(mMockRunner); } + + @Test + public void testNotReallyStarted() throws Exception { + final WindowState win = createWindow(null /* parent */, TYPE_BASE_APPLICATION, "testWin"); + mController.createAnimationAdapter(win.mAppToken, + new Point(50, 100), new Rect(50, 100, 150, 150)); + mController.goodToGo(); + verifyZeroInteractions(mMockRunner); + } + + @Test + public void testOneNotStarted() throws Exception { + final WindowState win1 = createWindow(null /* parent */, TYPE_BASE_APPLICATION, "testWin1"); + final WindowState win2 = createWindow(null /* parent */, TYPE_BASE_APPLICATION, "testWin2"); + mController.createAnimationAdapter(win1.mAppToken, + new Point(50, 100), new Rect(50, 100, 150, 150)); + final AnimationAdapter adapter = mController.createAnimationAdapter(win2.mAppToken, + new Point(50, 100), new Rect(50, 100, 150, 150)); + adapter.startAnimation(mMockLeash, mMockTransaction, mFinishedCallback); + mController.goodToGo(); + sWm.mAnimator.executeAfterPrepareSurfacesRunnables(); + final ArgumentCaptor appsCaptor = + ArgumentCaptor.forClass(RemoteAnimationTarget[].class); + final ArgumentCaptor finishedCaptor = + ArgumentCaptor.forClass(IRemoteAnimationFinishedCallback.class); + verify(mMockRunner).onAnimationStart(appsCaptor.capture(), finishedCaptor.capture()); + assertEquals(1, appsCaptor.getValue().length); + assertEquals(mMockLeash, appsCaptor.getValue()[0].leash); + } } diff --git a/services/tests/servicestests/src/com/android/server/wm/SurfaceAnimatorTest.java b/services/tests/servicestests/src/com/android/server/wm/SurfaceAnimatorTest.java index 6506872458587..16b84581de398 100644 --- a/services/tests/servicestests/src/com/android/server/wm/SurfaceAnimatorTest.java +++ b/services/tests/servicestests/src/com/android/server/wm/SurfaceAnimatorTest.java @@ -134,6 +134,7 @@ public class SurfaceAnimatorTest extends WindowTestsBase { mAnimatable.mSurfaceAnimator.startAnimation(mTransaction, mSpec, true /* hidden */); verifyZeroInteractions(mSpec); assertAnimating(mAnimatable); + assertTrue(mAnimatable.mSurfaceAnimator.isAnimationStartDelayed()); mAnimatable.mSurfaceAnimator.endDelayingAnimationStart(); verify(mSpec).startAnimation(any(), any(), any()); }