Refactor offset handling in CVH

There is no logic change.

By refactoring the offset change into a separate class, we can make it
much easier to reason about and add new tests. For example,
https://chromium-review.googlesource.com/c/chromium/src/+/980513
can use this test.

Bug: 825858
Change-Id: I3465057c8b7fed637f61a2a02a03d5048be8c387
Reviewed-on: https://chromium-review.googlesource.com/c/1335273
Commit-Queue: Changwan Ryu <changwan@chromium.org>
Reviewed-by: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608846}

chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java

@@ -59,7 +59,6 @@ import org.chromium.content_public.browser.WebContents;
 import org.chromium.ui.KeyboardVisibilityDelegate;
 import org.chromium.ui.UiUtils;
 import org.chromium.ui.base.EventForwarder;
-import org.chromium.ui.base.SPenSupport;
 import org.chromium.ui.base.WindowAndroid;
 import org.chromium.ui.resources.ResourceManager;
 import org.chromium.ui.resources.dynamics.DynamicResourceLoader;
@@ -80,6 +79,7 @@ public class CompositorViewHolder extends FrameLayout
                    KeyboardExtensionSizeManager.Observer {
     private static final long SYSTEM_UI_VIEWPORT_UPDATE_DELAY_MS = 500;
 
+    private EventOffsetHandler mEventOffsetHandler;
     private boolean mIsKeyboardShowing;
 
     private final Invalidator mInvalidator = new Invalidator();
@@ -122,7 +122,6 @@ public class CompositorViewHolder extends FrameLayout
     private TabObserver mTabObserver;
 
     // Cache objects that should not be created frequently.
-    private final RectF mCacheViewport = new RectF();
     private final Rect mCacheRect = new Rect();
     private final Point mCachePoint = new Point();
 
@@ -205,6 +204,27 @@ public class CompositorViewHolder extends FrameLayout
     }
 
     private void internalInit() {
+        mEventOffsetHandler =
+                new EventOffsetHandler(new EventOffsetHandler.EventOffsetHandlerDelegate() {
+                    // Cache objects that should not be created frequently.
+                    private final RectF mCacheViewport = new RectF();
+
+                    @Override
+                    public RectF getViewport() {
+                        if (mLayoutManager != null) mLayoutManager.getViewportPixel(mCacheViewport);
+                        return mCacheViewport;
+                    }
+
+                    @Override
+                    public void setCurrentTouchEventOffsets(float x, float y) {
+                        if (mTabVisible == null) return;
+                        WebContents webContents = mTabVisible.getWebContents();
+                        if (webContents == null) return;
+                        EventForwarder forwarder = webContents.getEventForwarder();
+                        forwarder.setCurrentTouchEventOffsets(x, y);
+                    }
+                });
+
         mTabObserver = new EmptyTabObserver() {
             @Override
             public void onContentChanged(Tab tab) {
@@ -490,7 +510,7 @@ public class CompositorViewHolder extends FrameLayout
 
         if (mLayoutManager == null) return false;
 
-        setContentViewMotionEventOffsets(e, false);
+        mEventOffsetHandler.onInterceptTouchEvent(e);
         return mLayoutManager.onInterceptTouchEvent(e, mIsKeyboardShowing);
     }
 
@@ -500,13 +520,13 @@ public class CompositorViewHolder extends FrameLayout
 
         if (mFullscreenManager != null) mFullscreenManager.onMotionEvent(e);
         boolean consumed = mLayoutManager != null && mLayoutManager.onTouchEvent(e);
-        setContentViewMotionEventOffsets(e, true);
+        mEventOffsetHandler.onTouchEvent(e);
         return consumed;
     }
 
     @Override
     public boolean onInterceptHoverEvent(MotionEvent e) {
-        setContentViewMotionEventOffsets(e, true);
+        mEventOffsetHandler.onInterceptHoverEvent(e);
         return super.onInterceptHoverEvent(e);
     }
 
@@ -522,20 +542,9 @@ public class CompositorViewHolder extends FrameLayout
 
     @Override
     public boolean dispatchDragEvent(DragEvent e) {
-        if (mTabVisible == null) return false;
-        WebContents webContents = mTabVisible.getWebContents();
-        if (webContents == null) return false;
-
-        if (mLayoutManager != null) mLayoutManager.getViewportPixel(mCacheViewport);
-        EventForwarder forwarder = webContents.getEventForwarder();
-        forwarder.setCurrentTouchEventOffsets(-mCacheViewport.left, -mCacheViewport.top);
+        mEventOffsetHandler.onPreDispatchDragEvent(e.getAction());
         boolean ret = super.dispatchDragEvent(e);
-
-        int action = e.getAction();
-        if (action == DragEvent.ACTION_DRAG_EXITED || action == DragEvent.ACTION_DRAG_ENDED
-                || action == DragEvent.ACTION_DROP) {
-            forwarder.setCurrentTouchEventOffsets(0.f, 0.f);
-        }
+        mEventOffsetHandler.onPostDispatchDragEvent(e.getAction());
         return ret;
     }
 
@@ -702,28 +711,6 @@ public class CompositorViewHolder extends FrameLayout
         }
     }
 
