gpu benchmarking swipe for touchpad

gpu benchmarking swipe for touchpad

Swipe function for touchpad is needed to test handling a fling on the
compositor thread when touchpad scroll latching is enabled. A follow up
cl will contain layout tests that use swipe to do a touchpad fling that
starts on compositor and changes to main thread later.

BUG=526463

[sahel, 2017-03-08 20:51:12]

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

[commit-bot: I haz the power, 2017-03-08 20:51:51]

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

[sahel, 2017-03-08 21:05:08]

Patchset #1 (id:1) has been deleted

[sahel, 2017-03-08 21:05:52]

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

[commit-bot: I haz the power, 2017-03-08 21:06:13]

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-03-08 21:45:52]

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

[commit-bot: I haz the power, 2017-03-08 21:45:53]

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_...)

[sahel, 2017-03-09 22:04:40]

Description was changed from

==========
gpu benchmarking swipe for touchpad

Swipe function for touchpad is needed to test handling a fling on the
compositor thread when touchpad scroll latching is enabled. A follow up
cl will contain layout tests that use swipe to do a touchpad fling that
starts on compositor and changes to main thread later.

BUG=526463
==========

to

==========
gpu benchmarking swipe for touchpad

Swipe function for touchpad is needed to test handling a fling on the
compositor thread when touchpad scroll latching is enabled. A follow up
cl will contain layout tests that use swipe to do a touchpad fling that
starts on compositor and changes to main thread later.

BUG=526463
==========

[sahel, 2017-03-09 22:04:40]

sahel@chromium.org changed reviewers:
+ bokan@chromium.org, dtapuska@chromium.org, tdresser@chromium.org

[sahel, 2017-03-09 22:09:55]

Right now the try bots are failing:
telemetry.internal.actions.action_runner_unittest.ActionRunnerTest.testSwipe

I have changed swipe function from gpu benchmarking extension to support
touchpad fling as well. I have also fixed the inconsistency between scroll
directions in swipe and smoothscrolby. Now both functions scroll down with
direction = "down" and positive pixels to scroll, both for touch and wheel
(touchpad). I need to land a cl in catapult to fix the testswipe function.

Please review this patch while the tests are still failing, I have two have both
cls ready to submit to minimize the time that I need to disable the test.

[bokan, 2017-03-10 14:41:57]

https://codereview.chromium.org/2742473002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/input/synthetic_gesture_target_aura.cc
(right):

https://codereview.chromium.org/2742473002/diff/20001/content/browser/rendere...
content/browser/renderer_host/input/synthetic_gesture_target_aura.cc:73:
gfx::Point(), timestamp, ui::EF_NONE, ui::EF_NONE);
Is this change related?

https://codereview.chromium.org/2742473002/diff/20001/content/browser/rendere...
content/browser/renderer_host/input/synthetic_gesture_target_aura.cc:173: if
(web_gesture.type() != blink::WebInputEvent::GestureFlingStart)
The style guide prohibits handing a failed DCHECK:
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...

Remove either the DCHECKs or the |if|s

https://codereview.chromium.org/2742473002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/input/synthetic_gesture_target_base.cc
(right):

https://codereview.chromium.org/2742473002/diff/20001/content/browser/rendere...
content/browser/renderer_host/input/synthetic_gesture_target_base.cc:116:
host_->ForwardGestureEventWithLatencyInfo(web_gesture, latency_info);
This base implementation is meant to work on Mac? Doesn't Mac's event stream for
flings differ significantly enough that this won't work?

https://codereview.chromium.org/2742473002/diff/20001/content/renderer/gpu/gp...
File content/renderer/gpu/gpu_benchmarking_extension.cc (right):

https://codereview.chromium.org/2742473002/diff/20001/content/renderer/gpu/gp...
content/renderer/gpu/gpu_benchmarking_extension.cc:394: if (set_velocity &&
!velocity_x && !velocity_y)
Ditto here for the DCHECK and |if|

https://codereview.chromium.org/2742473002/diff/20001/content/renderer/gpu/gp...
content/renderer/gpu/gpu_benchmarking_extension.cc:400: if (direction == "down")
{
Question: do we really need this direction parameter? Couldn't we just agree on
a convention that positive distance/velocity means "direction that causes
scrolling downwards" and use a pair of floats to represent the distance?

We used to do something similar in EventHandler way back and we've converted to
just use this convention. It's much simpler. (Not necessarily requesting it for
this patch)

