Define frame subscription interface and implementation on Mac

Define frame subscription interface and implementation on Mac

Define RenderWidgetHostViewFrameSubscriber for listening to frame
presentation events by platform frame presenter. This will enable
a push model for generating captured frames from a RenderView.

Also implement this on Mac.

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

[ncarter (slow), 2013-02-16 02:45:22]

https://codereview.chromium.org/12277023/diff/2001/content/port/browser/rende...
File content/port/browser/render_widget_host_view_frame_subscriber.h (right):

https://codereview.chromium.org/12277023/diff/2001/content/port/browser/rende...
content/port/browser/render_widget_host_view_frame_subscriber.h:43: //
media::VideoFrame::RGB32 and media::VideoFrame::YV12 are supported.
Why support RGB32 destinations? Who is that for?

https://codereview.chromium.org/12277023/diff/2001/content/port/browser/rende...
content/port/browser/render_widget_host_view_frame_subscriber.h:58: base::Time
timestamp) = 0;
What happens in the event of errors? Seems like the subscriber needs to be
notified that the capture operation is no longer in flight.

I'd caution you against relying on the "no longer needed callback" that
VideoFrame has, because the subscriber is going to be an intermediary between
the allocator of the frames (VideoCaptureController leases shared memory to us)
and the filler of the frames (the RWHVP). So by letting the refcount fall to
zero, the RWHVP will be able tell the VideoCaptureController that a frame is
recycled, but the Subscriber might think it's still outstanding.

https://codereview.chromium.org/12277023/diff/2001/content/port/browser/rende...
File content/port/browser/render_widget_host_view_port.h (right):

https://codereview.chromium.org/12277023/diff/2001/content/port/browser/rende...
content/port/browser/render_widget_host_view_port.h:192: // Begin subscribing
for presentation events and captured frames.
My familiarity with the render_(view|widget).* code is still far from expert,
but right now this interface feels wrongly factored to me. I don't have a better
solution, but I'll offer some problems I see.

https://codereview.chromium.org/12277023/diff/2001/content/port/browser/rende...
content/port/browser/render_widget_host_view_port.h:194: // presentation thread
which might not be the UI thread.
It feels somewhat unstructured, that the code to interact with the
FrameSubscriber is going to have to be implemented on every platform, and that
the threading behavior is going to be different. Are we 100% sure that we really
can't make the "should capture frame being presented" decision on the UI thread,
and forward the decision to present thread?

https://codereview.chromium.org/12277023/diff/2001/content/port/browser/rende...
content/port/browser/render_widget_host_view_port.h:196: // TODO(hclam):
Implement this API on all platforms.
How does the user of this mechanism know whether BeginFrameSubscription is
supported on his platform?

https://codereview.chromium.org/12277023/diff/2001/content/port/browser/rende...
content/port/browser/render_widget_host_view_port.h:203:
RenderWidgetHostViewFrameSubscriber* subscriber) {}
If ownership of |subscriber| passes to the RWHVP, then isn't it impossible for
the caller to retain the subscriber pointer, to pass into this method?

[Alpha Left Google, 2013-02-16 06:39:22]

I see now the concerns are:

1. How to recycle VideoFrame back to WebContentsVideoCaptureDevice.
2. Threading.
3. Ending a subscription.

I'll address these concerns in the next patchset.

[ncarter (slow), 2013-02-16 19:13:52]

On 2013/02/16 06:39:22, Alpha wrote:
> I see now the concerns are:
> 
> 1. How to recycle VideoFrame back to WebContentsVideoCaptureDevice.
> 2. Threading.
> 3. Ending a subscription.
> 
> I'll address these concerns in the next patchset.

There's one other thing too -- which is, how does the caller decide to use frame
subscription instead of some other mechanism?

[Alpha Left Google, 2013-02-20 21:34:50]

https://codereview.chromium.org/12277023/diff/2001/content/port/browser/rende...
File content/port/browser/render_widget_host_view_frame_subscriber.h (right):

https://codereview.chromium.org/12277023/diff/2001/content/port/browser/rende...
content/port/browser/render_widget_host_view_frame_subscriber.h:43: //
media::VideoFrame::RGB32 and media::VideoFrame::YV12 are supported.
On 2013/02/16 02:45:22, ncarter wrote:
> Why support RGB32 destinations? Who is that for?

