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

Issue 637393 link

Starred by 5 users
Status: Fixed
Owner:
█ chongz@chromium.org
Last visit > 30 days ago
Closed: Oct 2016
Cc:
dtapu...@chromium.org
tdres...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocking:
issue 625689



Sign in to add a comment

Refactor event handling path from |InputEventFilter| to |InputHandlerProxy| to be callback-based

Project Member Reported by chongz@chromium.org, Aug 12 2016 
In order to implement VSync Aligned Input on compositor we need to refactor the event passing path to be callback based.

We also need to changed the logic of |current_overscroll_params_| since the current intercepting logic only works for synchronized event handling.

So the planned steps are:
  1. Move |DidOverscrollParams| from |content::| to |ui::|
  2. Move |ScopedWebInputEvent| from |content::| to |ui::|
  3. Change the path to callback-based. e.g. Passing |ScopedWebInputEvent| along |InputEventFilter|->|InputHandlerProxy| (as well as |CompositorMusConnection|->|InputHandlerProxy|), and pass back |ScopedWebInputEvent|+|DidOverscrollParams| in the callbacks.

Note:
We have to choose the callback-based implementation instead of making it an interface of the client. The reasons are:
  1. |CompositorMusConnection| is not a client but also sending events to |InputHandlerManager|.
  2. |CompositorMusConnection| is already callback-based, and it's hard to make it interop with interface-based implementation.
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 16 2016 

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

commit a8ba91fcbea546ad0922f99aa89fc19670c3b129
Author: chongz <chongz@chromium.org>
Date: Tue Aug 16 21:39:17 2016

Move |DidOverscrollParams| from |content::| to "ui/events/blink"

This CL moves |DidOverscrollParams| from |content::| to |ui::|, so it
can be accessed by |InputHandlerProxy| in "ui/events/blink".

Adds dependency:
  1. content/common : ui/events/blink
  2. content/renderer/mus : ui/events/blink

BUG= 637393 

