Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(194)

Issue 2814043004: MouseWheelEventQueue should respect ScrollBegin/End state from RenderWidgetHost. (Closed)

Created:
3 years, 8 months ago by wjmaclean
Modified:
3 years, 7 months ago
CC:
chromium-reviews, jam, darin-cc_chromium.org, dtapuska+chromiumwatch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

MouseWheelEventQueue should respect ScrollBegin/End state from RenderWidgetHost. This CL modifies MouseWheelEventQueue to refer to it's client to know if a (non-synthetic) GestureScrollBegin/End are required, instead of hard-coding that state in the Queue itself. This is important since OOPIF scroll bubbling inserts its own Begin/Ends as needed, without the Queue's involvement. This CL has been split off from https://codereview.chromium.org/2785533003/ to simplify review/landing of that CL. BUG=701178

Patch Set 1 #

Total comments: 9

Patch Set 2 : Rename function, make it const. #

Total comments: 6

Patch Set 3 : Rename function. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+83 lines, -39 lines) Patch
M content/browser/renderer_host/input/input_router_client.h View 1 1 chunk +4 lines, -0 lines 0 comments Download
M content/browser/renderer_host/input/input_router_impl.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M content/browser/renderer_host/input/input_router_impl.cc View 1 2 1 chunk +5 lines, -0 lines 0 comments Download
M content/browser/renderer_host/input/input_router_impl_perftest.cc View 1 1 chunk +3 lines, -0 lines 0 comments Download
M content/browser/renderer_host/input/mock_input_router_client.h View 1 2 chunks +2 lines, -0 lines 0 comments Download
M content/browser/renderer_host/input/mock_input_router_client.cc View 1 2 chunks +15 lines, -2 lines 0 comments Download
M content/browser/renderer_host/input/mouse_wheel_event_queue.h View 1 2 2 chunks +1 line, -7 lines 0 comments Download
M content/browser/renderer_host/input/mouse_wheel_event_queue.cc View 1 2 9 chunks +11 lines, -18 lines 0 comments Download
M content/browser/renderer_host/input/mouse_wheel_event_queue_unittest.cc View 1 2 3 chunks +15 lines, -1 line 0 comments Download
M content/browser/renderer_host/render_widget_host_impl.h View 1 2 chunks +4 lines, -6 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_impl.cc View 1 2 chunks +18 lines, -3 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_input_event_router.cc View 1 2 chunks +4 lines, -2 lines 0 comments Download

Depends on Patchset:

Messages

Total messages: 24 (11 generated)
wjmaclean
I've split this part of https://codereview.chromium.org/2785533003/ into it's own CL, as requested by bokan@, but ...
3 years, 8 months ago (2017-04-12 18:26:44 UTC) #5
dtapuska
https://codereview.chromium.org/2814043004/diff/1/content/browser/renderer_host/input/input_router_client.h File content/browser/renderer_host/input/input_router_client.h (right): https://codereview.chromium.org/2814043004/diff/1/content/browser/renderer_host/input/input_router_client.h#newcode64 content/browser/renderer_host/input/input_router_client.h:64: virtual bool is_in_gesture_scroll(blink::WebGestureDevice device) = 0; This is not ...
3 years, 8 months ago (2017-04-12 19:24:46 UTC) #7
wjmaclean
See responses below. I'll upload a new patch once I'm out of my meetings, about ...
3 years, 8 months ago (2017-04-12 19:47:35 UTC) #10
wjmaclean
https://codereview.chromium.org/2814043004/diff/1/content/browser/renderer_host/input/mouse_wheel_event_queue.cc File content/browser/renderer_host/input/mouse_wheel_event_queue.cc (left): https://codereview.chromium.org/2814043004/diff/1/content/browser/renderer_host/input/mouse_wheel_event_queue.cc#oldcode226 content/browser/renderer_host/input/mouse_wheel_event_queue.cc:226: SendScrollBegin(scroll_update, false); On 2017/04/12 19:47:35, wjmaclean wrote: > On ...
3 years, 8 months ago (2017-04-12 19:48:42 UTC) #11
wjmaclean
Comments addressed, ptal?
3 years, 8 months ago (2017-04-12 21:12:18 UTC) #13
bokan
https://codereview.chromium.org/2814043004/diff/20001/content/browser/renderer_host/input/mouse_wheel_event_queue.cc File content/browser/renderer_host/input/mouse_wheel_event_queue.cc (left): https://codereview.chromium.org/2814043004/diff/20001/content/browser/renderer_host/input/mouse_wheel_event_queue.cc#oldcode287 content/browser/renderer_host/input/mouse_wheel_event_queue.cc:287: needs_scroll_begin_ = true; I don't understand this code well, ...
3 years, 8 months ago (2017-04-12 22:27:20 UTC) #17
wjmaclean
https://codereview.chromium.org/2814043004/diff/20001/content/browser/renderer_host/input/mouse_wheel_event_queue.cc File content/browser/renderer_host/input/mouse_wheel_event_queue.cc (left): https://codereview.chromium.org/2814043004/diff/20001/content/browser/renderer_host/input/mouse_wheel_event_queue.cc#oldcode287 content/browser/renderer_host/input/mouse_wheel_event_queue.cc:287: needs_scroll_begin_ = true; On 2017/04/12 22:27:20, bokan wrote: > ...
3 years, 8 months ago (2017-04-13 13:09:43 UTC) #18
dtapuska
https://codereview.chromium.org/2814043004/diff/20001/content/browser/renderer_host/input/input_router_impl.h File content/browser/renderer_host/input/input_router_impl.h (right): https://codereview.chromium.org/2814043004/diff/20001/content/browser/renderer_host/input/input_router_impl.h#newcode119 content/browser/renderer_host/input/input_router_impl.h:119: bool IsInGestureScroll() const override; This is still missing the ...
3 years, 8 months ago (2017-04-13 13:39:48 UTC) #19
wjmaclean
https://codereview.chromium.org/2814043004/diff/20001/content/browser/renderer_host/input/input_router_impl.h File content/browser/renderer_host/input/input_router_impl.h (right): https://codereview.chromium.org/2814043004/diff/20001/content/browser/renderer_host/input/input_router_impl.h#newcode119 content/browser/renderer_host/input/input_router_impl.h:119: bool IsInGestureScroll() const override; On 2017/04/13 13:39:47, dtapuska wrote: ...
3 years, 8 months ago (2017-04-13 14:17:11 UTC) #20
dtapuska
On 2017/04/13 14:17:11, wjmaclean wrote: > https://codereview.chromium.org/2814043004/diff/20001/content/browser/renderer_host/input/input_router_impl.h > File content/browser/renderer_host/input/input_router_impl.h (right): > > https://codereview.chromium.org/2814043004/diff/20001/content/browser/renderer_host/input/input_router_impl.h#newcode119 > ...
3 years, 8 months ago (2017-04-13 14:41:52 UTC) #21
wjmaclean
On 2017/04/13 14:41:52, dtapuska wrote: > > Can you do testing on Mac to see ...
3 years, 8 months ago (2017-04-13 14:43:05 UTC) #22
wjmaclean
On 2017/04/13 14:43:05, wjmaclean wrote: > On 2017/04/13 14:41:52, dtapuska wrote: > > > > ...
3 years, 8 months ago (2017-04-13 19:43:43 UTC) #23
wjmaclean
3 years, 7 months ago (2017-04-28 19:17:20 UTC) #24
Message was sent while issue was closed.
I'm closing this issue as (i) I'm not sure the approach is Mac-compatible, and
(ii) this will hopefully be addressed in the revised scroll latching code.

Powered by Google App Engine
This is Rietveld 408576698