Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(6)

Issue 2882443002: Tiimer based phase info generated for mouse wheel events. (Closed)

Created:
6 months, 1 week ago by sahel
Modified:
5 months, 4 weeks ago
Reviewers:
bokan, dtapuska, tdresser
CC:
chromium-reviews, yusukes+watch_chromium.org, shuchen+watch_chromium.org, jam, dtapuska+chromiumwatch_chromium.org, blink-reviews-api_chromium.org, jbauman+watch_chromium.org, nona+watch_chromium.org, dglazkov+blink, darin-cc_chromium.org, piman+watch_chromium.org, blink-reviews, kalyank, danakj+watch_chromium.org, James Su, kinuko+watch
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Timer based phase info generated for mouse wheel events. In RenderWidgetHostViewAura and RenderWidgetHostViewEventHandler timer based phase info is added to WebMouseWheelEvents when they are generated from ui events. This phase info is used in MouseWheelEventQueue to generate GSB and GSE events when wheel scroll latching is enabled. When latching is enabled, no wheel event with phase = |kPhaseEnded| will be sent before a wheel event with momentum_phase = |kPhaseBegan|. This cl contains aura only changes. Mac specific changes will be implemented as a different cl. BUG=526463 TEST=RenderWidgetHostViewAuraWheelScrollLatchingEnabledTest. TimerBasedWheelEventPhaseInfo/TouchpadFlingStartStopsWheelPhaseTimer/ GSBWithTouchSourceStopsWheelScrollSequence Review-Url: https://codereview.chromium.org/2882443002 Cr-Commit-Position: refs/heads/master@{#473288} Committed: https://chromium.googlesource.com/chromium/src/+/d18f2226063d54e2f67afbcdf63347a70a94e66b

Patch Set 1 #

Patch Set 2 : use of phase instead of syntheticPhase #

Patch Set 3 : failing input_router_impl_unittest fixed. #

Patch Set 4 : debouncing queue disabled to count sent messages properly #

Total comments: 32

Patch Set 5 : comments addressed #

Total comments: 6

