Handle touchpad flings with passive event listeners on compositor thread.

Handle touchpad flings with passive event listeners on compositor thread.

Touchpad flings that have only passive event listeners were forced to be
main thread. Handle event injection to the main thread event queue
for when we get passive events so it can be entirely done on the
compositor thread.

BUG=646592
TBR=ben@chromium.org

Committed: https://crrev.com/1bcb284c49ca00b7ebf70d5de2cdbd2809a57a22
Cr-Commit-Position: refs/heads/master@{#419870}

[dtapuska, 2016-09-14 18:48:06]

The CQ bit was checked by dtapuska@chromium.org to run a CQ dry run

[dtapuska, 2016-09-14 18:48:32]

Description was changed from

==========
Handle touchpad flings with passive event listeners on compositor thread.

Touchpad flings that have only passive event listeners were forced to be
main thread. Handle event injection to the main thread event queue
for when we get passive events so it can be entirely done on the
compositor thread.

BUG=646592
==========

to

==========
Handle touchpad flings with passive event listeners on compositor thread.

Touchpad flings that have only passive event listeners were forced to be
main thread. Handle event injection to the main thread event queue
for when we get passive events so it can be entirely done on the
compositor thread.

BUG=646592
==========

[dtapuska, 2016-09-14 18:48:33]

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

[commit-bot: I haz the power, 2016-09-14 18:48:43]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dtapuska, 2016-09-14 18:48:56]

tdresser@ PTAL. I'm open to changing the callback names if you have better
suggestions.

[tdresser, 2016-09-14 18:58:08]

Looks pretty decent to me. Tests?

https://codereview.chromium.org/2341873002/diff/1/content/renderer/input/inpu...
File content/renderer/input/input_event_filter.cc (right):

https://codereview.chromium.org/2341873002/diff/1/content/renderer/input/inpu...
content/renderer/input/input_event_filter.cc:117:
iter->second->HandleEvent(std::move(event), ui::LatencyInfo(),
I'm worried we might accidentally use this on a non-synthetic event, and then
end up with bogus LatencyInfo.

Options:
1) Rename to make it clear this is synthetic.
2) Pass an empty LatencyInfo along from the callsite.
3) Hope this never happens.

I'm in favor of #2.

https://codereview.chromium.org/2341873002/diff/1/ui/events/blink/input_handl...
File ui/events/blink/input_handler_proxy_client.h (right):

https://codereview.chromium.org/2341873002/diff/1/ui/events/blink/input_handl...
ui/events/blink/input_handler_proxy_client.h:34: // via
|TransferActiveWheelFlingAnimation|.
This comment doesn't obviously connect with the method its commenting on. Can
you make the connection clearer?

[commit-bot: I haz the power, 2016-09-14 19:03:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-14 19:03:20]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dtapuska, 2016-09-15 20:42:06]

The CQ bit was checked by dtapuska@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-15 20:42:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dtapuska, 2016-09-15 20:43:04]

Modified the tests as well. PTAL.

https://codereview.chromium.org/2341873002/diff/1/content/renderer/input/inpu...
File content/renderer/input/input_event_filter.cc (right):

https://codereview.chromium.org/2341873002/diff/1/content/renderer/input/inpu...
content/renderer/input/input_event_filter.cc:117:
iter->second->HandleEvent(std::move(event), ui::LatencyInfo(),
On 2016/09/14 18:58:07, tdresser (OOO Sept 16-21) wrote:
> I'm worried we might accidentally use this on a non-synthetic event, and then
> end up with bogus LatencyInfo.
> 
> Options:
> 1) Rename to make it clear this is synthetic.
> 2) Pass an empty LatencyInfo along from the callsite.
> 3) Hope this never happens.
> 
> I'm in favor of #2.

Approach #2 done.

https://codereview.chromium.org/2341873002/diff/1/ui/events/blink/input_handl...
File ui/events/blink/input_handler_proxy_client.h (right):

