cc: Add support for sending BeginFrames for video.

cc: Add support for sending BeginFrames for video.

This CL adds support for plumbing BeginFrames to
VideoFrameProviderClientImpl if it opts into it. This is required for
upcoming video rendering changes to improve video smoothness.

BUG=456991
Committed: https://crrev.com/7d073dc78a7578aababc032237a54e55a86d3480
Cr-Commit-Position: refs/heads/master@{#325550}

[sunnyps, 2015-04-07 21:03:00]

sunnyps@chromium.org changed reviewers:
+ brianderson@chromium.org

[sunnyps, 2015-04-07 21:03:01]

PTAL

[sunnyps, 2015-04-07 21:03:53]

sunnyps@chromium.org changed reviewers:
+ danakj@chromium.org

[sunnyps, 2015-04-07 21:03:53]

danakj@: Could you PTAL at LTHI/video changes? Thanks!

[danakj, 2015-04-07 21:15:12]

Is there a design doc/bug explaining what the VideoFrameProviderClient is going
to do with tab-visibility?

[sunnyps, 2015-04-07 21:25:05]

On 2015/04/07 21:15:12, danakj wrote:
> Is there a design doc/bug explaining what the VideoFrameProviderClient is
going
> to do with tab-visibility?

There's no doc as such but here's a link to the CL that changes the video
rendering side of things: https://codereview.chromium.org/1053113002/

There are two reasons for telling the video renderer about tab visibility:

1. To accurately count the number of dropped frames: In a model where the
compositor drives the renderer by asking for a video frame on each vsync, the
video renderer can't tell if a frame was dropped because the compositor was too
late or if the tab was invisible and we stopped asking for vsyncs.
2. To keep driving itself when the compositor stops driving it. Decoding video
frames must continue while the tab is invisible.

Dale could probably explain the reasons better.

[danakj, 2015-04-07 21:30:35]

On Tue, Apr 7, 2015 at 2:25 PM, <sunnyps@chromium.org> wrote:

> On 2015/04/07 21:15:12, danakj wrote:
>
>> Is there a design doc/bug explaining what the VideoFrameProviderClient is
>>
> going
>
>> to do with tab-visibility?
>>
>
> There's no doc as such but here's a link to the CL that changes the video
> rendering side of things: https://codereview.chromium.org/1053113002/
>
> There are two reasons for telling the video renderer about tab visibility:
>
> 1. To accurately count the number of dropped frames: In a model where the
> compositor drives the renderer by asking for a video frame on each vsync,
> the
> video renderer can't tell if a frame was dropped because the compositor
> was too
> late or if the tab was invisible and we stopped asking for vsyncs.
>

Why does it care?


> 2. To keep driving itself when the compositor stops driving it. Decoding
> video
> frames must continue while the tab is invisible.
>

Or, why does it keep making frames when not visible? This seems like a
whole lot of bad CPU for background tabs?

Why does this visibility need to come through the compositor thread? Does
it need to be in-band with the requests for video frames? Why can't the
place that calls LayerTreeHost::SetVisible() also notify the media system?
Or even a LayerTreeHost notification? Plumbing through the whole compositor
to the media system seems weird to me, why do we want to do that?


>
> Dale could probably explain the reasons better.
>
> https://codereview.chromium.org/1039533002/
>

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

[DaleCurtis, 2015-04-07 22:17:16]

On 2015/04/07 21:30:35, danakj wrote:
> On Tue, Apr 7, 2015 at 2:25 PM, <mailto:sunnyps@chromium.org> wrote:
> 
> > On 2015/04/07 21:15:12, danakj wrote:
> >
> >> Is there a design doc/bug explaining what the VideoFrameProviderClient is
> >>
> > going
> >
> >> to do with tab-visibility?
> >>
> >
> > There's no doc as such but here's a link to the CL that changes the video
> > rendering side of things: https://codereview.chromium.org/1053113002/
> >
> > There are two reasons for telling the video renderer about tab visibility:
> >
> > 1. To accurately count the number of dropped frames: In a model where the
> > compositor drives the renderer by asking for a video frame on each vsync,
> > the
> > video renderer can't tell if a frame was dropped because the compositor
> > was too
> > late or if the tab was invisible and we stopped asking for vsyncs.
> >
> 
> Why does it care?

Because it's a JS visible stat that web applications use for evaluating their
playback experience. If someone puts a tab in the background, the stat is going
to be useless; it has been a source of Pri-0 bugs from partner companies in the
past.

> 
> 
> > 2. To keep driving itself when the compositor stops driving it. Decoding
> > video
> > frames must continue while the tab is invisible.
> >
> 
> Or, why does it keep making frames when not visible? This seems like a
> whole lot of bad CPU for background tabs?
> 
> Why does this visibility need to come through the compositor thread? Does
> it need to be in-band with the requests for video frames? Why can't the
> place that calls LayerTreeHost::SetVisible() also notify the media system?
> Or even a LayerTreeHost notification? Plumbing through the whole compositor
> to the media system seems weird to me, why do we want to do that?

I don't like it either; but we need some signal if the compositor is going to
stop BeginFrame notifications based on visibility. Ideally we'd prefer to
continue to get low frequency updates, say every 100ms instead with a flag
indicating they're delayed updates. Sunny said you guys don't want to do that in
the compositor any more, so we need to do it at the player level. We need this
for a few reasons:
- We can't let the decoder get too far behind or we'll hit user visible issues
when the tab comes back into view; i.e. a stuck frame as the decoder furiously
spins to catch up (which may be a while).
- We can't let the renderer get too far behind because that's JS visible when
copying to canvas / webgl / etc. Clients may then XHR those frames somewhere /
do any number of crazy things.
- We can't just spin up a low frequency task without letting the video renderer
know that it's in some new mode where it shouldn't count dropped frames (for the
reasons mentioned above).

I'm not concerned with the implementation details of the notification. I don't
even care if it's relays the concept of visibility. I'd be fine if it just said
something like OnBeginFrameNotifications suspended. The only important part is
that it always occur before BeginFrame notifications restart and after they've
stopped. Otherwise it'll be a pain to ensure we can install a fake pump without
reentrancy issues.

> 
> 
> >
> > Dale could probably explain the reasons better.
> >
> > https://codereview.chromium.org/1039533002/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[brianderson, 2015-04-07 22:34:58]

On 2015/04/07 22:17:16, DaleCurtis wrote:
> On 2015/04/07 21:30:35, danakj wrote:
> > On Tue, Apr 7, 2015 at 2:25 PM, <mailto:sunnyps@chromium.org> wrote:
> > 
> > > On 2015/04/07 21:15:12, danakj wrote:
> > >
> > >> Is there a design doc/bug explaining what the VideoFrameProviderClient is
> > >>
> > > going
> > >
> > >> to do with tab-visibility?
> > >>
> > >
> > > There's no doc as such but here's a link to the CL that changes the video
> > > rendering side of things: https://codereview.chromium.org/1053113002/
> > >
> > > There are two reasons for telling the video renderer about tab visibility:
> > >
> > > 1. To accurately count the number of dropped frames: In a model where the
> > > compositor drives the renderer by asking for a video frame on each vsync,
> > > the
> > > video renderer can't tell if a frame was dropped because the compositor
> > > was too
> > > late or if the tab was invisible and we stopped asking for vsyncs.
> > >
> > 
> > Why does it care?
> 
> Because it's a JS visible stat that web applications use for evaluating their
> playback experience. If someone puts a tab in the background, the stat is
going
> to be useless; it has been a source of Pri-0 bugs from partner companies in
the
> past.
> 
> > 
> > 
> > > 2. To keep driving itself when the compositor stops driving it. Decoding
> > > video
> > > frames must continue while the tab is invisible.
> > >
> > 
> > Or, why does it keep making frames when not visible? This seems like a
> > whole lot of bad CPU for background tabs?
> > 
> > Why does this visibility need to come through the compositor thread? Does
> > it need to be in-band with the requests for video frames? Why can't the
> > place that calls LayerTreeHost::SetVisible() also notify the media system?
> > Or even a LayerTreeHost notification? Plumbing through the whole compositor
> > to the media system seems weird to me, why do we want to do that?
> 
> I don't like it either; but we need some signal if the compositor is going to
> stop BeginFrame notifications based on visibility. Ideally we'd prefer to
> continue to get low frequency updates, say every 100ms instead with a flag
> indicating they're delayed updates. Sunny said you guys don't want to do that
in
> the compositor any more, so we need to do it at the player level. We need this
> for a few reasons:
> - We can't let the decoder get too far behind or we'll hit user visible issues
> when the tab comes back into view; i.e. a stuck frame as the decoder furiously
> spins to catch up (which may be a while).
> - We can't let the renderer get too far behind because that's JS visible when
> copying to canvas / webgl / etc. Clients may then XHR those frames somewhere /
> do any number of crazy things.
> - We can't just spin up a low frequency task without letting the video
renderer
> know that it's in some new mode where it shouldn't count dropped frames (for
the
> reasons mentioned above).
> 
> I'm not concerned with the implementation details of the notification. I don't
> even care if it's relays the concept of visibility. I'd be fine if it just
said
> something like OnBeginFrameNotifications suspended. The only important part is
> that it always occur before BeginFrame notifications restart and after they've
> stopped. Otherwise it'll be a pain to ensure we can install a fake pump
without
> reentrancy issues.

Would it work if you were to receive BeginFrames while not visible?
If so, maybe we could have the VideoFrameProvider use the cc::Scheduler's
SetChildrenNeedBeginFrames logic.

We've run into monotonicity issues with switching BeginFrame sources in the
compositor. I imagine you'd want to avoid them in the VideoFrameProvider too
(such as jumping backwards a frame on becoming visible), in which case the
easiest way is to just always use the same source.

> 
> > 
> > 
> > >
> > > Dale could probably explain the reasons better.
> > >
> > > https://codereview.chromium.org/1039533002/
> > >
> > 
> > To unsubscribe from this group and stop receiving emails from it, send an
> email
> > to mailto:chromium-reviews+unsubscribe@chromium.org.

[DaleCurtis, 2015-04-07 22:43:00]

On 2015/04/07 22:34:58, brianderson wrote:
> On 2015/04/07 22:17:16, DaleCurtis wrote:
> > On 2015/04/07 21:30:35, danakj wrote:
> > > On Tue, Apr 7, 2015 at 2:25 PM, <mailto:sunnyps@chromium.org> wrote:
> > > 
> > > > On 2015/04/07 21:15:12, danakj wrote:
> > > >
> > > >> Is there a design doc/bug explaining what the VideoFrameProviderClient
is
> > > >>
> > > > going
> > > >
> > > >> to do with tab-visibility?
> > > >>
> > > >
> > > > There's no doc as such but here's a link to the CL that changes the
video
> > > > rendering side of things: https://codereview.chromium.org/1053113002/
> > > >
> > > > There are two reasons for telling the video renderer about tab
visibility:
> > > >
> > > > 1. To accurately count the number of dropped frames: In a model where
the
> > > > compositor drives the renderer by asking for a video frame on each
vsync,
> > > > the
> > > > video renderer can't tell if a frame was dropped because the compositor
> > > > was too
> > > > late or if the tab was invisible and we stopped asking for vsyncs.
> > > >
> > > 
> > > Why does it care?
> > 
> > Because it's a JS visible stat that web applications use for evaluating
their
> > playback experience. If someone puts a tab in the background, the stat is
> going
> > to be useless; it has been a source of Pri-0 bugs from partner companies in
> the
> > past.
> > 
> > > 
> > > 
> > > > 2. To keep driving itself when the compositor stops driving it. Decoding
> > > > video
> > > > frames must continue while the tab is invisible.
> > > >
> > > 
> > > Or, why does it keep making frames when not visible? This seems like a
> > > whole lot of bad CPU for background tabs?
> > > 
> > > Why does this visibility need to come through the compositor thread? Does
> > > it need to be in-band with the requests for video frames? Why can't the
> > > place that calls LayerTreeHost::SetVisible() also notify the media system?
> > > Or even a LayerTreeHost notification? Plumbing through the whole
compositor
> > > to the media system seems weird to me, why do we want to do that?
> > 
> > I don't like it either; but we need some signal if the compositor is going
to
> > stop BeginFrame notifications based on visibility. Ideally we'd prefer to
> > continue to get low frequency updates, say every 100ms instead with a flag
> > indicating they're delayed updates. Sunny said you guys don't want to do
that
> in
> > the compositor any more, so we need to do it at the player level. We need
this
> > for a few reasons:
> > - We can't let the decoder get too far behind or we'll hit user visible
issues
> > when the tab comes back into view; i.e. a stuck frame as the decoder
furiously
> > spins to catch up (which may be a while).
> > - We can't let the renderer get too far behind because that's JS visible
when
> > copying to canvas / webgl / etc. Clients may then XHR those frames somewhere
/
> > do any number of crazy things.
> > - We can't just spin up a low frequency task without letting the video
> renderer
> > know that it's in some new mode where it shouldn't count dropped frames (for
> the
> > reasons mentioned above).
> > 
> > I'm not concerned with the implementation details of the notification. I
don't
> > even care if it's relays the concept of visibility. I'd be fine if it just
> said
> > something like OnBeginFrameNotifications suspended. The only important part
is
> > that it always occur before BeginFrame notifications restart and after
they've
> > stopped. Otherwise it'll be a pain to ensure we can install a fake pump
> without
> > reentrancy issues.
> 
> Would it work if you were to receive BeginFrames while not visible?
> If so, maybe we could have the VideoFrameProvider use the cc::Scheduler's
> SetChildrenNeedBeginFrames logic.
> 
> We've run into monotonicity issues with switching BeginFrame sources in the
> compositor. I imagine you'd want to avoid them in the VideoFrameProvider too
> (such as jumping backwards a frame on becoming visible), in which case the
> easiest way is to just always use the same source.
> 

That would be awesome if it's possible. We really don't care about visibility,
only that we stop receiving regular callbacks which has implications for the
rest of the media pump in various JS visible ways. If you're going to pump the
callbacks at less than the vsync frequency though (which I assume you want to
for power reasons), we'll need a signal of some sorts. We're happy to expose a
"next frame time" hint which could be used to coordinate the optimal callback
frequency.

> > 
> > > 
> > > 
> > > >
> > > > Dale could probably explain the reasons better.
> > > >
> > > > https://codereview.chromium.org/1039533002/
> > > >
> > > 
> > > To unsubscribe from this group and stop receiving emails from it, send an
> > email
> > > to mailto:chromium-reviews+unsubscribe@chromium.org.

[sunnyps, 2015-04-07 23:33:43]

I looked at how LayerTreeHost::SetVisible gets called from blink and it's not
pretty. Hooking that up to the media stack seems like like it would be very
hard.

OTOH as Dale explained the video frame controller doesn't care about visibility,
it cares if it's client is going to keep delivering BeginFrames. Visibility just
happens to be the only thing that's relevant in this case. WDYT about passing
the SetNeedsBeginFrames(...) call from the Scheduler through ThreadProxy and
LTHI to the VideoFrameController?

One thing I don't know is if LTHI should cache the value of the last
SetNeedsBeginFrames call. This matters if we're able to commit (and create
VideoFrameController) while SetNeedsBeginFrames is false (because of the tab
being invisible say).