Patch Set 6 : review comments addressed. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+718 lines, -226 lines) Patch
M content/browser/renderer_host/input/input_router_impl_unittest.cc View 1 2 3 chunks +26 lines, -5 lines 0 comments Download
M content/browser/renderer_host/input/mouse_wheel_event_queue.h View 4 chunks +0 lines, -8 lines 0 comments Download
M content/browser/renderer_host/input/mouse_wheel_event_queue.cc View 1 6 chunks +24 lines, -62 lines 0 comments Download
M content/browser/renderer_host/input/mouse_wheel_event_queue_unittest.cc View 1 2 3 4 17 chunks +156 lines, -80 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_aura_unittest.cc View 1 2 3 4 5 33 chunks +403 lines, -60 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_base.h View 1 2 chunks +6 lines, -0 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_base.cc View 1 2 chunks +3 lines, -0 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_event_handler.h View 1 2 3 4 4 chunks +13 lines, -0 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_event_handler.cc View 1 2 3 4 5 6 chunks +82 lines, -11 lines 0 comments Download
M ui/events/blink/web_input_event_traits.cc View 1 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 50 (34 generated)
sahel
dtapuska@chromium.org: Please review changes in bokan@chromium.org: Please review changes in
6 months, 1 week ago (2017-05-11 14:54:47 UTC) #2
bokan
Hey Sahel, So you mentioned that the reason you added a separate synthetic field was ...
6 months, 1 week ago (2017-05-11 20:46:03 UTC) #3
sahel
On 2017/05/11 20:46:03, bokan wrote: > Hey Sahel, > > So you mentioned that the ...
6 months, 1 week ago (2017-05-11 20:54:48 UTC) #4
bokan
On 2017/05/11 20:54:48, sahel wrote: > On 2017/05/11 20:46:03, bokan wrote: > > Hey Sahel, ...
6 months, 1 week ago (2017-05-11 21:53:56 UTC) #5
sahel
On 2017/05/11 21:53:56, bokan wrote: > On 2017/05/11 20:54:48, sahel wrote: > > On 2017/05/11 ...
6 months, 1 week ago (2017-05-12 18:28:27 UTC) #6
bokan
On 2017/05/12 18:28:27, sahel wrote: > On 2017/05/11 21:53:56, bokan wrote: > > On 2017/05/11 ...
6 months, 1 week ago (2017-05-12 18:41:00 UTC) #7
dtapuska
On 2017/05/12 18:41:00, bokan wrote: > On 2017/05/12 18:28:27, sahel wrote: > > On 2017/05/11 ...
6 months, 1 week ago (2017-05-12 19:58:24 UTC) #8
sahel
tdresser@chromium.org: Please review changes in
6 months ago (2017-05-17 15:19:50 UTC) #17
tdresser
https://codereview.chromium.org/2882443002/diff/60001/content/browser/renderer_host/input/mouse_wheel_event_queue_unittest.cc File content/browser/renderer_host/input/mouse_wheel_event_queue_unittest.cc (right): https://codereview.chromium.org/2882443002/diff/60001/content/browser/renderer_host/input/mouse_wheel_event_queue_unittest.cc#newcode320 content/browser/renderer_host/input/mouse_wheel_event_queue_unittest.cc:320: kWheelScrollGlobalY, 5, 5, 0, high_precision); Would it make sense ...
6 months ago (2017-05-18 13:49:06 UTC) #25
bokan
https://codereview.chromium.org/2882443002/diff/60001/content/browser/renderer_host/render_widget_host_view_base.cc File content/browser/renderer_host/render_widget_host_view_base.cc (right): https://codereview.chromium.org/2882443002/diff/60001/content/browser/renderer_host/render_widget_host_view_base.cc#newcode44 content/browser/renderer_host/render_widget_host_view_base.cc:44: wheel_scroll_latching_enabled_(base::FeatureList::IsEnabled( Why do we need to store this? Can ...
6 months ago (2017-05-18 16:47:01 UTC) #28
sahel
https://codereview.chromium.org/2882443002/diff/60001/content/browser/renderer_host/input/mouse_wheel_event_queue_unittest.cc File content/browser/renderer_host/input/mouse_wheel_event_queue_unittest.cc (right): https://codereview.chromium.org/2882443002/diff/60001/content/browser/renderer_host/input/mouse_wheel_event_queue_unittest.cc#newcode320 content/browser/renderer_host/input/mouse_wheel_event_queue_unittest.cc:320: kWheelScrollGlobalY, 5, 5, 0, high_precision); On 2017/05/18 13:49:05, tdresser ...
6 months ago (2017-05-18 18:15:15 UTC) #32
bokan
Please wrap the commit message at 72 columns. Otherwise, lgtm https://codereview.chromium.org/2882443002/diff/60001/content/browser/renderer_host/render_widget_host_view_event_handler.cc File content/browser/renderer_host/render_widget_host_view_event_handler.cc (right): https://codereview.chromium.org/2882443002/diff/60001/content/browser/renderer_host/render_widget_host_view_event_handler.cc#newcode553 ...
6 months ago (2017-05-19 15:52:59 UTC) #35
tdresser
LGTM https://codereview.chromium.org/2882443002/diff/100001/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc File content/browser/renderer_host/render_widget_host_view_aura_unittest.cc (right): https://codereview.chromium.org/2882443002/diff/100001/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc#newcode4779 content/browser/renderer_host/render_widget_host_view_aura_unittest.cc:4779: WebMouseWheelEvent::kPhaseChanged); Is it worth using the same trick ...
6 months ago (2017-05-19 16:17:33 UTC) #37
sahel
https://codereview.chromium.org/2882443002/diff/100001/content/browser/renderer_host/input/mouse_wheel_event_queue_unittest.cc File content/browser/renderer_host/input/mouse_wheel_event_queue_unittest.cc (right): https://codereview.chromium.org/2882443002/diff/100001/content/browser/renderer_host/input/mouse_wheel_event_queue_unittest.cc#newcode422 content/browser/renderer_host/input/mouse_wheel_event_queue_unittest.cc:422: // |kPhaseEnded| will be sent before a wheel event ...
6 months ago (2017-05-19 17:39:16 UTC) #41
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2882443002/120001
6 months ago (2017-05-19 19:43:29 UTC) #47
commit-bot: I haz the power
6 months ago (2017-05-19 19:49:00 UTC) #50
Message was sent while issue was closed.
Committed patchset #6 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/d18f2226063d54e2f67afbcdf633...

Powered by Google App Engine
This is Rietveld efc10ee0f