https://codereview.chromium.org/2742473002/diff/20001/content/renderer/gpu/gp...
File content/renderer/gpu/gpu_benchmarking_extension.h (right):

https://codereview.chromium.org/2742473002/diff/20001/content/renderer/gpu/gp...
content/renderer/gpu/gpu_benchmarking_extension.h:55: // threaded compositing is
enabled.
Is this because of a difference in how main thread flings are handled?

[sahel, 2017-03-10 19:42:45]

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

[commit-bot: I haz the power, 2017-03-10 19:43:30]

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-03-10 20:40:34]

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

[commit-bot: I haz the power, 2017-03-10 20:40:35]

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

[sahel, 2017-03-10 21:51:42]

https://codereview.chromium.org/2742473002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/input/synthetic_gesture_target_aura.cc
(right):

https://codereview.chromium.org/2742473002/diff/20001/content/browser/rendere...
content/browser/renderer_host/input/synthetic_gesture_target_aura.cc:73:
gfx::Point(), timestamp, ui::EF_NONE, ui::EF_NONE);
On 2017/03/10 14:41:57, bokan wrote:
> Is this change related?

Yes, it is. It used to use eventTimeForNow instead of the event timestamp.
Because SyntheticSmoothMoveGesture::ForwardMouseWheelInputEvents used to call
itself recursively with the same timestamp. The result was receiving synthetic
wheel events at the same time.

I had to make sure that wheel events are generated with different timestamps and
the fling is sent after mouse wheels (similar to touch). Because otherwise fling
would have been handled before GSB that was generated because of handling the
first wheel event.

https://codereview.chromium.org/2742473002/diff/20001/content/browser/rendere...
content/browser/renderer_host/input/synthetic_gesture_target_aura.cc:173: if
(web_gesture.type() != blink::WebInputEvent::GestureFlingStart)
On 2017/03/10 14:41:57, bokan wrote:
> The style guide prohibits handing a failed DCHECK:
>
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...
> 
> Remove either the DCHECKs or the |if|s

Done.

https://codereview.chromium.org/2742473002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/input/synthetic_gesture_target_base.cc
(right):

https://codereview.chromium.org/2742473002/diff/20001/content/browser/rendere...
content/browser/renderer_host/input/synthetic_gesture_target_base.cc:116:
host_->ForwardGestureEventWithLatencyInfo(web_gesture, latency_info);
On 2017/03/10 14:41:57, bokan wrote:
> This base implementation is meant to work on Mac? Doesn't Mac's event stream
for
> flings differ significantly enough that this won't work?

Swipe for touchpad sends a bunch of mouse wheel events followed by a fling
event. The only platform that has this event sequence is ChromeOS.
In aura implementation I generate a ui event from the fling gesture event.
But in the base implementation I just forward the GFS event itself.

Does it work on mac or not? yes it does, but it doesn't simulate the event
sequence on mac and it's not intended to do so. This is used for testing
parts of the code that handle touchpad fling.

https://codereview.chromium.org/2742473002/diff/20001/content/renderer/gpu/gp...
File content/renderer/gpu/gpu_benchmarking_extension.cc (right):

https://codereview.chromium.org/2742473002/diff/20001/content/renderer/gpu/gp...
content/renderer/gpu/gpu_benchmarking_extension.cc:394: if (set_velocity &&
!velocity_x && !velocity_y)
On 2017/03/10 14:41:57, bokan wrote:
> Ditto here for the DCHECK and |if|

Done.

https://codereview.chromium.org/2742473002/diff/20001/content/renderer/gpu/gp...
content/renderer/gpu/gpu_benchmarking_extension.cc:400: if (direction == "down")
{
On 2017/03/10 14:41:57, bokan wrote:
> Question: do we really need this direction parameter? Couldn't we just agree
on
> a convention that positive distance/velocity means "direction that causes
> scrolling downwards" and use a pair of floats to represent the distance?
> 
> We used to do something similar in EventHandler way back and we've converted
to
> just use this convention. It's much simpler. (Not necessarily requesting it
for
> this patch)

I am not sure why we have it here, but one reason can be for scrolling both
horizontally and vertically. Instead of having deltas in x and y direction, here
we have pixels to scroll and direction.

https://codereview.chromium.org/2742473002/diff/20001/content/renderer/gpu/gp...
File content/renderer/gpu/gpu_benchmarking_extension.h (right):

