cc: Implement DelegatingRender::drawFrame() method.

cc: Implement DelegatingRender::drawFrame() method.

The method sends the render passes it is given to the parent compositor via
the output surface. It should include a valid frame metadata with it.

Tests:
DelegatingRendererTestDraw.runSingleThread
DelegatingRendererTestDraw.runMultiThread
DelegatingRendererTestResources.runSingleThread
DelegatingRendererTestResources.runMultiThread

R=piman
BUG=123444
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=177073

[danakj, 2013-01-08 03:24:42]

+jamesr for webkit/

[piman, 2013-01-08 03:41:08]

LGTM!

https://codereview.chromium.org/11777025/diff/4001/cc/test/render_pass_test_c...
File cc/test/render_pass_test_common.h (right):

https://codereview.chromium.org/11777025/diff/4001/cc/test/render_pass_test_c...
cc/test/render_pass_test_common.h:30: cc::ResourceProvider*, RenderPass::Id
childPass);
nit: child_pass

[aelias_OOO_until_Jul13, 2013-01-08 03:44:46]

Metadata change lgtm

[danakj, 2013-01-08 03:53:40]

thanks for quick review!

https://codereview.chromium.org/11777025/diff/4001/cc/test/render_pass_test_c...
File cc/test/render_pass_test_common.h (right):

https://codereview.chromium.org/11777025/diff/4001/cc/test/render_pass_test_c...
cc/test/render_pass_test_common.h:30: cc::ResourceProvider*, RenderPass::Id
childPass);
On 2013/01/08 03:41:08, piman wrote:
> nit: child_pass

Done.

[piman, 2013-01-08 04:22:23]

https://codereview.chromium.org/11777025/diff/9001/content/browser/renderer_h...
File content/browser/renderer_host/render_widget_host_impl.h (right):

https://codereview.chromium.org/11777025/diff/9001/content/browser/renderer_h...
content/browser/renderer_host/render_widget_host_impl.h:559: void
OnSwapCompositorFrame(cc::CompositorFrame& frame);
Why is this const going away?

[danakj, 2013-01-08 15:28:25]

https://codereview.chromium.org/11777025/diff/9001/content/browser/renderer_h...
File content/browser/renderer_host/render_widget_host_impl.h (right):

https://codereview.chromium.org/11777025/diff/9001/content/browser/renderer_h...
content/browser/renderer_host/render_widget_host_impl.h:559: void
OnSwapCompositorFrame(cc::CompositorFrame& frame);
On 2013/01/08 04:22:23, piman wrote:
> Why is this const going away?

Because the compositor frame has scoped ptrs, and I want to record what was in
the frame for testing. To do that I either:
- Make a clone of all the render passes/resources/etc.
- Make it non-const and steal them.

I think non-const seems reasonable, when you SendToParent, you give up rights to
the stuff in it, and the OutputSurface can do what it wants with it. Do you
agree?

[jamesr, 2013-01-08 17:08:27]

On 2013/01/08 15:28:25, danakj wrote:
>
https://codereview.chromium.org/11777025/diff/9001/content/browser/renderer_h...
> File content/browser/renderer_host/render_widget_host_impl.h (right):
> 
>
https://codereview.chromium.org/11777025/diff/9001/content/browser/renderer_h...
> content/browser/renderer_host/render_widget_host_impl.h:559: void
> OnSwapCompositorFrame(cc::CompositorFrame& frame);
> On 2013/01/08 04:22:23, piman wrote:
> > Why is this const going away?
> 
> Because the compositor frame has scoped ptrs, and I want to record what was in
> the frame for testing. To do that I either:
> - Make a clone of all the render passes/resources/etc.
> - Make it non-const and steal them.
> 
> I think non-const seems reasonable, when you SendToParent, you give up rights
to
> the stuff in it, and the OutputSurface can do what it wants with it. Do you
> agree?

Use a pointer if it's mutable - chromium style is that parameters are always
const ref or pointers

[danakj, 2013-01-08 17:52:40]

