Track plugin input event latency

Track plugin input event latency

This CL uses LatencyInfo to track the plugin input event latency.

1.Each plugin input event has an associated LatencyInfo which includes
some important components from its according ui::Event/WebInputEvent's
LatencyInfo. To do that, during the scope of
RenderWidget::OnHandleInputEvent(WebInputEvent, latency_info)
we first cache the WebInputEvent's latency_info, then if the input
event needs to be routed to plugin, we copy the important components
from the cached latency_info into the plugin input event's LatencyInfo.

2.A private API InputEventPrivate::TraceInputLatency(bool has_damage) is exposed.

3.If the event is believed to cause rendering damage, private_event.TraceInputLatency(true)
can be called, and the input event's LatencyInfo will be sent to renderer with next
plugin frame and be tracked until it reaches screen.
If the event is believed to not cause any rendering damage,
private_event.TraceInputLatency(false) can be called, and the LatencyInfo tracking ends
right at the call.

BUG=355719
TEST=with custom test touch drawing plugin, input-to-swapbuffer latency
     is shown correctly in trace viewer.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=272990

[piman, 2014-04-25 01:59:43]

https://codereview.chromium.org/252663002/diff/20001/content/renderer/pepper/...
File content/renderer/pepper/pepper_plugin_instance_impl.cc (right):

https://codereview.chromium.org/252663002/diff/20001/content/renderer/pepper/...
content/renderer/pepper/pepper_plugin_instance_impl.cc:1102:
render_frame_->GetRenderWidget()->current_event_latency_info();
In the future (e.g. after the blink merge), I'm hoping the LatencyInfo can stay
together with the WebInputEvent as it flows through webkit. This is a bit of a
hack for now...

https://codereview.chromium.org/252663002/diff/20001/content/renderer/pepper/...
content/renderer/pepper/pepper_plugin_instance_impl.cc:2005: cc::LayerTreeHost*
layer_tree_host = texture_layer_->layer_tree_host();
I'd rather we went to the compositor via the RenderWidget, otherwise this is
racy.
When newly created, the TextureLayer won't be added to the tree until the next
layout, meaning its LTH will be NULL.

Maybe ideally, we'd go through
RenderWidgetCompositor::CreateLatencyInfoSwapPromiseMonitor like
RenderWidget::OnHandleInputEvent

https://codereview.chromium.org/252663002/diff/20001/content/renderer/render_...
File content/renderer/render_widget.cc (right):

https://codereview.chromium.org/252663002/diff/20001/content/renderer/render_...
content/renderer/render_widget.cc:936: current_event_latency_info_ =
&latency_info;
I fear potential early outs (some could be added later). This function is huge.
Can you use an AutoReset<LatencyInfo> for RAII goodness?

https://codereview.chromium.org/252663002/diff/20001/ppapi/proxy/plugin_globa...
File ppapi/proxy/plugin_globals.h (right):

https://codereview.chromium.org/252663002/diff/20001/ppapi/proxy/plugin_globa...
ppapi/proxy/plugin_globals.h:141: void AddLatencyInfo(ui::LatencyInfo
latency_info, PP_Instance instance);
How about stashing all this state/logic on the InstanceData instead?

https://codereview.chromium.org/252663002/diff/20001/ppapi/proxy/ppapi_messag...
File ppapi/proxy/ppapi_messages.h (right):

https://codereview.chromium.org/252663002/diff/20001/ppapi/proxy/ppapi_messag...
ppapi/proxy/ppapi_messages.h:130: // We can eliminate the duplication if we have
a content_common_nacl.gyp build.
Can we just move the definition into some, say, ui_param_traits.h?

[sadrul, 2014-04-25 12:09:12]

https://codereview.chromium.org/252663002/diff/20001/ui/events/latency_info.h
File ui/events/latency_info.h (right):

https://codereview.chromium.org/252663002/diff/20001/ui/events/latency_info.h...
ui/events/latency_info.h:73: LATENCY_COMPONENT_TYPE_LAST =
LATENCY_INFO_LIST_TERMINATED_OVERFLOW_COMPONENT
You need to update this.

[Yufeng Shen (Slow to review), 2014-04-25 20:42:41]

I moved all the ppapi/ side latency monitoring code to 
the host side so it is much cleaner.

PTAL.

https://codereview.chromium.org/252663002/diff/20001/content/renderer/pepper/...
File content/renderer/pepper/pepper_plugin_instance_impl.cc (right):

https://codereview.chromium.org/252663002/diff/20001/content/renderer/pepper/...
content/renderer/pepper/pepper_plugin_instance_impl.cc:1102:
render_frame_->GetRenderWidget()->current_event_latency_info();
On 2014/04/25 01:59:43, piman wrote:
> In the future (e.g. after the blink merge), I'm hoping the LatencyInfo can
stay
> together with the WebInputEvent as it flows through webkit. This is a bit of a
> hack for now...

Right. I always want to couple WebInputEvent with LatencyInfo so we have more
latency coverage along the input path. It's just this requires more plumbing
rework and need a stronger use case to convince the blink reviewer. That's why I
take this hacky solution in this CL.

https://codereview.chromium.org/252663002/diff/20001/content/renderer/pepper/...
content/renderer/pepper/pepper_plugin_instance_impl.cc:2005: cc::LayerTreeHost*
layer_tree_host = texture_layer_->layer_tree_host();
On 2014/04/25 01:59:43, piman wrote:
> I'd rather we went to the compositor via the RenderWidget, otherwise this is
> racy.
> When newly created, the TextureLayer won't be added to the tree until the next
> layout, meaning its LTH will be NULL.

done.
It now goes through render_frame_->GetRenderWidget()->compositor()

> 
> Maybe ideally, we'd go through
> RenderWidgetCompositor::CreateLatencyInfoSwapPromiseMonitor like
> RenderWidget::OnHandleInputEvent

https://codereview.chromium.org/252663002/diff/20001/content/renderer/render_...
File content/renderer/render_widget.cc (right):

https://codereview.chromium.org/252663002/diff/20001/content/renderer/render_...
content/renderer/render_widget.cc:936: current_event_latency_info_ =
&latency_info;
On 2014/04/25 01:59:43, piman wrote:
> I fear potential early outs (some could be added later). This function is
huge.
> Can you use an AutoReset<LatencyInfo> for RAII goodness?