Dropped RGB32, we'll do YV12 in SW as fallback for mac initially.

https://codereview.chromium.org/12277023/diff/2001/content/port/browser/rende...
content/port/browser/render_widget_host_view_frame_subscriber.h:58: base::Time
timestamp) = 0;
On 2013/02/16 02:45:22, ncarter wrote:
> What happens in the event of errors? Seems like the subscriber needs to be
> notified that the capture operation is no longer in flight.
> 
> I'd caution you against relying on the "no longer needed callback" that
> VideoFrame has, because the subscriber is going to be an intermediary between
> the allocator of the frames (VideoCaptureController leases shared memory to
us)
> and the filler of the frames (the RWHVP). So by letting the refcount fall to
> zero, the RWHVP will be able tell the VideoCaptureController that a frame is
> recycled, but the Subscriber might think it's still outstanding.

I've changed this interface to accept a boolean. This means the frame contains
invalid content. This allows the platform layer to return a frame when it is not
able to capture a frame or the presenter is shutting down.

https://codereview.chromium.org/12277023/diff/2001/content/port/browser/rende...
File content/port/browser/render_widget_host_view_port.h (right):

https://codereview.chromium.org/12277023/diff/2001/content/port/browser/rende...
content/port/browser/render_widget_host_view_port.h:194: // presentation thread
which might not be the UI thread.
On 2013/02/16 02:45:22, ncarter wrote:
> It feels somewhat unstructured, that the code to interact with the
> FrameSubscriber is going to have to be implemented on every platform, and that
> the threading behavior is going to be different. Are we 100% sure that we
really
> can't make the "should capture frame being presented" decision on the UI
thread,
> and forward the decision to present thread?

No we can't guarantee that we can make the decision on the UI thread. In fact
even doing it on the UI thread is not the best. The best to do is to let the
amount of resources to do rate limiting and in this case the limiting factor is
on the IO thread for shared memory.

I envision it will look like this:

[ Presentation thread]  Presenter -> pull frames synchronously from -> Frame
Subscriber

[ UI thread ] Frame Subscriber -> pull frames asynchronously from -> WCVCD
                   WCVCD -> pull frames asynchronously from ->
VideoCaptureManager

[ IO Thread ] VideoCaptureManager -> pull frames synchronously from -> Shared
memory

I imagine FrameSubscriber to be a simple implementation that has:
1. An initial set of frames to fill in
2. A scoped_refptr to WCVCD and MessageLoopProxy where WCVCD lives

Once FrameSubscriber is constructed with above items the ownership is
transferred to the presenter, it is then never called on threads other than the
presentation thread. Everything is asynchronous except for the end points.

https://codereview.chromium.org/12277023/diff/2001/content/port/browser/rende...
content/port/browser/render_widget_host_view_port.h:196: // TODO(hclam):
Implement this API on all platforms.
On 2013/02/16 02:45:22, ncarter wrote:
> How does the user of this mechanism know whether BeginFrameSubscription is
> supported on his platform?

I added CanSubscribeFrame().

https://codereview.chromium.org/12277023/diff/2001/content/port/browser/rende...
content/port/browser/render_widget_host_view_port.h:203:
RenderWidgetHostViewFrameSubscriber* subscriber) {}
On 2013/02/16 02:45:22, ncarter wrote:
> If ownership of |subscriber| passes to the RWHVP, then isn't it impossible for
> the caller to retain the subscriber pointer, to pass into this method?

The ownership is transferred but WCVCD should keep a const FrameSubscriber*
const for terminating the subscription. It uses as a handle but should be never
be called by WCVCD again. I'm open to suggestions if you think there's something
better can be done here.

[Alpha Left Google, 2013-02-22 19:33:27]

+piman

Antoine: I have discussed with Nick and we are both comfortable about this
interface. Please take a look. If this is good I'll work on the mac
implementation of this API.

[piman, 2013-02-22 20:45:47]

https://codereview.chromium.org/12277023/diff/15001/content/port/browser/rend...
File content/port/browser/render_widget_host_view_frame_subscriber.h (right):

https://codereview.chromium.org/12277023/diff/15001/content/port/browser/rend...
content/port/browser/render_widget_host_view_frame_subscriber.h:28: // Objects
of this class live on the UI thread.
How will this work on non-aura windows?