-    private void setContentViewMotionEventOffsets(MotionEvent e, boolean canClear) {
-        // TODO(dtrainor): Factor this out to LayoutDriver.
-        if (e == null || mTabVisible == null) return;
-
-        WebContents webContents = mTabVisible.getWebContents();
-        if (webContents == null) return;
-
-        int actionMasked = SPenSupport.convertSPenEventAction(e.getActionMasked());
-
-        if (actionMasked == MotionEvent.ACTION_DOWN
-                || actionMasked == MotionEvent.ACTION_HOVER_ENTER
-                || actionMasked == MotionEvent.ACTION_HOVER_MOVE) {
-            if (mLayoutManager != null) mLayoutManager.getViewportPixel(mCacheViewport);
-            webContents.getEventForwarder().setCurrentTouchEventOffsets(
-                    -mCacheViewport.left, -mCacheViewport.top);
-        } else if (canClear && (actionMasked == MotionEvent.ACTION_UP
-                                       || actionMasked == MotionEvent.ACTION_CANCEL
-                                       || actionMasked == MotionEvent.ACTION_HOVER_EXIT)) {
-            webContents.getEventForwarder().setCurrentTouchEventOffsets(0.f, 0.f);
-        }
-    }
-
     private void onViewportChanged() {
         if (mLayoutManager != null) mLayoutManager.onViewportChanged();
     }

chrome/android/java/src/org/chromium/chrome/browser/compositor/EventOffsetHandler.java

+// Copyright 2018 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;
+
+import android.graphics.RectF;
+import android.view.DragEvent;
+import android.view.MotionEvent;
+import android.view.View;
+import android.view.ViewGroup;
+
+import org.chromium.ui.base.SPenSupport;
+
+/**
+ * A class to update motion event offset while dragging. This is needed to compensate the change
+ * caused by top control.
+ */
+public class EventOffsetHandler {
+    /**
+     * A delegate for EventOffsetHandler.
+     */
+    interface EventOffsetHandlerDelegate {
+        RectF getViewport();
+        void setCurrentTouchEventOffsets(float left, float top);
+    }
+
+    private final EventOffsetHandlerDelegate mDelegate;
+
+    public EventOffsetHandler(EventOffsetHandlerDelegate delegate) {
+        mDelegate = delegate;
+    }
+
+    /**
+     * Call this before handling onDispatchDragEvent.
+     * @param action Drag event action.
+     */
+    public void onPreDispatchDragEvent(int action) {
+        RectF viewport = mDelegate.getViewport();
+        setTouchEventOffsets(-viewport.left, -viewport.top);
+    }
+
+    /**
+     * Call this after handling onDispatchDragEvent.
+     * @param action Drag event action.
+     */
+    public void onPostDispatchDragEvent(int action) {
+        if (action == DragEvent.ACTION_DRAG_EXITED || action == DragEvent.ACTION_DRAG_ENDED
+                || action == DragEvent.ACTION_DROP) {
+            setTouchEventOffsets(0.f, 0.f);
+        }
+    }
+
+    /** See {@link ViewGroup#onInterceptTouchEvent(MotionEvent)}. */
+    public void onInterceptTouchEvent(MotionEvent e) {
+        setContentViewMotionEventOffsets(e, false);
+    }
+
+    /** See {@link View#onTouchEvent(MotionEvent)}. */
+    public void onTouchEvent(MotionEvent e) {
+        setContentViewMotionEventOffsets(e, true);
+    }
+
+    /** See {@link ViewGroup#onInterceptHoverEvent(MotionEvent)}. */
+    public void onInterceptHoverEvent(MotionEvent e) {
+        setContentViewMotionEventOffsets(e, true);
+    }
+
+    private void setContentViewMotionEventOffsets(MotionEvent e, boolean canClear) {
+        int actionMasked = SPenSupport.convertSPenEventAction(e.getActionMasked());
+        if (actionMasked == MotionEvent.ACTION_DOWN
+                || actionMasked == MotionEvent.ACTION_HOVER_ENTER
+                || actionMasked == MotionEvent.ACTION_HOVER_MOVE) {
+            RectF viewport = mDelegate.getViewport();
+            setTouchEventOffsets(-viewport.left, -viewport.top);
+        } else if (canClear
+                && (actionMasked == MotionEvent.ACTION_UP
+                           || actionMasked == MotionEvent.ACTION_CANCEL
+                           || actionMasked == MotionEvent.ACTION_HOVER_EXIT)) {
+            setTouchEventOffsets(0.f, 0.f);
+        }
+    }
+
+    private void setTouchEventOffsets(float x, float y) {
+        mDelegate.setCurrentTouchEventOffsets(x, y);
+    }
+}
\ No newline at end of file