Done.

https://codereview.chromium.org/252663002/diff/20001/ppapi/proxy/plugin_globa...
File ppapi/proxy/plugin_globals.h (right):

https://codereview.chromium.org/252663002/diff/20001/ppapi/proxy/plugin_globa...
ppapi/proxy/plugin_globals.h:141: void AddLatencyInfo(ui::LatencyInfo
latency_info, PP_Instance instance);
On 2014/04/25 01:59:43, piman wrote:
> How about stashing all this state/logic on the InstanceData instead?

All the logic/data are now moved to the host side.

https://codereview.chromium.org/252663002/diff/20001/ui/events/latency_info.h
File ui/events/latency_info.h (right):

https://codereview.chromium.org/252663002/diff/20001/ui/events/latency_info.h...
ui/events/latency_info.h:73: LATENCY_COMPONENT_TYPE_LAST =
LATENCY_INFO_LIST_TERMINATED_OVERFLOW_COMPONENT
On 2014/04/25 12:09:12, sadrul wrote:
> You need to update this.

Done.

[Yufeng Shen (Slow to review), 2014-04-25 20:44:34]

On 2014/04/25 20:42:41, Yufeng Shen wrote:
> I moved all the ppapi/ side latency monitoring code to 
> the host side so it is much cleaner.
> 
> PTAL.
> 

Also I am only tracking 2D damage for now. Logic for handling
3D damage will be in a separate CL once I get more familiar
with how that works.

[piman, 2014-04-25 20:58:05]

I don't think doing it on the host side will give you the right results.

Since the renderer and the plugin are asynchronous wrt each other, the input
event message and the invalidation messages (from a previous event, or some
other behavior) can cross each other, and so you would pick the newer latency
info for an invalidation from an older event.

[chromium-reviews, 2014-04-25 21:08:04]

Do we have the same problem even if we do the tracking in plugin side that
we
can't be sure if the invalidation message is from the current input event
or a
previous input event or some other behavior ? Do you think is there a better
way to track the casualty ?


On Fri, Apr 25, 2014 at 4:58 PM, <piman@chromium.org> wrote:

> I don't think doing it on the host side will give you the right results.
>
> Since the renderer and the plugin are asynchronous wrt each other, the
> input
> event message and the invalidation messages (from a previous event, or some
> other behavior) can cross each other, and so you would pick the newer
> latency
> info for an invalidation from an older event.
>
> https://codereview.chromium.org/252663002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2014-04-25 21:46:28]

A more accurate way I can think of is to expose an API that the plugin code
can use to indicate if a event has caused invalidation. Then it is the
plugin writer's
responsibility to make sure the API is called in the event handling code,
if the
plugin writer wants the input latency to be tracked.

This way the usability of the tracking system is reduced. But since our goal
was to have a baseline plugin input latency (so we will write the baseline
plugin
ourselves) and compare the latency with other plugins which are also
written by
us, we have the full control of adding extra instrumenting code to the
plugin's event
handling code, this maybe a good solution for us.

What do you think ?


On Fri, Apr 25, 2014 at 5:08 PM, Yufeng Shen <miletus@google.com> wrote:

> Do we have the same problem even if we do the tracking in plugin side that
> we
> can't be sure if the invalidation message is from the current input event
> or a
> previous input event or some other behavior ? Do you think is there a
> better
> way to track the casualty ?
>
>
>
> On Fri, Apr 25, 2014 at 4:58 PM, <piman@chromium.org> wrote:
>
>> I don't think doing it on the host side will give you the right results.
>>
>> Since the renderer and the plugin are asynchronous wrt each other, the
>> input
>> event message and the invalidation messages (from a previous event, or
>> some
>> other behavior) can cross each other, and so you would pick the newer
>> latency
>> info for an invalidation from an older event.
>>
>> https://codereview.chromium.org/252663002/
>>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dmichael (off chromium), 2014-04-25 22:24:15]

On Fri, Apr 25, 2014 at 3:46 PM, 'Yufeng Shen <miletus@google.com>' via
Chromium-reviews <chromium-reviews@chromium.org> wrote:

> A more accurate way I can think of is to expose an API that the plugin code
> can use to indicate if a event has caused invalidation. Then it is the
> plugin writer's
> responsibility to make sure the API is called in the event handling code,
> if the
> plugin writer wants the input latency to be tracked.
>
> This way the usability of the tracking system is reduced. But since our
> goal
> was to have a baseline plugin input latency (so we will write the baseline
> plugin
> ourselves) and compare the latency with other plugins which are also
> written by
> us, we have the full control of adding extra instrumenting code to the
> plugin's event
> handling code, this maybe a good solution for us.
>
> What do you think ?
>
If that would solve your immediate need, that would give you the most
accurate data. You could make an api in ppapi/api/private for it.


>
>
> On Fri, Apr 25, 2014 at 5:08 PM, Yufeng Shen <miletus@google.com> wrote:
>
>> Do we have the same problem even if we do the tracking in plugin side
>> that we
>> can't be sure if the invalidation message is from the current input event
>> or a
>> previous input event or some other behavior ? Do you think is there a
>> better
>> way to track the casualty ?
>>
> The problem is definitely less severe if you do it in the plugin.

In the renderer, you don't actually know if the plugin has even received
the last input event you sent when you receive a message to invalidate.

*If* we measure for general plugins (I could imagine tracking something in
UMA, for example), we should probably do it in the plugin. I don't know
whether that's something we should spend much time on right now, though.


>
>>
>>
>> On Fri, Apr 25, 2014 at 4:58 PM, <piman@chromium.org> wrote:
>>
>>> I don't think doing it on the host side will give you the right results.
>>>
>>> Since the renderer and the plugin are asynchronous wrt each other, the
>>> input
>>> event message and the invalidation messages (from a previous event, or
>>> some
>>> other behavior) can cross each other, and so you would pick the newer
>>> latency
>>> info for an invalidation from an older event.
>>>
>>> https://codereview.chromium.org/252663002/
>>>
>>
>>
>  To unsubscribe from this group and stop receiving emails from it, send an
> email to chromium-reviews+unsubscribe@chromium.org.
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[piman, 2014-04-25 22:25:45]

