Android mouse events shouldn't appear as TouchEvents

content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java

 // Copyright 2012 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.content.browser;
 
 import android.annotation.SuppressLint;
 import android.annotation.TargetApi;
 import android.app.Activity;
 import android.app.SearchManager;
@@ skipping 1001 matching lines @@
     public boolean isGamepadAPIActive() {
         return GamepadList.isGamepadAPIActive();
     }
 
     // End FrameLayout overrides.
 
     /**
      * @see View#onTouchEvent(MotionEvent)
      */
     public boolean onTouchEvent(MotionEvent event) {
+        // TODO(mustaq): Should we include MotionEvent.TOOL_TYPE_STYLUS here?
+        // crbug.com/592082
+        if (event.getToolType(0) == MotionEvent.TOOL_TYPE_MOUSE) {
+            // Pre-Andorid-L mouse button info is incomplete

[aelias_OOO_until_Jul13, 2016/11/09 02:43:14]

"Mouse button info is incomplete on L and below"

[mustaq, 2016/11/09 17:04:33]

Done.

+            int apiVersion = Build.VERSION.SDK_INT;
+            if (apiVersion >= android.os.Build.VERSION_CODES.M) {
+                return sendMouseEvent(event);
+            }
+        }
+
         final boolean isTouchHandleEvent = false;
-        return onTouchEventImpl(event, isTouchHandleEvent);
+        return sendTouchEvent(event, isTouchHandleEvent);
+    }
+
+    @SuppressLint("NewApi")

[aelias_OOO_until_Jul13, 2016/11/09 02:43:14]

I think it would work to do @TargetApi(Build.VERSION_CODES.M) instead?

[mustaq, 2016/11/09 17:04:33]

Done, thanks.