[danakj, 2015-04-07 23:37:53]

We don't commit while not visible. The scheduler shouldn't let this happen.

On Tue, Apr 7, 2015 at 4:33 PM, <sunnyps@chromium.org> wrote:

> I looked at how LayerTreeHost::SetVisible gets called from blink and it's
> not
> pretty. Hooking that up to the media stack seems like like it would be very
> hard.
>
> OTOH as Dale explained the video frame controller doesn't care about
> visibility,
> it cares if it's client is going to keep delivering BeginFrames.
> Visibility just
> happens to be the only thing that's relevant in this case. WDYT about
> passing
> the SetNeedsBeginFrames(...) call from the Scheduler through ThreadProxy
> and
> LTHI to the VideoFrameController?
>
> One thing I don't know is if LTHI should cache the value of the last
> SetNeedsBeginFrames call. This matters if we're able to commit (and create
> VideoFrameController) while SetNeedsBeginFrames is false (because of the
> tab
> being invisible say).
>
> https://codereview.chromium.org/1039533002/
>

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

[sunnyps, 2015-04-08 00:11:47]

On 2015/04/07 23:37:53, danakj wrote:
> We don't commit while not visible. The scheduler shouldn't let this happen.
> 

Do you think this addresses your concerns about LTHI passing tab visibility
information to video frame controller?

