Replace LayoutUpdateHost usages with a Runnable

This patch replaces usages of the layout LayoutUpdateHost that only use
the requestUpdate method to instead depend on a Runnable.

Bug: 1070281
Change-Id: I4af2b265cd5d3e3121afb8134729ed7a79ac09cf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2490237Reviewed-by: Mei Liang <meiliang@chromium.org>
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819872}

chrome/android/chrome_junit_test_java_sources.gni

@@ -45,7 +45,6 @@ chrome_junit_test_java_sources = [
   "junit/src/org/chromium/chrome/browser/compositor/animation/CompositorAnimationHandlerTest.java",
   "junit/src/org/chromium/chrome/browser/compositor/animation/CompositorAnimatorTest.java",
   "junit/src/org/chromium/chrome/browser/compositor/layouts/CompositorModelChangeProcessorUnitTest.java",
-  "junit/src/org/chromium/chrome/browser/compositor/layouts/MockLayoutUpdateHost.java",
   "junit/src/org/chromium/chrome/browser/compositor/layouts/SceneOverlayTest.java",
   "junit/src/org/chromium/chrome/browser/compositor/layouts/StaticLayoutUnitTest.java",
   "junit/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelperTest.java",

chrome/android/java/src/org/chromium/chrome/browser/compositor/animation/CompositorAnimationHandler.java

@@ -10,8 +10,6 @@ import android.animation.AnimatorListenerAdapter;
 import androidx.annotation.NonNull;
 import androidx.annotation.VisibleForTesting;
 
-import org.chromium.chrome.browser.compositor.layouts.LayoutUpdateHost;
-
 import java.util.ArrayList;
 
 /**
@@ -25,8 +23,8 @@ public class CompositorAnimationHandler {
     /** A list of all the handler's animators. */
     private final ArrayList<CompositorAnimator> mAnimators = new ArrayList<>();
 
-    /** This handler's update host. */
-    private final LayoutUpdateHost mUpdateHost;
+    /** A means of requesting a render from the compositor. */
+    private final Runnable mRenderRequestRunnable;
 
     /**
      * A cached copy of the list of {@link CompositorAnimator}s to prevent allocating a new list
@@ -45,12 +43,12 @@ public class CompositorAnimationHandler {
 
     /**
      * Default constructor.
-     * @param host A {@link LayoutUpdateHost} responsible for requesting frames when an animation
-     *             updates.
+     * @param renderRequestRunnable A {@link Runnable} responsible for requesting frames when an
+     *                              animation updates.
      */
-    public CompositorAnimationHandler(@NonNull LayoutUpdateHost host) {
-        assert host != null;
-        mUpdateHost = host;
+    public CompositorAnimationHandler(@NonNull Runnable renderRequestRunnable) {
+        assert renderRequestRunnable != null;
+        mRenderRequestRunnable = renderRequestRunnable;
     }
 
     /**
@@ -72,7 +70,7 @@ public class CompositorAnimationHandler {
         mAnimators.add(animator);
 
         if (!mWasUpdateRequestedForAnimationStart) {
-            mUpdateHost.requestUpdate();
+            mRenderRequestRunnable.run();
             mWasUpdateRequestedForAnimationStart = true;
         }
 
@@ -114,7 +112,7 @@ public class CompositorAnimationHandler {
         }
         mCachedList.clear();
 
-        mUpdateHost.requestUpdate();
+        mRenderRequestRunnable.run();
 
         return mAnimators.isEmpty();
     }

chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/CompositorModelChangeProcessor.java

@@ -4,6 +4,7 @@
 
 package org.chromium.chrome.browser.compositor.layouts;
 
+import androidx.annotation.NonNull;
 import androidx.annotation.VisibleForTesting;
 
 import org.chromium.base.Callback;
@@ -28,10 +29,11 @@ public class CompositorModelChangeProcessor<V extends SceneLayer> {
      * request another frame.
      */
     public static class FrameRequestSupplier extends ObservableSupplierImpl<Long> {
-        private final LayoutUpdateHost mHost;
+        @NonNull
+        private final Runnable mRenderRequestRunnable;
 
-        public FrameRequestSupplier(LayoutUpdateHost host) {
-            mHost = host;
+        public FrameRequestSupplier(@NonNull Runnable renderRequestRunnable) {
+            mRenderRequestRunnable = renderRequestRunnable;
         }
 
         /**
@@ -39,7 +41,7 @@ public class CompositorModelChangeProcessor<V extends SceneLayer> {
          */
         @VisibleForTesting
         void request() {
-            mHost.requestUpdate();
+            mRenderRequestRunnable.run();
         }
     }
 

chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManager.java

@@ -287,11 +287,12 @@ public class LayoutManager implements LayoutUpdateHost, LayoutProvider,
         assert contentContainer != null;
         mContentContainer = contentContainer;
 
-        mAnimationHandler = new CompositorAnimationHandler(this);
+        mAnimationHandler = new CompositorAnimationHandler(this::requestUpdate);
 
         mOverlayPanelManager = new OverlayPanelManager();
 
-        mFrameRequestSupplier = new CompositorModelChangeProcessor.FrameRequestSupplier(this);
+        mFrameRequestSupplier =
+                new CompositorModelChangeProcessor.FrameRequestSupplier(this::requestUpdate);
 
         mLayoutStateProviderOneshotSupplier.set(this);
     }

chrome/android/junit/src/org/chromium/chrome/browser/compositor/animation/CompositorAnimationHandlerTest.java

@@ -12,8 +12,6 @@ import org.junit.runner.RunWith;
 import org.robolectric.annotation.Config;
 
 import org.chromium.base.test.BaseRobolectricTestRunner;
-import org.chromium.chrome.browser.compositor.layouts.LayoutUpdateHost;
-import org.chromium.chrome.browser.compositor.layouts.MockLayoutUpdateHost;
 
 /**
  * Unit tests for {@link CompositorAnimator}.
@@ -25,13 +23,11 @@ public class CompositorAnimationHandlerTest {
     private static final long SLOW_DURATION_MS = 1000;
 
     private CompositorAnimationHandler mAnimations;
-    private LayoutUpdateHost mUpdateHost;
 
     @Test
     @SmallTest
     public void testConcurrentAnimationsFinishSeparately() {
-        mUpdateHost = new MockLayoutUpdateHostWithAnimationHandler(mAnimations);
-        mAnimations = new CompositorAnimationHandler(mUpdateHost);
+        mAnimations = new CompositorAnimationHandler(() -> {});
 
         CompositorAnimator mFastAnimation =
                 CompositorAnimator.ofFloat(mAnimations, 0.f, 1.f, FAST_DURATION_MS, null);
@@ -53,18 +49,4 @@ public class CompositorAnimationHandlerTest {
         Assert.assertFalse(mFastAnimation.isRunning());
         Assert.assertFalse(mSlowAnimation.isRunning());
     }
-
-    /** A mock implementation of {@link LayoutUpdateHost} with animation handler. */
-    private class MockLayoutUpdateHostWithAnimationHandler extends MockLayoutUpdateHost {
-        private CompositorAnimationHandler mAnimationHandler;
-
-        MockLayoutUpdateHostWithAnimationHandler(CompositorAnimationHandler animationHandler) {
-            mAnimationHandler = animationHandler;
-        }
-
-        @Override
-        public CompositorAnimationHandler getAnimationHandler() {
-            return mAnimationHandler;
-        }
-    }
 }
\ No newline at end of file

chrome/android/junit/src/org/chromium/chrome/browser/compositor/animation/CompositorAnimatorTest.java

@@ -22,8 +22,6 @@ import org.robolectric.annotation.Config;
 import org.chromium.base.MathUtils;
 import org.chromium.base.test.BaseRobolectricTestRunner;
 import org.chromium.base.test.util.CallbackHelper;
-import org.chromium.chrome.browser.compositor.layouts.LayoutUpdateHost;
-import org.chromium.chrome.browser.compositor.layouts.MockLayoutUpdateHost;
 
 import java.util.ArrayList;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -37,16 +35,6 @@ import java.util.concurrent.atomic.AtomicInteger;
         // specification once we upgrade to a version in which it works. crbug.com/774357
         sdk = Build.VERSION_CODES.N_MR1)
 public final class CompositorAnimatorTest {
-    /** A mock implementation of {@link LayoutUpdateHost} that tracks update requests. */
-    private static class MockLayoutUpdateHostWithCallback extends MockLayoutUpdateHost {
-        private final CallbackHelper mUpdateCallbackHelper = new CallbackHelper();
-
-        @Override
-        public void requestUpdate() {
-            mUpdateCallbackHelper.notifyCalled();
-        }
-    }
-
     /** An animation update listener that counts calls to its methods. */
     private static class TestUpdateListener implements CompositorAnimator.AnimatorUpdateListener {
         private final CallbackHelper mUpdateCallbackHelper = new CallbackHelper();
@@ -81,8 +69,7 @@ public final class CompositorAnimatorTest {
         }
     }
 
-    /** A mock {@link LayoutUpdateHost} to track update requests. */
-    private MockLayoutUpdateHostWithCallback mHost;
+    private final CallbackHelper mRequestRenderCallbackHelper = new CallbackHelper();
 
     /** The handler that is responsible for managing all {@link CompositorAnimator}s. */
     private CompositorAnimationHandler mHandler;
@@ -96,9 +83,8 @@ public final class CompositorAnimatorTest {
     @Before
     public void setUp() {
         MockitoAnnotations.initMocks(this);
-        mHost = new MockLayoutUpdateHostWithCallback();
 
-        mHandler = new CompositorAnimationHandler(mHost);
+        mHandler = new CompositorAnimationHandler(mRequestRenderCallbackHelper::notifyCalled);
 
         mUpdateListener = new TestUpdateListener();
         mListener = new TestAnimatorListener();
@@ -118,7 +104,7 @@ public final class CompositorAnimatorTest {
         animator.addListener(mListener);
 
         assertEquals("No updates should have been requested.", 0,
-                mHost.mUpdateCallbackHelper.getCallCount());
+                mRequestRenderCallbackHelper.getCallCount());
         assertEquals(
                 "There should be no active animations.", 0, mHandler.getActiveAnimationCount());
         assertEquals("The 'start' event should not have been called.", 0,
@@ -127,7 +113,7 @@ public final class CompositorAnimatorTest {
         animator.start();
 
         assertEquals("One update should have been requested.", 1,
-                mHost.mUpdateCallbackHelper.getCallCount());
+                mRequestRenderCallbackHelper.getCallCount());
         assertEquals(
                 "There should be one active animation.", 1, mHandler.getActiveAnimationCount());
         assertEquals("The 'start' event should have been called.", 1,
@@ -141,7 +127,7 @@ public final class CompositorAnimatorTest {
         animator.addListener(mListener);
 
         assertEquals("No updates should have been requested.", 0,
-                mHost.mUpdateCallbackHelper.getCallCount());
+                mRequestRenderCallbackHelper.getCallCount());
         assertEquals(
                 "There should be no active animations.", 0, mHandler.getActiveAnimationCount());
         assertEquals("The 'end' event should not have been called.", 0,
@@ -150,14 +136,14 @@ public final class CompositorAnimatorTest {
         animator.start();
 
         assertEquals("One update should have been requested.", 1,
-                mHost.mUpdateCallbackHelper.getCallCount());
+                mRequestRenderCallbackHelper.getCallCount());
         assertEquals(
                 "There should be one active animation.", 1, mHandler.getActiveAnimationCount());
 
         mHandler.pushUpdate(15);
 
         assertEquals("Two updates should have been requested", 2,
-                mHost.mUpdateCallbackHelper.getCallCount());
+                mRequestRenderCallbackHelper.getCallCount());
         assertEquals(
                 "There should be no active animations.", 0, mHandler.getActiveAnimationCount());
         assertEquals("The 'cancel' event should not have been called.", 0,
@@ -173,7 +159,7 @@ public final class CompositorAnimatorTest {
         animator.addListener(mListener);
 
         assertEquals("No updates should have been requested.", 0,
-                mHost.mUpdateCallbackHelper.getCallCount());
+                mRequestRenderCallbackHelper.getCallCount());
         assertEquals(
                 "There should be no active animations.", 0, mHandler.getActiveAnimationCount());
         assertEquals("The 'end' event should not have been called.", 0,
@@ -182,14 +168,14 @@ public final class CompositorAnimatorTest {
         animator.start();
 
         assertEquals("One update should have been requested.", 1,
-                mHost.mUpdateCallbackHelper.getCallCount());
+                mRequestRenderCallbackHelper.getCallCount());
         assertEquals(
                 "There should be one active animation.", 1, mHandler.getActiveAnimationCount());
 
         animator.cancel();
 
         assertEquals("One update should have been requested.", 1,
-                mHost.mUpdateCallbackHelper.getCallCount());
+                mRequestRenderCallbackHelper.getCallCount());
         assertEquals(
                 "There should be no active animations.", 0, mHandler.getActiveAnimationCount());
         assertEquals("The 'cancel' event should have been called.", 1,

chrome/android/junit/src/org/chromium/chrome/browser/compositor/layouts/CompositorModelChangeProcessorUnitTest.java

@@ -9,6 +9,7 @@ import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -17,10 +18,12 @@ import org.mockito.MockitoAnnotations;
 import org.robolectric.annotation.Config;
 
 import org.chromium.base.test.BaseRobolectricTestRunner;
+import org.chromium.base.test.util.CallbackHelper;
 import org.chromium.chrome.browser.compositor.scene_layer.SceneLayer;
 import org.chromium.ui.modelutil.PropertyModel;
 import org.chromium.ui.modelutil.PropertyModelChangeProcessor;
 
+import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 /**
@@ -32,12 +35,12 @@ public class CompositorModelChangeProcessorUnitTest {
     private static final PropertyModel.WritableBooleanPropertyKey PROPERTY_CHANGED =
             new PropertyModel.WritableBooleanPropertyKey();
 
+    private final CallbackHelper mRequestRenderCallbackHelper = new CallbackHelper();
+
     @Mock
     private SceneLayer mView;
     @Mock
     private PropertyModelChangeProcessor.ViewBinder mViewBinder;
-    @Mock
-    private LayoutUpdateHost mLayoutUpdateHost;
 
     private CompositorModelChangeProcessor.FrameRequestSupplier mFrameSupplier;
     private CompositorModelChangeProcessor mCompositorMCP;
@@ -48,7 +51,8 @@ public class CompositorModelChangeProcessorUnitTest {
     public void setUp() {
         MockitoAnnotations.initMocks(this);
 
-        mFrameSupplier = new CompositorModelChangeProcessor.FrameRequestSupplier(mLayoutUpdateHost);
+        mFrameSupplier = new CompositorModelChangeProcessor.FrameRequestSupplier(
+                mRequestRenderCallbackHelper::notifyCalled);
         mModel = new PropertyModel(PROPERTY_CHANGED);
 
         mCompositorMCP = CompositorModelChangeProcessor.create(
@@ -56,9 +60,10 @@ public class CompositorModelChangeProcessorUnitTest {
     }
 
     @Test
-    public void testBindAndRequestFrame() {
+    public void testBindAndRequestFrame() throws TimeoutException {
+        int callCount = mRequestRenderCallbackHelper.getCallCount();
         mModel.set(PROPERTY_CHANGED, mPropertyChangedValue.getAndSet(!mPropertyChangedValue.get()));
-        verify(mLayoutUpdateHost).requestUpdate();
+        mRequestRenderCallbackHelper.waitForCallback(callCount, 1);
 
         mFrameSupplier.set(System.currentTimeMillis());
         verify(mViewBinder).bind(eq(mModel), eq(mView), eq(null));
@@ -66,17 +71,20 @@ public class CompositorModelChangeProcessorUnitTest {
 
     @Test
     public void testBindAndNoRequestFrame() {
+        int callCount = mRequestRenderCallbackHelper.getCallCount();
         mFrameSupplier.set(System.currentTimeMillis());
 
         verify(mViewBinder).bind(eq(mModel), eq(mView), eq(null));
-        verify(mLayoutUpdateHost, never()).requestUpdate();
+        Assert.assertEquals("A render should not have been requested!", callCount,
+                mRequestRenderCallbackHelper.getCallCount());
     }
 
     @Test
-    public void testRequestFrameAndNoBindOnPropertyChanged() {
+    public void testRequestFrameAndNoBindOnPropertyChanged() throws TimeoutException {
+        int callCount = mRequestRenderCallbackHelper.getCallCount();
         mModel.set(PROPERTY_CHANGED, mPropertyChangedValue.getAndSet(!mPropertyChangedValue.get()));
+        mRequestRenderCallbackHelper.waitForCallback(callCount, 1);
 
-        verify(mLayoutUpdateHost).requestUpdate();
         verify(mViewBinder, never()).bind(any(), any(), any());
     }
 }
\ No newline at end of file

chrome/android/junit/src/org/chromium/chrome/browser/compositor/layouts/MockLayoutUpdateHost.java

-// Copyright 2017 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-package org.chromium.chrome.browser.compositor.layouts;
-
-import org.chromium.chrome.browser.compositor.animation.CompositorAnimationHandler;
-import org.chromium.chrome.browser.compositor.layouts.components.LayoutTab;
-
-/** A mock implementation of {@link LayoutUpdateHost}. */
-public class MockLayoutUpdateHost implements LayoutUpdateHost {
-    @Override
-    public void requestUpdate() {}
-
-    @Override
-    public void startHiding(int nextTabId, boolean hintAtTabSelection) {}
-
-    @Override
-    public void doneHiding() {}
-
-    @Override
-    public void doneShowing() {}
-
-    @Override
-    public boolean isActiveLayout(Layout layout) {
-        return true;
-    }
-
-    @Override
-    public void initLayoutTabFromHost(final int tabId) {}
-
-    @Override
-    public LayoutTab createLayoutTab(int id, boolean incognito, boolean showCloseButton,
-            boolean isTitleNeeded, float maxContentWidth, float maxContentHeight) {
-        return null;
-    }
-
-    @Override
-    public void releaseTabLayout(int id) {}
-
-    @Override
-    public void releaseResourcesForTab(int tabId) {}
-
-    @Override
-    public CompositorAnimationHandler getAnimationHandler() {
-        return null;
-    }
-}

chrome/android/junit/src/org/chromium/chrome/browser/compositor/layouts/StaticLayoutUnitTest.java

@@ -127,7 +127,7 @@ public class StaticLayoutUnitTest {
     public void setUp() {
         MockitoAnnotations.initMocks(this);
 
-        mCompositorAnimationHandler = new CompositorAnimationHandler(mUpdateHost);
+        mCompositorAnimationHandler = new CompositorAnimationHandler(mUpdateHost::requestUpdate);
         CompositorAnimationHandler.setTestingMode(true);
 
         mTab1 = prepareTab(TAB1_ID, TAB1_URL);