input: Dispatch synthesized events at regular intervals.

input: Dispatch synthesized events at regular intervals.

Instead of dispatching events at begin-frame + offset, dispatch events
at regular intervals (~16ms). The timer starts at an offset from the
first begin frame.

BUG=722921
Review-Url: https://codereview.chromium.org/2886263002
Cr-Commit-Position: refs/heads/master@{#473922}
Committed: https://chromium.googlesource.com/chromium/src/+/a6251cb3214b42349e7208d285a5e2e01276fe6f

[sadrul, 2017-05-17 20:30:04]

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

[commit-bot: I haz the power, 2017-05-17 20:31:35]

Dry run: CQ is trying da patch.

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

[sadrul, 2017-05-17 20:33:06]

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

[sadrul, 2017-05-17 20:34:21]

Patchset #2 (id:20001) has been deleted

[commit-bot: I haz the power, 2017-05-17 20:34:51]

Dry run: CQ is trying da patch.

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

[sadrul, 2017-05-17 20:35:45]

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

[sadrul, 2017-05-17 20:35:46]

This is the next step I had in mind after
https://codereview.chromium.org/2890603002

[commit-bot: I haz the power, 2017-05-17 21:24:39]

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

[commit-bot: I haz the power, 2017-05-17 21:24:40]

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

[sadrul, 2017-05-18 17:04:44]

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

[commit-bot: I haz the power, 2017-05-18 17:05:45]

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, 2017-05-18 17:34:37]

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

[commit-bot: I haz the power, 2017-05-18 17:34:38]

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

[sadrul, 2017-05-19 18:00:58]

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

[commit-bot: I haz the power, 2017-05-19 18:01:56]

Dry run: CQ is trying da patch.

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

[sadrul, 2017-05-19 18:08:53]

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

[commit-bot: I haz the power, 2017-05-19 18:09:13]

Dry run: CQ is trying da patch.

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

[sadrul, 2017-05-19 18:16:26]

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

[commit-bot: I haz the power, 2017-05-19 18:17:03]

Dry run: CQ is trying da patch.

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

[sadrul, 2017-05-19 18:17:35]

I have updated this to be on ToT, so that we can just directly do 'dispatch at
regular intervals'. PTAL.

[dtapuska, 2017-05-19 18:32:27]

https://codereview.chromium.org/2886263002/diff/100001/content/browser/render...
File content/browser/renderer_host/input/synthetic_gesture_controller.cc
(right):

https://codereview.chromium.org/2886263002/diff/100001/content/browser/render...
content/browser/renderer_host/input/synthetic_gesture_controller.cc:52:
constexpr base::TimeDelta kSynthesizedDispatchDelay =
I think we need an explanation around why 2 is a *good* time. (Basically
identifying we felt 2 was good there is not a lot of experimentation that went
into it).

https://codereview.chromium.org/2886263002/diff/100001/content/browser/render...
content/browser/renderer_host/input/synthetic_gesture_controller.cc:85:
RequestBeginFrameIfNecessary();
Could this be a DCHECK(dispatch_timer_.IsRunning()) instead?

https://codereview.chromium.org/2886263002/diff/100001/content/browser/render...
content/browser/renderer_host/input/synthetic_gesture_controller.cc:92:
RequestBeginFrameIfNecessary();
same here.

[sadrul, 2017-05-19 18:46:03]

https://codereview.chromium.org/2886263002/diff/100001/content/browser/render...
File content/browser/renderer_host/input/synthetic_gesture_controller.cc
(right):

https://codereview.chromium.org/2886263002/diff/100001/content/browser/render...
content/browser/renderer_host/input/synthetic_gesture_controller.cc:52:
constexpr base::TimeDelta kSynthesizedDispatchDelay =
On 2017/05/19 18:32:27, dtapuska wrote:
> I think we need an explanation around why 2 is a *good* time. (Basically
> identifying we felt 2 was good there is not a lot of experimentation that went
> into it).

Added a note. PTAL.

https://codereview.chromium.org/2886263002/diff/100001/content/browser/render...
content/browser/renderer_host/input/synthetic_gesture_controller.cc:85:
RequestBeginFrameIfNecessary();
On 2017/05/19 18:32:27, dtapuska wrote:
> Could this be a DCHECK(dispatch_timer_.IsRunning()) instead?

I had a DCHECK() at the beginning of the function here. But the unit-tests
tripped on it (because they directly dispatch the events [1]). I can add the
DCHECK() back and update the test instead though.

[1]
https://cs.chromium.org/chromium/src/content/browser/renderer_host/input/synt...

[sadrul, 2017-05-19 19:01:04]

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

[dtapuska, 2017-05-19 19:01:56]

On 2017/05/19 18:46:03, sadrul wrote:
>
https://codereview.chromium.org/2886263002/diff/100001/content/browser/render...
> File content/browser/renderer_host/input/synthetic_gesture_controller.cc
> (right):
> 
>
https://codereview.chromium.org/2886263002/diff/100001/content/browser/render...
> content/browser/renderer_host/input/synthetic_gesture_controller.cc:52:
> constexpr base::TimeDelta kSynthesizedDispatchDelay =
> On 2017/05/19 18:32:27, dtapuska wrote:
> > I think we need an explanation around why 2 is a *good* time. (Basically
> > identifying we felt 2 was good there is not a lot of experimentation that
went
> > into it).
> 
> Added a note. PTAL.
> 
>
https://codereview.chromium.org/2886263002/diff/100001/content/browser/render...
> content/browser/renderer_host/input/synthetic_gesture_controller.cc:85:
> RequestBeginFrameIfNecessary();
> On 2017/05/19 18:32:27, dtapuska wrote:
> > Could this be a DCHECK(dispatch_timer_.IsRunning()) instead?
> 
> I had a DCHECK() at the beginning of the function here. But the unit-tests
> tripped on it (because they directly dispatch the events [1]). I can add the
> DCHECK() back and update the test instead though.
> 
> [1]
>
https://cs.chromium.org/chromium/src/content/browser/renderer_host/input/synt...