https://codereview.chromium.org/2742473002/diff/20001/content/renderer/gpu/gp...
content/renderer/gpu/gpu_benchmarking_extension.h:55: // threaded compositing is
enabled.
On 2017/03/10 14:41:57, bokan wrote:
> Is this because of a difference in how main thread flings are handled?

I tried to debug it to see what the cause is. It seems like when a GFS is
handled by main thread, the call back function of swipe is never called.

One can still use swipe for fling on main thread, but there shouldn't be
any dependency to the call back function e.g. if you check scrollTop
in the callback function recursively and decide to finish the test based
on its value, the test will timeout because the callback function is never
called.

I changed the comment to make it more precise.

[bokan, 2017-03-13 13:43:13]

Thanks, non-OWNER lgtm

https://codereview.chromium.org/2742473002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/input/synthetic_gesture_target_aura.cc
(right):

https://codereview.chromium.org/2742473002/diff/20001/content/browser/rendere...
content/browser/renderer_host/input/synthetic_gesture_target_aura.cc:73:
gfx::Point(), timestamp, ui::EF_NONE, ui::EF_NONE);
On 2017/03/10 21:51:41, sahel wrote:
> On 2017/03/10 14:41:57, bokan wrote:
> > Is this change related?
> 
> Yes, it is. It used to use eventTimeForNow instead of the event timestamp.
> Because SyntheticSmoothMoveGesture::ForwardMouseWheelInputEvents used to call
> itself recursively with the same timestamp. The result was receiving synthetic
> wheel events at the same time.
> 
> I had to make sure that wheel events are generated with different timestamps
and
> the fling is sent after mouse wheels (similar to touch). Because otherwise
fling
> would have been handled before GSB that was generated because of handling the
> first wheel event.

Acknowledged.

https://codereview.chromium.org/2742473002/diff/20001/content/renderer/gpu/gp...
File content/renderer/gpu/gpu_benchmarking_extension.h (right):

https://codereview.chromium.org/2742473002/diff/20001/content/renderer/gpu/gp...
content/renderer/gpu/gpu_benchmarking_extension.h:55: // threaded compositing is
enabled.
On 2017/03/10 21:51:41, sahel wrote:
> On 2017/03/10 14:41:57, bokan wrote:
> > Is this because of a difference in how main thread flings are handled?
> 
> I tried to debug it to see what the cause is. It seems like when a GFS is
> handled by main thread, the call back function of swipe is never called.
> 
> One can still use swipe for fling on main thread, but there shouldn't be
> any dependency to the call back function e.g. if you check scrollTop
> in the callback function recursively and decide to finish the test based
> on its value, the test will timeout because the callback function is never
> called.
> 
> I changed the comment to make it more precise. 

Acknowledged.

[sahel, 2017-03-15 20:21:53]

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

[commit-bot: I haz the power, 2017-03-15 20:22:37]

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-03-15 21:27:36]

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

[commit-bot: I haz the power, 2017-03-15 21:27:37]

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_...)

[sahel, 2017-03-21 15:01:04]

sahel@chromium.org changed reviewers:
+ jbauman@chromium.org

[sahel, 2017-03-21 15:01:04]