chrome/android/java_sources.gni

@@ -214,6 +214,7 @@ chrome_java_sources = [
   "java/src/org/chromium/chrome/browser/compositor/CompositorSurfaceManagerImpl.java",
   "java/src/org/chromium/chrome/browser/compositor/CompositorView.java",
   "java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java",
+  "java/src/org/chromium/chrome/browser/compositor/EventOffsetHandler.java",
   "java/src/org/chromium/chrome/browser/compositor/Invalidator.java",
   "java/src/org/chromium/chrome/browser/compositor/LayerTitleCache.java",
   "java/src/org/chromium/chrome/browser/compositor/TitleCache.java",
@@ -2316,6 +2317,7 @@ chrome_junit_test_java_sources = [
   "junit/src/org/chromium/chrome/browser/compositor/animation/CompositorAnimatorTest.java",
   "junit/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelperTest.java",
   "junit/src/org/chromium/chrome/browser/compositor/CompositorSurfaceManagerImplTest.java",
+  "junit/src/org/chromium/chrome/browser/compositor/EventOffsetHandlerTest.java",
   "junit/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulatorTest.java",
   "junit/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchContextForTest.java",
   "junit/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchContextTest.java",

chrome/android/junit/src/org/chromium/chrome/browser/compositor/EventOffsetHandlerTest.java

+// Copyright 2018 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;
+
+import static org.junit.Assert.assertEquals;
+
+import android.graphics.RectF;
+import android.view.DragEvent;
+import android.view.MotionEvent;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.annotation.Config;
+
+import org.chromium.base.test.BaseRobolectricTestRunner;
+
+/**
+ * Unit tests for the EventOffsetHandler.
+ */
+@RunWith(BaseRobolectricTestRunner.class)
+@Config(manifest = Config.NONE)
+public class EventOffsetHandlerTest {
+    private EventOffsetHandler mHandler;
+    private EventOffsetHandler.EventOffsetHandlerDelegate mDelegate =
+            new EventOffsetHandler.EventOffsetHandlerDelegate() {
+                @Override
+                public RectF getViewport() {
+                    return mViewport;
+                }
+
+                @Override
+                public void setCurrentTouchEventOffsets(float x, float y) {
+                    mOffsetX = x;
+                    mOffsetY = y;
+                }
+
+            };
+
+    private RectF mViewport;
+    private float mOffsetX;
+    private float mOffsetY;
+
+    private void assertOffsets(float x, float y) {
+        assertEquals(x, mOffsetX, 0.0);
+        assertEquals(y, mOffsetY, 0.0);
+    }
+
+    @Before
+    public void setUp() {
+        mHandler = new EventOffsetHandler(mDelegate);
+        mViewport = new RectF(100, 200, 600, 800);
+        assertOffsets(0, 0);
+    }
+
+    @Test
+    public void testOffsetChangesWhileDragging() {
+        mHandler.onPreDispatchDragEvent(DragEvent.ACTION_DRAG_STARTED);
+        mHandler.onPostDispatchDragEvent(DragEvent.ACTION_DRAG_STARTED);
+
+        // Viewport position has been negated.
+        assertOffsets(-100, -200);
+
+        MotionEvent motionStart = MotionEvent.obtain(0, 0, MotionEvent.ACTION_DOWN, 100, 100, 0);
+        mHandler.onInterceptTouchEvent(motionStart);
+
+        assertOffsets(-100, -200);
+
+        MotionEvent motionEnd = MotionEvent.obtain(0, 0, MotionEvent.ACTION_UP, 100, 100, 0);
+        mHandler.onInterceptTouchEvent(motionStart);
+
+        assertOffsets(-100, -200);
+
+        mHandler.onPreDispatchDragEvent(DragEvent.ACTION_DRAG_ENDED);
+        mHandler.onPostDispatchDragEvent(DragEvent.ACTION_DRAG_ENDED);
+        assertOffsets(0, 0);
+    }
+}