lgtm

[sadrul, 2017-05-19 19:02:49]

https://codereview.chromium.org/2886263002/diff/100001/content/browser/render...
File content/browser/renderer_host/input/synthetic_gesture_controller.cc
(right):

https://codereview.chromium.org/2886263002/diff/100001/content/browser/render...
content/browser/renderer_host/input/synthetic_gesture_controller.cc:85:
RequestBeginFrameIfNecessary();
On 2017/05/19 18:46:03, sadrul wrote:
> On 2017/05/19 18:32:27, dtapuska wrote:
> > Could this be a DCHECK(dispatch_timer_.IsRunning()) instead?
> 
> I had a DCHECK() at the beginning of the function here. But the unit-tests
> tripped on it (because they directly dispatch the events [1]). I can add the
> DCHECK() back and update the test instead though.
> 
> [1]
>
https://cs.chromium.org/chromium/src/content/browser/renderer_host/input/synt...

I have gone ahead and added the DCHECK() back, and updated the test to have a
ScopedTaskEnvironment, and start/stop the timer in the test.

[commit-bot: I haz the power, 2017-05-19 19:02:51]

Dry run: CQ is trying da patch.

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

[dtapuska, 2017-05-19 19:05:47]

On 2017/05/19 19:02:49, sadrul wrote:
>
https://codereview.chromium.org/2886263002/diff/100001/content/browser/render...
> File content/browser/renderer_host/input/synthetic_gesture_controller.cc
> (right):
> 
>
https://codereview.chromium.org/2886263002/diff/100001/content/browser/render...
> content/browser/renderer_host/input/synthetic_gesture_controller.cc:85:
> RequestBeginFrameIfNecessary();
> On 2017/05/19 18:46:03, sadrul wrote:
> > On 2017/05/19 18:32:27, dtapuska wrote:
> > > Could this be a DCHECK(dispatch_timer_.IsRunning()) instead?
> > 
> > I had a DCHECK() at the beginning of the function here. But the unit-tests
> > tripped on it (because they directly dispatch the events [1]). I can add the
> > DCHECK() back and update the test instead though.
> > 
> > [1]
> >
>
https://cs.chromium.org/chromium/src/content/browser/renderer_host/input/synt...
> 
> I have gone ahead and added the DCHECK() back, and updated the test to have a
> ScopedTaskEnvironment, and start/stop the timer in the test.

still lgtm. Thanks for adding the dcheck. I think this is better.

[dtapuska, 2017-05-19 19:05:48]

On 2017/05/19 19:02:49, sadrul wrote:
>
https://codereview.chromium.org/2886263002/diff/100001/content/browser/render...
> File content/browser/renderer_host/input/synthetic_gesture_controller.cc
> (right):
> 
>
https://codereview.chromium.org/2886263002/diff/100001/content/browser/render...
> content/browser/renderer_host/input/synthetic_gesture_controller.cc:85:
> RequestBeginFrameIfNecessary();
> On 2017/05/19 18:46:03, sadrul wrote:
> > On 2017/05/19 18:32:27, dtapuska wrote:
> > > Could this be a DCHECK(dispatch_timer_.IsRunning()) instead?
> > 
> > I had a DCHECK() at the beginning of the function here. But the unit-tests
> > tripped on it (because they directly dispatch the events [1]). I can add the
> > DCHECK() back and update the test instead though.
> > 
> > [1]
> >
>
https://cs.chromium.org/chromium/src/content/browser/renderer_host/input/synt...
> 
> I have gone ahead and added the DCHECK() back, and updated the test to have a
> ScopedTaskEnvironment, and start/stop the timer in the test.

still lgtm. Thanks for adding the dcheck. I think this is better.

[commit-bot: I haz the power, 2017-05-19 21:38:43]

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

[commit-bot: I haz the power, 2017-05-19 21:38:44]

Dry run: This issue passed the CQ dry run.

[sadrul, 2017-05-23 02:54:50]

The CQ bit was checked by sadrul@chromium.org

[commit-bot: I haz the power, 2017-05-23 02:55:09]

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, 2017-05-23 03:51:15]

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

[commit-bot: I haz the power, 2017-05-23 03:51:16]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[sadrul, 2017-05-23 15:08:31]

The CQ bit was checked by sadrul@chromium.org

[commit-bot: I haz the power, 2017-05-23 15:09:07]

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, 2017-05-23 16:06:33]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1495552111987240,
"parent_rev": "a0d07deeaa973b547aab5cc88918bb15432f093d", "commit_rev":
"a6251cb3214b42349e7208d285a5e2e01276fe6f"}

[commit-bot: I haz the power, 2017-05-23 16:06:53]

Description was changed from

==========
input: Dispatch synthesized events at regular intervals.

Instead of dispatching events at begin-frame + offset, dispatch events
at regular intervals (~16ms). The timer starts at an offset from the
first begin frame.

BUG=722921
==========

to

==========
input: Dispatch synthesized events at regular intervals.

Instead of dispatching events at begin-frame + offset, dispatch events
at regular intervals (~16ms). The timer starts at an offset from the
first begin frame.

BUG=722921

Review-Url: https://codereview.chromium.org/2886263002
Cr-Commit-Position: refs/heads/master@{#473922}
Committed:
https://chromium.googlesource.com/chromium/src/+/a6251cb3214b42349e7208d285a5...
==========

[commit-bot: I haz the power, 2017-05-23 16:06:54]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/a6251cb3214b42349e7208d285a5...