jbauman@chromium.org: Please review changes in 
content/renderer/gpu/*

dcheng@chromium.org: Please review changes in
content/common/input/input_param_traits_unittest.cc
content/common/input_messages.h

Please note that the try bots are failing now, because of
telemetry.internal.actions.action_runner_unittest.ActionRunnerTest.testSwipe
test. I have a cl that changes this test.
:https://codereview.chromium.org/2754203005/
I want to have both cls ready for submitting to minimize the time that I the
test is disabled.

[jbauman, 2017-03-21 19:19:35]

lgtm

[tdresser, 2017-03-21 19:20:03]

https://codereview.chromium.org/2742473002/diff/60001/content/browser/rendere...
File content/browser/renderer_host/input/synthetic_gesture_target_aura.cc
(right):

https://codereview.chromium.org/2742473002/diff/60001/content/browser/rendere...
content/browser/renderer_host/input/synthetic_gesture_target_aura.cc:168: void
SyntheticGestureTargetAura::DispatchTouchpadGestureFlingStartToPlatform(
Comment that this aligns with the Chrome OS model of touchpad fling.

https://codereview.chromium.org/2742473002/diff/60001/content/renderer/gpu/gp...
File content/renderer/gpu/gpu_benchmarking_extension.cc (right):

https://codereview.chromium.org/2742473002/diff/60001/content/renderer/gpu/gp...
content/renderer/gpu/gpu_benchmarking_extension.cc:394: return false;
Why return false here? We could interpret 0 velocity as just not triggering a
fling, couldn't we?

https://codereview.chromium.org/2742473002/diff/60001/content/renderer/gpu/gp...
File content/renderer/gpu/gpu_benchmarking_extension.h (right):

https://codereview.chromium.org/2742473002/diff/60001/content/renderer/gpu/gp...
content/renderer/gpu/gpu_benchmarking_extension.h:56: // thread.
Why will the callback only be called when the GFS is handled on the compositor?

[sahel, 2017-03-21 21:08:44]

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

[commit-bot: I haz the power, 2017-03-21 21:09:34]

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-03-21 22:09:54]

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

[commit-bot: I haz the power, 2017-03-21 22:09:55]

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_...)

[sahel, 2017-03-22 14:43:25]

https://codereview.chromium.org/2742473002/diff/60001/content/browser/rendere...
File content/browser/renderer_host/input/synthetic_gesture_target_aura.cc
(right):

https://codereview.chromium.org/2742473002/diff/60001/content/browser/rendere...
content/browser/renderer_host/input/synthetic_gesture_target_aura.cc:168: void
SyntheticGestureTargetAura::DispatchTouchpadGestureFlingStartToPlatform(
On 2017/03/21 19:20:02, tdresser wrote:
> Comment that this aligns with the Chrome OS model of touchpad fling.

Done.

https://codereview.chromium.org/2742473002/diff/60001/content/renderer/gpu/gp...
File content/renderer/gpu/gpu_benchmarking_extension.cc (right):

https://codereview.chromium.org/2742473002/diff/60001/content/renderer/gpu/gp...
content/renderer/gpu/gpu_benchmarking_extension.cc:394: return false;
On 2017/03/21 19:20:02, tdresser wrote:
> Why return false here? We could interpret 0 velocity as just not triggering a
> fling, couldn't we?

If prevent_fling is false we must trigger a fling.

https://codereview.chromium.org/2742473002/diff/60001/content/renderer/gpu/gp...
File content/renderer/gpu/gpu_benchmarking_extension.h (right):

https://codereview.chromium.org/2742473002/diff/60001/content/renderer/gpu/gp...
content/renderer/gpu/gpu_benchmarking_extension.h:56: // thread.
On 2017/03/21 19:20:02, tdresser wrote:
> Why will the callback only be called when the GFS is handled on the
compositor?

I traced back the messages. When the compositor handles the fling, a
notification (DidStopFlinging) calls
RenderWidgetHostImpl::OnSyntheticGestureCompleted(). This function in turn
causes the callback to get executed.

On the other hand when the fling is handled by main thread, there is some
plumbing to notify layer_tree_host_impl that the fling on main is over. But it's
not plumbed to call OnSyntheticGestureCompleted().

[tdresser, 2017-03-22 15:21:29]

https://codereview.chromium.org/2742473002/diff/60001/content/renderer/gpu/gp...
File content/renderer/gpu/gpu_benchmarking_extension.cc (right):

https://codereview.chromium.org/2742473002/diff/60001/content/renderer/gpu/gp...
content/renderer/gpu/gpu_benchmarking_extension.cc:394: return false;
On 2017/03/22 14:43:25, sahel wrote:
> On 2017/03/21 19:20:02, tdresser wrote:
> > Why return false here? We could interpret 0 velocity as just not triggering
a
> > fling, couldn't we?
> 
> If prevent_fling is false we must trigger a fling.

Could we always trigger a fling unless the velocity is 0, and get rid of the
prevent_fling flag?

https://codereview.chromium.org/2742473002/diff/60001/content/renderer/gpu/gp...
File content/renderer/gpu/gpu_benchmarking_extension.h (right):

https://codereview.chromium.org/2742473002/diff/60001/content/renderer/gpu/gp...
content/renderer/gpu/gpu_benchmarking_extension.h:56: // thread.
On 2017/03/22 14:43:25, sahel wrote:
> On 2017/03/21 19:20:02, tdresser wrote:
> > Why will the callback only be called when the GFS is handled on the
> compositor?
> 
> I traced back the messages. When the compositor handles the fling, a
> notification (DidStopFlinging) calls
> RenderWidgetHostImpl::OnSyntheticGestureCompleted(). This function in turn
> causes the callback to get executed.
> 
> On the other hand when the fling is handled by main thread, there is some
> plumbing to notify layer_tree_host_impl that the fling on main is over. But
it's
> not plumbed to call OnSyntheticGestureCompleted().

Yikes. Can you file a bug for this? Maybe assign it to Lan?

[sahel, 2017-03-22 16:04:04]

https://codereview.chromium.org/2742473002/diff/60001/content/renderer/gpu/gp...
File content/renderer/gpu/gpu_benchmarking_extension.cc (right):

https://codereview.chromium.org/2742473002/diff/60001/content/renderer/gpu/gp...
content/renderer/gpu/gpu_benchmarking_extension.cc:394: return false;
On 2017/03/22 15:21:29, tdresser wrote:
> On 2017/03/22 14:43:25, sahel wrote:
> > On 2017/03/21 19:20:02, tdresser wrote:
> > > Why return false here? We could interpret 0 velocity as just not
triggering
> a
> > > fling, couldn't we?
> > 
> > If prevent_fling is false we must trigger a fling.
> 
> Could we always trigger a fling unless the velocity is 0, and get rid of the
> prevent_fling flag?

BeginSmoothScroll is called both with smoothScrollBy and swipe but the first one
calls it with prevent_fling=true and the second one with prevent_fling=false.

https://codereview.chromium.org/2742473002/diff/60001/content/renderer/gpu/gp...
File content/renderer/gpu/gpu_benchmarking_extension.h (right):

https://codereview.chromium.org/2742473002/diff/60001/content/renderer/gpu/gp...
content/renderer/gpu/gpu_benchmarking_extension.h:56: // thread.
On 2017/03/22 15:21:29, tdresser wrote:
> On 2017/03/22 14:43:25, sahel wrote:
> > On 2017/03/21 19:20:02, tdresser wrote:
> > > Why will the callback only be called when the GFS is handled on the
> > compositor?
> > 
> > I traced back the messages. When the compositor handles the fling, a
> > notification (DidStopFlinging) calls
> > RenderWidgetHostImpl::OnSyntheticGestureCompleted(). This function in turn
> > causes the callback to get executed.
> > 
> > On the other hand when the fling is handled by main thread, there is some
> > plumbing to notify layer_tree_host_impl that the fling on main is over. But
> it's
> > not plumbed to call OnSyntheticGestureCompleted().
> 
> Yikes. Can you file a bug for this? Maybe assign it to Lan?

Done.

[tdresser, 2017-03-22 19:44:02]

LGTM.

https://codereview.chromium.org/2742473002/diff/80001/content/renderer/gpu/gp...
File content/renderer/gpu/gpu_benchmarking_extension.cc (right):

https://codereview.chromium.org/2742473002/diff/80001/content/renderer/gpu/gp...
content/renderer/gpu/gpu_benchmarking_extension.cc:436: gesture_params->velocity
= velocity;
Instead of all the ternary operators above, could you just make executing this
conditional on set_velocity?

https://codereview.chromium.org/2742473002/diff/80001/content/renderer/gpu/gp...
File content/renderer/gpu/gpu_benchmarking_extension.h (right):

https://codereview.chromium.org/2742473002/diff/80001/content/renderer/gpu/gp...
content/renderer/gpu/gpu_benchmarking_extension.h:56: // thread.
Mention the bug here.

[sahel, 2017-03-22 20:16:42]

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

[commit-bot: I haz the power, 2017-03-22 20:17:46]

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-03-22 21:26:30]

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

[commit-bot: I haz the power, 2017-03-22 21:26:31]

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

[sahel, 2017-03-22 21:30:28]

mkwst@chromium.org please review changes at:
content/common/input/input_param_traits_unittest.cc
content/common/input_messages.h

Please note that the try bots are failing now, because of
telemetry.internal.actions.action_runner_unittest.ActionRunnerTest.testSwipe
test. I have a cl that changes this test.
:https://codereview.chromium.org/2754203005/
I want to have both cls ready for submitting to minimize the time that I the
test is disabled.

https://codereview.chromium.org/2742473002/diff/80001/content/renderer/gpu/gp...
File content/renderer/gpu/gpu_benchmarking_extension.cc (right):

https://codereview.chromium.org/2742473002/diff/80001/content/renderer/gpu/gp...
content/renderer/gpu/gpu_benchmarking_extension.cc:436: gesture_params->velocity
= velocity;
On 2017/03/22 19:44:02, tdresser wrote:
> Instead of all the ternary operators above, could you just make executing this
> conditional on set_velocity?

Done.

https://codereview.chromium.org/2742473002/diff/80001/content/renderer/gpu/gp...
File content/renderer/gpu/gpu_benchmarking_extension.h (right):

https://codereview.chromium.org/2742473002/diff/80001/content/renderer/gpu/gp...
content/renderer/gpu/gpu_benchmarking_extension.h:56: // thread.
On 2017/03/22 19:44:02, tdresser wrote:
> Mention the bug here.

Done.

[tdresser, 2017-03-23 15:47:33]

https://codereview.chromium.org/2742473002/diff/100001/content/renderer/gpu/g...
File content/renderer/gpu/gpu_benchmarking_extension.cc (right):

https://codereview.chromium.org/2742473002/diff/100001/content/renderer/gpu/g...
content/renderer/gpu/gpu_benchmarking_extension.cc:437: if (set_velocity)
I suppose we don't even need the condition here, seeing as velocity will be 0,0
if set_velocity is false.

[sahel, 2017-03-24 18:46:31]

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

[commit-bot: I haz the power, 2017-03-24 18:47:19]

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-03-24 19:05:42]

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

[commit-bot: I haz the power, 2017-03-24 19:05:43]

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

[sahel, 2017-03-24 19:14:06]

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

[commit-bot: I haz the power, 2017-03-24 19:14:54]

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

[sahel, 2017-03-24 20:18:37]

sahel@chromium.org changed reviewers:
+ tsepez@chromium.org

[sahel, 2017-03-24 20:21:57]

tsepez@chromium.org please review the following files:
content/common/input_messages.h
content/common/input/input_param_traits_unittest.cc

Please note that the try bots are failing now, because of
telemetry.internal.actions.action_runner_unittest.ActionRunnerTest.testSwipe
test. I have a cl that changes this test.
:https://codereview.chromium.org/2754203005/
I want to have both cls ready for submitting to minimize the time that I the
test is disabled

https://codereview.chromium.org/2742473002/diff/100001/content/renderer/gpu/g...
File content/renderer/gpu/gpu_benchmarking_extension.cc (right):

https://codereview.chromium.org/2742473002/diff/100001/content/renderer/gpu/g...
content/renderer/gpu/gpu_benchmarking_extension.cc:437: if (set_velocity)
On 2017/03/23 15:47:33, tdresser wrote:
> I suppose we don't even need the condition here, seeing as velocity will be
0,0
> if set_velocity is false.

Done.

[commit-bot: I haz the power, 2017-03-24 20:24:59]

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

[commit-bot: I haz the power, 2017-03-24 20:25:00]

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

[Tom Sepez, 2017-03-24 20:27:53]

lgtm

[sahel, 2017-04-25 16:06:50]

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

[commit-bot: I haz the power, 2017-04-25 16:07:21]

Dry run: CQ is trying da patch.

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

[Tom Sepez, 2017-04-25 17:03:36]

messages LGTM

[commit-bot: I haz the power, 2017-04-25 17:38:36]

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

[commit-bot: I haz the power, 2017-04-25 17:38:38]

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

[sahel, 2017-04-25 18:05:22]

sahel@chromium.org changed reviewers:
+ nednguyen@google.com

[sahel, 2017-04-25 18:07:18]

@nednguyen
Please review changes in:
tools/perf/page_sets/*

[nednguyen, 2017-04-25 23:32:59]

On 2017/04/25 18:07:18, sahel wrote:
> @nednguyen
> Please review changes in:
> tools/perf/page_sets/*

Can you explain why you reverse the direction of those gesture?

[sahel, 2017-04-26 19:47:13]

On 2017/04/25 23:32:59, nednguyen wrote:
> On 2017/04/25 18:07:18, sahel wrote:
> > @nednguyen
> > Please review changes in:
> > tools/perf/page_sets/*
> 
> Can you explain why you reverse the direction of those gesture?

To make Swipe consistent with SmoothScrollBy I had to reverse the labels, now
both functions use direction='down/right' with positive distance to increase
scrollTop/Left.
The default behavior of the Swipe is not changed (It increases scrollLeft by
default), but the label for that is right rather than left.
To preserve the same behavior in tests, I had to change the direction labels.