On Fri, Apr 25, 2014 at 2:46 PM, Yufeng Shen <miletus@google.com> wrote:

> A more accurate way I can think of is to expose an API that the plugin code
> can use to indicate if a event has caused invalidation. Then it is the
> plugin writer's
> responsibility to make sure the API is called in the event handling code,
> if the
> plugin writer wants the input latency to be tracked.
>

Yes, I think that's the only way you'll get truly accurate data.


>
> This way the usability of the tracking system is reduced. But since our
> goal
> was to have a baseline plugin input latency (so we will write the baseline
> plugin
> ourselves) and compare the latency with other plugins which are also
> written by
> us, we have the full control of adding extra instrumenting code to the
> plugin's event
> handling code, this maybe a good solution for us.
>
> What do you think ?
>

I think from the plugin side, the common cases are:
1- plugin gets event, kicks an invalidation/draw directly from the handler
(modulo throttling / callbacks)
2- plugin gets event, handles it but doesn't draw yet, instead kicks a
timer (or waits for the previous frame's callback).

In either of those 2 cases, the code you had in the previous patch would
give you the right result, so I think it's not a bad thing to have as a
baseline.



>
>
> On Fri, Apr 25, 2014 at 5:08 PM, Yufeng Shen <miletus@google.com> wrote:
>
>> Do we have the same problem even if we do the tracking in plugin side
>> that we
>> can't be sure if the invalidation message is from the current input event
>> or a
>> previous input event or some other behavior ? Do you think is there a
>> better
>> way to track the casualty ?
>>
>>
>>
>> On Fri, Apr 25, 2014 at 4:58 PM, <piman@chromium.org> wrote:
>>
>>> I don't think doing it on the host side will give you the right results.
>>>
>>> Since the renderer and the plugin are asynchronous wrt each other, the
>>> input
>>> event message and the invalidation messages (from a previous event, or
>>> some
>>> other behavior) can cross each other, and so you would pick the newer
>>> latency
>>> info for an invalidation from an older event.
>>>
>>> https://codereview.chromium.org/252663002/
>>>
>>
>>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Yufeng Shen (Slow to review), 2014-05-01 21:29:51]

Based on your guys's comments, I decide to go with adding a new private API 

InputEventPrivate::TraceInputLatency(bool has_damage)

for tracking the input latency.

1. If the event is believed to cause rendering damage,
private_event.TraceInputLatency(true) 
can be called, and the input event's LatencyInfo will be sent to renderer with
next 
plugin frame and be tracked until it reaches screen. 

2. If the event is believed to not cause any rendering damage,
private_event.TraceInputLatency(false) 
can be called, and the LatencyInfo tracking ends right at the call. 


Also an example touch drawing plugin which demos how to use the new API is
added:
ppapi/examples/input/touch_event_latency_tracing.cc

PTAL.

https://codereview.chromium.org/252663002/diff/20001/ppapi/proxy/ppapi_messag...
File ppapi/proxy/ppapi_messages.h (right):

https://codereview.chromium.org/252663002/diff/20001/ppapi/proxy/ppapi_messag...
ppapi/proxy/ppapi_messages.h:130: // We can eliminate the duplication if we have
a content_common_nacl.gyp build.
On 2014/04/25 01:59:43, piman wrote:
> Can we just move the definition into some, say, ui_param_traits.h?

moved it into 
ui/events/latency_info_param_traits_macros.h

[Yufeng Shen (Slow to review), 2014-05-05 19:41:09]

David, how does this API change look to you ?

[dmichael (off chromium), 2014-05-05 20:18:50]

The API looks reasonable at a glance to me. I've just started looking at the
rest.

[dmichael (off chromium), 2014-05-05 22:20:32]

https://codereview.chromium.org/252663002/diff/80001/content/renderer/pepper/...
File content/renderer/pepper/pepper_plugin_instance_impl.cc (right):

https://codereview.chromium.org/252663002/diff/80001/content/renderer/pepper/...
content/renderer/pepper/pepper_plugin_instance_impl.cc:1127:
InitLatencyInfo(&events[i].latency_info,
You're doing some work here even if the plugin isn't doing any latency tracking,
right?

Would it make sense to add another PPAPI call to PPB_InputEvent_Private,
something like:
void StartTrackingLatency(PP_Instance instance)
...that only gets called once at startup?

Also...  can you limit when this API works? There's currently no permissions
check; is there maybe an origin to whitelist or something?

https://codereview.chromium.org/252663002/diff/80001/ppapi/api/private/ppb_in...
File ppapi/api/private/ppb_input_event_private.idl (right):

https://codereview.chromium.org/252663002/diff/80001/ppapi/api/private/ppb_in...
ppapi/api/private/ppb_input_event_private.idl:27: * the resulted plugin frame
eventually reaches screen.
There are some caveats with this in your current patch:
1) Graphics3D is not part of it.
2) Graphics2D only works if it's bound. That makes sense, but there is a weird
case that could be surprising. If the plugin has 2 Graphics2Ds, one bound, and
one not...  the unbound one can steal the latency info objects, and never does
anything with them.

This is probably OK for a private interface, but probably worth documenting.

https://codereview.chromium.org/252663002/diff/80001/ppapi/examples/input/tou...
File ppapi/examples/input/touch_event_latency_tracing.cc (right):

https://codereview.chromium.org/252663002/diff/80001/ppapi/examples/input/tou...
ppapi/examples/input/touch_event_latency_tracing.cc:1: // Copyright (c) 2012 The
Chromium Authors. All rights reserved.
nit: 2014
tinier nit: I've heard it's preferable to omit the "(c)" part now. (Here and
other new files)


Seeing as this is a private interface, do we really need an example? Maybe it
would be better to have an automated test?

[Yufeng Shen (Slow to review), 2014-05-07 17:12:13]

1.StartTrackingLatency() added.
2.Example removed.
3.Browser test added.

PTAL.

https://codereview.chromium.org/252663002/diff/80001/content/renderer/pepper/...
File content/renderer/pepper/pepper_plugin_instance_impl.cc (right):