Review-Url: https://codereview.chromium.org/2240273002
Cr-Commit-Position: refs/heads/master@{#412344}

[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/browser/android/overscroll_controller_android.cc
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/browser/android/overscroll_controller_android.h
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/browser/android/synchronous_compositor_host.cc
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/browser/android/synchronous_compositor_host.h
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/browser/renderer_host/input/input_router_client.h
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/browser/renderer_host/input/input_router_impl.cc
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/browser/renderer_host/input/input_router_impl.h
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/browser/renderer_host/input/input_router_impl_perftest.cc
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/browser/renderer_host/input/input_router_impl_unittest.cc
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/browser/renderer_host/input/mock_input_router_client.cc
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/browser/renderer_host/input/mock_input_router_client.h
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/browser/renderer_host/render_widget_host_view_base.h
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/common/BUILD.gn
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/common/android/sync_compositor_messages.h
[delete] https://crrev.com/ba4c3aede798239f5f02d78322051ffc80bb752f/content/common/input/did_overscroll_params.cc
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/common/input/input_event_ack.cc
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/common/input/input_event_ack.h
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/common/input_messages.h
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/content_common.gypi
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/renderer/input/input_event_filter.cc
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/renderer/input/input_event_filter.h
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/renderer/input/input_handler_manager.cc
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/renderer/input/input_handler_manager.h
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/renderer/input/input_handler_manager_client.h
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/renderer/input/input_handler_wrapper.cc
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/renderer/input/render_widget_input_handler.cc
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/renderer/input/render_widget_input_handler.h
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/renderer/input/render_widget_input_handler_delegate.h
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/renderer/mus/BUILD.gn
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/renderer/mus/compositor_mus_connection.cc
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/renderer/mus/compositor_mus_connection.h
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/renderer/mus/compositor_mus_connection_unittest.cc
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/renderer/mus/render_widget_mus_connection.cc
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/renderer/mus/render_widget_mus_connection.h
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/renderer/render_widget.cc
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/renderer/render_widget.h
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/content/renderer/render_widget_unittest.cc
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/ui/events/blink/BUILD.gn
[add] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/ui/events/blink/did_overscroll_params.cc
[rename] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/ui/events/blink/did_overscroll_params.h
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/ui/events/blink/input_handler_proxy.cc
[modify] https://crrev.com/a8ba91fcbea546ad0922f99aa89fc19670c3b129/ui/events/blink/input_handler_proxy.h
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 17 2016 

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

commit 4a975194104af57c2c05f8dc0144e94a79dc2076
Author: chongz <chongz@chromium.org>
Date: Wed Aug 17 17:49:39 2016

Move |ScopedWebInputEvent| and |WebInputEventTraits| from
|content::| to "ui/events/blink"

This CL moves |ScopedWebInputEvent| and |WebInputEventTraits|
from |content::| to "ui/events/blink", so they can be accessed by
|InputHandlerProxy|.

Adds dependency:
  1. content/common : ui/events/blink
  2. content/renderer/mus : ui/events/blink

Concerns:
  1. Moving |WebInputEventTraits| seems redundant since we only
need |WebInputEventTraits::Delete/Clone|.
  2. "ui/events/blink" seems to be static linked, so it will cause each
file to grow.

After discussion with dtapuska@ and tdresser@ it doesn't really
matter whether we move it entirely or only part of it.
So the easier solution is to move it entirely for now, and handle it
later if it's actually causing problems.

BUG= 637393 

Review-Url: https://codereview.chromium.org/2240983003
Cr-Commit-Position: refs/heads/master@{#412579}

[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/browser/renderer_host/input/gesture_event_queue.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/browser/renderer_host/input/input_router_impl.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/browser/renderer_host/input/input_router_impl_perftest.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/browser/renderer_host/input/input_router_impl_unittest.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/browser/renderer_host/input/mouse_wheel_event_queue.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/browser/renderer_host/input/render_widget_host_latency_tracker.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/browser/renderer_host/input/synthetic_gesture_target_base.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/browser/renderer_host/input/touch_emulator_unittest.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/browser/renderer_host/input/web_input_event_util_unittest.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/browser/renderer_host/render_widget_host_unittest.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/browser/renderer_host/render_widget_host_view_mac_unittest.mm
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/common/content_param_traits.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/common/input/event_with_latency_info.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/common/input/event_with_latency_info.h
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/common/input/gesture_event_stream_validator.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/common/input/input_event.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/common/input/input_event.h
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/common/input/input_event_stream_validator.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/common/input/input_param_traits.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/common/input/input_param_traits.h
[delete] https://crrev.com/e5a7ff50b3c0cb72e06a3ea7af481d7f41f872a9/content/common/input/scoped_web_input_event.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/common/input/touch_event_stream_validator.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/common/input/web_touch_event_traits.h
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/content_common.gypi
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/content_tests.gypi
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/renderer/input/input_event_filter.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/renderer/input/input_event_filter_unittest.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/renderer/input/input_handler_manager.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/renderer/input/main_thread_event_queue.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/renderer/input/main_thread_event_queue.h
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/renderer/input/main_thread_event_queue_unittest.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/renderer/input/render_widget_input_handler.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/renderer/mus/compositor_mus_connection.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/renderer/render_widget.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/renderer/render_widget_unittest.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/content/test/BUILD.gn
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/ui/events/BUILD.gn
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/ui/events/blink/BUILD.gn
[add] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/ui/events/blink/scoped_web_input_event.cc
[rename] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/ui/events/blink/scoped_web_input_event.h
[rename] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/ui/events/blink/web_input_event_traits.cc
[rename] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/ui/events/blink/web_input_event_traits.h
[rename] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/ui/events/blink/web_input_event_traits_unittest.cc
[modify] https://crrev.com/4a975194104af57c2c05f8dc0144e94a79dc2076/ui/events/events_unittests.gyp
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 10 2016 

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

commit 78f276e96eac5a447149fafac0b4953eb5936979
Author: chongz <chongz@chromium.org>
Date: Sat Sep 10 01:08:50 2016

Refactor compositor event handling path to be callback-based

(A pre-work for compositor VSync aligned input.)

This CL adds |DidHandleInputEventAndOverscroll| method, and will
be passed to |HandleInputEvent| as callback.
(It's hard to use interface-based implementation as
|CompositorMusConnection| already has callback-based method
baked in.)

New event path:
1. |InputEventFilter|                |DidForwardToHandlerAndOverscroll|
          V                                        ^
   |InputHandlerManager|             |DidHandleInputEventAndOverscroll|
          V                                        ^
   |InputHandlerProxy|        ->      (handle event)

2. |CompositorMusConnection|         |DidHandle...AndOverscroll|
          V                                        ^
   |InputHandlerManager|             |DidHandleInputEventAndOverscroll|
          V                                        ^
   |InputHandlerProxy|        ->      (handle event)

3. (other sources)           ..Wrapper->..Manager->..Filter::DidOverscroll
          V                                        ^
   |InputHandlerProxy|        ->      (did overscroll)

Note about overscroll:
Only one of |DidOverscroll| or |DidHandleInputEventAndOverscroll|
will be fired, and it's decided by whether it's caused by an InputEvent.

Note about event queue:
Events are still handled synchronously in |InputHandlerProxy|, following
up CLs will support event queue.

Design Doc: (@chromium.org)
https://docs.google.com/a/chromium.org/document/d/1SUPPJkqxEYMRso3Jw1wZnqD3g1dmif0ic-x5aQkpHBo/edit?usp=sharing

BUG= 637393 

Review-Url: https://codereview.chromium.org/2265393002
Cr-Commit-Position: refs/heads/master@{#417788}

[modify] https://crrev.com/78f276e96eac5a447149fafac0b4953eb5936979/content/renderer/input/input_event_filter.cc
[modify] https://crrev.com/78f276e96eac5a447149fafac0b4953eb5936979/content/renderer/input/input_event_filter.h
[modify] https://crrev.com/78f276e96eac5a447149fafac0b4953eb5936979/content/renderer/input/input_event_filter_unittest.cc
[modify] https://crrev.com/78f276e96eac5a447149fafac0b4953eb5936979/content/renderer/input/input_handler_manager.cc
[modify] https://crrev.com/78f276e96eac5a447149fafac0b4953eb5936979/content/renderer/input/input_handler_manager.h
[modify] https://crrev.com/78f276e96eac5a447149fafac0b4953eb5936979/content/renderer/input/input_handler_manager_client.h
[modify] https://crrev.com/78f276e96eac5a447149fafac0b4953eb5936979/content/renderer/input/input_handler_wrapper.h
[modify] https://crrev.com/78f276e96eac5a447149fafac0b4953eb5936979/content/renderer/mus/compositor_mus_connection.cc
[modify] https://crrev.com/78f276e96eac5a447149fafac0b4953eb5936979/content/renderer/mus/compositor_mus_connection.h
[modify] https://crrev.com/78f276e96eac5a447149fafac0b4953eb5936979/content/renderer/mus/compositor_mus_connection_unittest.cc
[modify] https://crrev.com/78f276e96eac5a447149fafac0b4953eb5936979/content/renderer/mus/render_widget_mus_connection.cc
[modify] https://crrev.com/78f276e96eac5a447149fafac0b4953eb5936979/content/renderer/mus/render_widget_mus_connection.h
[modify] https://crrev.com/78f276e96eac5a447149fafac0b4953eb5936979/ui/events/blink/input_handler_proxy.cc
[modify] https://crrev.com/78f276e96eac5a447149fafac0b4953eb5936979/ui/events/blink/input_handler_proxy.h
[modify] https://crrev.com/78f276e96eac5a447149fafac0b4953eb5936979/ui/events/blink/input_handler_proxy_client.h

Comment 4 by chongz@chromium.org, Oct 13 2016

Status: Fixed (was: Started)
►

Sign in to add a comment