Moving RenderSurface creation outside of CalcDrawProps

cc/trees/damage_tracker_unittest.cc

Index: cc/trees/damage_tracker_unittest.cc
diff --git a/cc/trees/damage_tracker_unittest.cc b/cc/trees/damage_tracker_unittest.cc
index b6a44094dc69c5ef0c0e4f1edff4f5fac7e68dc1..7eaa2f93ba105e287f12085e529ecff72d1f469c 100644
--- a/cc/trees/damage_tracker_unittest.cc
+++ b/cc/trees/damage_tracker_unittest.cc
@@ -86,7 +86,7 @@ class DamageTrackerTest : public testing::Test {
     root->SetBounds(gfx::Size(500, 500));
     root->SetContentBounds(gfx::Size(500, 500));
     root->SetDrawsContent(true);
-    root->CreateRenderSurface();
+    root->SetHasRenderSurface(true);
     root->render_surface()->SetContentRect(gfx::Rect(0, 0, 500, 500));
 
     child->SetPosition(gfx::PointF(100.f, 100.f));
@@ -118,7 +118,7 @@ class DamageTrackerTest : public testing::Test {
     root->SetBounds(gfx::Size(500, 500));
     root->SetContentBounds(gfx::Size(500, 500));
     root->SetDrawsContent(true);
-    root->CreateRenderSurface();
+    root->SetHasRenderSurface(true);
     root->render_surface()->SetContentRect(gfx::Rect(0, 0, 500, 500));
 
     child1->SetPosition(gfx::PointF(100.f, 100.f));
@@ -130,7 +130,7 @@ class DamageTrackerTest : public testing::Test {
     // ForceRenderSurface may be redundant here.
     child1->SetOpacity(0.5f);

[danakj, 2014/09/03 19:30:33]

does the opacity really matter anymore? do we need to bother setting it?

[awoloszyn, 2014/09/09 15:31:36]

Done.

     child1->SetDrawsContent(false);
-    child1->SetForceRenderSurface(true);
+    child1->SetHasRenderSurface(true);
 
     child2->SetPosition(gfx::PointF(11.f, 11.f));
     child2->SetBounds(gfx::Size(18, 18));
@@ -141,6 +141,7 @@ class DamageTrackerTest : public testing::Test {
     grand_child1->SetBounds(gfx::Size(6, 8));
     grand_child1->SetContentBounds(gfx::Size(6, 8));
     grand_child1->SetDrawsContent(true);
+    grand_child1->SetHasRenderSurface(true);

[danakj, 2014/09/03 19:30:34]

The comment at the top of this method suggests this shouldn't have a surface.
Why did you add one?

[awoloszyn, 2014/09/09 15:31:36]

This was an over-eager fix to some other tests. I have moved the creation into
the appropriate tests.

 
     grand_child2->SetPosition(gfx::PointF(190.f, 190.f));
     grand_child2->SetBounds(gfx::Size(6, 8));
@@ -543,6 +544,7 @@ TEST_F(DamageTrackerTest, VerifyDamageForImageFilter) {
   // Setting the filter will damage the whole surface.
   ClearDamageForAllSurfaces(root.get());
   child->SetFilters(filters);
+  child->SetHasRenderSurface(true);
   EmulateDrawingOneFrame(root.get());
   root_damage_rect =
           root->render_surface()->damage_tracker()->current_damage_rect();
@@ -927,7 +929,7 @@ TEST_F(DamageTrackerTest, VerifyDamageForAddingAndRemovingRenderSurfaces) {
   //         exposed.
   ClearDamageForAllSurfaces(root.get());
   child1->SetOpacity(1.f);

[danakj, 2014/09/03 19:30:33]

do we need to do this anymore?

[awoloszyn, 2014/09/09 15:31:36]

Have removed the SetOpacity, but we must keep the SetHasRenderSurface(false)
since it was set to true in CreateAndSetUpTestTreeWithTwoSurfaces.

-  child1->SetForceRenderSurface(false);
+  child1->SetHasRenderSurface(false);
   EmulateDrawingOneFrame(root.get());
 
   // Sanity check that there is only one surface now.
@@ -953,7 +955,8 @@ TEST_F(DamageTrackerTest, VerifyDamageForAddingAndRemovingRenderSurfaces) {
   // Then change the tree so that the render surface is added back.
   ClearDamageForAllSurfaces(root.get());
   child1->SetOpacity(0.5f);

[danakj, 2014/09/03 19:30:34]

and this?

[awoloszyn, 2014/09/09 15:31:36]

Same as above, SetOpacity removed.

-  child1->SetForceRenderSurface(true);
+  child1->SetHasRenderSurface(true);
+
   EmulateDrawingOneFrame(root.get());
 
   // Sanity check that there is a new surface now.
@@ -1112,7 +1115,6 @@ TEST_F(DamageTrackerTest, VerifyDamageForReplica) {
   ASSERT_EQ(old_content_rect.height(),
             child1->render_surface()->content_rect().height());
 
-  EXPECT_FALSE(grand_child1->render_surface());

[danakj, 2014/09/03 19:30:33]

this seems like it should be there and we shouldn't have put a surface on it
from the comments

[awoloszyn, 2014/09/09 15:31:36]

Done.

   child_damage_rect =
           child1->render_surface()->damage_tracker()->current_damage_rect();
   root_damage_rect =
@@ -1153,13 +1155,11 @@ TEST_F(DamageTrackerTest, VerifyDamageForMask) {
     grand_child->SetBounds(gfx::Size(2, 2));
     grand_child->SetContentBounds(gfx::Size(2, 2));
     grand_child->SetDrawsContent(true);
+    child->SetHasRenderSurface(true);

[danakj, 2014/09/03 19:30:33]

move this up to the SetMaskLayer() call on L1144? does the opacity call on L1150
still need to be there?

[awoloszyn, 2014/09/09 15:31:36]

Done. And correct, we no longer need the opacity call.

     child->AddChild(grand_child.Pass());
   }
   EmulateDrawingOneFrame(root.get());
 
-  // Sanity check that a new surface was created for the child.
-  ASSERT_TRUE(child->render_surface());
-
   // CASE 1: the update_rect on a mask layer should damage the entire target
   //         surface.
   ClearDamageForAllSurfaces(root.get());
@@ -1377,7 +1377,12 @@ TEST_F(DamageTrackerTest, VerifyDamageForEmptyLayerList) {
   // tracker does not crash when it receives an empty layer_list.
 
   scoped_ptr<LayerImpl> root = LayerImpl::Create(host_impl_.active_tree(), 1);
-  root->CreateRenderSurface();
+  root->SetHasRenderSurface(true);
+
+  {
+    LayerImplList render_surface_layer_list;
+    ExecuteCalculateDrawProperties(root.get(), &render_surface_layer_list);

[danakj, 2014/09/03 19:30:33]

why is this needed?

[awoloszyn, 2014/09/09 15:31:36]

Since we now only set render_targets during CDP we have to run CDP in order to
set them up. (Since we can have a render_surface without a render_target). I
have switched this to the simpler root->draw_properties().draw_target =
root.get();

+  }
 
   ASSERT_TRUE(root == root->render_target());
   RenderSurfaceImpl* target_surface = root->render_surface();