https://codereview.chromium.org/252663002/diff/80001/content/renderer/pepper/...
content/renderer/pepper/pepper_plugin_instance_impl.cc:1127:
InitLatencyInfo(&events[i].latency_info,
On 2014/05/05 22:20:32, dmichael wrote:
> You're doing some work here even if the plugin isn't doing any latency
tracking,
> right?
> 
> Would it make sense to add another PPAPI call to PPB_InputEvent_Private,
> something like:
> void StartTrackingLatency(PP_Instance instance)
> ...that only gets called once at startup?
> 

done.

> Also...  can you limit when this API works? There's currently no permissions
> check; is there maybe an origin to whitelist or something?

I am not sure how to do this. I looked at the whitelist example for UMA and it
seems that they are based on the host url. The primary usage for the latency
tracking I want to do is to setup a telemetry test that serve a simple touch
drawing plugin, simulate some touch events, and gather the input latency for it.
Using URL to do whitelist seems strange for me.

Is there a way to do filtering based on plugin type, e.g. 
type="application/x-ppapi-example-input" ?

https://codereview.chromium.org/252663002/diff/80001/ppapi/api/private/ppb_in...
File ppapi/api/private/ppb_input_event_private.idl (right):

https://codereview.chromium.org/252663002/diff/80001/ppapi/api/private/ppb_in...
ppapi/api/private/ppb_input_event_private.idl:27: * the resulted plugin frame
eventually reaches screen.
On 2014/05/05 22:20:32, dmichael wrote:
> There are some caveats with this in your current patch:
> 1) Graphics3D is not part of it.
> 2) Graphics2D only works if it's bound. That makes sense, but there is a weird
> case that could be surprising. If the plugin has 2 Graphics2Ds, one bound, and
> one not...  the unbound one can steal the latency info objects, and never does
> anything with them.
> 
> This is probably OK for a private interface, but probably worth documenting.

Extra notes added.

[dmichael (off chromium), 2014-05-07 22:59:23]