https://codereview.chromium.org/12277023/diff/15001/content/port/browser/rend...
content/port/browser/render_widget_host_view_frame_subscriber.h:59: // space
conversion if needed.
So, that leaves the RWHV responsible for doing the readback. In our discussions,
for the Aura case I thought we settled on passing a texture to the receiving
renderer, that would do the readback.

The former adds quite a bit of complexity to the compositor, since it then needs
to babysit the asynchronous readback, and deal with the platform fallback.

We can chat more, but is it conceivable that for Aura we'd use the
NATIVE_TEXTURE format (or something like that)?

https://codereview.chromium.org/12277023/diff/15001/content/port/browser/rend...
content/port/browser/render_widget_host_view_frame_subscriber.h:68: gfx::Rect*
dst_subrect,
do we really need dst_subrect, or simply dst_size? Or even base it on storage's
size ?

https://codereview.chromium.org/12277023/diff/15001/content/port/browser/rend...
File content/port/browser/render_widget_host_view_port.h (right):

https://codereview.chromium.org/12277023/diff/15001/content/port/browser/rend...
content/port/browser/render_widget_host_view_port.h:193: virtual bool
CanSubscribeFrame() const { return false; }
note: all methods in this class are pure virtual. You can move the default
implementations to RenderWidgetHostViewBase

[Alpha Left Google, 2013-02-22 21:05:38]

https://codereview.chromium.org/12277023/diff/15001/content/port/browser/rend...
File content/port/browser/render_widget_host_view_frame_subscriber.h (right):

https://codereview.chromium.org/12277023/diff/15001/content/port/browser/rend...
content/port/browser/render_widget_host_view_frame_subscriber.h:28: // Objects
of this class live on the UI thread.
On 2013/02/22 20:45:47, piman wrote:
> How will this work on non-aura windows?

Present has to go through UI thread somehow, we plan to plant the output
VideoFrame here:
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...

Such that UI thread RWHVP can decide whether to have presenter do the copy.

https://codereview.chromium.org/12277023/diff/15001/content/port/browser/rend...
content/port/browser/render_widget_host_view_frame_subscriber.h:59: // space
conversion if needed.
On 2013/02/22 20:45:47, piman wrote:
> So, that leaves the RWHV responsible for doing the readback. In our
discussions,
> for the Aura case I thought we settled on passing a texture to the receiving
> renderer, that would do the readback.
> 
> The former adds quite a bit of complexity to the compositor, since it then
needs
> to babysit the asynchronous readback, and deal with the platform fallback.
> 
> We can chat more, but is it conceivable that for Aura we'd use the
> NATIVE_TEXTURE format (or something like that)?

Without our team we went back and forth with this. The conclusion is that for
Aura we should try to use the existing architecture, i.e. browser process do the
read back and hand it off to the extension renderer. This will have an extra
memcpy but simplify a whole lot as the first attempt.

https://codereview.chromium.org/12277023/diff/15001/content/port/browser/rend...
content/port/browser/render_widget_host_view_frame_subscriber.h:68: gfx::Rect*
dst_subrect,
On 2013/02/22 20:45:47, piman wrote:
> do we really need dst_subrect, or simply dst_size? Or even base it on
storage's
> size ?

Yes because of letter boxing. Implementation of FrameSubscriber can implement
letter boxing logic and RWHVP doesn't have to know this.

https://codereview.chromium.org/12277023/diff/15001/content/port/browser/rend...
File content/port/browser/render_widget_host_view_port.h (right):

https://codereview.chromium.org/12277023/diff/15001/content/port/browser/rend...
content/port/browser/render_widget_host_view_port.h:193: virtual bool
CanSubscribeFrame() const { return false; }
On 2013/02/22 20:45:47, piman wrote:
> note: all methods in this class are pure virtual. You can move the default
> implementations to RenderWidgetHostViewBase

Okay will do.

[piman, 2013-02-22 21:18:54]

https://codereview.chromium.org/12277023/diff/15001/content/port/browser/rend...
File content/port/browser/render_widget_host_view_frame_subscriber.h (right):

https://codereview.chromium.org/12277023/diff/15001/content/port/browser/rend...
content/port/browser/render_widget_host_view_frame_subscriber.h:28: // Objects
of this class live on the UI thread.
On 2013/02/22 21:05:38, Alpha wrote:
> On 2013/02/22 20:45:47, piman wrote:
> > How will this work on non-aura windows?
> 
> Present has to go through UI thread somehow,

