Run all LayerTreeHost tests with a delegating renderer.

cc: Run all LayerTreeHost tests with a delegating renderer.

Unless they choose to opt out.

The VideoResourceUpdater was deleting resources when the ResourceProvider
shut down, which was not ideal. Instead, it deletes them when
it shuts down itself (before the ResourceProvider). This works
out fine, since anyone else using it will be using it through a
mailbox.

DelegatingRenderer needs to use and export the resources in the
quads it is given inside of DrawFrame() as they may be deleted
afterward, instead of waiting until SwapBuffers().

Depends on: https://codereview.chromium.org/14960006/

TEST=cc_unittests
R=jamesr, piman
BUG=123444
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=199496

[piman, 2013-05-08 19:24:36]

https://codereview.chromium.org/14925009/diff/4001/cc/layers/heads_up_display...
File cc/layers/heads_up_display_layer_impl.cc (right):

https://codereview.chromium.org/14925009/diff/4001/cc/layers/heads_up_display...
cc/layers/heads_up_display_layer_impl.cc:173: }
Looks like this function doesn't do anything anymore. Remove?

https://codereview.chromium.org/14925009/diff/4001/cc/output/delegating_rende...
File cc/output/delegating_renderer.cc (right):

https://codereview.chromium.org/14925009/diff/4001/cc/output/delegating_rende...
cc/output/delegating_renderer.cc:154:
resource_provider_->PrepareSendToParent(resources, &out_data.resource_list);
I really meant for this to happen in SwapBuffers so that we don't pack resources
into mailboxes that we won't need anyway if we're not going to send that frame.

https://codereview.chromium.org/14925009/diff/4001/cc/trees/layer_tree_host_u...
File cc/trees/layer_tree_host_unittest_delegated.cc (right):