> On Tue, Apr 7, 2015 at 4:33 PM, <mailto:sunnyps@chromium.org> wrote:
> 
> > I looked at how LayerTreeHost::SetVisible gets called from blink and it's
> > not
> > pretty. Hooking that up to the media stack seems like like it would be very
> > hard.
> >
> > OTOH as Dale explained the video frame controller doesn't care about
> > visibility,
> > it cares if it's client is going to keep delivering BeginFrames.
> > Visibility just
> > happens to be the only thing that's relevant in this case. WDYT about
> > passing
> > the SetNeedsBeginFrames(...) call from the Scheduler through ThreadProxy
> > and
> > LTHI to the VideoFrameController?
> >
> > One thing I don't know is if LTHI should cache the value of the last
> > SetNeedsBeginFrames call. This matters if we're able to commit (and create
> > VideoFrameController) while SetNeedsBeginFrames is false (because of the
> > tab
> > being invisible say).
> >
> > https://codereview.chromium.org/1039533002/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[danakj, 2015-04-08 18:11:48]

On Tue, Apr 7, 2015 at 5:11 PM, <sunnyps@chromium.org> wrote:

> On 2015/04/07 23:37:53, danakj wrote:
>
>> We don't commit while not visible. The scheduler shouldn't let this
>> happen.
>>
>
>
> Do you think this addresses your concerns about LTHI passing tab visibility
> information to video frame controller?


