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

This reverts commit 6a340039.

Reason for revert: Causes test failure, see https://ci.chromium.org/ui/p/chromium/builders/ci/android-bfcache-rel/7275/overview

Original change's description:
> [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}

TBR=pavely@chromium.org,twellington@chromium.org,sinansahin@google.com,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com

Change-Id: Id20e63d9c11d00514ce5f40796a724aa760535e4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1159429
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2630448Reviewed-by: Sky Malice <skym@chromium.org>
Reviewed-by: Wenyu Fu <wenyufu@chromium.org>
Commit-Queue: Wenyu Fu <wenyufu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843780}

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

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

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

@@ -5,7 +5,6 @@
 package org.chromium.components.messages;
 
 import org.chromium.base.supplier.Supplier;
-import org.chromium.ui.base.WindowAndroid;
 import org.chromium.ui.modelutil.PropertyModel;
 
 /**
@@ -16,27 +15,23 @@ 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 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.
+     * @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.
      */
-    public MessageDispatcherImpl(MessageContainer messageContainer,
-            Supplier<Integer> messageMaxTranslation, WindowAndroid windowAndroid) {
+    public MessageDispatcherImpl(
+            MessageContainer messageContainer, Supplier<Integer> messageMaxTranslation) {
         mMessageContainer = messageContainer;
         mMessageMaxTranslationSupplier = messageMaxTranslation;
-        mWindowAndroid = windowAndroid;
     }
 
     @Override
     public void enqueueMessage(PropertyModel messageProperties) {
-        MessageStateHandler messageStateHandler =
-                new SingleActionMessage(mMessageContainer, messageProperties, this::dismissMessage,
-                        mMessageMaxTranslationSupplier, mWindowAndroid);
+        MessageStateHandler messageStateHandler = new SingleActionMessage(mMessageContainer,
+                messageProperties, this::dismissMessage, mMessageMaxTranslationSupplier);
         mMessageQueueManager.enqueueMessage(messageStateHandler, messageProperties);
     }
 

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

@@ -17,12 +17,11 @@ 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, WindowAndroid windowAndroid) {
-        return new MessageDispatcherImpl(container, messageMaxTranslation, windowAndroid);
+    public static ManagedMessageDispatcher createMessageDispatcher(
+            MessageContainer container, Supplier<Integer> messageMaxTranslation) {
+        return new MessageDispatcherImpl(container, messageMaxTranslation);
     }
 
     /**

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

@@ -11,7 +11,6 @@ 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;
 
@@ -33,17 +32,15 @@ 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, WindowAndroid windowAndroid) {
+            Runnable messageDismissed) {
         mView = view;
         mModel = model;
         PropertyModelChangeProcessor.create(model, view, MessageBannerViewBinder::bind);
         mMediator = new MessageBannerMediator(
-                model, maxTranslationSupplier, resources, messageDismissed, windowAndroid);
+                model, maxTranslationSupplier, resources, messageDismissed);
         view.setSwipeHandler(mMediator);
         ViewCompat.replaceAccessibilityAction(
                 view, AccessibilityActionCompat.ACTION_DISMISS, null, (v, c) -> {

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

@@ -25,7 +25,6 @@ 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;
@@ -70,7 +69,6 @@ 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
@@ -84,7 +82,7 @@ class MessageBannerMediator implements SwipeHandler {
      * Constructs the message banner mediator.
      */
     MessageBannerMediator(PropertyModel model, Supplier<Integer> maxTranslationSupplier,
-            Resources resources, Runnable messageDismissed, WindowAndroid windowAndroid) {
+            Resources resources, Runnable messageDismissed) {
         mModel = model;
         mMaxTranslationYSupplier = maxTranslationSupplier;
         mVerticalHideThresholdPx =
@@ -98,7 +96,6 @@ class MessageBannerMediator implements SwipeHandler {
                     screenWidth / 2);
         };
         mMessageDismissed = messageDismissed;
-        mWindowAndroid = windowAndroid;
     }
 
     /**
@@ -110,7 +107,8 @@ class MessageBannerMediator implements SwipeHandler {
             mModel.set(TRANSLATION_Y, -mMaxTranslationYSupplier.get());
         }
         cancelAnyAnimations();
-        startAnimation(true, 0, false, messageShown);
+        mAnimation = createAnimation(true, 0, false, messageShown);
+        mAnimation.start();
     }
 
     /**
@@ -124,7 +122,8 @@ class MessageBannerMediator implements SwipeHandler {
         }
 
         cancelAnyAnimations();
-        startAnimation(true, -mMaxTranslationYSupplier.get(), false, messageHidden);
+        mAnimation = createAnimation(true, -mMaxTranslationYSupplier.get(), false, messageHidden);
+        mAnimation.start();
     }
 
     void setOnTouchRunnable(Runnable runnable) {
@@ -190,8 +189,9 @@ class MessageBannerMediator implements SwipeHandler {
                     ? 0
                     : MathUtils.flipSignIf(mMaxHorizontalTranslationPx.get(), translationX < 0);
         }
-        startAnimation(
+        mAnimation = createAnimation(
                 isVertical, translateTo, false, translateTo != 0 ? mMessageDismissed : () -> {});
+        mAnimation.start();
     }
 
     @Override
@@ -222,8 +222,9 @@ class MessageBannerMediator implements SwipeHandler {
 
         // TODO(crbug.com/1157213): See if we can use velocity to change the animation
         // speed/duration.
-        startAnimation(isVertical(mSwipeDirection), translateTo, velocity != 0,
+        mAnimation = createAnimation(isVertical(mSwipeDirection), translateTo, velocity != 0,
                 translateTo != 0 ? mMessageDismissed : () -> {});
+        mAnimation.start();
     }
 
     @Override
@@ -235,13 +236,14 @@ class MessageBannerMediator implements SwipeHandler {
     // endregion
 
     /**
-     * Create and start an animation.
+     * Create 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 void startAnimation(
+    private Animator createAnimation(
             boolean vertical, float translateTo, boolean didFling, Runnable onEndCallback) {
         final long duration = translateTo == 0 ? ENTER_DURATION_MS : EXIT_DURATION_MS;
 
@@ -284,8 +286,7 @@ class MessageBannerMediator implements SwipeHandler {
             }
         });
 
-        mAnimation = animatorSet;
-        mWindowAndroid.startAnimationOverContent(mAnimation);
+        return animatorSet;
     }
 
     private void cancelAnyAnimations() {

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

@@ -7,15 +7,12 @@ 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;
@@ -35,7 +32,6 @@ 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}. */
@@ -58,8 +54,6 @@ public class MessageBannerMediatorUnitTest {
     private Runnable mShownRunnable;
     @Mock
     private Runnable mHiddenRunnable;
-    @Mock
-    private WindowAndroid mWindowAndroid;
 
     private MessageBannerMediator mMediator;
     private PropertyModel mModel;
@@ -78,14 +72,8 @@ 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, mWindowAndroid);
+                mModel, mMaxTranslationSupplier, mResources, mDismissedRunnable);
         when(mMaxTranslationSupplier.get()).thenReturn(100);
     }
 

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

