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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 week, 6 days ago by sahel
Modified:
4 days, 22 hours 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
Commit queue not available (can’t edit this change).

Messages

Total messages: 50 (34 generated)
sahel
dtapuska@chromium.org: Please review changes in bokan@chromium.org: Please review changes in
1 week, 6 days 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 ...
1 week, 5 days 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 ...
1 week, 5 days 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, ...
1 week, 5 days 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 ...
1 week, 4 days 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 ...
1 week, 4 days 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 ...
1 week, 4 days ago (2017-05-12 19:58:24 UTC) #8
sahel
tdresser@chromium.org: Please review changes in
1 week 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 days, 4 hours 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 days, 1 hour 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 ...
5 days, 23 hours 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 ...
5 days, 1 hour 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 ...
5 days, 1 hour 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 ...
5 days 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
4 days, 22 hours ago (2017-05-19 19:43:29 UTC) #47
commit-bot: I haz the power
4 days, 22 hours 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...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 650457f06