https://codereview.chromium.org/2341873002/diff/1/ui/events/blink/input_handl...
ui/events/blink/input_handler_proxy_client.h:34: // via
|TransferActiveWheelFlingAnimation|.
On 2016/09/14 18:58:08, tdresser (OOO Sept 16-21) wrote:
> This comment doesn't obviously connect with the method its commenting on. Can
> you make the connection clearer?

Done.

[commit-bot: I haz the power, 2016-09-15 21:54:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-15 21:54:29]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Rick Byers, 2016-09-16 14:00:54]

LGTM with nits

https://codereview.chromium.org/2341873002/diff/40001/ui/events/blink/input_h...
File ui/events/blink/input_handler_proxy_unittest.cc (right):

https://codereview.chromium.org/2341873002/diff/40001/ui/events/blink/input_h...
ui/events/blink/input_handler_proxy_unittest.cc:243: void DidStartFlinging()
override{};
nit: why the semicolons?  Definitions don't normally have semicolons, right
(only declarations)?

https://codereview.chromium.org/2341873002/diff/40001/ui/events/blink/input_h...
ui/events/blink/input_handler_proxy_unittest.cc:1028:
DispatchNonBlockingEventToMainThread_(testing::_, testing::_))
nit: Don't you want to still test the parameters of the wheel event, since they
were generated synthetically from the fling?

[dtapuska, 2016-09-19 19:19:11]

The CQ bit was checked by dtapuska@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-19 19:20:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dtapuska, 2016-09-19 19:37:39]

https://codereview.chromium.org/2341873002/diff/40001/ui/events/blink/input_h...
File ui/events/blink/input_handler_proxy_unittest.cc (right):

https://codereview.chromium.org/2341873002/diff/40001/ui/events/blink/input_h...
ui/events/blink/input_handler_proxy_unittest.cc:243: void DidStartFlinging()
override{};
On 2016/09/16 14:00:54, Rick Byers wrote:
> nit: why the semicolons?  Definitions don't normally have semicolons, right
> (only declarations)?

Err; I initially tried to mock this entire class; done.

https://codereview.chromium.org/2341873002/diff/40001/ui/events/blink/input_h...
ui/events/blink/input_handler_proxy_unittest.cc:1028:
DispatchNonBlockingEventToMainThread_(testing::_, testing::_))
On 2016/09/16 14:00:54, Rick Byers (slow until 9-26) wrote:
> nit: Don't you want to still test the parameters of the wheel event, since
they
> were generated synthetically from the fling?

Done.

[commit-bot: I haz the power, 2016-09-19 20:37:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-19 20:37:06]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[dtapuska, 2016-09-20 13:33:47]

The CQ bit was checked by dtapuska@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-20 13:34:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-20 14:23:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-20 14:23:02]

Dry run: This issue passed the CQ dry run.

[dtapuska, 2016-09-20 14:26:32]

The CQ bit was checked by dtapuska@chromium.org

[dtapuska, 2016-09-20 14:26:33]

The patchset sent to the CQ was uploaded after l-g-t-m from rbyers@chromium.org
Link to the patchset: https://codereview.chromium.org/2341873002/#ps80001
(title: "Fix windows build")

[commit-bot: I haz the power, 2016-09-20 14:26:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dtapuska, 2016-09-20 14:28:06]

dtapuska@chromium.org changed reviewers:
+ ben@chromium.org, wez@chromium.org

[dtapuska, 2016-09-20 14:28:07]

wez@chromium.org: Please review changes in 