No, it doesn't in the general case. On windows, the UI thread cannot be involved
in the presentation path from the renderer - it'd cause deadlocks.

> we plan to plant the output
> VideoFrame here:
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...

That path is only taken to patch OS-level damage (e.g. window exposure), and not
generally involved in the renderer path.

> Such that UI thread RWHVP can decide whether to have presenter do the copy.

Given the above, I don't think so...

https://codereview.chromium.org/12277023/diff/15001/content/port/browser/rend...
content/port/browser/render_widget_host_view_frame_subscriber.h:59: // space
conversion if needed.
On 2013/02/22 21:05:38, Alpha wrote:
> On 2013/02/22 20:45:47, piman wrote:
> > So, that leaves the RWHV responsible for doing the readback. In our
> discussions,
> > for the Aura case I thought we settled on passing a texture to the receiving
> > renderer, that would do the readback.
> > 
> > The former adds quite a bit of complexity to the compositor, since it then
> needs
> > to babysit the asynchronous readback, and deal with the platform fallback.
> > 
> > We can chat more, but is it conceivable that for Aura we'd use the
> > NATIVE_TEXTURE format (or something like that)?
> 
> Without our team we went back and forth with this. The conclusion is that for
> Aura we should try to use the existing architecture, i.e. browser process do
the
> read back and hand it off to the extension renderer. This will have an extra
> memcpy but simplify a whole lot as the first attempt.

"simplyfy" = maybe for you, but really it means moving the complexity to another
place.

https://codereview.chromium.org/12277023/diff/15001/content/port/browser/rend...
content/port/browser/render_widget_host_view_frame_subscriber.h:68: gfx::Rect*
dst_subrect,
On 2013/02/22 21:05:38, Alpha wrote:
> On 2013/02/22 20:45:47, piman wrote:
> > do we really need dst_subrect, or simply dst_size? Or even base it on
> storage's
> > size ?
> 
> Yes because of letter boxing. Implementation of FrameSubscriber can implement
> letter boxing logic and RWHVP doesn't have to know this.

Not sure I understand... why would we want to carry around and send over the
network a bunch of zeros? Can't letter-boxing be handled on the presentation
path?

[Alpha Left Google, 2013-02-22 22:45:45]

> No, it doesn't in the general case. On windows, the UI thread cannot be
involved
> in the presentation path from the renderer - it'd cause deadlocks.
> 
> > we plan to plant the output
> > VideoFrame here:
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...
> 
> That path is only taken to patch OS-level damage (e.g. window exposure), and
not
> generally involved in the renderer path.
> 
> > Such that UI thread RWHVP can decide whether to have presenter do the copy.
> 
> Given the above, I don't think so...

After discussing with Antoine I think we should move this object to IO thread
for Windows and Mac here:

https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/gp...

This is the place where we are sure a presentation is going to happen. Like we
discussed before this FrameSubscriber object can forward messages to wherever
WebContentsVideoCaptureDevice lives.

