Ensure that we don't report huge mouse movement deltas for mouse enter and leave events.

content/browser/renderer_host/render_widget_host_view_aura.cc

 // Copyright (c) 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.
 
 #include "content/browser/renderer_host/render_widget_host_view_aura.h"
 
 #include <set>
 #include <utility>
 
 #include "base/auto_reset.h"
@@ skipping 1299 matching lines @@
     window_->ReleaseCapture();
 
 #if defined(OS_WIN)
   ::ClipCursor(NULL);
 #endif
 
   // Ensure that the global mouse position is updated here to its original
   // value. If we don't do this then the synthesized mouse move which is posted
   // after the cursor is moved ends up getting a large movement delta which is
   // not what sites expect. The delta is computed in the
   // ModifyEventMovementAndCoords function.

[sky, 2016/10/05 15:49:55]

If you update aura::Env and nuke global_mouse_position_ does that means this
goes away? I'm a bit confused by some of this code and would like to understand
why it is the way it is rather than working around it.

[ananta, 2016/10/05 21:42:17]

It looks like the mouse events sent to blink require the movement X and Y deltas
which are computed via the difference between the global mouse position in the
current mouse event and its last position, which is saved in
global_mouse_position_. When the mouse lock state is reset we force the cursor
back to where it was originally placed which is represented by the
unlock_global_mouse_position_ member.

Removing global_mouse_position_ while good, is sadly not going to get rid of
this code here and the modify mouse event function.

   global_mouse_position_ = unlocked_global_mouse_position_;
   window_->MoveCursorTo(unlocked_mouse_position_);
 
   aura::client::CursorClient* cursor_client =
       aura::client::GetCursorClient(root_window);
   if (cursor_client) {
     cursor_client->UnlockCursor();
     cursor_client->ShowCursor();
   }
 
@@ skipping 537 matching lines @@
         IsFractionalScaleFactor(current_device_scale_factor_)) {
       if (event->type() == ui::ET_MOUSE_MOVED ||
           event->type() == ui::ET_MOUSE_DRAGGED) {
         if ((abs(mouse_event.x - center.x()) <= 2) &&
             (abs(mouse_event.y - center.y()) <= 2)) {
           is_move_to_center_event = true;
         }
       }
     }
 
-    ModifyEventMovementAndCoords(&mouse_event);
+    ModifyEventMovementAndCoords(&mouse_event, event);
 
     bool should_not_forward = is_move_to_center_event && synthetic_move_sent_;
     if (should_not_forward) {
       synthetic_move_sent_ = false;
     } else {
       // Check if the mouse has reached the border and needs to be centered.
       if (ShouldMoveToCenter()) {
         synthetic_move_sent_ = true;
         window_->MoveCursorTo(center);
       }
@@ skipping 58 matching lines @@
         popup_child_host_view_ && popup_child_host_view_->NeedsInputGrab();
     if (CanRendererHandleEvent(event, mouse_locked_, is_selection_popup) &&
         !(event->flags() & ui::EF_FROM_TOUCH)) {
       // Confirm existing composition text on mouse press, to make sure
       // the input caret won't be moved with an ongoing composition text.
       if (event->type() == ui::ET_MOUSE_PRESSED)
         FinishImeCompositionSession();
 
       blink::WebMouseEvent mouse_event = ui::MakeWebMouseEvent(
           *event, base::Bind(&GetScreenLocationFromEvent));
-      ModifyEventMovementAndCoords(&mouse_event);
+      ModifyEventMovementAndCoords(&mouse_event, event);
       if (ShouldRouteEvent(event)) {
         host_->delegate()->GetInputEventRouter()->RouteMouseEvent(
             this, &mouse_event, *event->latency());
       } else {
         ProcessMouseEvent(mouse_event, *event->latency());
       }
 
       // Ensure that we get keyboard focus on mouse down as a plugin window may
       // have grabbed keyboard focus.
       if (event->type() == ui::ET_MOUSE_PRESSED)
@@ skipping 525 matching lines @@
   if (!has_composition_text_)
     return;
 
   if (!!text_input_manager_ && !!text_input_manager_->GetActiveWidget()) {
     text_input_manager_->GetActiveWidget()->ImeFinishComposingText(false);
   }
   ImeCancelComposition();
 }
 
 void RenderWidgetHostViewAura::ModifyEventMovementAndCoords(
-    blink::WebMouseEvent* event) {
+    blink::WebMouseEvent* event,
+    ui::MouseEvent* ui_mouse_event) {

[sky, 2016/10/06 16:33:36]

Make this take a const&, in fact can both args be const& ?

[ananta, 2016/10/06 21:15:18]

Changed ui_mouse_event to be a const ref. The other one can't be as it is
updated below.

Regarding tests, it may be a touch tricky to write a non flaky one, because the
only way I could get this to reliably repro is to hover the mouse right around
the edge of the window border. 

Will see if an interactive test is doable in a future patchset.

   // If the mouse has just entered, we must report zero movementX/Y. Hence we
   // reset any global_mouse_position set previously.
-  if (event->type == blink::WebInputEvent::MouseEnter ||
-      event->type == blink::WebInputEvent::MouseLeave)
-    global_mouse_position_.SetPoint(event->globalX, event->globalY);
+  if (ui_mouse_event->type() == ui::ET_MOUSE_ENTERED ||
+      ui_mouse_event->type() == ui::ET_MOUSE_EXITED) {
+      global_mouse_position_.SetPoint(event->globalX, event->globalY);

[sky, 2016/10/04 18:17:55]

This is the first I've seen of ModifyEventMovementAndCoords. Can you elaborate
on why we need global_mouse_position_ per RWHVA? aura::Env has a
last_mouse_location, can that be used? Screen also has GetCursorScreenPoint(),
which uses GetCursorPos() where as last_mouse_location() is the last mouse
location seen in chrome.

[ananta, 2016/10/04 19:44:52]

The global_mouse_position technically can be possibly replaced with the aura
global. I would prefer to do that in a separate patch.

+  }
 
   // Movement is computed by taking the difference of the new cursor position
   // and the previous. Under mouse lock the cursor will be warped back to the
   // center so that we are not limited by clipping boundaries.
   // We do not measure movement as the delta from cursor to center because
   // we may receive more mouse movement events before our warp has taken
   // effect.
   event->movementX = event->globalX - global_mouse_position_.x();
   event->movementY = event->globalY - global_mouse_position_.y();
 
@@ skipping 523 matching lines @@
           ->GetTextSelection(focused_view)
           ->GetSelectedText(&selected_text)) {
     // Set the CLIPBOARD_TYPE_SELECTION to the ui::Clipboard.
     ui::ScopedClipboardWriter clipboard_writer(ui::CLIPBOARD_TYPE_SELECTION);
     clipboard_writer.WriteText(selected_text);
   }
 #endif  // defined(USE_X11) && !defined(OS_CHROMEOS)
 }
 
 }  // namespace content