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

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

Created:
11 months, 2 weeks ago by sahel
Modified:
11 months 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. #

Messages

Total messages: 50 (34 generated)
sahel
dtapuska@chromium.org: Please review changes in bokan@chromium.org: Please review changes in
11 months, 2 weeks 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 ...
11 months, 2 weeks 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 ...
11 months, 2 weeks 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, ...
11 months, 2 weeks 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 ...
11 months, 2 weeks 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 ...
11 months, 2 weeks 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 ...
11 months, 2 weeks ago (2017-05-12 19:58:24 UTC) #8
sahel
tdresser@chromium.org: Please review changes in
11 months, 2 weeks 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 ...
11 months, 1 week 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 ...
11 months, 1 week 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 ...
11 months, 1 week 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 ...
11 months, 1 week 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 ...
11 months, 1 week 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 ...
11 months, 1 week 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
11 months, 1 week ago (2017-05-19 19:43:29 UTC) #47
commit-bot: I haz the power
11 months, 1 week 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 408576698