New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 796649 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 796651



Sign in to add a comment

Separate targeting logic from dispatch logic in RenderWidgetHostInputEventRouter

Project Member Reported by sadrul@chromium.org, Dec 20 2017

Issue description

The event-targeting and event-dispatching code in RenderWidgetHostInputEventRouter is pretty tightly coupled right now. Separate the two phases, so that it's easier to introduce asynchronous event targeting.
 

Comment 1 by sadrul@chromium.org, Dec 20 2017

Blocking: 796651
Components: Internals>Sandbox>SiteIsolation
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f454e4b2e578ab6725d8450fc163625c6e0051e4

commit f454e4b2e578ab6725d8450fc163625c6e0051e4
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Wed Dec 20 19:36:19 2017

event router: Some code consolidation.

Notable changes:
. ProcessKeyboardEvent() is never actually called. So remove it.
. Make ProcessMouseEvent and ProcessTouchEvent non-virtuals, and provide
  impl in in the base class. Also, add pre-dispatch steps for these
  events. ChildFrame and Guest impls override the pre-dispatch to focus
  their frames on mouse-down and touch-down.
. Make ProcessMouseWheelEvent non-virtual, and provide impl in the base
  class.
. Keep ProcessGestureEvent virtual, but consolidate impls into the base
  class. I would make this non-virtual too, but the ChildFrame impl needs
  to override the event being dispatched. So for now, I kept this a
  virtual. An alternative would be to provide a pre-dispatch phase, which
  can mutate the event being dispatched.

BUG= 796649 

Change-Id: I5ab082bbb7eb2136fc39e2dcb7472e6352afce68
Reviewed-on: https://chromium-review.googlesource.com/836267
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525411}
[modify] https://crrev.com/f454e4b2e578ab6725d8450fc163625c6e0051e4/content/browser/frame_host/render_widget_host_view_guest.cc
[modify] https://crrev.com/f454e4b2e578ab6725d8450fc163625c6e0051e4/content/browser/frame_host/render_widget_host_view_guest.h
[modify] https://crrev.com/f454e4b2e578ab6725d8450fc163625c6e0051e4/content/browser/renderer_host/render_widget_host_input_event_router_unittest.cc
[modify] https://crrev.com/f454e4b2e578ab6725d8450fc163625c6e0051e4/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/f454e4b2e578ab6725d8450fc163625c6e0051e4/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/f454e4b2e578ab6725d8450fc163625c6e0051e4/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/f454e4b2e578ab6725d8450fc163625c6e0051e4/content/browser/renderer_host/render_widget_host_view_aura.h
[modify] https://crrev.com/f454e4b2e578ab6725d8450fc163625c6e0051e4/content/browser/renderer_host/render_widget_host_view_base.cc
[modify] https://crrev.com/f454e4b2e578ab6725d8450fc163625c6e0051e4/content/browser/renderer_host/render_widget_host_view_base.h
[modify] https://crrev.com/f454e4b2e578ab6725d8450fc163625c6e0051e4/content/browser/renderer_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/f454e4b2e578ab6725d8450fc163625c6e0051e4/content/browser/renderer_host/render_widget_host_view_child_frame.h
[modify] https://crrev.com/f454e4b2e578ab6725d8450fc163625c6e0051e4/content/browser/renderer_host/render_widget_host_view_mac.h
[modify] https://crrev.com/f454e4b2e578ab6725d8450fc163625c6e0051e4/content/browser/renderer_host/render_widget_host_view_mac.mm

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d2ac364261f069fdf4552658fbf3c5d24cf2c19d

commit d2ac364261f069fdf4552658fbf3c5d24cf2c19d
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Fri Dec 22 02:26:55 2017

event router: Separate targeting code for mouse events.

This change splits out the code to find the target for a mouse event and
clearly separates it from the dispatching code, and other code can
mutates the state of the event-router. This is a pre-cursor to using
asynchronous viz-assisted hit-testing for hit-testing.

BUG= 796649 

