Fix render state restore bug in DirectRenderer

cc/trees/layer_tree_host_pixeltest_readback.cc

Index: cc/trees/layer_tree_host_pixeltest_readback.cc
diff --git a/cc/trees/layer_tree_host_pixeltest_readback.cc b/cc/trees/layer_tree_host_pixeltest_readback.cc
index 6d3c6ec318412c474befc5ebd523cc01a42eb350..03018f2d6fe98657d97372c97800a28d6a2cda08 100644
--- a/cc/trees/layer_tree_host_pixeltest_readback.cc
+++ b/cc/trees/layer_tree_host_pixeltest_readback.cc
@@ -1151,9 +1151,9 @@ class LayerTreeHostReadbackNonFirstNonRootRenderPassPixelTest
 TEST_P(LayerTreeHostReadbackNonFirstNonRootRenderPassPixelTest,
        ReadbackNonRootOrFirstLayer) {
   // This test has 3 render passes with the copy request on the render pass in
-  // the middle. Doing a copy request can be destructive of state, so for render
-  // passes after the first drawn the code path is different. This verifies the
-  // non-first and non-root path. See http://crbug.com/99393 for more info.
+  // the middle. This test caught an issue where copy requests on non-root
+  // non-first render passes were being treated differently from the first
+  // render pass.
   scoped_refptr<SolidColorLayer> background =
       CreateSolidColorLayer(gfx::Rect(200, 200), SK_ColorGREEN);
 
@@ -1171,6 +1171,30 @@ TEST_P(LayerTreeHostReadbackNonFirstNonRootRenderPassPixelTest,
       base::FilePath(FILE_PATH_LITERAL("green_with_blue_corner.png")));
 }
 
+TEST_P(LayerTreeHostReadbackNonFirstNonRootRenderPassPixelTest,
+       MultipleReadbackNonRootOrFirstLayer) {

[danakj, 2015/02/09 18:01:17]

The test has multiple requests on blue which becomes the first renderpass drawn
in the renderer, so this isnt quite right for the name.
MultipleReadbacksOnLayer?

[ericrk, 2015/02/09 19:46:14]

Done.

+  // This test has 3 render passes with multiple copy requests on the second
+  // render pass. For every copy request after the first, state must be
+  // restored via a call to UseRenderPass (see http://crbug.com/99393). This
+  // test ensures that the renderer correctly handles cases where
+  // UseRenderPass is called multiple times for a single layer.
+  scoped_refptr<SolidColorLayer> background =
+      CreateSolidColorLayer(gfx::Rect(200, 200), SK_ColorGREEN);
+
+  scoped_refptr<SolidColorLayer> blue =
+      CreateSolidColorLayer(gfx::Rect(150, 150, 50, 50), SK_ColorBLUE);
+  blue->RequestCopyOfOutput(CopyOutputRequest::CreateBitmapRequest(
+      base::Bind(&IgnoreReadbackResult)));
+  blue->RequestCopyOfOutput(CopyOutputRequest::CreateBitmapRequest(

[danakj, 2015/02/09 18:01:17]

I think we can simplify this test a bit, by just putting one extra request like
this on background and removing blue? That should still test that codepath
right?

[ericrk, 2015/02/09 19:46:14]

Done.

+      base::Bind(&IgnoreReadbackResult)));
+  background->AddChild(blue);
+
+  RunReadbackTestWithReadbackTarget(
+      GetParam() ? PIXEL_TEST_GL : PIXEL_TEST_SOFTWARE, READBACK_DEFAULT,

[danakj, 2015/02/09 18:01:17]

Hm, it occured to me if only one version of this test failed, i'd have no idea
if /0 and /1 (the test suffixes) meant GL or Software.. Do you think we can
change the param type from bool to this enum type directly? Then we can see the
order clearly in the definition of the TEST_P?

[ericrk, 2015/02/09 19:46:14]

Good call - done.

+      background, background.get(),
+      base::FilePath(FILE_PATH_LITERAL("green_with_blue_corner.png")));
+}
+
 INSTANTIATE_TEST_CASE_P(
     LayerTreeHostReadbackNonFirstNonRootRenderPassPixelTests,
     LayerTreeHostReadbackNonFirstNonRootRenderPassPixelTest,