SetBeginFrameNeeded is for the begin frame source right? It appears to be
based on things like "do you need a commit". Is this going to be calling
the media system to tell it about begin frames constantly as commits happen
and such to enable/disable begin frames?

I'm not fully convinced that this should be tied to the scheduler like
this.  Media wants to know when backgrounded, this has nothing to do with
compositor. Are you saying there are other situations that it cares about
other than tab backgrounded?

I was picturing notifying the media code from somewhere like this:
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/r...

Which is where we tell other various systems the tab is backgrounded.


>
>
>  On Tue, Apr 7, 2015 at 4:33 PM, <mailto:sunnyps@chromium.org> wrote:
>>
>
>  > I looked at how LayerTreeHost::SetVisible gets called from blink and
>> it's
>> > not
>> > pretty. Hooking that up to the media stack seems like like it would be
>> very
>> > hard.
>> >
>> > OTOH as Dale explained the video frame controller doesn't care about
>> > visibility,
>> > it cares if it's client is going to keep delivering BeginFrames.
>> > Visibility just
>> > happens to be the only thing that's relevant in this case. WDYT about
>> > passing
>> > the SetNeedsBeginFrames(...) call from the Scheduler through ThreadProxy
>> > and
>> > LTHI to the VideoFrameController?
>> >
>> > One thing I don't know is if LTHI should cache the value of the last
>> > SetNeedsBeginFrames call. This matters if we're able to commit (and
>> create
>> > VideoFrameController) while SetNeedsBeginFrames is false (because of the
>> > tab
>> > being invisible say).
>> >
>> > https://codereview.chromium.org/1039533002/
>> >
>>
>
>  To unsubscribe from this group and stop receiving emails from it, send an
>>
> email
>
>> to mailto:chromium-reviews+unsubscribe@chromium.org.
>>
>
>
>
> https://codereview.chromium.org/1039533002/
>

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