+    private boolean sendMouseEvent(MotionEvent event) {
+        TraceEvent.begin("sendMouseEvent");
+
+        MotionEvent offsetEvent = createOffsetMotionEvent(event);
+        try {
+            mContainerView.removeCallbacks(mFakeMouseMoveRunnable);

[aelias_OOO_until_Jul13, 2016/11/09 02:43:14]

That reminds me that this thing exists... could you add a "TODO(mustaq): Delete
this, see http://crbug.com/492738" next to uses of it in this file?

[mustaq, 2016/11/09 17:04:32]

Commented the main usage instead. Does it look enough?

+            if (mNativeContentViewCore != 0) {

[aelias_OOO_until_Jul13, 2016/11/09 02:43:14]

Please do "if (mNativeContentViewCore == 0) return false;" in order to avoid the
indentation, and also because returning false seems more correct than returning
true.

[mustaq, 2016/11/09 17:04:32]

Done.

[mustaq, 2016/11/09 17:04:32]

Done.

+                int eventAction = event.getActionMasked();
+
+                // For mousedown and mouseup events, we use ACTION_BUTTON_PRESS
+                // and ACTION_BUTTON_RELEASE respectively because they provide
+                // info about the changed-button.
+                if (eventAction == MotionEvent.ACTION_DOWN
+                        || eventAction == MotionEvent.ACTION_UP) {
+                    return false;
+                }
+
+                // The method getActionButton() is defined only for

[aelias_OOO_until_Jul13, 2016/11/09 02:43:14]

What happens if you call getActionButton() on one of the other event types? 
Does it crash/assert/log?  If it doesn't have any undesirable side effects and
just returns 0 anyway, then please delete the unnecessary if statement below.

[mustaq, 2016/11/09 17:04:32]

Assuming it neither throws nor emits any logs, even if we found the return value
to be 0 in our test cases, there is no guarantee of it across all devices or in
future OSes. Since MouseEvent.button (=action button value) is crucial for web
compat, I would prefer avoiding the unspeced behavior here. Wdyt?

[aelias_OOO_until_Jul13, 2016/11/09 18:57:03]

That seems excessively paranoid.  Almost no Android API is specced or tested,
therefore you could make a similar argument about any Android API whatsoever and
justify an infinite amount of unnecessary filtering cruft that way.  Also, I
have no reason to believe OEMs or future Android releases will touch this and
would be fine to triage a bug arising from this if they did (it would likely
come along with other interesting mouse changes we'd want to pay attention to).

A common cause of hairy code complexity is fear -- weird branches accumulate
because people are afraid of breaking a hypothetical scenario.  I'm OK taking a
low risk of an unknown bug to keep the code healthy.

[mustaq, 2016/11/09 19:55:59]

Just tested on an N7/M, even with chorded button presses, and it just works. So
let's go for the simplest code.

+                // ACTION_BUTTON_PRESS and ACTION_BUTTON_RELEASE
+                int changedButton = 0;
+                if (eventAction == MotionEvent.ACTION_BUTTON_PRESS
+                        || eventAction == MotionEvent.ACTION_BUTTON_RELEASE) {
+                    changedButton = event.getActionButton();
+                }
+
+                nativeSendMouseEvent(mNativeContentViewCore, event.getEventTime(), eventAction,
+                        offsetEvent.getX(), offsetEvent.getY(), event.getPointerId(0),
+                        event.getPressure(0), event.getOrientation(0),
+                        event.getAxisValue(MotionEvent.AXIS_TILT, 0), changedButton,
+                        event.getButtonState(), event.getMetaState(), event.getToolType(0));
+            }
+            return true;
+        } finally {
+            offsetEvent.recycle();
+            TraceEvent.end("sendMouseEvent");
+        }
     }
 
     /**
      * Called by PopupWindow-based touch handles.
      * @param event the MotionEvent targeting the handle.
      */
     public boolean onTouchHandleEvent(MotionEvent event) {
         final boolean isTouchHandleEvent = true;
-        return onTouchEventImpl(event, isTouchHandleEvent);
+        return sendTouchEvent(event, isTouchHandleEvent);
     }
 
-    private boolean onTouchEventImpl(MotionEvent event, boolean isTouchHandleEvent) {
-        TraceEvent.begin("onTouchEvent");
+    private boolean sendTouchEvent(MotionEvent event, boolean isTouchHandleEvent) {
+        TraceEvent.begin("sendTouchEvent");
         try {
             int eventAction = event.getActionMasked();
 
             if (eventAction == MotionEvent.ACTION_DOWN) {
                 cancelRequestToScrollFocusedEditableNodeIntoView();
             }
 
             if (SPenSupport.isSPenSupported(mContext)) {
                 eventAction = SPenSupport.convertSPenEventAction(eventAction);
             }
@@ skipping 38 matching lines @@
                     event.getRawX(), event.getRawY(),
                     event.getToolType(0),
                     pointerCount > 1 ? event.getToolType(1) : MotionEvent.TOOL_TYPE_UNKNOWN,
                     event.getButtonState(),
                     event.getMetaState(),
                     isTouchHandleEvent);
 
             if (offset != null) offset.recycle();
             return consumed;
         } finally {
-            TraceEvent.end("onTouchEvent");
+            TraceEvent.end("sendTouchEvent");
         }
     }
 
     @CalledByNative
     private void requestDisallowInterceptTouchEvent() {
         mContainerView.requestDisallowInterceptTouchEvent(true);
     }
 
     private static boolean isValidTouchEventActionForNative(int eventAction) {
         // Only these actions have any effect on gesture detection.  Other
@@ skipping 460 matching lines @@
 
     /**
      * @see View#onHoverEvent(MotionEvent)
      * Mouse move events are sent on hover enter, hover move and hover exit.
      * They are sent on hover exit because sometimes it acts as both a hover
      * move and hover exit.
      */
     public boolean onHoverEvent(MotionEvent event) {
         TraceEvent.begin("onHoverEvent");
 
+        int eventAction = event.getActionMasked();
+
+        // Work around Android bug where the x, y coordinates of a hover exit

[aelias_OOO_until_Jul13, 2016/11/09 02:43:14]

This if statement is now no-op given your other early-return below.

[aelias_OOO_until_Jul13, 2016/11/09 20:08:51]

Looks like you missed my previous comment.  Please delete this if statement
before you land.

[mustaq, 2016/11/09 20:22:58]

Oops, done.

+        // event are incorrect when touch exploration is on.
+        if (mTouchExplorationEnabled && eventAction == MotionEvent.ACTION_HOVER_EXIT) {
+            return true;
+        }
+
+        // Ignore ACTION_HOVER_ENTER & ACTION_HOVER_EXIT: every mouse-down on
+        // Android follows a hover-exit and is followed by a hover-enter. The
+        // MotionEvent spec seems to support this behavior indirectly.
+        if (eventAction == MotionEvent.ACTION_HOVER_ENTER
+                || eventAction == MotionEvent.ACTION_HOVER_EXIT) {
+            return false;
+        }
+
         MotionEvent offset = createOffsetMotionEvent(event);
         try {
             if (mBrowserAccessibilityManager != null && !mIsObscuredByAnotherView) {
                 return mBrowserAccessibilityManager.onHoverEvent(offset);
             }
 
-            // Work around Android bug where the x, y coordinates of a hover exit
-            // event are incorrect when touch exploration is on.
-            if (mTouchExplorationEnabled && offset.getAction() == MotionEvent.ACTION_HOVER_EXIT) {
-                return true;
-            }
-
             // TODO(lanwei): Remove this switch once experimentation is complete -
             // crbug.com/418188
             if (event.getToolType(0) == MotionEvent.TOOL_TYPE_FINGER) {
                 if (mEnableTouchHover == null) {
                     mEnableTouchHover =
                             CommandLine.getInstance().hasSwitch(ContentSwitches.ENABLE_TOUCH_HOVER);
                 }
                 if (!mEnableTouchHover.booleanValue()) return false;
             }
 
             mContainerView.removeCallbacks(mFakeMouseMoveRunnable);
             if (mNativeContentViewCore != 0) {
-                nativeSendMouseMoveEvent(mNativeContentViewCore, offset.getEventTime(),
-                        offset.getX(), offset.getY(), event.getToolType(0));
+                nativeSendMouseEvent(mNativeContentViewCore, event.getEventTime(), eventAction,
+                        offset.getX(), offset.getY(), event.getPointerId(0), event.getPressure(0),
+                        event.getOrientation(0), event.getAxisValue(MotionEvent.AXIS_TILT, 0),
+                        0 /* changedButton */, event.getButtonState(), event.getMetaState(),
+                        event.getToolType(0));
             }
             return true;
         } finally {
             offset.recycle();
             TraceEvent.end("onHoverEvent");
         }
     }
 
     /**
      * @see View#onGenericMotionEvent(MotionEvent)
      */
     public boolean onGenericMotionEvent(MotionEvent event) {
         if (GamepadList.onGenericMotionEvent(event)) return true;
         if ((event.getSource() & InputDevice.SOURCE_CLASS_POINTER) != 0) {
             mLastFocalEventX = event.getX();
             mLastFocalEventY = event.getY();
-            switch (event.getAction()) {
+            switch (event.getActionMasked()) {
                 case MotionEvent.ACTION_SCROLL:
                     if (mNativeContentViewCore == 0) return false;
 
                     nativeSendMouseWheelEvent(mNativeContentViewCore, event.getEventTime(),
                             event.getX(), event.getY(),
                             event.getAxisValue(MotionEvent.AXIS_HSCROLL),
                             event.getAxisValue(MotionEvent.AXIS_VSCROLL),
                             mRenderCoordinates.getWheelScrollFactor());
 
                     mContainerView.removeCallbacks(mFakeMouseMoveRunnable);
                     // Send a delayed onMouseMove event so that we end
                     // up hovering over the right position after the scroll.
                     final MotionEvent eventFakeMouseMove = MotionEvent.obtain(event);
                     mFakeMouseMoveRunnable = new Runnable() {
                         @Override
                         public void run() {
                             onHoverEvent(eventFakeMouseMove);
                             eventFakeMouseMove.recycle();
                         }
                     };
                     mContainerView.postDelayed(mFakeMouseMoveRunnable, 250);
                     return true;
+                case MotionEvent.ACTION_BUTTON_PRESS:
+                case MotionEvent.ACTION_BUTTON_RELEASE:
+                    // TODO(mustaq): Should we include MotionEvent.TOOL_TYPE_STYLUS here?
+                    // crbug.com/592082
+                    if (event.getToolType(0) == MotionEvent.TOOL_TYPE_MOUSE) {
+                        return sendMouseEvent(event);
+                    }
             }
         } else if ((event.getSource() & InputDevice.SOURCE_CLASS_JOYSTICK) != 0) {
             if (mJoystickScrollProvider.onMotion(event)) return true;
             if (mJoystickZoomProvider == null) {
                 mJoystickZoomProvider =
                         new JoystickZoomProvider(this, new SystemAnimationIntervalProvider());
             }
             if (mJoystickZoomProvider.onMotion(event)) return true;
         }
         return mContainerViewInternals.super_onGenericMotionEvent(event);
@@ skipping 1691 matching lines @@
             int pointerId0, int pointerId1,
             float touchMajor0, float touchMajor1,
             float touchMinor0, float touchMinor1,
             float orientation0, float orientation1,
             float tilt0, float tilt1,
             float rawX, float rawY,
             int androidToolType0, int androidToolType1,
             int androidButtonState, int androidMetaState,
             boolean isTouchHandleEvent);
 
-    private native int nativeSendMouseMoveEvent(
-            long nativeContentViewCoreImpl, long timeMs, float x, float y, int toolType);
+    private native int nativeSendMouseEvent(long nativeContentViewCoreImpl, long timeMs, int action,
+            float x, float y, int pointerId, float pressure, float orientaton, float tilt,
+            int changedButton, int buttonState, int metaState, int toolType);
 
     private native int nativeSendMouseWheelEvent(long nativeContentViewCoreImpl, long timeMs,
             float x, float y, float ticksX, float ticksY, float pixelsPerTick);
 
     private native void nativeScrollBegin(long nativeContentViewCoreImpl, long timeMs, float x,
             float y, float hintX, float hintY, boolean targetViewport);
 
     private native void nativeScrollEnd(long nativeContentViewCoreImpl, long timeMs);
 
     private native void nativeScrollBy(
@@ skipping 62 matching lines @@
             String textTrackTextShadow, String textTrackTextSize);
 
     private native void nativeExtractSmartClipData(long nativeContentViewCoreImpl,
             int x, int y, int w, int h);
 
     private native void nativeSetBackgroundOpaque(long nativeContentViewCoreImpl, boolean opaque);
     private native boolean nativeIsTouchDragDropEnabled(long nativeContentViewCoreImpl);
     private native void nativeOnDragEvent(long nativeContentViewCoreImpl, int action, int x, int y,
             int screenX, int screenY, String[] mimeTypes, String content);
 }