blimp/*

ben@chromium.org: Please review changes in 

content/renderer/mus/*

[commit-bot: I haz the power, 2016-09-20 14:34:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-20 14:34:18]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Wez, 2016-09-20 20:24:24]

https://codereview.chromium.org/2341873002/diff/80001/blimp/client/core/input...
File blimp/client/core/input/blimp_input_handler_wrapper.cc (right):

https://codereview.chromium.org/2341873002/diff/80001/blimp/client/core/input...
blimp/client/core/input/blimp_input_handler_wrapper.cc:99:
DCHECK(compositor_thread_checker_.CalledOnValidThread());
Let's DCHECK the assumption that this is only handling fling events, which this
code won't be seeing, right?

[Wez, 2016-09-20 20:25:01]

LGTM w/ my comment addressed :)

[dtapuska, 2016-09-20 21:13:21]

Description was changed from

==========
Handle touchpad flings with passive event listeners on compositor thread.

Touchpad flings that have only passive event listeners were forced to be
main thread. Handle event injection to the main thread event queue
for when we get passive events so it can be entirely done on the
compositor thread.

BUG=646592
==========

to

==========
Handle touchpad flings with passive event listeners on compositor thread.

Touchpad flings that have only passive event listeners were forced to be
main thread. Handle event injection to the main thread event queue
for when we get passive events so it can be entirely done on the
compositor thread.

BUG=646592
TBR=ben@chromium.org
==========

[dtapuska, 2016-09-20 21:13:38]

The CQ bit was checked by dtapuska@chromium.org

[dtapuska, 2016-09-20 21:13:39]

The patchset sent to the CQ was uploaded after l-g-t-m from rbyers@chromium.org,
wez@chromium.org
Link to the patchset: https://codereview.chromium.org/2341873002/#ps100001
(title: "Add dcheck for blimp")

[dtapuska, 2016-09-20 21:14:01]

https://codereview.chromium.org/2341873002/diff/80001/blimp/client/core/input...
File blimp/client/core/input/blimp_input_handler_wrapper.cc (right):

https://codereview.chromium.org/2341873002/diff/80001/blimp/client/core/input...
blimp/client/core/input/blimp_input_handler_wrapper.cc:99:
DCHECK(compositor_thread_checker_.CalledOnValidThread());
On 2016/09/20 20:24:24, Wez wrote:
> Let's DCHECK the assumption that this is only handling fling events, which
this
> code won't be seeing, right?

Done.

[commit-bot: I haz the power, 2016-09-20 21:14:25]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-20 22:13:11]

Description was changed from

==========
Handle touchpad flings with passive event listeners on compositor thread.

Touchpad flings that have only passive event listeners were forced to be
main thread. Handle event injection to the main thread event queue
for when we get passive events so it can be entirely done on the
compositor thread.

BUG=646592
TBR=ben@chromium.org
==========

to

==========
Handle touchpad flings with passive event listeners on compositor thread.

Touchpad flings that have only passive event listeners were forced to be
main thread. Handle event injection to the main thread event queue
for when we get passive events so it can be entirely done on the
compositor thread.

BUG=646592
TBR=ben@chromium.org
==========

[commit-bot: I haz the power, 2016-09-20 22:13:12]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-09-20 22:16:35]

Description was changed from

==========
Handle touchpad flings with passive event listeners on compositor thread.

Touchpad flings that have only passive event listeners were forced to be
main thread. Handle event injection to the main thread event queue
for when we get passive events so it can be entirely done on the
compositor thread.

BUG=646592
TBR=ben@chromium.org
==========

to

==========
Handle touchpad flings with passive event listeners on compositor thread.

Touchpad flings that have only passive event listeners were forced to be
main thread. Handle event injection to the main thread event queue
for when we get passive events so it can be entirely done on the
compositor thread.

BUG=646592
TBR=ben@chromium.org

Committed: https://crrev.com/1bcb284c49ca00b7ebf70d5de2cdbd2809a57a22
Cr-Commit-Position: refs/heads/master@{#419870}
==========

[commit-bot: I haz the power, 2016-09-20 22:16:36]

Patchset 6 (id:??) landed as
https://crrev.com/1bcb284c49ca00b7ebf70d5de2cdbd2809a57a22
Cr-Commit-Position: refs/heads/master@{#419870}