[DaleCurtis, 2015-04-11 16:22:10]

I had a thought last night about how we might be able to avoid the compositor
doing background pumping: we can use a "lost button" (countdown) timer on the
media thread to handle expiring frames which are too old and requesting new
frames. Frames expired in this way wouldn't be counted as dropped frames.
BeginFrame callbacks would reset the timer. I'll play around with this next week
to see if it's viable.

[DaleCurtis, 2015-04-13 18:32:42]

dalecurtis@chromium.org changed reviewers:
+ dalecurtis@chromium.org

[DaleCurtis, 2015-04-13 18:32:43]

I believe my approach will be viable, so I've nuked the concept of visibility on
the video related interfaces from my landing pad CL:
https://codereview.chromium.org/1053113002/

We'll pump the media loop within the VideoRenderer using a countdown timer based
on frame duration and buffer extents when BeginFrame calls stop.

For WebGL/Canvas use cases we'll limit frame update to every 250ms when the page
is in the background; we can tweak the time limit as necessary for clients in
the wild.

[sunnyps, 2015-04-13 22:43:54]

Removed visibility updates from this CL.

PTAL.

[brianderson, 2015-04-16 01:42:23]

lgtm

https://codereview.chromium.org/1039533002/diff/60001/cc/scheduler/scheduler.h
File cc/scheduler/scheduler.h (right):

https://codereview.chromium.org/1039533002/diff/60001/cc/scheduler/scheduler....
cc/scheduler/scheduler.h:175: void SetVideoNeedsBeginFrames(bool
video_needs_begin_frames);
Group with SetChildrenNeedBeginFrames?