On 2014/05/07 17:12:13, Yufeng Shen wrote:
> 1.StartTrackingLatency() added.
> 2.Example removed.
> 3.Browser test added.
> 
> PTAL.
> 
>
https://codereview.chromium.org/252663002/diff/80001/content/renderer/pepper/...
> File content/renderer/pepper/pepper_plugin_instance_impl.cc (right):
> 
>
https://codereview.chromium.org/252663002/diff/80001/content/renderer/pepper/...
> content/renderer/pepper/pepper_plugin_instance_impl.cc:1127:
> InitLatencyInfo(&events[i].latency_info,
> On 2014/05/05 22:20:32, dmichael wrote:
> > You're doing some work here even if the plugin isn't doing any latency
> tracking,
> > right?
> > 
> > Would it make sense to add another PPAPI call to PPB_InputEvent_Private,
> > something like:
> > void StartTrackingLatency(PP_Instance instance)
> > ...that only gets called once at startup?
> > 
> 
> done.
> 
> > Also...  can you limit when this API works? There's currently no permissions
> > check; is there maybe an origin to whitelist or something?
> 
> I am not sure how to do this. I looked at the whitelist example for UMA and it
> seems that they are based on the host url. The primary usage for the latency
> tracking I want to do is to setup a telemetry test that serve a simple touch
> drawing plugin, simulate some touch events, and gather the input latency for
it.
> Using URL to do whitelist seems strange for me.
> 
> Is there a way to do filtering based on plugin type, e.g. 
> type="application/x-ppapi-example-input" ?
Are you using a trusted plugin, or are you wanting to test this in NaCl/PNaCl?

If it's a trusted plugin, then you can just move your interface back to
ppapi/thunk/interfaces_ppb_private.h, because trusted plugins get private
permissions anyway.

If you want it to work in NaCl/PNaCl, then using the MIME type isn't going to do
the right thing. But if it's *only* for a test, then maybe (again) you can move
your API back to ppapi/thunk/interfaces_ppb_private.h. If you run chrome with
--enable-nacl, you might get private permissions (I don't remember).

Let me know the following, and we'll work from there about how to do the
permissions properly:
- Are you making a trusted or NaCl plugin? Or both?
- In what context does this plugin run? Is it shipped to users, or is it purely
for testing?

[Yufeng Shen (Slow to review), 2014-05-07 23:40:57]

On 2014/05/07 22:59:23, dmichael wrote:
> On 2014/05/07 17:12:13, Yufeng Shen wrote:
> > 1.StartTrackingLatency() added.
> > 2.Example removed.
> > 3.Browser test added.
> > 
> > PTAL.
> > 
> >
>
https://codereview.chromium.org/252663002/diff/80001/content/renderer/pepper/...
> > File content/renderer/pepper/pepper_plugin_instance_impl.cc (right):
> > 
> >
>
https://codereview.chromium.org/252663002/diff/80001/content/renderer/pepper/...
> > content/renderer/pepper/pepper_plugin_instance_impl.cc:1127:
> > InitLatencyInfo(&events[i].latency_info,
> > On 2014/05/05 22:20:32, dmichael wrote:
> > > You're doing some work here even if the plugin isn't doing any latency
> > tracking,
> > > right?
> > > 
> > > Would it make sense to add another PPAPI call to PPB_InputEvent_Private,
> > > something like:
> > > void StartTrackingLatency(PP_Instance instance)
> > > ...that only gets called once at startup?
> > > 
> > 
> > done.
> > 
> > > Also...  can you limit when this API works? There's currently no
permissions
> > > check; is there maybe an origin to whitelist or something?
> > 
> > I am not sure how to do this. I looked at the whitelist example for UMA and
it
> > seems that they are based on the host url. The primary usage for the latency
> > tracking I want to do is to setup a telemetry test that serve a simple touch
> > drawing plugin, simulate some touch events, and gather the input latency for
> it.
> > Using URL to do whitelist seems strange for me.
> > 
> > Is there a way to do filtering based on plugin type, e.g. 
> > type="application/x-ppapi-example-input" ?
> Are you using a trusted plugin, or are you wanting to test this in NaCl/PNaCl?
> 
> If it's a trusted plugin, then you can just move your interface back to
> ppapi/thunk/interfaces_ppb_private.h, because trusted plugins get private
> permissions anyway.
> 
> If you want it to work in NaCl/PNaCl, then using the MIME type isn't going to
do
> the right thing. But if it's *only* for a test, then maybe (again) you can
move
> your API back to ppapi/thunk/interfaces_ppb_private.h. If you run chrome with
> --enable-nacl, you might get private permissions (I don't remember).
> 
> Let me know the following, and we'll work from there about how to do the
> permissions properly:
> - Are you making a trusted or NaCl plugin? Or both?

I think the test plugin I am going to make is just a trusted plugin.

> - In what context does this plugin run? Is it shipped to users, or is it
purely
> for testing?

It will only be for testing. The goal is to benchmark the input latency for a
simple
plugin. So I will make a simple touch drawing plugin and load it only in
telemetry test
environment

I guess moving the interface to ppapi/thunk/interfaces_ppb_private.h  is enough
? I have
updated the CL to do that.

[dmichael (off chromium), 2014-05-08 21:47:07]

https://codereview.chromium.org/252663002/diff/140001/content/renderer/pepper...
File content/renderer/pepper/pepper_plugin_instance_impl.cc (right):

https://codereview.chromium.org/252663002/diff/140001/content/renderer/pepper...
content/renderer/pepper/pepper_plugin_instance_impl.cc:2551:
is_tracking_latency_ = true;
Please add a permissions check for this. It might look like this:

if (module_->permissions().HasPermission(ppapi::PERMISSION_PRIVATE))
  is_tracking_latency_ = true;

https://codereview.chromium.org/252663002/diff/140001/ppapi/cpp/private/insta...
File ppapi/cpp/private/instance_private.h (right):

https://codereview.chromium.org/252663002/diff/140001/ppapi/cpp/private/insta...
ppapi/cpp/private/instance_private.h:51: void StartTrackingLatency (PP_Instance
instance);
Could you just put this inside InputEventPrivate as a static method?
i.e.:
static StartTrackingLatency(const InstanceHandle& instance);

https://codereview.chromium.org/252663002/diff/140001/ppapi/shared_impl/ppb_i...
File ppapi/shared_impl/ppb_input_event_shared.cc (right):

https://codereview.chromium.org/252663002/diff/140001/ppapi/shared_impl/ppb_i...
ppapi/shared_impl/ppb_input_event_shared.cc:198: 0, NULL))
So the call to TraceInputLatency is coming from the plugin process. But the
renderer is where the LatencyInfo is made for the time the input event gets
created.

Does this ui::LatencyInfo automagically work across processes like this? It's a
bit magical looking to me.

[Yufeng Shen (Slow to review), 2014-05-08 23:49:39]

https://codereview.chromium.org/252663002/diff/140001/content/renderer/pepper...
File content/renderer/pepper/pepper_plugin_instance_impl.cc (right):

https://codereview.chromium.org/252663002/diff/140001/content/renderer/pepper...
content/renderer/pepper/pepper_plugin_instance_impl.cc:2551:
is_tracking_latency_ = true;
On 2014/05/08 21:47:07, dmichael wrote:
> Please add a permissions check for this. It might look like this:
> 
> if (module_->permissions().HasPermission(ppapi::PERMISSION_PRIVATE))
>   is_tracking_latency_ = true;

Done.

https://codereview.chromium.org/252663002/diff/140001/ppapi/cpp/private/insta...
File ppapi/cpp/private/instance_private.h (right):

https://codereview.chromium.org/252663002/diff/140001/ppapi/cpp/private/insta...
ppapi/cpp/private/instance_private.h:51: void StartTrackingLatency (PP_Instance
instance);
On 2014/05/08 21:47:07, dmichael wrote:
> Could you just put this inside InputEventPrivate as a static method?
> i.e.:
> static StartTrackingLatency(const InstanceHandle& instance);

Done.

https://codereview.chromium.org/252663002/diff/140001/ppapi/shared_impl/ppb_i...
File ppapi/shared_impl/ppb_input_event_shared.cc (right):

https://codereview.chromium.org/252663002/diff/140001/ppapi/shared_impl/ppb_i...
ppapi/shared_impl/ppb_input_event_shared.cc:198: 0, NULL))
On 2014/05/08 21:47:07, dmichael wrote:
> So the call to TraceInputLatency is coming from the plugin process. But the
> renderer is where the LatencyInfo is made for the time the input event gets
> created.
> 
> Does this ui::LatencyInfo automagically work across processes like this? It's
a
> bit magical looking to me.

hmm, not sure what confuses you. The LatencyInfo is added to
InputEventData  as a data member. The input event and the LatencyInfo are
created in renderer and at that time we saved some components in it (e.g. this
BEGIN_PLUGIN_COMPONENT). Then the input event/LatencyInfo get passed/copied into
plugin process through IPC. Now if we call
TraceInputLatency(true) on it, we cached the LatencyInfo and later on send it
back to renderer through IPC.

[Rick Byers, 2014-05-09 13:24:57]