@@ -13,7 +13,6 @@ 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;
 
 /**
@@ -30,7 +29,6 @@ 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.
@@ -40,19 +38,14 @@ 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,
-            WindowAndroid windowAndroid) {
+            Callback<PropertyModel> dismissHandler, Supplier<Integer> maxTranslationSupplier) {
         mModel = model;
         mContainer = container;
         mDismissHandler = dismissHandler;
         mAutoDismissTimer = new MessageAutoDismissTimer(getAutoDismissDuration());
         mMaxTranslationSupplier = maxTranslationSupplier;
-        mWindowAndroid = windowAndroid;
 
         mModel.set(
                 MessageBannerProperties.PRIMARY_BUTTON_CLICK_LISTENER, this::handlePrimaryAction);
@@ -68,7 +61,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), mWindowAndroid);
+                    mContainer.getResources(), mDismissHandler.bind(mModel));
         }
         mContainer.addMessage(mView);
 

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

@@ -18,7 +18,6 @@ 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;
@@ -27,7 +26,6 @@ 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;
@@ -39,8 +37,6 @@ 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) -> {};
@@ -59,8 +55,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, mWindowAndroid);
+        SingleActionMessage message =
+                new SingleActionMessage(container, model, mEmptyDismissCallback, () -> 0);
         final MessageBannerCoordinator messageBanner = Mockito.mock(MessageBannerCoordinator.class);
         doNothing().when(messageBanner).show(any(Runnable.class));
         doNothing().when(messageBanner).setOnTouchRunnable(any(Runnable.class));
@@ -91,8 +87,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, mWindowAndroid);
+        SingleActionMessage message1 =
+                new SingleActionMessage(container, m1, mEmptyDismissCallback, () -> 0);
         final MessageBannerCoordinator messageBanner1 =
                 Mockito.mock(MessageBannerCoordinator.class);
         doNothing().when(messageBanner1).show(any(Runnable.class));
@@ -100,8 +96,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, mWindowAndroid);
+        SingleActionMessage message2 =
+                new SingleActionMessage(container, m2, mEmptyDismissCallback, () -> 0);
         final MessageBannerCoordinator messageBanner2 =
                 Mockito.mock(MessageBannerCoordinator.class);
         doNothing().when(messageBanner2).show(any(Runnable.class));