[Messages] Fix clipping of message banner because of SurfaceView hole

This CL delegates the Animator#start calls in MessageBannerMediator to
WindowAndroid#startAnimationOverContent.

Bug: 1159429
Change-Id: I865e62718efa46eae8e1bf733271854475edaa00
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2628497
Commit-Queue: Sinan Sahin <sinansahin@google.com>
Reviewed-by: Theresa  <twellington@chromium.org>
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843685}

chrome/android/java/src/org/chromium/chrome/browser/ui/RootUiCoordinator.java

@@ -438,8 +438,9 @@ public class RootUiCoordinator
             MessageContainer container = mActivity.findViewById(R.id.message_container);
             mMessageContainerCoordinator =
                     new MessageContainerCoordinator(container, getBrowserControlsManager());
-            mMessageDispatcher = MessagesFactory.createMessageDispatcher(
-                    container, mMessageContainerCoordinator::getMessageMaxTranslation);
+            mMessageDispatcher = MessagesFactory.createMessageDispatcher(container,
+                    mMessageContainerCoordinator::getMessageMaxTranslation,
+                    mActivity.getWindowAndroid());
             mMessageQueueMediator = new ChromeMessageQueueMediator(
                     mActivity.getBrowserControlsManager(), mMessageContainerCoordinator,
                     mActivity.getFullscreenManager(), mLayoutStateProviderOneShotSupplier,

components/messages/android/internal/java/src/org/chromium/components/messages/MessageDispatcherImpl.java

@@ -5,6 +5,7 @@
 package org.chromium.components.messages;
 
 import org.chromium.base.supplier.Supplier;
+import org.chromium.ui.base.WindowAndroid;
 import org.chromium.ui.modelutil.PropertyModel;
 
 /**
@@ -15,23 +16,27 @@ public class MessageDispatcherImpl implements ManagedMessageDispatcher {
     private final MessageQueueManager mMessageQueueManager = new MessageQueueManager();
     private final MessageContainer mMessageContainer;
     private final Supplier<Integer> mMessageMaxTranslationSupplier;
+    private final WindowAndroid mWindowAndroid;
 
     /**
      * Build a new message dispatcher
      * @param messageContainer A container view for displaying message banners.
-     * @param messageMaxTranslationSupplier A {@link Supplier} that supplies the maximum translation
-     *         Y value the message banner can have as a result of the animations or the gestures.
+     * @param messageMaxTranslation A {@link Supplier} that supplies the maximum translation Y value
+     *         the message banner can have as a result of the animations or the gestures.
+     * @param windowAndroid The {@link WindowAndroid} with which the Message is associated.
      */
-    public MessageDispatcherImpl(
-            MessageContainer messageContainer, Supplier<Integer> messageMaxTranslation) {
+    public MessageDispatcherImpl(MessageContainer messageContainer,
+            Supplier<Integer> messageMaxTranslation, WindowAndroid windowAndroid) {
         mMessageContainer = messageContainer;
         mMessageMaxTranslationSupplier = messageMaxTranslation;
+        mWindowAndroid = windowAndroid;
     }
 
     @Override
     public void enqueueMessage(PropertyModel messageProperties) {
-        MessageStateHandler messageStateHandler = new SingleActionMessage(mMessageContainer,
-                messageProperties, this::dismissMessage, mMessageMaxTranslationSupplier);
+        MessageStateHandler messageStateHandler =
+                new SingleActionMessage(mMessageContainer, messageProperties, this::dismissMessage,
+                        mMessageMaxTranslationSupplier, mWindowAndroid);
         mMessageQueueManager.enqueueMessage(messageStateHandler, messageProperties);
     }
 

components/messages/android/internal/java/src/org/chromium/components/messages/MessagesFactory.java

@@ -17,11 +17,12 @@ public class MessagesFactory {
      *         to the MessageContainer. When messages are shown, they will be animated down the
      *         screen, starting at the negative |messageMaxTranslation| y translation to the resting
      *         position in the MessageContainer.
+     * @param windowAndroid The {@link WindowAndroid} with which the Message is associated.
      * @return The constructed ManagedMessageDispatcher.
      */
-    public static ManagedMessageDispatcher createMessageDispatcher(
-            MessageContainer container, Supplier<Integer> messageMaxTranslation) {
-        return new MessageDispatcherImpl(container, messageMaxTranslation);
+    public static ManagedMessageDispatcher createMessageDispatcher(MessageContainer container,
+            Supplier<Integer> messageMaxTranslation, WindowAndroid windowAndroid) {
+        return new MessageDispatcherImpl(container, messageMaxTranslation, windowAndroid);
     }
 
     /**

components/messages/android/java/src/org/chromium/components/messages/MessageBannerCoordinator.java

@@ -11,6 +11,7 @@ import androidx.core.view.ViewCompat;
 import androidx.core.view.accessibility.AccessibilityNodeInfoCompat.AccessibilityActionCompat;
 
 import org.chromium.base.supplier.Supplier;
+import org.chromium.ui.base.WindowAndroid;
 import org.chromium.ui.modelutil.PropertyModel;
 import org.chromium.ui.modelutil.PropertyModelChangeProcessor;
 
@@ -32,15 +33,17 @@ class MessageBannerCoordinator {
      * @param resources The {@link Resources}.
      * @param messageDismissed The {@link Runnable} that will run if and when the user dismisses the
      *         message.
+     * @param windowAndroid The {@link WindowAndroid} that will be used to start the animations so
+     *         the message is not clipped as a result of some Android SurfaceView optimization.
      */
     MessageBannerCoordinator(MessageBannerView view, PropertyModel model,
             Supplier<Integer> maxTranslationSupplier, Resources resources,
-            Runnable messageDismissed) {
+            Runnable messageDismissed, WindowAndroid windowAndroid) {
         mView = view;
         mModel = model;
         PropertyModelChangeProcessor.create(model, view, MessageBannerViewBinder::bind);
         mMediator = new MessageBannerMediator(
-                model, maxTranslationSupplier, resources, messageDismissed);
+                model, maxTranslationSupplier, resources, messageDismissed, windowAndroid);
         view.setSwipeHandler(mMediator);
         ViewCompat.replaceAccessibilityAction(
                 view, AccessibilityActionCompat.ACTION_DISMISS, null, (v, c) -> {

components/messages/android/java/src/org/chromium/components/messages/MessageBannerMediator.java

@@ -25,6 +25,7 @@ import org.chromium.components.browser_ui.widget.animation.CancelAwareAnimatorLi
 import org.chromium.components.browser_ui.widget.animation.Interpolators;
 import org.chromium.components.browser_ui.widget.gesture.SwipeGestureListener.ScrollDirection;
 import org.chromium.components.browser_ui.widget.gesture.SwipeGestureListener.SwipeHandler;
+import org.chromium.ui.base.WindowAndroid;
 import org.chromium.ui.modelutil.PropertyModel;
 import org.chromium.ui.modelutil.PropertyModel.WritableFloatPropertyKey;
 import org.chromium.ui.modelutil.PropertyModelAnimatorFactory;
@@ -69,6 +70,7 @@ class MessageBannerMediator implements SwipeHandler {
     private final float mHorizontalHideThresholdPx;
     private final Supplier<Float> mMaxHorizontalTranslationPx;
     private final Runnable mMessageDismissed;
+    private final WindowAndroid mWindowAndroid;
 
     private Animator mAnimation;
     @State
@@ -82,7 +84,7 @@ class MessageBannerMediator implements SwipeHandler {
      * Constructs the message banner mediator.
      */
     MessageBannerMediator(PropertyModel model, Supplier<Integer> maxTranslationSupplier,
-            Resources resources, Runnable messageDismissed) {
+            Resources resources, Runnable messageDismissed, WindowAndroid windowAndroid) {
         mModel = model;
         mMaxTranslationYSupplier = maxTranslationSupplier;
         mVerticalHideThresholdPx =
@@ -96,6 +98,7 @@ class MessageBannerMediator implements SwipeHandler {
                     screenWidth / 2);
         };
         mMessageDismissed = messageDismissed;
+        mWindowAndroid = windowAndroid;
     }
 
     /**
@@ -107,8 +110,7 @@ class MessageBannerMediator implements SwipeHandler {
             mModel.set(TRANSLATION_Y, -mMaxTranslationYSupplier.get());
         }
         cancelAnyAnimations();
-        mAnimation = createAnimation(true, 0, false, messageShown);
-        mAnimation.start();
+        startAnimation(true, 0, false, messageShown);
     }
 
     /**
@@ -122,8 +124,7 @@ class MessageBannerMediator implements SwipeHandler {
         }
 
         cancelAnyAnimations();
-        mAnimation = createAnimation(true, -mMaxTranslationYSupplier.get(), false, messageHidden);
-        mAnimation.start();
+        startAnimation(true, -mMaxTranslationYSupplier.get(), false, messageHidden);
     }
 
     void setOnTouchRunnable(Runnable runnable) {
@@ -189,9 +190,8 @@ class MessageBannerMediator implements SwipeHandler {
                     ? 0
                     : MathUtils.flipSignIf(mMaxHorizontalTranslationPx.get(), translationX < 0);
         }
-        mAnimation = createAnimation(
+        startAnimation(
                 isVertical, translateTo, false, translateTo != 0 ? mMessageDismissed : () -> {});
-        mAnimation.start();
     }
 
     @Override
@@ -222,9 +222,8 @@ class MessageBannerMediator implements SwipeHandler {
 
         // TODO(crbug.com/1157213): See if we can use velocity to change the animation
         // speed/duration.
-        mAnimation = createAnimation(isVertical(mSwipeDirection), translateTo, velocity != 0,
+        startAnimation(isVertical(mSwipeDirection), translateTo, velocity != 0,
                 translateTo != 0 ? mMessageDismissed : () -> {});
-        mAnimation.start();
     }
 
     @Override
@@ -236,14 +235,13 @@ class MessageBannerMediator implements SwipeHandler {
     // endregion
 
     /**
-     * Create an animation.
+     * Create and start an animation.
      * @param vertical Whether the message is being animated vertically.
      * @param translateTo Target translation value for the animation.
      * @param didFling Whether the animation is the result of a fling gesture.
      * @param onEndCallback Callback that will be called after the animation.
-     * @return The {@link Animator}
      */
-    private Animator createAnimation(
+    private void startAnimation(
             boolean vertical, float translateTo, boolean didFling, Runnable onEndCallback) {
         final long duration = translateTo == 0 ? ENTER_DURATION_MS : EXIT_DURATION_MS;
 
@@ -286,7 +284,8 @@ class MessageBannerMediator implements SwipeHandler {
             }
         });
 
-        return animatorSet;
+        mAnimation = animatorSet;
+        mWindowAndroid.startAnimationOverContent(mAnimation);
     }
 
     private void cancelAnyAnimations() {

components/messages/android/java/src/org/chromium/components/messages/MessageBannerMediatorUnitTest.java

@@ -7,12 +7,15 @@ package org.chromium.components.messages;
 import static android.os.Looper.getMainLooper;
 
 import static org.junit.Assert.assertEquals;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 import static org.robolectric.Shadows.shadowOf;
 import static org.robolectric.annotation.LooperMode.Mode.PAUSED;
 
+import android.animation.Animator;
 import android.content.res.Resources;
 import android.util.DisplayMetrics;
 import android.view.MotionEvent;
@@ -32,6 +35,7 @@ import org.chromium.base.MathUtils;
 import org.chromium.base.supplier.Supplier;
 import org.chromium.base.test.BaseRobolectricTestRunner;
 import org.chromium.components.browser_ui.widget.gesture.SwipeGestureListener.ScrollDirection;
+import org.chromium.ui.base.WindowAndroid;
 import org.chromium.ui.modelutil.PropertyModel;
 
 /** Unit tests for {@link MessageBannerMediator}. */
@@ -54,6 +58,8 @@ public class MessageBannerMediatorUnitTest {
     private Runnable mShownRunnable;
     @Mock
     private Runnable mHiddenRunnable;
+    @Mock
+    private WindowAndroid mWindowAndroid;
 
     private MessageBannerMediator mMediator;
     private PropertyModel mModel;
@@ -72,8 +78,14 @@ public class MessageBannerMediatorUnitTest {
                 .thenReturn(24);
         when(mResources.getDimensionPixelSize(R.dimen.message_max_horizontal_translation))
                 .thenReturn(120);
+        doAnswer(invocation -> {
+            ((Animator) invocation.getArguments()[0]).start();
+            return null;
+        })
+                .when(mWindowAndroid)
+                .startAnimationOverContent(any(Animator.class));
         mMediator = new MessageBannerMediator(
-                mModel, mMaxTranslationSupplier, mResources, mDismissedRunnable);
+                mModel, mMaxTranslationSupplier, mResources, mDismissedRunnable, mWindowAndroid);
         when(mMaxTranslationSupplier.get()).thenReturn(100);
     }
 

components/messages/android/java/src/org/chromium/components/messages/SingleActionMessage.java

@@ -13,6 +13,7 @@ import androidx.annotation.VisibleForTesting;
 
 import org.chromium.base.Callback;
 import org.chromium.base.supplier.Supplier;
+import org.chromium.ui.base.WindowAndroid;
 import org.chromium.ui.modelutil.PropertyModel;
 
 /**
@@ -29,6 +30,7 @@ public class SingleActionMessage implements MessageStateHandler {
     private final Callback<PropertyModel> mDismissHandler;
     private MessageAutoDismissTimer mAutoDismissTimer;
     private final Supplier<Integer> mMaxTranslationSupplier;
+    private final WindowAndroid mWindowAndroid;
 
     /**
      * @param container The container holding messages.
@@ -38,14 +40,19 @@ public class SingleActionMessage implements MessageStateHandler {
      *         property model.
      * @param maxTranslationSupplier A {@link Supplier} that supplies the maximum translation Y
      *         value the message banner can have as a result of the animations or the gestures.
+     * @param windowAndroid The {@link WindowAndroid} that will be used by the message banner to
+     *         start the animations so the message is not clipped as a result of some Android
+     *         SurfaceView optimization.
      */
     public SingleActionMessage(MessageContainer container, PropertyModel model,
-            Callback<PropertyModel> dismissHandler, Supplier<Integer> maxTranslationSupplier) {
+            Callback<PropertyModel> dismissHandler, Supplier<Integer> maxTranslationSupplier,
+            WindowAndroid windowAndroid) {
         mModel = model;
         mContainer = container;
         mDismissHandler = dismissHandler;
         mAutoDismissTimer = new MessageAutoDismissTimer(getAutoDismissDuration());
         mMaxTranslationSupplier = maxTranslationSupplier;
+        mWindowAndroid = windowAndroid;
 
         mModel.set(
                 MessageBannerProperties.PRIMARY_BUTTON_CLICK_LISTENER, this::handlePrimaryAction);
@@ -61,7 +68,7 @@ public class SingleActionMessage implements MessageStateHandler {
             mView = (MessageBannerView) LayoutInflater.from(mContainer.getContext())
                             .inflate(R.layout.message_banner_view, mContainer, false);
             mMessageBanner = new MessageBannerCoordinator(mView, mModel, mMaxTranslationSupplier,
-                    mContainer.getResources(), mDismissHandler.bind(mModel));
+                    mContainer.getResources(), mDismissHandler.bind(mModel), mWindowAndroid);
         }
         mContainer.addMessage(mView);
 

components/messages/android/java/src/org/chromium/components/messages/SingleActionMessageTest.java

@@ -18,6 +18,7 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
+import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.junit.MockitoJUnit;
 import org.mockito.junit.MockitoRule;
@@ -26,6 +27,7 @@ import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.base.Callback;
 import org.chromium.base.test.BaseJUnit4ClassRunner;
 import org.chromium.base.test.util.CallbackHelper;
+import org.chromium.ui.base.WindowAndroid;
 import org.chromium.ui.modelutil.PropertyModel;
 import org.chromium.ui.test.util.DummyUiActivityTestCase;
 import org.chromium.ui.util.AccessibilityUtil;
@@ -37,6 +39,8 @@ import org.chromium.ui.util.AccessibilityUtil;
 public class SingleActionMessageTest extends DummyUiActivityTestCase {
     @Rule
     public MockitoRule mMockitoRule = MockitoJUnit.rule();
+    @Mock
+    private WindowAndroid mWindowAndroid;
 
     private CallbackHelper mDismissCallback;
     private Callback<PropertyModel> mEmptyDismissCallback = (model) -> {};
@@ -55,8 +59,8 @@ public class SingleActionMessageTest extends DummyUiActivityTestCase {
     public void testAddAndRemoveSingleActionMessage() throws Exception {
         MessageContainer container = new MessageContainer(getActivity(), null);
         PropertyModel model = createBasicSingleActionMessageModel();
-        SingleActionMessage message =
-                new SingleActionMessage(container, model, mEmptyDismissCallback, () -> 0);
+        SingleActionMessage message = new SingleActionMessage(
+                container, model, mEmptyDismissCallback, () -> 0, mWindowAndroid);
         final MessageBannerCoordinator messageBanner = Mockito.mock(MessageBannerCoordinator.class);
         doNothing().when(messageBanner).show(any(Runnable.class));
         doNothing().when(messageBanner).setOnTouchRunnable(any(Runnable.class));
@@ -87,8 +91,8 @@ public class SingleActionMessageTest extends DummyUiActivityTestCase {
         MessageContainer container = new MessageContainer(getActivity(), null);
         PropertyModel m1 = createBasicSingleActionMessageModel();
         PropertyModel m2 = createBasicSingleActionMessageModel();
-        SingleActionMessage message1 =
-                new SingleActionMessage(container, m1, mEmptyDismissCallback, () -> 0);
+        SingleActionMessage message1 = new SingleActionMessage(
+                container, m1, mEmptyDismissCallback, () -> 0, mWindowAndroid);
         final MessageBannerCoordinator messageBanner1 =
                 Mockito.mock(MessageBannerCoordinator.class);
         doNothing().when(messageBanner1).show(any(Runnable.class));
@@ -96,8 +100,8 @@ public class SingleActionMessageTest extends DummyUiActivityTestCase {
         view1.setId(R.id.message_banner);
         message1.setMessageBannerForTesting(messageBanner1);
         message1.setViewForTesting(view1);
-        SingleActionMessage message2 =
-                new SingleActionMessage(container, m2, mEmptyDismissCallback, () -> 0);
+        SingleActionMessage message2 = new SingleActionMessage(
+                container, m2, mEmptyDismissCallback, () -> 0, mWindowAndroid);
         final MessageBannerCoordinator messageBanner2 =
                 Mockito.mock(MessageBannerCoordinator.class);
         doNothing().when(messageBanner2).show(any(Runnable.class));