Fix double lock of texture resource in software renderer

If in software rendering mode, a copy request
of the layer is not in the root or first render pass,
there will be double locking resource crash.
Now we first release old lock and then try to
obtain lock again.

R=danakj@chromium.org

TEST=SoftwareRendererTest.DoubleLockTextureCrash

Review URL: https://codereview.chromium.org/899183003

Cr-Commit-Position: refs/heads/master@{#315155}

cc/output/gl_renderer.cc

@@ -2712,6 +2712,8 @@ bool GLRenderer::BindFramebufferToTexture(DrawingFrame* frame,
                                           const gfx::Rect& target_rect) {
   DCHECK(texture->id());
 
+  // Explicitly release lock, otherwise we can crash when try to lock
+  // same texture again.
   current_framebuffer_lock_ = nullptr;
 
   SetStencilEnabled(false);

cc/output/software_renderer.cc

@@ -160,6 +160,11 @@ bool SoftwareRenderer::BindFramebufferToTexture(
     DrawingFrame* frame,
     const ScopedResource* texture,
     const gfx::Rect& target_rect) {
+  DCHECK(texture->id());
+
+  // Explicitly release lock, otherwise we can crash when try to lock
+  // same texture again.
+  current_framebuffer_lock_ = nullptr;
   current_framebuffer_lock_ = make_scoped_ptr(
       new ResourceProvider::ScopedWriteLockSoftware(
           resource_provider_, texture->id()));

cc/output/software_renderer_unittest.cc

@@ -135,7 +135,7 @@ TEST_F(SoftwareRendererTest, SolidColorQuad) {
   scoped_ptr<SkBitmap> output =
       DrawAndCopyOutput(&list, device_scale_factor, device_viewport_rect);
   EXPECT_EQ(outer_rect.width(), output->info().fWidth);
-  EXPECT_EQ(outer_rect.width(), output->info().fHeight);
+  EXPECT_EQ(outer_rect.height(), output->info().fHeight);
 
   EXPECT_EQ(SK_ColorYELLOW, output->getColor(0, 0));
   EXPECT_EQ(SK_ColorYELLOW,
@@ -233,7 +233,7 @@ TEST_F(SoftwareRendererTest, TileQuad) {
   scoped_ptr<SkBitmap> output =
       DrawAndCopyOutput(&list, device_scale_factor, device_viewport_rect);
   EXPECT_EQ(outer_rect.width(), output->info().fWidth);
-  EXPECT_EQ(outer_rect.width(), output->info().fHeight);
+  EXPECT_EQ(outer_rect.height(), output->info().fHeight);
 
   EXPECT_EQ(SK_ColorYELLOW, output->getColor(0, 0));
   EXPECT_EQ(SK_ColorYELLOW,
@@ -308,7 +308,7 @@ TEST_F(SoftwareRendererTest, TileQuadVisibleRect) {
   scoped_ptr<SkBitmap> output =
       DrawAndCopyOutput(&list, device_scale_factor, device_viewport_rect);
   EXPECT_EQ(tile_rect.width(), output->info().fWidth);
-  EXPECT_EQ(tile_rect.width(), output->info().fHeight);
+  EXPECT_EQ(tile_rect.height(), output->info().fHeight);
 
   // Check portion of tile not in visible rect isn't drawn.
   const unsigned int kTransparent = SK_ColorTRANSPARENT;
@@ -350,7 +350,7 @@ TEST_F(SoftwareRendererTest, ShouldClearRootRenderPass) {
   scoped_ptr<SkBitmap> output =
       DrawAndCopyOutput(&list, device_scale_factor, device_viewport_rect);
   EXPECT_EQ(device_viewport_rect.width(), output->info().fWidth);
-  EXPECT_EQ(device_viewport_rect.width(), output->info().fHeight);
+  EXPECT_EQ(device_viewport_rect.height(), output->info().fHeight);
 
   EXPECT_EQ(SK_ColorGREEN, output->getColor(0, 0));
   EXPECT_EQ(SK_ColorGREEN,
@@ -372,7 +372,7 @@ TEST_F(SoftwareRendererTest, ShouldClearRootRenderPass) {
 
   output = DrawAndCopyOutput(&list, device_scale_factor, device_viewport_rect);
   EXPECT_EQ(device_viewport_rect.width(), output->info().fWidth);
-  EXPECT_EQ(device_viewport_rect.width(), output->info().fHeight);
+  EXPECT_EQ(device_viewport_rect.height(), output->info().fHeight);
 
   // If we didn't clear, the borders should still be green.
   EXPECT_EQ(SK_ColorGREEN, output->getColor(0, 0));
@@ -417,7 +417,7 @@ TEST_F(SoftwareRendererTest, RenderPassVisibleRect) {
   scoped_ptr<SkBitmap> output =
       DrawAndCopyOutput(&list, device_scale_factor, device_viewport_rect);
   EXPECT_EQ(device_viewport_rect.width(), output->info().fWidth);
-  EXPECT_EQ(device_viewport_rect.width(), output->info().fHeight);
+  EXPECT_EQ(device_viewport_rect.height(), output->info().fHeight);
 
   EXPECT_EQ(SK_ColorGREEN, output->getColor(0, 0));
   EXPECT_EQ(SK_ColorGREEN,

cc/trees/layer_tree_host_pixeltest_readback.cc

@@ -1144,6 +1144,38 @@ TEST_F(LayerTreeHostReadbackPixelTest, ReadbackNonRootLayerOutsideViewport) {
       base::FilePath(FILE_PATH_LITERAL("green_with_blue_corner.png")));
 }
 
+class LayerTreeHostReadbackNonFirstNonRootRenderPassPixelTest
+    : public LayerTreeHostReadbackPixelTest,
+      public testing::WithParamInterface<bool> {};
+
+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.
+  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)));
+  background->AddChild(blue);
+
+  RunReadbackTestWithReadbackTarget(
+      GetParam() ? PIXEL_TEST_GL : PIXEL_TEST_SOFTWARE,
+      READBACK_DEFAULT,
+      background,
+      background.get(),
+      base::FilePath(FILE_PATH_LITERAL("green_with_blue_corner.png")));
+}
+
+INSTANTIATE_TEST_CASE_P(
+    LayerTreeHostReadbackNonFirstNonRootRenderPassPixelTests,
+    LayerTreeHostReadbackNonFirstNonRootRenderPassPixelTest,
+    testing::Bool());
+
 }  // namespace
 }  // namespace cc