The same can apply to Linux and Aura. Right now these two platforms handle
GpuHostMsg_AcceleratedSurfaceBuffersSwapped on the UI thread but it's simple to
go through IO thread as well. (The message goes through GpuProcessHost anyway,
it just doesn't handle it but let UI thread handle it later).

> > Without our team we went back and forth with this. The conclusion is that
for
> > Aura we should try to use the existing architecture, i.e. browser process do
> the
> > read back and hand it off to the extension renderer. This will have an extra
> > memcpy but simplify a whole lot as the first attempt.
> 
> "simplyfy" = maybe for you, but really it means moving the complexity to
another
> place.

Looks like Antoine has strong opinion not to have browser process do the copy in
Aura. We'll leave this open for now.

> > 
> > Yes because of letter boxing. Implementation of FrameSubscriber can
implement
> > letter boxing logic and RWHVP doesn't have to know this.
> 
> Not sure I understand... why would we want to carry around and send over the
> network a bunch of zeros? Can't letter-boxing be handled on the presentation
> path?

Looks like GL doesn't support strides for glReadPixels, so having the dest
subrect will force a memcpy for some cases in the API. Drop this for now.

[ncarter (slow), 2013-02-22 23:26:57]

> Looks like GL doesn't support strides for glReadPixels, so having the dest
> subrect will force a memcpy for some cases in the API. Drop this for now.

glReadPixels supporting strides has nothing to do with dst_subrect. dst_subrect
describes a region of the VideoFrame; the VideoFrame isn't the target of
glReadPixels.

[piman, 2013-02-22 23:56:27]

On Fri, Feb 22, 2013 at 3:26 PM, <nick@chromium.org> wrote:

>  Looks like GL doesn't support strides for glReadPixels, so having the dest
>> subrect will force a memcpy for some cases in the API. Drop this for now.
>>
>
> glReadPixels supporting strides has nothing to do with dst_subrect.
> dst_subrect
> describes a region of the VideoFrame; the VideoFrame isn't the target of
> glReadPixels.
>

Then it forces an extra copy.


>
>
https://chromiumcodereview.**appspot.com/12277023/<https://chromiumcodereview...
>

[ncarter (slow), 2013-02-23 00:08:09]

On 2013/02/22 23:56:27, piman wrote:
> On Fri, Feb 22, 2013 at 3:26 PM, <mailto:nick@chromium.org> wrote:
> 
> >  Looks like GL doesn't support strides for glReadPixels, so having the dest
> >> subrect will force a memcpy for some cases in the API. Drop this for now.
> >>
> >
> > glReadPixels supporting strides has nothing to do with dst_subrect.
> > dst_subrect
> > describes a region of the VideoFrame; the VideoFrame isn't the target of
> > glReadPixels.
> >
> 
> Then it forces an extra copy.
> 
> 
> >
> >
>
https://chromiumcodereview.**appspot.com/12277023/%3Chttps://chromiumcoderevi...>
> >

Are you talking about glReadPixels for Aura, or for the Mac implementation? I
was assuming we were talking about Mac.

[ncarter (slow), 2013-02-23 00:12:45]

On 2013/02/23 00:08:09, ncarter wrote:
> On 2013/02/22 23:56:27, piman wrote:
> > On Fri, Feb 22, 2013 at 3:26 PM, <mailto:nick@chromium.org> wrote:
> > 
> > >  Looks like GL doesn't support strides for glReadPixels, so having the
dest
> > >> subrect will force a memcpy for some cases in the API. Drop this for now.
> > >>
> > >
> > > glReadPixels supporting strides has nothing to do with dst_subrect.
> > > dst_subrect
> > > describes a region of the VideoFrame; the VideoFrame isn't the target of
> > > glReadPixels.
> > >
> > 
> > Then it forces an extra copy.
> > 
> > 
> > >
> > >
> >
>
https://chromiumcodereview.**appspot.com/12277023/%253Chttps://chromiumcodere...>
> > >
> 
> Are you talking about glReadPixels for Aura, or for the Mac implementation? I
> was assuming we were talking about Mac.

(on mac the target of the readpixels is a PBO. We memcpy from there into the
VideoFrame. The memcpy can support strides. VideoFrame itself has a stride
per-plane, so dst_rect isn't the only source of stride)

[piman, 2013-02-23 00:44:32]

On Fri, Feb 22, 2013 at 4:08 PM, <nick@chromium.org> wrote:

> On 2013/02/22 23:56:27, piman wrote:
>
>  On Fri, Feb 22, 2013 at 3:26 PM, <mailto:nick@chromium.org> wrote:
>>
>
>  >  Looks like GL doesn't support strides for glReadPixels, so having the
>> dest
>> >> subrect will force a memcpy for some cases in the API. Drop this for
>> now.
>> >>
>> >
>> > glReadPixels supporting strides has nothing to do with dst_subrect.
>> > dst_subrect
>> > describes a region of the VideoFrame; the VideoFrame isn't the target of
>> > glReadPixels.
>> >
>>
>
>  Then it forces an extra copy.
>>
>
>
>  >
>> >
>>
>
> https://chromiumcodereview.**a**ppspot.com/12277023/%3Chttps:/**
>
/chromiumcodereview.appspot.**com/12277023/<http://appspot.com/12277023/%3Chttps://chromiumcodereview.appspot.com/12277023/>
> >
>
>> >
>>
>
> Are you talking about glReadPixels for Aura, or for the Mac
> implementation? I
> was assuming we were talking about Mac.
>

Talking in general, but I do have Aura in mind, yes.
If you're already burning a copy, then I guess it doesn't matter in that
case.

>
>
https://chromiumcodereview.**appspot.com/12277023/<https://chromiumcodereview...
>

[Alpha Left Google, 2013-02-23 17:00:54]

Sorry for going a full circle.

For Windows and Mac it is best for now to have the destRect, because the
presenter has to do a memcpy anyway. If destRect is not specified, then later in
the pipeline we'll have to a memcpy. I think it's best for now to have destRect
with Windows and Mac in mind. For Aura we'll revisit this interface again, it
not all clear yet how the implementation underneath for Aura is going to be.

[Alpha Left Google, 2013-03-02 02:17:24]

This is good for review now.

piman: Please look at
content/port/browser/*

thakis: Please look at
content/browser/renderer_host/*

[piman, 2013-03-02 03:26:07]

https://codereview.chromium.org/12277023/diff/30001/content/browser/renderer_...
File content/browser/renderer_host/render_widget_host_view_base.cc (right):

https://codereview.chromium.org/12277023/diff/30001/content/browser/renderer_...
content/browser/renderer_host/render_widget_host_view_base.cc:454: return true;
Should this be false?

[Alpha Left Google, 2013-03-02 05:21:24]

https://codereview.chromium.org/12277023/diff/30001/content/browser/renderer_...
File content/browser/renderer_host/render_widget_host_view_base.cc (right):

https://codereview.chromium.org/12277023/diff/30001/content/browser/renderer_...
content/browser/renderer_host/render_widget_host_view_base.cc:454: return true;
On 2013/03/02 03:26:08, piman wrote:
> Should this be false?

Right this should be false.

[piman, 2013-03-02 05:44:20]

lgtm

[Nico, 2013-03-04 08:35:22]

This looks really good. One question about why CGLSetCurrentContext(0) was
removed, other than that this looks good.

https://codereview.chromium.org/12277023/diff/26010/content/browser/renderer_...
File content/browser/renderer_host/compositing_iosurface_mac.mm (left):

https://codereview.chromium.org/12277023/diff/26010/content/browser/renderer_...
content/browser/renderer_host/compositing_iosurface_mac.mm:549:
CGLSetCurrentContext(0);
Why is this no longer needed?

https://codereview.chromium.org/12277023/diff/26010/content/browser/renderer_...
File content/browser/renderer_host/compositing_iosurface_mac.mm (right):

https://codereview.chromium.org/12277023/diff/26010/content/browser/renderer_...
content/browser/renderer_host/compositing_iosurface_mac.mm:455: base::Closure
copy_done_cb;
nit: style guide says "no abbreviations"

https://codereview.chromium.org/12277023/diff/26010/content/browser/renderer_...
content/browser/renderer_host/compositing_iosurface_mac.mm:592: // Asynchornous
copy started, return a null closure.
spelling

[Alpha Left Google, 2013-03-04 18:43:18]

https://codereview.chromium.org/12277023/diff/26010/content/browser/renderer_...
File content/browser/renderer_host/compositing_iosurface_mac.mm (left):

https://codereview.chromium.org/12277023/diff/26010/content/browser/renderer_...
content/browser/renderer_host/compositing_iosurface_mac.mm:549:
CGLSetCurrentContext(0);
On 2013/03/04 08:35:22, Nico wrote:
> Why is this no longer needed?

CopyVideoFrameInternal() should now always be called when CGL context is set, so
it's the caller's responsibility to call CGLSetCurrentContext(0).

https://codereview.chromium.org/12277023/diff/26010/content/browser/renderer_...
File content/browser/renderer_host/compositing_iosurface_mac.mm (right):

https://codereview.chromium.org/12277023/diff/26010/content/browser/renderer_...
content/browser/renderer_host/compositing_iosurface_mac.mm:455: base::Closure
copy_done_cb;
On 2013/03/04 08:35:22, Nico wrote:
> nit: style guide says "no abbreviations"

okay, done.

https://codereview.chromium.org/12277023/diff/26010/content/browser/renderer_...
content/browser/renderer_host/compositing_iosurface_mac.mm:455: base::Closure
copy_done_cb;
On 2013/03/04 08:35:22, Nico wrote:
> nit: style guide says "no abbreviations"

Done.

[Nico, 2013-03-04 18:44:49]

Ah, ok. lgtm.

[commit-bot: I haz the power, 2013-03-05 01:33:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hclam@chromium.org/12277023/24004

[commit-bot: I haz the power, 2013-03-05 06:03:01]

Change committed as 186108