On 2014/05/07 23:40:57, Yufeng Shen wrote:
> On 2014/05/07 22:59:23, dmichael wrote:
> > On 2014/05/07 17:12:13, Yufeng Shen wrote:
> > > 1.StartTrackingLatency() added.
> > > 2.Example removed.
> > > 3.Browser test added.
> > > 
> > > PTAL.
> > > 
> > >
> >
>
https://codereview.chromium.org/252663002/diff/80001/content/renderer/pepper/...
> > > File content/renderer/pepper/pepper_plugin_instance_impl.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/252663002/diff/80001/content/renderer/pepper/...
> > > content/renderer/pepper/pepper_plugin_instance_impl.cc:1127:
> > > InitLatencyInfo(&events[i].latency_info,
> > > On 2014/05/05 22:20:32, dmichael wrote:
> > > > You're doing some work here even if the plugin isn't doing any latency
> > > tracking,
> > > > right?
> > > > 
> > > > Would it make sense to add another PPAPI call to PPB_InputEvent_Private,
> > > > something like:
> > > > void StartTrackingLatency(PP_Instance instance)
> > > > ...that only gets called once at startup?
> > > > 
> > > 
> > > done.
> > > 
> > > > Also...  can you limit when this API works? There's currently no
> permissions
> > > > check; is there maybe an origin to whitelist or something?
> > > 
> > > I am not sure how to do this. I looked at the whitelist example for UMA
and
> it
> > > seems that they are based on the host url. The primary usage for the
latency
> > > tracking I want to do is to setup a telemetry test that serve a simple
touch
> > > drawing plugin, simulate some touch events, and gather the input latency
for
> > it.
> > > Using URL to do whitelist seems strange for me.
> > > 
> > > Is there a way to do filtering based on plugin type, e.g. 
> > > type="application/x-ppapi-example-input" ?
> > Are you using a trusted plugin, or are you wanting to test this in
NaCl/PNaCl?
> > 
> > If it's a trusted plugin, then you can just move your interface back to
> > ppapi/thunk/interfaces_ppb_private.h, because trusted plugins get private
> > permissions anyway.
> > 
> > If you want it to work in NaCl/PNaCl, then using the MIME type isn't going
to
> do
> > the right thing. But if it's *only* for a test, then maybe (again) you can
> move
> > your API back to ppapi/thunk/interfaces_ppb_private.h. If you run chrome
with
> > --enable-nacl, you might get private permissions (I don't remember).
> > 
> > Let me know the following, and we'll work from there about how to do the
> > permissions properly:
> > - Are you making a trusted or NaCl plugin? Or both?
> 
> I think the test plugin I am going to make is just a trusted plugin.
> 
> > - In what context does this plugin run? Is it shipped to users, or is it
> purely
> > for testing?
> 
> It will only be for testing. The goal is to benchmark the input latency for a
> simple
> plugin. So I will make a simple touch drawing plugin and load it only in
> telemetry test
> environment
> 
> I guess moving the interface to ppapi/thunk/interfaces_ppb_private.h  is
enough
> ? I have
> updated the CL to do that.

Note though that a big part of the motivation for doing this work is to
demonstrate to plugin authors that it is possible to have low-input latency
plugins and that they can measure the latency of input through their plugin if
necessary.  I'm OK with starting with a trusted plugin model, but we should plan
on developing a good story for how other plugin authors can measure input
latency through their plugin.  If, in the short term, that story is "produce a
modified version of chromium that considers your plugin trusted" then that's
probably OK for now.

[chromium-reviews, 2014-05-09 13:43:06]

On Fri, May 9, 2014 at 9:24 AM, <rbyers@chromium.org> wrote:

> On 2014/05/07 23:40:57, Yufeng Shen wrote:
>
>> On 2014/05/07 22:59:23, dmichael wrote:
>> > On 2014/05/07 17:12:13, Yufeng Shen wrote:
>> > > 1.StartTrackingLatency() added.
>> > > 2.Example removed.
>> > > 3.Browser test added.
>> > >
>> > > PTAL.
>> > >
>> > >
>> >
>>
>
> https://codereview.chromium.org/252663002/diff/80001/
> content/renderer/pepper/pepper_plugin_instance_impl.cc
>
>> > > File content/renderer/pepper/pepper_plugin_instance_impl.cc (right):
>> > >
>> > >
>> >
>>
>
> https://codereview.chromium.org/252663002/diff/80001/
> content/renderer/pepper/pepper_plugin_instance_impl.cc#newcode1127
>
>> > > content/renderer/pepper/pepper_plugin_instance_impl.cc:1127:
>> > > InitLatencyInfo(&events[i].latency_info,
>> > > On 2014/05/05 22:20:32, dmichael wrote:
>> > > > You're doing some work here even if the plugin isn't doing any
>> latency
>> > > tracking,
>> > > > right?
>> > > >
>> > > > Would it make sense to add another PPAPI call to
>> PPB_InputEvent_Private,
>> > > > something like:
>> > > > void StartTrackingLatency(PP_Instance instance)
>> > > > ...that only gets called once at startup?
>> > > >
>> > >
>> > > done.
>> > >
>> > > > Also...  can you limit when this API works? There's currently no
>> permissions
>> > > > check; is there maybe an origin to whitelist or something?
>> > >
>> > > I am not sure how to do this. I looked at the whitelist example for
>> UMA
>>
> and
>
>> it
>> > > seems that they are based on the host url. The primary usage for the
>>
> latency
>
>> > > tracking I want to do is to setup a telemetry test that serve a simple
>>
> touch
>
>> > > drawing plugin, simulate some touch events, and gather the input
>> latency
>>
> for
>
>> > it.
>> > > Using URL to do whitelist seems strange for me.
>> > >
>> > > Is there a way to do filtering based on plugin type, e.g.
>> > > type="application/x-ppapi-example-input" ?
>> > Are you using a trusted plugin, or are you wanting to test this in
>>
> NaCl/PNaCl?
>
>> >
>> > If it's a trusted plugin, then you can just move your interface back to
>> > ppapi/thunk/interfaces_ppb_private.h, because trusted plugins get
>> private
>> > permissions anyway.
>> >
>> > If you want it to work in NaCl/PNaCl, then using the MIME type isn't
>> going
>>
> to
>
>> do
>> > the right thing. But if it's *only* for a test, then maybe (again) you
>> can
>> move
>> > your API back to ppapi/thunk/interfaces_ppb_private.h. If you run
>> chrome
>>
> with
>
>> > --enable-nacl, you might get private permissions (I don't remember).
>> >
>> > Let me know the following, and we'll work from there about how to do the
>> > permissions properly:
>> > - Are you making a trusted or NaCl plugin? Or both?
>>
>
>  I think the test plugin I am going to make is just a trusted plugin.
>>
>
>  > - In what context does this plugin run? Is it shipped to users, or is it
>> purely
>> > for testing?
>>
>
>  It will only be for testing. The goal is to benchmark the input latency
>> for a
>> simple
>> plugin. So I will make a simple touch drawing plugin and load it only in
>> telemetry test
>> environment
>>
>
>  I guess moving the interface to ppapi/thunk/interfaces_ppb_private.h  is
>>
> enough
>
>> ? I have
>> updated the CL to do that.
>>
>
> Note though that a big part of the motivation for doing this work is to
> demonstrate to plugin authors that it is possible to have low-input latency
> plugins and that they can measure the latency of input through their
> plugin if
> necessary.  I'm OK with starting with a trusted plugin model, but we
> should plan
> on developing a good story for how other plugin authors can measure input
> latency through their plugin.  If, in the short term, that story is
> "produce a
> modified version of chromium that considers your plugin trusted" then
> that's
> probably OK for now.
>
>
For the record, I will build the test plugin as PNaCl for easier test
deployment
. he permission for the private API still remains in the
ppapi/thunk/interfaces_ppb_private.h.


>
> https://codereview.chromium.org/252663002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dmichael (off chromium), 2014-05-09 15:19:32]

lgtm

https://codereview.chromium.org/252663002/diff/140001/ppapi/shared_impl/ppb_i...
File ppapi/shared_impl/ppb_input_event_shared.cc (right):

https://codereview.chromium.org/252663002/diff/140001/ppapi/shared_impl/ppb_i...
ppapi/shared_impl/ppb_input_event_shared.cc:198: 0, NULL))
On 2014/05/08 23:49:40, Yufeng Shen wrote:
> On 2014/05/08 21:47:07, dmichael wrote:
> > So the call to TraceInputLatency is coming from the plugin process. But the
> > renderer is where the LatencyInfo is made for the time the input event gets
> > created.
> > 
> > Does this ui::LatencyInfo automagically work across processes like this?
It's
> a
> > bit magical looking to me.
> 
> hmm, not sure what confuses you. The LatencyInfo is added to
> InputEventData  as a data member. The input event and the LatencyInfo are
> created in renderer and at that time we saved some components in it (e.g. this
> BEGIN_PLUGIN_COMPONENT). Then the input event/LatencyInfo get passed/copied
into
> plugin process through IPC. Now if we call
> TraceInputLatency(true) on it, we cached the LatencyInfo and later on send it
> back to renderer through IPC.
> 
>  
OK, got it. I was forgetting about it riding along as a data member when the
input event gets sent.

https://codereview.chromium.org/252663002/diff/160001/ppapi/cpp/private/input...
File ppapi/cpp/private/input_event_private.h (right):

https://codereview.chromium.org/252663002/diff/160001/ppapi/cpp/private/input...
ppapi/cpp/private/input_event_private.h:21: static void StartTrackingLatency
(const PP_Instance& instance);
tiny nit: For consistency at least, it might be better to use InstanceHandle
instead of PP_Instance here. Doesn't make much difference, though.

[Yufeng Shen (Slow to review), 2014-05-12 21:53:06]

piman@ : mind taking a look at content/  ?

sadrul@: mind taking a look at ui/events/ ?

https://codereview.chromium.org/252663002/diff/160001/ppapi/cpp/private/input...
File ppapi/cpp/private/input_event_private.h (right):

https://codereview.chromium.org/252663002/diff/160001/ppapi/cpp/private/input...
ppapi/cpp/private/input_event_private.h:21: static void StartTrackingLatency
(const PP_Instance& instance);
On 2014/05/09 15:19:33, dmichael wrote:
> tiny nit: For consistency at least, it might be better to use InstanceHandle
> instead of PP_Instance here. Doesn't make much difference, though.

Done.

[piman, 2014-05-12 22:10:35]

https://codereview.chromium.org/252663002/diff/180001/content/common/content_...
File content/common/content_param_traits_macros.h (right):

https://codereview.chromium.org/252663002/diff/180001/content/common/content_...
content/common/content_param_traits_macros.h:39: #include
"ui/events/latency_info_param_traits_macros.h"
include at the top

https://codereview.chromium.org/252663002/diff/180001/ppapi/proxy/ppapi_messa...
File ppapi/proxy/ppapi_messages.h (right):

https://codereview.chromium.org/252663002/diff/180001/ppapi/proxy/ppapi_messa...
ppapi/proxy/ppapi_messages.h:80: #endif
ppapi can't depend on content.

[Yufeng Shen (Slow to review), 2014-05-13 00:16:15]

https://codereview.chromium.org/252663002/diff/180001/ppapi/proxy/ppapi_messa...
File ppapi/proxy/ppapi_messages.h (right):

https://codereview.chromium.org/252663002/diff/180001/ppapi/proxy/ppapi_messa...
ppapi/proxy/ppapi_messages.h:80: #endif
On 2014/05/12 22:10:36, piman wrote:
> ppapi can't depend on content.

I am not sure how to do this correctly.

so currently latency_info's ipc traits is in 
content/common/content_param_traits_macros.h

If I just copy the IPC traits stuff over from content_param_traits_macros.h,
there will be redefinition error
because ppapi_messages.h collides with content_param_traits_macros.h
in a non-nacl build.

Also I can't move the IPC traits into ui/events/ since that will
add dependency of ui on ipc.

[piman, 2014-05-13 00:31:40]

On Mon, May 12, 2014 at 5:16 PM, <miletus@chromium.org> wrote:

>
> https://codereview.chromium.org/252663002/diff/180001/
> ppapi/proxy/ppapi_messages.h
> File ppapi/proxy/ppapi_messages.h (right):
>
> https://codereview.chromium.org/252663002/diff/180001/
> ppapi/proxy/ppapi_messages.h#newcode80
> ppapi/proxy/ppapi_messages.h:80: #endif
> On 2014/05/12 22:10:36, piman wrote:
>
>> ppapi can't depend on content.
>>
>
> I am not sure how to do this correctly.
>
> so currently latency_info's ipc traits is in
> content/common/content_param_traits_macros.h
>
> If I just copy the IPC traits stuff over from
> content_param_traits_macros.h, there will be redefinition error
> because ppapi_messages.h collides with content_param_traits_macros.h
> in a non-nacl build.
>
> Also I can't move the IPC traits into ui/events/ since that will
> add dependency of ui on ipc.
>

What we've done in gpu/ is to create a gpu/ipc that depends on both gpu and
ipc, and then use that in the various places we need messages with gpu data
structures (among others, ppapi/proxy).


>
> https://codereview.chromium.org/252663002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Yufeng Shen (Slow to review), 2014-05-13 01:35:33]

On 2014/05/13 00:31:40, piman wrote:
> On Mon, May 12, 2014 at 5:16 PM, <mailto:miletus@chromium.org> wrote:
> 
> >
> > https://codereview.chromium.org/252663002/diff/180001/
> > ppapi/proxy/ppapi_messages.h
> > File ppapi/proxy/ppapi_messages.h (right):
> >
> > https://codereview.chromium.org/252663002/diff/180001/
> > ppapi/proxy/ppapi_messages.h#newcode80
> > ppapi/proxy/ppapi_messages.h:80: #endif
> > On 2014/05/12 22:10:36, piman wrote:
> >
> >> ppapi can't depend on content.
> >>
> >
> > I am not sure how to do this correctly.
> >
> > so currently latency_info's ipc traits is in
> > content/common/content_param_traits_macros.h
> >
> > If I just copy the IPC traits stuff over from
> > content_param_traits_macros.h, there will be redefinition error
> > because ppapi_messages.h collides with content_param_traits_macros.h
> > in a non-nacl build.
> >
> > Also I can't move the IPC traits into ui/events/ since that will
> > add dependency of ui on ipc.
> >
> 
> What we've done in gpu/ is to create a gpu/ipc that depends on both gpu and
> ipc, and then use that in the various places we need messages with gpu data
> structures (among others, ppapi/proxy).
> 
> 


Sadrul:  can we move the latency info ipc traits stuff into ui/events/ so they
stay together with latency_info.* and have both regular and nacl gyp for it.
This will introduce ui/events's dependency on ipc, is that OK ?



> >
> > https://codereview.chromium.org/252663002/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[sadrul, 2014-05-14 03:15:18]

> > > Also I can't move the IPC traits into ui/events/ since that will
> > > add dependency of ui on ipc.
> > 
> > What we've done in gpu/ is to create a gpu/ipc that depends on both gpu and
> > ipc, and then use that in the various places we need messages with gpu data
> > structures (among others, ppapi/proxy).
> 
> Sadrul:  can we move the latency info ipc traits stuff into ui/events/ so they
> stay together with latency_info.* and have both regular and nacl gyp for it.
> This will introduce ui/events's dependency on ipc, is that OK ?
> 

Can you do what piman@ suggested, and have ui/events/ipc/ with its own gypi?

[Yufeng Shen (Slow to review), 2014-05-20 22:44:30]

On 2014/05/14 03:15:18, sadrul wrote:
> > > > Also I can't move the IPC traits into ui/events/ since that will
> > > > add dependency of ui on ipc.
> > > 
> > > What we've done in gpu/ is to create a gpu/ipc that depends on both gpu
and
> > > ipc, and then use that in the various places we need messages with gpu
data
> > > structures (among others, ppapi/proxy).
> > 
> > Sadrul:  can we move the latency info ipc traits stuff into ui/events/ so
they
> > stay together with latency_info.* and have both regular and nacl gyp for it.
> > This will introduce ui/events's dependency on ipc, is that OK ?
> > 
> 
> Can you do what piman@ suggested, and have ui/events/ipc/ with its own gypi?



I had a separate CL https://codereview.chromium.org/291443006/   which moves the
latency_info ipc stuff into ui/events/ipc.
Now ppapi/proxy/ppapi_messages.h  only needs to include
"ui/events/ipc/latency_info_param_traits.h".

piman, sadrul, PTAL.

[piman, 2014-05-20 23:07:59]

lgtm

[Yufeng Shen (Slow to review), 2014-05-20 23:22:21]

add  asvitkine@  for OWNER of  tools/metrics/

[Alexei Svitkine (slow), 2014-05-21 05:03:19]

histograms.xml lgtm

[sadrul, 2014-05-21 05:15:13]

lgtm

[Yufeng Shen (Slow to review), 2014-05-21 23:22:05]

The CQ bit was checked by miletus@chromium.org

[commit-bot: I haz the power, 2014-05-21 23:23:58]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/miletus@chromium.org/252663002/200001

[Yufeng Shen (Slow to review), 2014-05-21 23:43:46]

oops, missing OWNER of ppapi/proxy/ppapi_messages.h

+ tsepez@

[Tom Sepez, 2014-05-21 23:47:55]

Messages LGTM.

[Yufeng Shen (Slow to review), 2014-05-21 23:50:18]

The CQ bit was unchecked by miletus@chromium.org

[Yufeng Shen (Slow to review), 2014-05-21 23:50:33]

The CQ bit was checked by miletus@chromium.org

[commit-bot: I haz the power, 2014-05-21 23:53:17]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/miletus@chromium.org/252663002/200001

[Yufeng Shen (Slow to review), 2014-05-22 00:43:21]

The CQ bit was checked by miletus@chromium.org

[commit-bot: I haz the power, 2014-05-22 00:48:53]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/miletus@chromium.org/252663002/220001

[Yufeng Shen (Slow to review), 2014-05-22 01:12:59]

The CQ bit was checked by miletus@chromium.org

[commit-bot: I haz the power, 2014-05-22 01:17:22]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/miletus@chromium.org/252663002/240001

[miletus1, 2014-05-22 03:47:41]

The CQ bit was checked by miletus@google.com

[commit-bot: I haz the power, 2014-05-22 03:50:16]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/miletus@chromium.org/252663002/260001

[commit-bot: I haz the power, 2014-05-22 09:54:02]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-22 10:04:41]

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

[commit-bot: I haz the power, 2014-05-22 10:04:42]

Try jobs failed on following builders:
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)

[Yufeng Shen (Slow to review), 2014-05-27 00:06:31]

The CQ bit was checked by miletus@chromium.org

[commit-bot: I haz the power, 2014-05-27 00:06:56]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/miletus@chromium.org/252663002/280001

[Yufeng Shen (Slow to review), 2014-05-27 00:12:51]

+sky@ for OWNER of

mojo/mojo_examples.gypi

[commit-bot: I haz the power, 2014-05-27 03:35:37]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)

[commit-bot: I haz the power, 2014-05-27 03:39:24]

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

[commit-bot: I haz the power, 2014-05-27 03:39:24]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)

[sky, 2014-05-27 16:41:17]

LGTM

[Yufeng Shen (Slow to review), 2014-05-27 16:41:37]

The CQ bit was checked by miletus@chromium.org

[commit-bot: I haz the power, 2014-05-27 16:42:10]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/miletus@chromium.org/252663002/280001

[commit-bot: I haz the power, 2014-05-27 16:56:47]

Change committed as 272990