Trackpad scrolling could lag on MacOSX

Trackpad scrolling could lag on MacOSX

Mouse wheel gesture scrolling with synthetic begin and ends
could saturate the gesture event queue. Ensure that
a synthetic gesture begin coalesces with a synthetic
gesture end so that the updates end up coalescing as well.
This prevents laggy scrolling on MacOSX when it takes
a long time per frame to scroll.

BUG=607340
Committed: https://crrev.com/dcd46c08887b38add468d81bf3b5bf4910d0f248
Cr-Commit-Position: refs/heads/master@{#390942}

[dtapuska, 2016-04-29 20:18:50]

Description was changed from

==========
Trackpad scrolling could lag on MacOSX

Mouse wheel gesture scrolling with synthetic begin and ends
could saturate the gesture event queue. Ensure that
a synthetic gesture begin coalesces with a synthetic
gesture end so that the updates end up coalescing as well.
This prevents laggy scrolling on MacOSX when it takes
a long time per frame to scroll.

BUG=607340
==========

to

==========
Trackpad scrolling could lag on MacOSX

Mouse wheel gesture scrolling with synthetic begin and ends
could saturate the gesture event queue. Ensure that
a synthetic gesture begin coalesces with a synthetic
gesture end so that the updates end up coalescing as well.
This prevents laggy scrolling on MacOSX when it takes
a long time per frame to scroll.

BUG=607340
==========

[dtapuska, 2016-04-29 20:18:50]

dtapuska@chromium.org changed reviewers:
+ tdresser@chromium.org

[tdresser, 2016-05-02 13:27:15]

LGTM with nits.

https://codereview.chromium.org/1932363002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/input/gesture_event_queue.cc (right):

https://codereview.chromium.org/1932363002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/input/gesture_event_queue.cc:203: // If a
synthetic scroll begin is encountered; it can cancel out a previous
; -> ,

https://codereview.chromium.org/1932363002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/input/gesture_event_queue.cc:207: if (synthetic &&
EventsInFlightCount() < coalesced_gesture_events_.size()) {
Might be worth factoring out |EventsInFlightCount() <
coalesced_gesture_events_.size()| into a well named bool.

Maybe something like |have_unsent_events|?

https://codereview.chromium.org/1932363002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/input/gesture_event_queue.h (right):

https://codereview.chromium.org/1932363002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/input/gesture_event_queue.h:120: bool
FilterScrollBegin(const GestureEventWithLatencyInfo& gesture_event);
The name |FilterScrollBegin| makes me think we'll just decide what to do with
the scroll begin event, whereas we actually remove the previous scroll end.

Maybe just OnScrollBegin?
Or you could move the synthetic check, and have it be OnSyntheticScrollBegin?

https://codereview.chromium.org/1932363002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/input/gesture_event_queue_unittest.cc
(right):

https://codereview.chromium.org/1932363002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/input/gesture_event_queue_unittest.cc:1161: //
Simulate gesture events.
The previous line doesn't add much information.

[dtapuska, 2016-05-02 13:57:25]

https://codereview.chromium.org/1932363002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/input/gesture_event_queue.cc (right):

https://codereview.chromium.org/1932363002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/input/gesture_event_queue.cc:203: // If a
synthetic scroll begin is encountered; it can cancel out a previous
On 2016/05/02 13:27:15, tdresser wrote:
> ; -> ,

Done.

https://codereview.chromium.org/1932363002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/input/gesture_event_queue.cc:207: if (synthetic &&
EventsInFlightCount() < coalesced_gesture_events_.size()) {
On 2016/05/02 13:27:15, tdresser wrote:
> Might be worth factoring out |EventsInFlightCount() <
> coalesced_gesture_events_.size()| into a well named bool.
> 
> Maybe something like |have_unsent_events|?

Done.

https://codereview.chromium.org/1932363002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/input/gesture_event_queue.h (right):

https://codereview.chromium.org/1932363002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/input/gesture_event_queue.h:120: bool
FilterScrollBegin(const GestureEventWithLatencyInfo& gesture_event);
On 2016/05/02 13:27:15, tdresser wrote:
> The name |FilterScrollBegin| makes me think we'll just decide what to do with
> the scroll begin event, whereas we actually remove the previous scroll end.
> 
> Maybe just OnScrollBegin?
> Or you could move the synthetic check, and have it be OnSyntheticScrollBegin?

Done.

https://codereview.chromium.org/1932363002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/input/gesture_event_queue_unittest.cc
(right):

https://codereview.chromium.org/1932363002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/input/gesture_event_queue_unittest.cc:1161: //
Simulate gesture events.
On 2016/05/02 13:27:15, tdresser wrote:
> The previous line doesn't add much information.

Done.

[dtapuska, 2016-05-02 13:58:58]

The CQ bit was checked by dtapuska@chromium.org

[dtapuska, 2016-05-02 13:58:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
tdresser@chromium.org
Link to the patchset: https://codereview.chromium.org/1932363002/#ps20001
(title: "Fix nits")

[commit-bot: I haz the power, 2016-05-02 13:59:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1932363002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1932363002/20001

[commit-bot: I haz the power, 2016-05-02 14:52:41]

Description was changed from

==========
Trackpad scrolling could lag on MacOSX

Mouse wheel gesture scrolling with synthetic begin and ends
could saturate the gesture event queue. Ensure that
a synthetic gesture begin coalesces with a synthetic
gesture end so that the updates end up coalescing as well.
This prevents laggy scrolling on MacOSX when it takes
a long time per frame to scroll.

BUG=607340
==========

to

==========
Trackpad scrolling could lag on MacOSX

Mouse wheel gesture scrolling with synthetic begin and ends
could saturate the gesture event queue. Ensure that
a synthetic gesture begin coalesces with a synthetic
gesture end so that the updates end up coalescing as well.
This prevents laggy scrolling on MacOSX when it takes
a long time per frame to scroll.

BUG=607340
==========

[commit-bot: I haz the power, 2016-05-02 14:52:44]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-05-02 14:54:12]

Description was changed from

==========
Trackpad scrolling could lag on MacOSX

Mouse wheel gesture scrolling with synthetic begin and ends
could saturate the gesture event queue. Ensure that
a synthetic gesture begin coalesces with a synthetic
gesture end so that the updates end up coalescing as well.
This prevents laggy scrolling on MacOSX when it takes
a long time per frame to scroll.

BUG=607340
==========

to

==========
Trackpad scrolling could lag on MacOSX

Mouse wheel gesture scrolling with synthetic begin and ends
could saturate the gesture event queue. Ensure that
a synthetic gesture begin coalesces with a synthetic
gesture end so that the updates end up coalescing as well.
This prevents laggy scrolling on MacOSX when it takes
a long time per frame to scroll.

BUG=607340

Committed: https://crrev.com/dcd46c08887b38add468d81bf3b5bf4910d0f248
Cr-Commit-Position: refs/heads/master@{#390942}
==========

[commit-bot: I haz the power, 2016-05-02 14:54:13]

Patchset 2 (id:??) landed as
https://crrev.com/dcd46c08887b38add468d81bf3b5bf4910d0f248
Cr-Commit-Position: refs/heads/master@{#390942}