From d42c56fece36299072476e8597183dc4b3db2423 Mon Sep 17 00:00:00 2001 From: Steve Elliott Date: Wed, 11 May 2022 11:29:00 -0400 Subject: [PATCH] Break pipeline re-entrant call due to dismiss anim Fixes: 224606120 Test: manual 1. Have one clearable notification in the shade 2. Tap "Clear All" 3. Receive an update to the notification before the animation has completed 4. Verify no crash Change-Id: Iefd021ac6bf6ca9c959a6f410ca96f51e77142cd --- .../src/com/android/systemui/SwipeHelper.java | 6 ++- .../row/StackScrollerDecorView.java | 47 ++++++++++++------- .../stack/NotificationStackScrollLayout.java | 23 ++++++--- 3 files changed, 51 insertions(+), 25 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/SwipeHelper.java b/packages/SystemUI/src/com/android/systemui/SwipeHelper.java index 3d0c08bb52374..fe6dbe5de8f0d 100644 --- a/packages/SystemUI/src/com/android/systemui/SwipeHelper.java +++ b/packages/SystemUI/src/com/android/systemui/SwipeHelper.java @@ -46,6 +46,8 @@ import com.android.systemui.plugins.statusbar.NotificationMenuRowPlugin; import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow; import com.android.wm.shell.animation.FlingAnimationUtils; +import java.util.function.Consumer; + public class SwipeHelper implements Gefingerpoken { static final String TAG = "com.android.systemui.SwipeHelper"; private static final boolean DEBUG = false; @@ -399,7 +401,7 @@ public class SwipeHelper implements Gefingerpoken { * @param useAccelerateInterpolator Should an accelerating Interpolator be used * @param fixedDuration If not 0, this exact duration will be taken */ - public void dismissChild(final View animView, float velocity, final Runnable endAction, + public void dismissChild(final View animView, float velocity, final Consumer endAction, long delay, boolean useAccelerateInterpolator, long fixedDuration, boolean isDismissAll) { final boolean canBeDismissed = mCallback.canChildBeDismissed(animView); @@ -487,7 +489,7 @@ public class SwipeHelper implements Gefingerpoken { resetSwipeState(); } if (endAction != null) { - endAction.run(); + endAction.accept(mCancelled); } if (!mDisableHwLayers) { animView.setLayerType(View.LAYER_TYPE_NONE, null); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/StackScrollerDecorView.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/StackScrollerDecorView.java index aa3e027fe1afb..7414bdc136723 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/StackScrollerDecorView.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/StackScrollerDecorView.java @@ -16,6 +16,7 @@ package com.android.systemui.statusbar.notification.row; +import android.animation.Animator; import android.animation.AnimatorListenerAdapter; import android.content.Context; import android.util.AttributeSet; @@ -25,6 +26,8 @@ import android.view.animation.Interpolator; import com.android.internal.annotations.VisibleForTesting; import com.android.systemui.animation.Interpolators; +import java.util.function.Consumer; + /** * A common base class for all views in the notification stack scroller which don't have a * background. @@ -48,7 +51,7 @@ public abstract class StackScrollerDecorView extends ExpandableView { }; private boolean mSecondaryAnimating = false; - private final Runnable mSecondaryVisibilityEndRunnable = () -> { + private final Consumer mSecondaryVisibilityEndRunnable = (cancelled) -> { mSecondaryAnimating = false; // If we were on screen, become GONE to avoid touches if (mSecondaryView == null) return; @@ -96,18 +99,20 @@ public abstract class StackScrollerDecorView extends ExpandableView { * @param animate True if we should fade to new visibility * @param runAfter Runnable to run after visibility updates */ - public void setContentVisible(boolean visible, boolean animate, Runnable runAfter) { + public void setContentVisible(boolean visible, boolean animate, Consumer runAfter) { if (mContentVisible != visible) { mContentAnimating = animate; mContentVisible = visible; - Runnable endRunnable = runAfter == null ? mContentVisibilityEndRunnable : () -> { + Consumer endRunnable = (cancelled) -> { mContentVisibilityEndRunnable.run(); - runAfter.run(); + if (runAfter != null) { + runAfter.accept(cancelled); + } }; setViewVisible(mContent, visible, animate, endRunnable); } else if (runAfter != null) { // Execute the runAfter runnable immediately if there's no animation to perform. - runAfter.run(); + runAfter.accept(true); } if (!mContentAnimating) { @@ -119,10 +124,6 @@ public abstract class StackScrollerDecorView extends ExpandableView { return mContentVisible; } - public void setVisible(boolean nowVisible, boolean animate) { - setVisible(nowVisible, animate, null); - } - /** * Make this view visible. If {@code false} is passed, the view will fade out it's content * and set the view Visibility to GONE. If only the content should be changed @@ -131,7 +132,7 @@ public abstract class StackScrollerDecorView extends ExpandableView { * @param nowVisible should the view be visible * @param animate should the change be animated. */ - public void setVisible(boolean nowVisible, boolean animate, Runnable runAfter) { + public void setVisible(boolean nowVisible, boolean animate) { if (mIsVisible != nowVisible) { mIsVisible = nowVisible; if (animate) { @@ -142,10 +143,10 @@ public abstract class StackScrollerDecorView extends ExpandableView { } else { setWillBeGone(true); } - setContentVisible(nowVisible, true /* animate */, runAfter); + setContentVisible(nowVisible, true /* animate */, null /* runAfter */); } else { setVisibility(nowVisible ? VISIBLE : GONE); - setContentVisible(nowVisible, false /* animate */, runAfter); + setContentVisible(nowVisible, false /* animate */, null /* runAfter */); setWillBeGone(false); notifyHeightChanged(false /* needsAnimation */); } @@ -166,7 +167,7 @@ public abstract class StackScrollerDecorView extends ExpandableView { } if (!mSecondaryAnimating) { - mSecondaryVisibilityEndRunnable.run(); + mSecondaryVisibilityEndRunnable.accept(true /* cancelled */); } } @@ -195,7 +196,7 @@ public abstract class StackScrollerDecorView extends ExpandableView { * @param endRunnable A runnable that is run when the animation is done. */ private void setViewVisible(View view, boolean nowVisible, - boolean animate, Runnable endRunnable) { + boolean animate, Consumer endRunnable) { if (view == null) { return; } @@ -211,7 +212,7 @@ public abstract class StackScrollerDecorView extends ExpandableView { if (!animate) { view.setAlpha(endValue); if (endRunnable != null) { - endRunnable.run(); + endRunnable.accept(true); } return; } @@ -222,7 +223,19 @@ public abstract class StackScrollerDecorView extends ExpandableView { .alpha(endValue) .setInterpolator(interpolator) .setDuration(mDuration) - .withEndAction(endRunnable); + .setListener(new AnimatorListenerAdapter() { + boolean mCancelled; + + @Override + public void onAnimationCancel(Animator animation) { + mCancelled = true; + } + + @Override + public void onAnimationEnd(Animator animation) { + endRunnable.accept(mCancelled); + } + }); } @Override @@ -231,7 +244,7 @@ public abstract class StackScrollerDecorView extends ExpandableView { Runnable onFinishedRunnable, AnimatorListenerAdapter animationListener) { // TODO: Use duration - setContentVisible(false, true /* animate */, onFinishedRunnable); + setContentVisible(false, true /* animate */, (cancelled) -> onFinishedRunnable.run()); return 0; } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java index 24369e7b860f1..4d325e154a4f2 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java @@ -1805,13 +1805,20 @@ public class NotificationStackScrollLayout extends ViewGroup implements Dumpable } @ShadeViewRefactor(RefactorComponent.STATE_RESOLVER) - public void dismissViewAnimated(View child, Runnable endRunnable, int delay, long duration) { + public void dismissViewAnimated( + View child, Consumer endRunnable, int delay, long duration) { if (child instanceof SectionHeaderView) { ((StackScrollerDecorView) child).setContentVisible( false /* visible */, true /* animate */, endRunnable); return; } - mSwipeHelper.dismissChild(child, 0, endRunnable, delay, true, duration, + mSwipeHelper.dismissChild( + child, + 0 /* velocity */, + endRunnable, + delay, + true /* useAccelerateInterpolator */, + duration, true /* isClearAll */); } @@ -5196,11 +5203,15 @@ public class NotificationStackScrollLayout extends ViewGroup implements Dumpable if (mClearAllListener != null) { mClearAllListener.onClearAll(selection); } - final Runnable dismissInBackend = () -> { - onClearAllAnimationsEnd(rowsToDismissInBackend, selection); + final Consumer dismissInBackend = (cancelled) -> { + if (cancelled) { + post(() -> onClearAllAnimationsEnd(rowsToDismissInBackend, selection)); + } else { + onClearAllAnimationsEnd(rowsToDismissInBackend, selection); + } }; if (viewsToAnimateAway.isEmpty()) { - dismissInBackend.run(); + dismissInBackend.accept(true); return; } // Disable normal animations @@ -5215,7 +5226,7 @@ public class NotificationStackScrollLayout extends ViewGroup implements Dumpable final int numItems = viewsToAnimateAway.size(); for (int i = numItems - 1; i >= 0; i--) { View view = viewsToAnimateAway.get(i); - Runnable endRunnable = null; + Consumer endRunnable = null; if (i == 0) { endRunnable = dismissInBackend; }