Change-Id: I58fdd4899f27bce341adb538833956090ceb38e1
Reviewed-on: https://chromium-review.googlesource.com/836811
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525904}
[modify] https://crrev.com/d2ac364261f069fdf4552658fbf3c5d24cf2c19d/content/browser/renderer_host/render_widget_host_input_event_router.cc
[modify] https://crrev.com/d2ac364261f069fdf4552658fbf3c5d24cf2c19d/content/browser/renderer_host/render_widget_host_input_event_router.h

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e061565c5cf93062f52f1ee665cdc33222a92b6e

commit e061565c5cf93062f52f1ee665cdc33222a92b6e
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Fri Dec 22 19:03:46 2017

event router: Remove an unnecessary overloaded function.

Remove the version of FindViewAtLocation that takes a gfx::Point. All
the callers can use the version that uses a gfx::PointF wihout having
to do extra work. So update the code to do that.

Also, make TargetData::delta a gfx::Vector2dF because blink event
locations are floats.

BUG= 796649 

Change-Id: Ib1f8d19671a642aed5f111b03a22a632521e33af
Reviewed-on: https://chromium-review.googlesource.com/841724
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526034}
[modify] https://crrev.com/e061565c5cf93062f52f1ee665cdc33222a92b6e/content/browser/renderer_host/render_widget_host_input_event_router.cc
[modify] https://crrev.com/e061565c5cf93062f52f1ee665cdc33222a92b6e/content/browser/renderer_host/render_widget_host_input_event_router.h

Project Member

Comment 6 by bugdroid1@chromium.org, Jan 5 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8feae8c3ffa845c0c3e798cc6f65c541474c800a

commit 8feae8c3ffa845c0c3e798cc6f65c541474c800a
Author: Ria Jiang <riajiang@chromium.org>
Date: Fri Jan 05 16:35:29 2018

Separate out targeting logic for mouse wheel events in event router.

This CL separates out targeting logic for mouse wheel events from
dispatching, updating event router states and sending gesture scroll end
event in RenderWidgetHostInputEventRouter::RouteMouseWheelEvent. Next
step is to change mouse wheel events to use the async hit-testing if
the returned target has ask flag set.

Bug:  796649 
Change-Id: I24959da5524945c4f392d0627dd40ee730e85fc7
Reviewed-on: https://chromium-review.googlesource.com/849554
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Ria Jiang <riajiang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527295}
[modify] https://crrev.com/8feae8c3ffa845c0c3e798cc6f65c541474c800a/content/browser/renderer_host/render_widget_host_input_event_router.cc
[modify] https://crrev.com/8feae8c3ffa845c0c3e798cc6f65c541474c800a/content/browser/renderer_host/render_widget_host_input_event_router.h

Comment 7 by sadrul@chromium.org, Jan 11 2018

Status: Fixed (was: Started)
riajiang@ took care of this for mouse-wheel events (crrev.com/527295).
wjmaclean@ took care of this for touch/gesture events (crrev.com/528375, crrev.com/528401). So we can call this fixed.
Project Member

Comment 8 by bugdroid1@chromium.org, Jan 11 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a7b0c543f513812f997f9aecfed06e43d6a15d8e

commit a7b0c543f513812f997f9aecfed06e43d6a15d8e
Author: Ria Jiang <riajiang@chromium.org>
Date: Thu Jan 11 17:04:31 2018

Separate out targeting logic for touchpad events in event router.

This CL separates out targeting logic for touchpad gesture events from
dispatching, updating event router states and sending gesture scroll end
event in RenderWidgetHostInputEventRouter::RouteTouchpadGestureEvent.
Next step is to change touchpad events to use the async hit-testing if
the returned target has ask flag set.

Bug:  796649 
Change-Id: Ib87e4225539e7ced4eca0bd3c7fbd3a56ba3babd
Reviewed-on: https://chromium-review.googlesource.com/855198
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Ria Jiang <riajiang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528646}
[modify] https://crrev.com/a7b0c543f513812f997f9aecfed06e43d6a15d8e/content/browser/renderer_host/render_widget_host_input_event_router.cc
[modify] https://crrev.com/a7b0c543f513812f997f9aecfed06e43d6a15d8e/content/browser/renderer_host/render_widget_host_input_event_router.h

Sign in to add a comment