On 2013/01/08 17:08:27, jamesr wrote:
> On 2013/01/08 15:28:25, danakj wrote:
> >
>
https://codereview.chromium.org/11777025/diff/9001/content/browser/renderer_h...
> > File content/browser/renderer_host/render_widget_host_impl.h (right):
> > 
> >
>
https://codereview.chromium.org/11777025/diff/9001/content/browser/renderer_h...
> > content/browser/renderer_host/render_widget_host_impl.h:559: void
> > OnSwapCompositorFrame(cc::CompositorFrame& frame);
> > On 2013/01/08 04:22:23, piman wrote:
> > > Why is this const going away?
> > 
> > Because the compositor frame has scoped ptrs, and I want to record what was
in
> > the frame for testing. To do that I either:
> > - Make a clone of all the render passes/resources/etc.
> > - Make it non-const and steal them.
> > 
> > I think non-const seems reasonable, when you SendToParent, you give up
rights
> to
> > the stuff in it, and the OutputSurface can do what it wants with it. Do you
> > agree?
> 
> Use a pointer if it's mutable - chromium style is that parameters are always
> const ref or pointers

Oooh right! I'll use scoped_ptr then.

[piman, 2013-01-08 18:06:35]

On Tue, Jan 8, 2013 at 7:28 AM, <danakj@chromium.org> wrote:

>
> https://codereview.chromium.**org/11777025/diff/9001/**
>
content/browser/renderer_host/**render_widget_host_impl.h<https://codereview.chromium.org/11777025/diff/9001/content/browser/renderer_host/render_widget_host_impl.h>
> File content/browser/renderer_host/**render_widget_host_impl.h (right):
>
> https://codereview.chromium.**org/11777025/diff/9001/**
>
content/browser/renderer_host/**render_widget_host_impl.h#**newcode559<https://codereview.chromium.org/11777025/diff/9001/content/browser/renderer_host/render_widget_host_impl.h#newcode559>
> content/browser/renderer_host/**render_widget_host_impl.h:559: void
> OnSwapCompositorFrame(cc::**CompositorFrame& frame);
> On 2013/01/08 04:22:23, piman wrote:
>
>> Why is this const going away?
>>
>
> Because the compositor frame has scoped ptrs, and I want to record what
> was in the frame for testing. To do that I either:
> - Make a clone of all the render passes/resources/etc.
> - Make it non-const and steal them.
>
> I think non-const seems reasonable, when you SendToParent, you give up
> rights to the stuff in it, and the OutputSurface can do what it wants
> with it. Do you agree?
>

I don't disagree with the OutputSurface ones, but this one is unrelated,
it's the one that gets called from the message handler in the browser
process.

Antoine


>
>
https://codereview.chromium.**org/11777025/<https://codereview.chromium.org/1...
>

[danakj, 2013-01-08 18:07:36]

On 2013/01/08 18:06:35, piman wrote:
> I don't disagree with the OutputSurface ones, but this one is unrelated,
> it's the one that gets called from the message handler in the browser
> process.

Oh crap, search+replace. Thanks, I'll revert that.

[danakj, 2013-01-08 19:06:53]

Ok, using a scoped_ptr was kinda annoying. There's no need for the
CompositorFrame to be long-lived. So just using a pointer as requested.

James can you PTAL? Thanks!

[jamesr, 2013-01-08 19:13:18]

webkit/ lgtm

[commit-bot: I haz the power, 2013-01-08 19:17:52]

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

[commit-bot: I haz the power, 2013-01-08 19:20:24]

Step "update" is always a major failure.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-01-08 19:22:55]

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

[commit-bot: I haz the power, 2013-01-08 19:32:07]

Step "update" is always a major failure.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-01-08 19:51:14]

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

[commit-bot: I haz the power, 2013-01-08 22:30:08]

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

[commit-bot: I haz the power, 2013-01-09 03:23:11]

Change committed as 175690

[commit-bot: I haz the power, 2013-01-09 05:01:20]

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

[danakj, 2013-01-09 05:01:44]

Removed the draw_quad.h header from render_pass.h

l_t_h_i.h includes render_pass.h. draw_quad.h has resource_provider.h now, and
this broke the mac build due to gl conflicts.

[commit-bot: I haz the power, 2013-01-09 07:28:25]

Retried try job too often on win_aura for step(s) content_browsertests

[commit-bot: I haz the power, 2013-01-09 14:22:48]

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

[commit-bot: I haz the power, 2013-01-09 14:23:02]

Failed to apply patch for cc/test/render_pass_test_common.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file cc/test/render_pass_test_common.h
  Hunk #1 FAILED at 19.
  1 out of 1 hunk FAILED -- saving rejects to file
cc/test/render_pass_test_common.h.rej

Patch:       cc/test/render_pass_test_common.h
Index: cc/test/render_pass_test_common.h
diff --git a/cc/test/render_pass_test_common.h
b/cc/test/render_pass_test_common.h
index
c565df022d82ed1c3d07fb57d8f01daca239d64b..6bc765db344bbbdf675dcfe12917400b08114cb5
100644
--- a/cc/test/render_pass_test_common.h
+++ b/cc/test/render_pass_test_common.h
@@ -19,14 +19,11 @@ class TestRenderPass : public cc::RenderPass {
     return make_scoped_ptr(new TestRenderPass);
   }
 
-  void AppendQuad(scoped_ptr<cc::DrawQuad> quad) {
-    quad_list.append(quad.Pass());
-  }
-  void AppendSharedQuadState(scoped_ptr<cc::SharedQuadState> state) {
-    shared_quad_state_list.append(state.Pass());
-  }
+  void AppendQuad(scoped_ptr<cc::DrawQuad> quad);
+  void AppendSharedQuadState(scoped_ptr<cc::SharedQuadState> state);
 
-  void AppendOneOfEveryQuadType(cc::ResourceProvider*);
+  void AppendOneOfEveryQuadType(
+      cc::ResourceProvider*, RenderPass::Id child_pass);
 
  protected:
   TestRenderPass() : RenderPass() {}

[commit-bot: I haz the power, 2013-01-09 14:31:16]

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

[commit-bot: I haz the power, 2013-01-09 14:49:07]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_clang_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...
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.

[slavi, 2013-01-09 20:58:49]

https://codereview.chromium.org/11777025/diff/31050/cc/direct_renderer.cc
File cc/direct_renderer.cc (right):

https://codereview.chromium.org/11777025/diff/31050/cc/direct_renderer.cc#new...
cc/direct_renderer.cc:11: #include "cc/draw_quad.h"
Is this include used?

[danakj, 2013-01-09 21:01:16]

https://codereview.chromium.org/11777025/diff/31050/cc/direct_renderer.cc
File cc/direct_renderer.cc (right):

https://codereview.chromium.org/11777025/diff/31050/cc/direct_renderer.cc#new...
cc/direct_renderer.cc:11: #include "cc/draw_quad.h"
On 2013/01/09 20:58:49, slavi wrote:
> Is this include used?

Well, it's here cuz of compile errors. It could perhaps be a forward
declaration, but is it worth that in a .cc?

[slavi, 2013-01-09 21:29:57]

On 2013/01/09 21:01:16, danakj wrote:
> https://codereview.chromium.org/11777025/diff/31050/cc/direct_renderer.cc
> File cc/direct_renderer.cc (right):
> 
>
https://codereview.chromium.org/11777025/diff/31050/cc/direct_renderer.cc#new...
> cc/direct_renderer.cc:11: #include "cc/draw_quad.h"
> On 2013/01/09 20:58:49, slavi wrote:
> > Is this include used?
> 
> Well, it's here cuz of compile errors. It could perhaps be a forward
> declaration, but is it worth that in a .cc?

I see. Nevermind, it was just a nit.

[commit-bot: I haz the power, 2013-01-09 22:13:38]

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

[danakj, 2013-01-10 00:47:12]

erm :(

[ RUN      ] DelegatingRendererTestResources.runSingleThread
[ERROR:io_surface_draw_quad.cc(53)] Not implemented reached in virtual void
cc::IOSurfaceDrawQuad::AppendResources(cc::ResourceProvider::ResourceIdArray*)
[ERROR:stream_video_draw_quad.cc(46)] Not implemented reached in virtual void
cc::StreamVideoDrawQuad::AppendResources(cc::ResourceProvider::ResourceIdArray*)
[ERROR:io_surface_draw_quad.cc(53)] Not implemented reached in virtual void
cc::IOSurfaceDrawQuad::AppendResources(cc::ResourceProvider::ResourceIdArray*)
[ERROR:stream_video_draw_quad.cc(46)] Not implemented reached in virtual void
cc::StreamVideoDrawQuad::AppendResources(cc::ResourceProvider::ResourceIdArray*)
[FATAL:resource_provider.cc(737)] Check failed: source->allocated. 
[FATAL:resource_provider.cc(737)] Check failed: source->allocated. 
[ CRASHED      ]

[commit-bot: I haz the power, 2013-01-11 03:53:15]

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

[commit-bot: I haz the power, 2013-01-11 03:53:32]

Failed to apply patch for cc/layer_tree_host_impl.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file cc/layer_tree_host_impl.cc
  Hunk #1 FAILED at 859.
  Hunk #2 succeeded at 997 (offset -81 lines).
  1 out of 2 hunks FAILED -- saving rejects to file
cc/layer_tree_host_impl.cc.rej

Patch:       cc/layer_tree_host_impl.cc
Index: cc/layer_tree_host_impl.cc
diff --git a/cc/layer_tree_host_impl.cc b/cc/layer_tree_host_impl.cc
index
44b305eb7e600738796578a9be697f76f044978b..5e431b4daed21696e16bf71f7a05f854e6efa878
100644
--- a/cc/layer_tree_host_impl.cc
+++ b/cc/layer_tree_host_impl.cc
@@ -859,19 +859,20 @@ void
LayerTreeHostImpl::OnCanDrawStateChangedForTree(LayerTreeImpl*)
 
 CompositorFrameMetadata LayerTreeHostImpl::makeCompositorFrameMetadata() const
 {
-    if (!rootScrollLayer())
-        return CompositorFrameMetadata();
-
     CompositorFrameMetadata metadata;
-    metadata.root_scroll_offset = rootScrollLayer()->scrollOffset() +
rootScrollLayer()->scrollDelta();
-    if (!m_settings.pageScalePinchZoomEnabled)
-        metadata.root_scroll_offset.Scale(1 /
m_pinchZoomViewport.pageScaleFactor());
     metadata.page_scale_factor = m_pinchZoomViewport.totalPageScaleFactor();
     metadata.viewport_size = m_pinchZoomViewport.bounds().size();
     metadata.root_layer_size = contentSize();
     metadata.min_page_scale_factor = m_pinchZoomViewport.minPageScaleFactor();
     metadata.max_page_scale_factor = m_pinchZoomViewport.maxPageScaleFactor();
 
+    if (!rootScrollLayer())
+      return metadata;
+
+    metadata.root_scroll_offset = rootScrollLayer()->scrollOffset() +
rootScrollLayer()->scrollDelta();
+    if (!m_settings.pageScalePinchZoomEnabled)
+        metadata.root_scroll_offset.Scale(1 /
m_pinchZoomViewport.pageScaleFactor());
+
     return metadata;
 }
 
@@ -1078,7 +1079,7 @@ bool
LayerTreeHostImpl::initializeRenderer(scoped_ptr<OutputSurface> outputSurfa
       m_tileManager.reset(new TileManager(this, resourceProvider.get(),
m_settings.numRasterThreads));
 
     if (outputSurface->Capabilities().has_parent_compositor)
-        m_renderer = DelegatingRenderer::Create(this, resourceProvider.get());
+        m_renderer = DelegatingRenderer::Create(this, outputSurface.get(),
resourceProvider.get());
     else if (outputSurface->Context3D())
         m_renderer = GLRenderer::create(this, outputSurface.get(),
resourceProvider.get());
     else if (outputSurface->SoftwareDevice())

[commit-bot: I haz the power, 2013-01-16 01:10:32]

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

[commit-bot: I haz the power, 2013-01-16 03:27:34]

Retried try job too often on win for step(s) compile