https://codereview.chromium.org/1039533002/diff/60001/cc/scheduler/scheduler_...
File cc/scheduler/scheduler_state_machine.cc (right):

https://codereview.chromium.org/1039533002/diff/60001/cc/scheduler/scheduler_...
cc/scheduler/scheduler_state_machine.cc:735: BeginFrameNeededForVideo() ||
ProactiveBeginFrameWanted());
To confirm: Video doesn't need BeginFrames when the Renderer isn't visible
because it has it's own timeout mechanism?

https://codereview.chromium.org/1039533002/diff/60001/cc/scheduler/scheduler_...
File cc/scheduler/scheduler_state_machine.h (right):

https://codereview.chromium.org/1039533002/diff/60001/cc/scheduler/scheduler_...
cc/scheduler/scheduler_state_machine.h:257: bool video_needs_begin_frames()
const { return video_needs_begin_frames_; }
Accesser below setter for consistency?

https://codereview.chromium.org/1039533002/diff/60001/cc/scheduler/video_fram...
File cc/scheduler/video_frame_controller.h (right):

https://codereview.chromium.org/1039533002/diff/60001/cc/scheduler/video_fram...
cc/scheduler/video_frame_controller.h:24: class CC_EXPORT VideoFrameController {
Can you add a comment indicating that we should consider moving this to a
straight up BeginFrameObserver when the BeginFrameSource supports multiple
observers and is used for children BeginFrames?

[sunnyps, 2015-04-16 20:56:11]

Thanks!

https://codereview.chromium.org/1039533002/diff/60001/cc/scheduler/scheduler.h
File cc/scheduler/scheduler.h (right):

https://codereview.chromium.org/1039533002/diff/60001/cc/scheduler/scheduler....
cc/scheduler/scheduler.h:175: void SetVideoNeedsBeginFrames(bool
video_needs_begin_frames);
On 2015/04/16 01:42:23, brianderson wrote:
> Group with SetChildrenNeedBeginFrames?

Done.

https://codereview.chromium.org/1039533002/diff/60001/cc/scheduler/scheduler_...
File cc/scheduler/scheduler_state_machine.cc (right):

https://codereview.chromium.org/1039533002/diff/60001/cc/scheduler/scheduler_...
cc/scheduler/scheduler_state_machine.cc:735: BeginFrameNeededForVideo() ||
ProactiveBeginFrameWanted());
On 2015/04/16 01:42:23, brianderson wrote:
> To confirm: Video doesn't need BeginFrames when the Renderer isn't visible
> because it has it's own timeout mechanism?

Yes, video doesn't need BeginFrames when the renderer is invisible.

https://codereview.chromium.org/1039533002/diff/60001/cc/scheduler/scheduler_...
File cc/scheduler/scheduler_state_machine.h (right):

https://codereview.chromium.org/1039533002/diff/60001/cc/scheduler/scheduler_...
cc/scheduler/scheduler_state_machine.h:257: bool video_needs_begin_frames()
const { return video_needs_begin_frames_; }
On 2015/04/16 01:42:23, brianderson wrote:
> Accesser below setter for consistency?

Done.

https://codereview.chromium.org/1039533002/diff/60001/cc/scheduler/video_fram...
File cc/scheduler/video_frame_controller.h (right):

https://codereview.chromium.org/1039533002/diff/60001/cc/scheduler/video_fram...
cc/scheduler/video_frame_controller.h:24: class CC_EXPORT VideoFrameController {
On 2015/04/16 01:42:23, brianderson wrote:
> Can you add a comment indicating that we should consider moving this to a
> straight up BeginFrameObserver when the BeginFrameSource supports multiple
> observers and is used for children BeginFrames?

Done.

[sunnyps, 2015-04-16 20:56:27]

The CQ bit was checked by sunnyps@chromium.org

[sunnyps, 2015-04-16 20:56:28]

The patchset sent to the CQ was uploaded after l-g-t-m from
brianderson@chromium.org
Link to the patchset: https://codereview.chromium.org/1039533002/#ps120001
(title: "Fix comment.")

[commit-bot: I haz the power, 2015-04-16 20:58:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1039533002/120001

[commit-bot: I haz the power, 2015-04-16 23:29:21]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-04-16 23:31:00]

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