From 87e5d55e98857bd43984c2395660d88ae20efcfc Mon Sep 17 00:00:00 2001 From: Winson Chung Date: Wed, 5 Apr 2017 11:49:38 -0700 Subject: [PATCH] Workaround for input ANR, always finish PiP menu activity. - Always finish the PiP menu activity when the interaction is complete (either the menu is hidden after showing, or when the user stops interacting with it and it was shown for the dismiss overlay) - Fix issue with bounds animation callback not working due to the app window being removed and not updating the app transition that its animation "finished" - Add additional logging throughout to trace PiP animation Bug: 36877782 Test: Enter PIP, tap to show menu, wait for it to hide, and then use wired headset button Change-Id: Ie88ba107d7fffdd182a4063ef4f324b58669d0ad --- .../systemui/pip/phone/PipMenuActivity.java | 22 +++++--------- .../pip/phone/PipMenuActivityController.java | 19 ++++++++++++ .../systemui/pip/phone/PipMotionHelper.java | 20 +++++++++---- .../systemui/pip/phone/PipTouchHandler.java | 29 +++++++++++++++---- .../com/android/server/wm/AppWindowToken.java | 7 +++++ .../server/wm/BoundsAnimationController.java | 15 +++++++--- .../server/wm/WindowSurfacePlacer.java | 2 ++ 7 files changed, 83 insertions(+), 31 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/pip/phone/PipMenuActivity.java b/packages/SystemUI/src/com/android/systemui/pip/phone/PipMenuActivity.java index fbd9f0c54725c..4e7cf723f662f 100644 --- a/packages/SystemUI/src/com/android/systemui/pip/phone/PipMenuActivity.java +++ b/packages/SystemUI/src/com/android/systemui/pip/phone/PipMenuActivity.java @@ -87,7 +87,9 @@ public class PipMenuActivity extends Activity { private boolean mMenuVisible; private boolean mAllowMenuTimeout = true; + private final List mActions = new ArrayList<>(); + private View mViewRoot; private Drawable mBackgroundDrawable; private View mMenuContainer; @@ -266,7 +268,6 @@ public class PipMenuActivity extends Activity { } notifyMenuVisibility(true); updateExpandButtonFromBounds(stackBounds, movementBounds); - setDecorViewVisibility(true); mMenuContainerAnimator = ObjectAnimator.ofFloat(mMenuContainer, View.ALPHA, mMenuContainer.getAlpha(), 1f); mMenuContainerAnimator.setInterpolator(Interpolators.ALPHA_IN); @@ -311,11 +312,15 @@ public class PipMenuActivity extends Activity { if (animationFinishedRunnable != null) { animationFinishedRunnable.run(); } - setDecorViewVisibility(false); + + finish(); } }); mMenuContainerAnimator.addUpdateListener(mMenuBgUpdateListener); mMenuContainerAnimator.start(); + } else { + // If the menu is not visible, just finish now + finish(); } } @@ -431,7 +436,6 @@ public class PipMenuActivity extends Activity { alpha = (int) (fraction * DISMISS_BACKGROUND_ALPHA * 255); } mBackgroundDrawable.setAlpha(alpha); - setDecorViewVisibility(alpha > 0); } private void notifyRegisterInputConsumer() { @@ -508,16 +512,4 @@ public class PipMenuActivity extends Activity { v.removeCallbacks(mFinishRunnable); v.postDelayed(mFinishRunnable, delay); } - - /** - * Sets the visibility of the root view of the window to disable drawing and touches for the - * activity. This differs from {@link Activity#setVisible(boolean)} in that it does not set - * the internal mVisibleFromClient state. - */ - private void setDecorViewVisibility(boolean visible) { - final View decorView = getWindow().getDecorView(); - if (decorView != null) { - decorView.setVisibility(visible ? View.VISIBLE : View.INVISIBLE); - } - } } diff --git a/packages/SystemUI/src/com/android/systemui/pip/phone/PipMenuActivityController.java b/packages/SystemUI/src/com/android/systemui/pip/phone/PipMenuActivityController.java index bcaa395288376..875fb141bb7b8 100644 --- a/packages/SystemUI/src/com/android/systemui/pip/phone/PipMenuActivityController.java +++ b/packages/SystemUI/src/com/android/systemui/pip/phone/PipMenuActivityController.java @@ -50,6 +50,7 @@ import java.util.List; public class PipMenuActivityController { private static final String TAG = "PipMenuActController"; + private static final boolean DEBUG = false; public static final String EXTRA_CONTROLLER_MESSENGER = "messenger"; public static final String EXTRA_ACTIONS = "actions"; @@ -195,6 +196,10 @@ public class PipMenuActivityController { * Updates the appearance of the menu and scrim on top of the PiP while dismissing. */ public void setDismissFraction(float fraction) { + if (DEBUG) { + Log.d(TAG, "setDismissFraction() hasActivity=" + (mToActivityMessenger != null) + + " fraction=" + fraction); + } if (mToActivityMessenger != null) { mTmpDismissFractionData.clear(); mTmpDismissFractionData.putFloat(EXTRA_DISMISS_FRACTION, fraction); @@ -216,6 +221,9 @@ public class PipMenuActivityController { * Shows the menu activity. */ public void showMenu(Rect stackBounds, Rect movementBounds, boolean allowMenuTimeout) { + if (DEBUG) { + Log.d(TAG, "showMenu() hasActivity=" + (mToActivityMessenger != null)); + } if (mToActivityMessenger != null) { Bundle data = new Bundle(); data.putParcelable(EXTRA_STACK_BOUNDS, stackBounds); @@ -238,6 +246,9 @@ public class PipMenuActivityController { * Pokes the menu, indicating that the user is interacting with it. */ public void pokeMenu() { + if (DEBUG) { + Log.d(TAG, "pokeMenu() hasActivity=" + (mToActivityMessenger != null)); + } if (mToActivityMessenger != null) { Message m = Message.obtain(); m.what = PipMenuActivity.MESSAGE_POKE_MENU; @@ -253,6 +264,9 @@ public class PipMenuActivityController { * Hides the menu activity. */ public void hideMenu() { + if (DEBUG) { + Log.d(TAG, "hideMenu() hasActivity=" + (mToActivityMessenger != null)); + } if (mToActivityMessenger != null) { Message m = Message.obtain(); m.what = PipMenuActivity.MESSAGE_HIDE_MENU; @@ -365,6 +379,10 @@ public class PipMenuActivityController { * Handles changes in menu visibility. */ private void onMenuVisibilityChanged(boolean visible, boolean resize) { + if (DEBUG) { + Log.d(TAG, "onMenuVisibilityChanged() mMenuVisible=" + mMenuVisible + + " menuVisible=" + visible + " resize=" + resize); + } if (visible) { mInputConsumerController.unregisterInputConsumer(); } else { @@ -389,6 +407,7 @@ public class PipMenuActivityController { final String innerPrefix = prefix + " "; pw.println(prefix + TAG); pw.println(innerPrefix + "mMenuVisible=" + mMenuVisible); + pw.println(innerPrefix + "mToActivityMessenger=" + mToActivityMessenger); pw.println(innerPrefix + "mListeners=" + mListeners.size()); } } diff --git a/packages/SystemUI/src/com/android/systemui/pip/phone/PipMotionHelper.java b/packages/SystemUI/src/com/android/systemui/pip/phone/PipMotionHelper.java index a14a71247da3b..fb8574da8e812 100644 --- a/packages/SystemUI/src/com/android/systemui/pip/phone/PipMotionHelper.java +++ b/packages/SystemUI/src/com/android/systemui/pip/phone/PipMotionHelper.java @@ -23,6 +23,7 @@ import static com.android.systemui.Interpolators.FAST_OUT_SLOW_IN; import static com.android.systemui.Interpolators.LINEAR_OUT_SLOW_IN; import android.animation.Animator; +import android.animation.Animator.AnimatorListener; import android.animation.AnimatorListenerAdapter; import android.animation.RectEvaluator; import android.animation.ValueAnimator; @@ -253,7 +254,7 @@ public class PipMotionHelper { * Flings the PiP to the closest snap target. */ Rect flingToSnapTarget(float velocity, float velocityX, float velocityY, Rect movementBounds, - AnimatorUpdateListener listener) { + AnimatorUpdateListener updateListener, AnimatorListener listener) { cancelAnimations(); Rect toBounds = mSnapAlgorithm.findClosestSnapBounds(movementBounds, mBounds, velocityX, velocityY); @@ -263,8 +264,11 @@ public class PipMotionHelper { mFlingAnimationUtils.apply(mBoundsAnimator, 0, distanceBetweenRectOffsets(mBounds, toBounds), velocity); - if (listener != null) { - mBoundsAnimator.addUpdateListener(listener); + if (updateListener != null) { + mBoundsAnimator.addUpdateListener(updateListener); + } + if (listener != null){ + mBoundsAnimator.addListener(listener); } mBoundsAnimator.start(); } @@ -274,14 +278,18 @@ public class PipMotionHelper { /** * Animates the PiP to the closest snap target. */ - Rect animateToClosestSnapTarget(Rect movementBounds, AnimatorUpdateListener listener) { + Rect animateToClosestSnapTarget(Rect movementBounds, AnimatorUpdateListener updateListener, + AnimatorListener listener) { cancelAnimations(); Rect toBounds = mSnapAlgorithm.findClosestSnapBounds(movementBounds, mBounds); if (!mBounds.equals(toBounds)) { mBoundsAnimator = createAnimationToBounds(mBounds, toBounds, SNAP_STACK_DURATION, FAST_OUT_SLOW_IN, mUpdateBoundsListener); - if (listener != null) { - mBoundsAnimator.addUpdateListener(listener); + if (updateListener != null) { + mBoundsAnimator.addUpdateListener(updateListener); + } + if (listener != null){ + mBoundsAnimator.addListener(listener); } mBoundsAnimator.start(); } diff --git a/packages/SystemUI/src/com/android/systemui/pip/phone/PipTouchHandler.java b/packages/SystemUI/src/com/android/systemui/pip/phone/PipTouchHandler.java index d68836c717e4c..161bdac77f89e 100644 --- a/packages/SystemUI/src/com/android/systemui/pip/phone/PipTouchHandler.java +++ b/packages/SystemUI/src/com/android/systemui/pip/phone/PipTouchHandler.java @@ -16,6 +16,8 @@ package com.android.systemui.pip.phone; +import android.animation.Animator; +import android.animation.AnimatorListenerAdapter; import android.animation.ValueAnimator; import android.animation.ValueAnimator.AnimatorUpdateListener; import android.app.IActivityManager; @@ -391,7 +393,10 @@ public class PipTouchHandler implements TunerService.Tunable { final float distance = bounds.bottom - target; fraction = Math.min(distance / bounds.height(), 1f); } - mMenuController.setDismissFraction(fraction); + if (Float.compare(fraction, 0f) != 0 || mMenuController.isMenuVisible()) { + // Update if the fraction > 0, or if fraction == 0 and the menu was already visible + mMenuController.setDismissFraction(fraction); + } } } @@ -611,22 +616,34 @@ public class PipTouchHandler implements TunerService.Tunable { setMinimizedStateInternal(false); } - // If the menu is still visible, and we aren't minimized, then just poke the menu - // so that it will timeout after the user stops touching it + AnimatorListenerAdapter postAnimationCallback = null; if (mMenuController.isMenuVisible()) { + // If the menu is still visible, and we aren't minimized, then just poke the + // menu so that it will timeout after the user stops touching it mMenuController.showMenu(mMotionHelper.getBounds(), mMovementBounds, true /* allowMenuTimeout */); + } else { + // If the menu is not visible, then we can still be showing the activity for the + // dismiss overlay, so just finish it after the animation completes + postAnimationCallback = new AnimatorListenerAdapter() { + @Override + public void onAnimationEnd(Animator animation) { + mMenuController.hideMenu(); + } + }; } if (isFling) { mMotionHelper.flingToSnapTarget(velocity, vel.x, vel.y, mMovementBounds, - mUpdateScrimListener); + mUpdateScrimListener, postAnimationCallback); } else { - mMotionHelper.animateToClosestSnapTarget(mMovementBounds, mUpdateScrimListener); + mMotionHelper.animateToClosestSnapTarget(mMovementBounds, mUpdateScrimListener, + postAnimationCallback); } } else if (mIsMinimized) { // This was a tap, so no longer minimized - mMotionHelper.animateToClosestSnapTarget(mMovementBounds, null /* listener */); + mMotionHelper.animateToClosestSnapTarget(mMovementBounds, null /* updateListener */, + null /* animatorListener */); setMinimizedStateInternal(false); } else if (!mIsMenuVisible) { mMenuController.showMenu(mMotionHelper.getBounds(), mMovementBounds, diff --git a/services/core/java/com/android/server/wm/AppWindowToken.java b/services/core/java/com/android/server/wm/AppWindowToken.java index 1decf4e19e9f6..a8664a5727ab4 100644 --- a/services/core/java/com/android/server/wm/AppWindowToken.java +++ b/services/core/java/com/android/server/wm/AppWindowToken.java @@ -505,6 +505,13 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree getController().removeStartingWindow(); } + // If this window was animating, then we need to ensure that the app transition notifies + // that animations have completed in WMS.handleAnimatingStoppedAndTransitionLocked(), so + // add to that list now + if (mAppAnimator.animating) { + mService.mNoAnimationNotifyOnTransitionFinished.add(token); + } + final TaskStack stack = getTask().mStack; if (delayed && !isEmpty()) { // set the token aside because it has an active animation to be finished diff --git a/services/core/java/com/android/server/wm/BoundsAnimationController.java b/services/core/java/com/android/server/wm/BoundsAnimationController.java index 9f0ed21004624..7b8057cafbb41 100644 --- a/services/core/java/com/android/server/wm/BoundsAnimationController.java +++ b/services/core/java/com/android/server/wm/BoundsAnimationController.java @@ -61,17 +61,21 @@ public class BoundsAnimationController { extends WindowManagerInternal.AppTransitionListener implements Runnable { public void onAppTransitionCancelledLocked() { + if (DEBUG) Slog.d(TAG, "onAppTransitionCancelledLocked:" + + " mFinishAnimationAfterTransition=" + mFinishAnimationAfterTransition); animationFinished(); } public void onAppTransitionFinishedLocked(IBinder token) { + if (DEBUG) Slog.d(TAG, "onAppTransitionFinishedLocked:" + + " mFinishAnimationAfterTransition=" + mFinishAnimationAfterTransition); animationFinished(); } private void animationFinished() { if (mFinishAnimationAfterTransition) { mHandler.removeCallbacks(this); - // This might end up calling into activity manager which will be bad since we have the - // window manager lock held at this point. Post a message to take care of the processing - // so we don't deadlock. + // This might end up calling into activity manager which will be bad since we have + // the window manager lock held at this point. Post a message to take care of the + // processing so we don't deadlock. mHandler.post(this); } } @@ -195,6 +199,7 @@ public class BoundsAnimationController { if (!mTarget.setPinnedStackSize(mTmpRect, mTmpTaskBounds)) { // Whoops, the target doesn't feel like animating anymore. Let's immediately finish // any further animation. + if (DEBUG) Slog.d(TAG, "animateUpdate: cancelled"); animation.cancel(); } } @@ -203,7 +208,9 @@ public class BoundsAnimationController { public void onAnimationEnd(Animator animation) { if (DEBUG) Slog.d(TAG, "onAnimationEnd: mTarget=" + mTarget + " mMoveToFullScreen=" + mMoveToFullScreen - + " mSkipAnimationEnd=" + mSkipAnimationEnd); + + " mSkipAnimationEnd=" + mSkipAnimationEnd + + " mFinishAnimationAfterTransition=" + mFinishAnimationAfterTransition + + " mAppTransitionIsRunning=" + mAppTransition.isRunning()); // There could be another animation running. For example in the // move to fullscreen case, recents will also be closing while the diff --git a/services/core/java/com/android/server/wm/WindowSurfacePlacer.java b/services/core/java/com/android/server/wm/WindowSurfacePlacer.java index 3cb96a1145fc6..ee2d5de710c14 100644 --- a/services/core/java/com/android/server/wm/WindowSurfacePlacer.java +++ b/services/core/java/com/android/server/wm/WindowSurfacePlacer.java @@ -441,6 +441,8 @@ class WindowSurfacePlacer { if (DEBUG_APP_TRANSITIONS) Slog.v(TAG, "Now closing app " + wtoken); appAnimator.clearThumbnail(); appAnimator.setNullAnimation(); + // TODO: Do we need to add to mNoAnimationNotifyOnTransitionFinished like above if not + // animating? wtoken.setVisibility(animLp, false, transit, false, voiceInteraction); wtoken.updateReportedVisibilityLocked(); // Force the allDrawn flag, because we want to start