https://codereview.chromium.org/14925009/diff/4001/cc/trees/layer_tree_host_u...
cc/trees/layer_tree_host_unittest_delegated.cc:506:
SINGLE_AND_MULTI_THREAD_DIRECT_RENDERER_TEST_F(
qq: what are the failures types in these tests with the delegating renderer?
We'll definitely want at some point to chain them, for e.g. <webview> and oop
iframes.

[danakj, 2013-05-08 19:28:03]

https://codereview.chromium.org/14925009/diff/4001/cc/output/delegating_rende...
File cc/output/delegating_renderer.cc (right):

https://codereview.chromium.org/14925009/diff/4001/cc/output/delegating_rende...
cc/output/delegating_renderer.cc:154:
resource_provider_->PrepareSendToParent(resources, &out_data.resource_list);
On 2013/05/08 19:24:36, piman wrote:
> I really meant for this to happen in SwapBuffers so that we don't pack
resources
> into mailboxes that we won't need anyway if we're not going to send that
frame.

Ah, hm. I moved it so tests would be symmetrical in that draw uses the same
number of textures regardless.

We should really not draw if we're not going to send the frame either..

But I can move this back to swap, and change the test to see how many textures
were used after swap instead of after draw.

https://codereview.chromium.org/14925009/diff/4001/cc/trees/layer_tree_host_u...
File cc/trees/layer_tree_host_unittest_delegated.cc (right):

https://codereview.chromium.org/14925009/diff/4001/cc/trees/layer_tree_host_u...
cc/trees/layer_tree_host_unittest_delegated.cc:506:
SINGLE_AND_MULTI_THREAD_DIRECT_RENDERER_TEST_F(
On 2013/05/08 19:24:36, piman wrote:
> qq: what are the failures types in these tests with the delegating renderer?
> We'll definitely want at some point to chain them, for e.g. <webview> and oop
> iframes.

Basically, when we send a frame with the FakeOutputSurface, the resources are
gone forever. We need some test apis on there to get resources sent back at the
right times.

[piman, 2013-05-08 19:35:15]

https://codereview.chromium.org/14925009/diff/4001/cc/output/delegating_rende...
File cc/output/delegating_renderer.cc (right):

https://codereview.chromium.org/14925009/diff/4001/cc/output/delegating_rende...
cc/output/delegating_renderer.cc:154:
resource_provider_->PrepareSendToParent(resources, &out_data.resource_list);
On 2013/05/08 19:28:03, danakj wrote:
> On 2013/05/08 19:24:36, piman wrote:
> > I really meant for this to happen in SwapBuffers so that we don't pack
> resources
> > into mailboxes that we won't need anyway if we're not going to send that
> frame.
> 
> Ah, hm. I moved it so tests would be symmetrical in that draw uses the same
> number of textures regardless.

Trying to understand: which test, and symmetrical wrt what?

> 
> We should really not draw if we're not going to send the frame either..

I guess it's a bit awkward here since we're not planning to support
CompositeAndReadback (at least as is), but GLRenderer does draw without sending
the frame in that case.
> 
> But I can move this back to swap, and change the test to see how many textures
> were used after swap instead of after draw.

https://codereview.chromium.org/14925009/diff/4001/cc/trees/layer_tree_host_u...
File cc/trees/layer_tree_host_unittest_delegated.cc (right):

https://codereview.chromium.org/14925009/diff/4001/cc/trees/layer_tree_host_u...
cc/trees/layer_tree_host_unittest_delegated.cc:506:
SINGLE_AND_MULTI_THREAD_DIRECT_RENDERER_TEST_F(
On 2013/05/08 19:28:03, danakj wrote:
> On 2013/05/08 19:24:36, piman wrote:
> > qq: what are the failures types in these tests with the delegating renderer?
> > We'll definitely want at some point to chain them, for e.g. <webview> and
oop
> > iframes.
> 
> Basically, when we send a frame with the FakeOutputSurface, the resources are
> gone forever. We need some test apis on there to get resources sent back at
the
> right times.

Got it, thanks. Sounds like a good candidate for a TODO and a bug.

[danakj, 2013-05-08 19:38:22]

https://codereview.chromium.org/14925009/diff/4001/cc/output/delegating_rende...
File cc/output/delegating_renderer.cc (right):

https://codereview.chromium.org/14925009/diff/4001/cc/output/delegating_rende...
cc/output/delegating_renderer.cc:154:
resource_provider_->PrepareSendToParent(resources, &out_data.resource_list);
On 2013/05/08 19:35:16, piman wrote:
> On 2013/05/08 19:28:03, danakj wrote:
> > On 2013/05/08 19:24:36, piman wrote:
> > > I really meant for this to happen in SwapBuffers so that we don't pack
> > resources
> > > into mailboxes that we won't need anyway if we're not going to send that
> > frame.
> > 
> > Ah, hm. I moved it so tests would be symmetrical in that draw uses the same
> > number of textures regardless.
> 
> Trying to understand: which test, and symmetrical wrt what?

LayerTreeHostTestAtomicCommit for example. Symmetrical to direct rendering.

ie. So when you finish DrawLayers in a unit test, the textures being used for
that frame will have been used. DrawLayers does the textury things. Swap just
does the shipping.

> 
> > 
> > We should really not draw if we're not going to send the frame either..
> 
> I guess it's a bit awkward here since we're not planning to support
> CompositeAndReadback (at least as is), but GLRenderer does draw without
sending
> the frame in that case.

Yeh, we don't really support any code path with delegating renderer that won't
swap, so it's a bit fuzzy what's right.

> > 
> > But I can move this back to swap, and change the test to see how many
textures
> > were used after swap instead of after draw.

[piman, 2013-05-08 19:47:00]

https://codereview.chromium.org/14925009/diff/4001/cc/output/delegating_rende...
File cc/output/delegating_renderer.cc (right):

https://codereview.chromium.org/14925009/diff/4001/cc/output/delegating_rende...
cc/output/delegating_renderer.cc:154:
resource_provider_->PrepareSendToParent(resources, &out_data.resource_list);
On 2013/05/08 19:38:23, danakj wrote:
> On 2013/05/08 19:35:16, piman wrote:
> > On 2013/05/08 19:28:03, danakj wrote:
> > > On 2013/05/08 19:24:36, piman wrote:
> > > > I really meant for this to happen in SwapBuffers so that we don't pack
> > > resources
> > > > into mailboxes that we won't need anyway if we're not going to send that
> > > frame.
> > > 
> > > Ah, hm. I moved it so tests would be symmetrical in that draw uses the
same
> > > number of textures regardless.
> > 
> > Trying to understand: which test, and symmetrical wrt what?
> 
> LayerTreeHostTestAtomicCommit for example. Symmetrical to direct rendering.

I see, we're trying to detect which texture is "used" by seeing if it gets bound
on the context, which happens, naturally, in GLRenderer when we draw it, and as
a side effect of sending it to the parent in the DelegatingRenderer case.
The latter is fragile though, because if we "draw" it in 2 successive frames,
we'll only send it once. The test you mentioned currently avoids the case but I
don't think it's a given...


Mmh, looking closer at this test in particular, I think it's trying to check
which textures are used for upload, which I think is orthogonal to this though.

> 
> ie. So when you finish DrawLayers in a unit test, the textures being used for
> that frame will have been used. DrawLayers does the textury things. Swap just
> does the shipping.
> 
> > 
> > > 
> > > We should really not draw if we're not going to send the frame either..
> > 
> > I guess it's a bit awkward here since we're not planning to support
> > CompositeAndReadback (at least as is), but GLRenderer does draw without
> sending
> > the frame in that case.
> 
> Yeh, we don't really support any code path with delegating renderer that won't
> swap, so it's a bit fuzzy what's right.
> 
> > > 
> > > But I can move this back to swap, and change the test to see how many
> textures
> > > were used after swap instead of after draw.
>

[danakj, 2013-05-08 23:35:30]

https://codereview.chromium.org/14925009/diff/4001/cc/output/delegating_rende...
File cc/output/delegating_renderer.cc (right):

https://codereview.chromium.org/14925009/diff/4001/cc/output/delegating_rende...
cc/output/delegating_renderer.cc:154:
resource_provider_->PrepareSendToParent(resources, &out_data.resource_list);
On 2013/05/08 19:47:00, piman wrote:
> On 2013/05/08 19:38:23, danakj wrote:
> > On 2013/05/08 19:35:16, piman wrote:
> > > On 2013/05/08 19:28:03, danakj wrote:
> > > > On 2013/05/08 19:24:36, piman wrote:
> > > > > I really meant for this to happen in SwapBuffers so that we don't pack
> > > > resources
> > > > > into mailboxes that we won't need anyway if we're not going to send
that
> > > > frame.
> > > > 
> > > > Ah, hm. I moved it so tests would be symmetrical in that draw uses the
> same
> > > > number of textures regardless.
> > > 
> > > Trying to understand: which test, and symmetrical wrt what?
> > 
> > LayerTreeHostTestAtomicCommit for example. Symmetrical to direct rendering.
> 
> I see, we're trying to detect which texture is "used" by seeing if it gets
bound
> on the context, which happens, naturally, in GLRenderer when we draw it, and
as
> a side effect of sending it to the parent in the DelegatingRenderer case.
> The latter is fragile though, because if we "draw" it in 2 successive frames,
> we'll only send it once. The test you mentioned currently avoids the case but
I
> don't think it's a given...

Ya, for sure, this test is making sure that two commits with dirty state use
different textures. I think that's valid here.

> 
> Mmh, looking closer at this test in particular, I think it's trying to check
> which textures are used for upload, which I think is orthogonal to this
though.

That's true, but I think it could equally check which textures are being sent to
the parent compositor, in the same way.

> 
> > 
> > ie. So when you finish DrawLayers in a unit test, the textures being used
for
> > that frame will have been used. DrawLayers does the textury things. Swap
just
> > does the shipping.
> > 
> > > 
> > > > 
> > > > We should really not draw if we're not going to send the frame either..
> > > 
> > > I guess it's a bit awkward here since we're not planning to support
> > > CompositeAndReadback (at least as is), but GLRenderer does draw without
> > sending
> > > the frame in that case.
> > 
> > Yeh, we don't really support any code path with delegating renderer that
won't
> > swap, so it's a bit fuzzy what's right.
> > 
> > > > 
> > > > But I can move this back to swap, and change the test to see how many
> > textures
> > > > were used after swap instead of after draw.
> > 
>

[danakj, 2013-05-08 23:43:46]

PTAL

https://codereview.chromium.org/14925009/diff/4001/cc/layers/heads_up_display...
File cc/layers/heads_up_display_layer_impl.cc (right):

https://codereview.chromium.org/14925009/diff/4001/cc/layers/heads_up_display...
cc/layers/heads_up_display_layer_impl.cc:173: }
On 2013/05/08 19:24:36, piman wrote:
> Looks like this function doesn't do anything anymore. Remove?

Done.

https://codereview.chromium.org/14925009/diff/4001/cc/trees/layer_tree_host_u...
File cc/trees/layer_tree_host_unittest_delegated.cc (right):

https://codereview.chromium.org/14925009/diff/4001/cc/trees/layer_tree_host_u...
cc/trees/layer_tree_host_unittest_delegated.cc:506:
SINGLE_AND_MULTI_THREAD_DIRECT_RENDERER_TEST_F(
On 2013/05/08 19:35:16, piman wrote:
> On 2013/05/08 19:28:03, danakj wrote:
> > On 2013/05/08 19:24:36, piman wrote:
> > > qq: what are the failures types in these tests with the delegating
renderer?
> > > We'll definitely want at some point to chain them, for e.g. <webview> and
> oop
> > > iframes.
> > 
> > Basically, when we send a frame with the FakeOutputSurface, the resources
are
> > gone forever. We need some test apis on there to get resources sent back at
> the
> > right times.
> 
> Got it, thanks. Sounds like a good candidate for a TODO and a bug.

Ya, true. https://code.google.com/p/chromium/issues/detail?id=239290

Added TODOs

[piman, 2013-05-08 23:49:49]

lgtm

[commit-bot: I haz the power, 2013-05-09 00:06:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/14925009/22001

[commit-bot: I haz the power, 2013-05-09 00:31:33]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_chromeos_clang.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-05-09 00:47:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/14925009/30001

[danakj, 2013-05-09 02:42:50]

PTAL.

I missed a few CreateOutputSurface() overrides.

https://codereview.chromium.org/14925009/diff/41001/cc/output/delegating_rend...
File cc/output/delegating_renderer.cc (right):

https://codereview.chromium.org/14925009/diff/41001/cc/output/delegating_rend...
cc/output/delegating_renderer.cc:155:
resource_provider_->PrepareSendToParent(resources, &out_data.resource_list);
DrawFrame needs to make this call. The resources here may not exist by the time
it gets to SwapBuffers if they don't get exported, as DidDraw() will delete the
resource it ships in the quad - it expects the quads to be consumed inside of
draw.

https://codereview.chromium.org/14925009/diff/41001/cc/resources/resource_pro...
File cc/resources/resource_provider.cc (right):

https://codereview.chromium.org/14925009/diff/41001/cc/resources/resource_pro...
cc/resources/resource_provider.cc:307: if (style == ForShutdown)
When the RP shuts down, it calls mailbox callbacks. The callback in
VideoResourceUpdater wants to delete the resource, which DCHECKs in here that
the resource exists still.

By not deleting lost resources, we can avoid this cycle and know that the RP is
gone.

I wasn't sure how else to deal with this, any better idea?

[danakj, 2013-05-09 02:43:11]

https://codereview.chromium.org/14925009/diff/41001/cc/resources/resource_pro...
File cc/resources/resource_provider.cc (right):

https://codereview.chromium.org/14925009/diff/41001/cc/resources/resource_pro...
cc/resources/resource_provider.cc:307: if (style == ForShutdown)
On 2013/05/09 02:42:50, danakj wrote:
> When the RP shuts down, it calls mailbox callbacks. The callback in
> VideoResourceUpdater wants to delete the resource, which DCHECKs in here that
> the resource exists still.
> 
> By not deleting lost resources, we can avoid this cycle and know that the RP
is
> gone.
> 
> I wasn't sure how else to deal with this, any better idea?

Oh, this we talked about and I reverted/fixed.

[piman, 2013-05-09 20:38:47]

We can't ignore CompositeAndReadback and pretend it doesn't exist...

https://codereview.chromium.org/14925009/diff/50002/cc/output/delegating_rend...
File cc/output/delegating_renderer.cc (right):

https://codereview.chromium.org/14925009/diff/50002/cc/output/delegating_rend...
cc/output/delegating_renderer.cc:136:
DCHECK(!frame_for_swap_buffers_.delegated_frame_data);
This DCHECK is not correct. If CompositeAndReadback is called, DrawFrame is
called without SwapBuffers, so nothing will have taken out delegated_frame_data.

It also means we'll leak resources in that case, and send a frame to the parent
with inexistent resources.

https://codereview.chromium.org/14925009/diff/50002/cc/output/delegating_rend...
cc/output/delegating_renderer.cc:166: NOTREACHED();
This is actually reached. The browser will have the renderer call it if the
in-browser thumbnailer fails (which it does currently). I also believe there's
an extension API that calls this. NOTIMPLEMENTED() is right.

https://codereview.chromium.org/14925009/diff/50002/cc/resources/video_resour...
File cc/resources/video_resource_updater.cc (right):

https://codereview.chromium.org/14925009/diff/50002/cc/resources/video_resour...
cc/resources/video_resource_updater.cc:33:
resource_provider_->DeleteResource(recycled_resources_.back().resource_id);
Don't you need to delete all resources here, rather than just the ones that came
back?

https://codereview.chromium.org/14925009/diff/50002/cc/trees/layer_tree_host_...
File cc/trees/layer_tree_host_unittest.cc (right):

https://codereview.chromium.org/14925009/diff/50002/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_unittest.cc:2829: layer_tree_host()->SetNeedsCommit();
The entire point of this test was to make sure CompositeAndReadback kept a
consistent state wrt resources/frames in flight/etc.

[danakj, 2013-05-09 22:42:37]

PTAL

https://codereview.chromium.org/14925009/diff/50002/cc/output/delegating_rend...
File cc/output/delegating_renderer.cc (right):

https://codereview.chromium.org/14925009/diff/50002/cc/output/delegating_rend...
cc/output/delegating_renderer.cc:136:
DCHECK(!frame_for_swap_buffers_.delegated_frame_data);
On 2013/05/09 20:38:47, piman wrote:
> This DCHECK is not correct. If CompositeAndReadback is called, DrawFrame is
> called without SwapBuffers, so nothing will have taken out
delegated_frame_data.
> 
> It also means we'll leak resources in that case, and send a frame to the
parent
> with inexistent resources.

This should be solved by https://codereview.chromium.org/14960006/

https://codereview.chromium.org/14925009/diff/50002/cc/output/delegating_rend...
cc/output/delegating_renderer.cc:166: NOTREACHED();
On 2013/05/09 20:38:47, piman wrote:
> This is actually reached. The browser will have the renderer call it if the
> in-browser thumbnailer fails (which it does currently). I also believe there's
> an extension API that calls this. NOTIMPLEMENTED() is right.

Similar by https://codereview.chromium.org/14960006/

https://codereview.chromium.org/14925009/diff/50002/cc/resources/video_resour...
File cc/resources/video_resource_updater.cc (right):

https://codereview.chromium.org/14925009/diff/50002/cc/resources/video_resour...
cc/resources/video_resource_updater.cc:33:
resource_provider_->DeleteResource(recycled_resources_.back().resource_id);
On 2013/05/09 20:38:47, piman wrote:
> Don't you need to delete all resources here, rather than just the ones that
came
> back?

Oh gosh, yes, thanks. Since the early out in Recycle now..

Also noticed that we should have the same early out in ReturnResource. That path
just isn't hit by the unit tests yet until we have mailbox videoframes.

I added all_resources_ to the VRU, and delete everything on destruction now.

https://codereview.chromium.org/14925009/diff/50002/cc/trees/layer_tree_host_...
File cc/trees/layer_tree_host_unittest.cc (right):

https://codereview.chromium.org/14925009/diff/50002/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_unittest.cc:2829: layer_tree_host()->SetNeedsCommit();
On 2013/05/09 20:38:47, piman wrote:
> The entire point of this test was to make sure CompositeAndReadback kept a
> consistent state wrt resources/frames in flight/etc.

I've restored this.

[piman, 2013-05-09 22:58:07]

LGTM, thanks

[commit-bot: I haz the power, 2013-05-10 12:35:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/14925009/74001

[commit-bot: I haz the power, 2013-05-10 12:35:34]

Failed to apply patch for cc/output/delegating_renderer.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file cc/output/delegating_renderer.cc
  Hunk #1 FAILED at 11.
  Hunk #2 succeeded at 124 with fuzz 2 (offset 1 line).
  Hunk #3 FAILED at 135.
  Hunk #4 FAILED at 160.
  3 out of 4 hunks FAILED -- saving rejects to file
cc/output/delegating_renderer.cc.rej

Patch:       cc/output/delegating_renderer.cc
Index: cc/output/delegating_renderer.cc
diff --git a/cc/output/delegating_renderer.cc b/cc/output/delegating_renderer.cc
index
2ee755a8a5dbafebc6b3044c3e103fbe730d76ff..c06536e0e73de03662d31c9ea6b907e8ac6a0e34
100644
--- a/cc/output/delegating_renderer.cc
+++ b/cc/output/delegating_renderer.cc
@@ -11,7 +11,6 @@
 #include "base/debug/trace_event.h"
 #include "base/string_util.h"
 #include "base/strings/string_split.h"
-#include "cc/output/compositor_frame.h"
 #include "cc/output/compositor_frame_ack.h"
 #include "cc/quads/checkerboard_draw_quad.h"
 #include "cc/quads/debug_border_draw_quad.h"
@@ -123,6 +122,8 @@ const RendererCapabilities&
DelegatingRenderer::Capabilities() const {
   return capabilities_;
 }
 
+bool DelegatingRenderer::CanReadPixels() const { return false; }
+
 static ResourceProvider::ResourceId AppendToArray(
     ResourceProvider::ResourceIdArray* array,
     ResourceProvider::ResourceId id) {
@@ -133,20 +134,16 @@ static ResourceProvider::ResourceId AppendToArray(
 void DelegatingRenderer::DrawFrame(
     RenderPassList* render_passes_in_draw_order) {
   TRACE_EVENT0("cc", "DelegatingRenderer::DrawFrame");
-  render_passes_for_swap_buffers_.swap(*render_passes_in_draw_order);
-  render_passes_in_draw_order->clear();
-}
 
-void DelegatingRenderer::SwapBuffers(const LatencyInfo& latency_info) {
-  TRACE_EVENT0("cc", "DelegatingRenderer::SwapBuffers");
+  DCHECK(!frame_for_swap_buffers_.delegated_frame_data);
 
-  CompositorFrame out_frame;
-  out_frame.metadata = client_->MakeCompositorFrameMetadata();
+  frame_for_swap_buffers_.metadata = client_->MakeCompositorFrameMetadata();
 
-  out_frame.delegated_frame_data = make_scoped_ptr(new DelegatedFrameData);
-  DelegatedFrameData& out_data = *out_frame.delegated_frame_data;
+  frame_for_swap_buffers_.delegated_frame_data =
+      make_scoped_ptr(new DelegatedFrameData);
+  DelegatedFrameData& out_data = *frame_for_swap_buffers_.delegated_frame_data;
   // Move the render passes and resources into the |out_frame|.
-  out_data.render_pass_list.swap(render_passes_for_swap_buffers_);
+  out_data.render_pass_list.swap(*render_passes_in_draw_order);
 
   // Collect all resource ids in the render passes into a ResourceIdArray.
   ResourceProvider::ResourceIdArray resources;
@@ -158,12 +155,17 @@ void DelegatingRenderer::SwapBuffers(const LatencyInfo&
latency_info) {
       render_pass->quad_list[j]->IterateResources(append_to_array);
   }
   resource_provider_->PrepareSendToParent(resources, &out_data.resource_list);
+}
+
+void DelegatingRenderer::SwapBuffers(const LatencyInfo& latency_info) {
+  TRACE_EVENT0("cc", "DelegatingRenderer::SwapBuffers");
 
-  output_surface_->SendFrameToParentCompositor(&out_frame);
+  output_surface_->SendFrameToParentCompositor(&frame_for_swap_buffers_);
+  DCHECK(!frame_for_swap_buffers_.delegated_frame_data);
 }
 
 void DelegatingRenderer::GetFramebufferPixels(void* pixels, gfx::Rect rect) {
-  NOTIMPLEMENTED();
+  NOTREACHED();
 }
 
 void DelegatingRenderer::ReceiveCompositorFrameAck(

[commit-bot: I haz the power, 2013-05-10 13:29:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/14925009/83001

[commit-bot: I haz the power, 2013-05-10 13:49:11]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_chromeos_clang.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-05-10 14:21:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/14925009/92001

[commit-bot: I haz the power, 2013-05-10 14:32:50]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...

[commit-bot: I haz the power, 2013-05-10 14:46:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/14925009/92001

[commit-bot: I haz the power, 2013-05-10 16:54:37]

Change committed as 199496