cc: Dirty the RenderSurfaceLayerList when taking copy requests.

When removing copy requests from a LayerImpl to fulfill the request,
this changes the RenderSurfaceLayerList that CalculateDrawProperties
would output. In particular, the layer would no longer keep itself as a
render target.

Since we fail to do this, the layer becomes !HasCopyRequest(), but keeps
itself as a target in the RenderSurfaceLayerList. Then when we go to
draw again (due to an animation or the like?) we skip appending the
layer's RenderPass since the subtree is hidden, but the layer is still
in the RenderSurfaceLayerList because we didn't recompute draw properties
and the layer copy request was keeping the hidden layer in the list.

This cognitive dissonance causes us to use a null RenderPass as
the target_render_pass in CalculateRenderPasses, which causes a crash as
soon as the (hidden) layer tries to append quads to the null surface.

R=vmpstr, weiliangc@chromium.org
BUG=439649

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

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

4 files changed, 111 insertions, 0 deletions

diff --git a/cc/layers/layer_impl.cc b/cc/layers/layer_impl.cc
index d9dee10..b090246 100644
--- a/cc/layers/layer_impl.cc
+++ b/cc/layers/layer_impl.cc
@@ -237,6 +237,7 @@ void LayerImpl::TakeCopyRequestsAndTransformToTarget(
   }
 
   layer_tree_impl()->RemoveLayerWithCopyOutputRequest(this);
+  layer_tree_impl()->set_needs_update_draw_properties();
 }
 
 void LayerImpl::ClearRenderSurfaceLayerList() {
diff --git a/cc/test/layer_tree_test.cc b/cc/test/layer_tree_test.cc
index a40a8c7..001aa13 100644
--- a/cc/test/layer_tree_test.cc
+++ b/cc/test/layer_tree_test.cc
@@ -573,6 +573,11 @@ void LayerTreeTest::PostCompositeImmediatelyToMainThread() {
                  main_thread_weak_ptr_));
 }
 
+void LayerTreeTest::PostEndTestToMainThread() {
+  main_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&LayerTreeTest::EndTest, main_thread_weak_ptr_));
+}
+
 void LayerTreeTest::WillBeginTest() {
   layer_tree_host_->SetLayerTreeHostClientReady();
 }
diff --git a/cc/test/layer_tree_test.h b/cc/test/layer_tree_test.h
index 4e38ae6..c009d4e 100644
--- a/cc/test/layer_tree_test.h
+++ b/cc/test/layer_tree_test.h
@@ -144,6 +144,7 @@ class LayerTreeTest : public testing::Test, public TestHooks {
   void PostSetVisibleToMainThread(bool visible);
   void PostSetNextCommitForcesRedrawToMainThread();
   void PostCompositeImmediatelyToMainThread();
+  void PostEndTestToMainThread();
 
   void DoBeginTest();
   void Timeout();
diff --git a/cc/trees/layer_tree_host_unittest_copyrequest.cc b/cc/trees/layer_tree_host_unittest_copyrequest.cc
index e4f944d..4d3a4be 100644
--- a/cc/trees/layer_tree_host_unittest_copyrequest.cc
+++ b/cc/trees/layer_tree_host_unittest_copyrequest.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "cc/layers/layer_iterator.h"
 #include "cc/output/copy_output_request.h"
 #include "cc/output/copy_output_result.h"
 #include "cc/test/fake_content_layer.h"
@@ -966,5 +967,108 @@ class LayerTreeHostCopyRequestTestShutdownBeforeCopy
 SINGLE_AND_MULTI_THREAD_DIRECT_RENDERER_TEST_F(
     LayerTreeHostCopyRequestTestShutdownBeforeCopy);
 
+class LayerTreeHostCopyRequestTestMultipleDrawsWithHiddenCopyRequest
+    : public LayerTreeHostCopyRequestTest {
+ protected:
+  void SetupTree() override {
+    scoped_refptr<FakeContentLayer> root = FakeContentLayer::Create(&client_);
+    root->SetBounds(gfx::Size(20, 20));
+
+    child_ = FakeContentLayer::Create(&client_);
+    child_->SetBounds(gfx::Size(10, 10));
+    root->AddChild(child_);
+    child_->SetHideLayerAndSubtree(true);
+
+    layer_tree_host()->SetRootLayer(root);
+    LayerTreeHostCopyRequestTest::SetupTree();
+  }
+
+  void BeginTest() override {
+    num_draws_ = 0;
+    copy_happened_ = false;
+    PostSetNeedsCommitToMainThread();
+  }
+
+  void DidCommitAndDrawFrame() override {
+    // Send a copy request after the first commit.
+    if (layer_tree_host()->source_frame_number() == 1) {
+      child_->RequestCopyOfOutput(
+          CopyOutputRequest::CreateBitmapRequest(base::Bind(
+              &LayerTreeHostCopyRequestTestMultipleDrawsWithHiddenCopyRequest::
+                  CopyOutputCallback,
+              base::Unretained(this))));
+    }
+  }
+
+  DrawResult PrepareToDrawOnThread(LayerTreeHostImpl* host_impl,
+                                   LayerTreeHostImpl::FrameData* frame_data,
+                                   DrawResult draw_result) override {
+    LayerImpl* root = host_impl->active_tree()->root_layer();
+    LayerImpl* child = root->children()[0];
+
+    bool saw_root = false;
+    bool saw_child = false;
+    for (LayerIterator<LayerImpl> it = LayerIterator<LayerImpl>::Begin(
+             frame_data->render_surface_layer_list);
+         it != LayerIterator<LayerImpl>::End(
+                   frame_data->render_surface_layer_list);
+         ++it) {
+      if (it.represents_itself()) {
+        if (*it == root)
+          saw_root = true;
+        else if (*it == child)
+          saw_child = true;
+        else
+          NOTREACHED();
+      }
+    }
+
+    ++num_draws_;
+    // The first draw has no copy request. The 2nd draw has a copy request, the
+    // 3rd should not again.
+    switch (num_draws_) {
+      case 1:
+        // Only the root layer draws, the child is hidden.
+        EXPECT_TRUE(saw_root);
+        EXPECT_FALSE(saw_child);
+        break;
+      case 2:
+        // Copy happening here, the child will draw.
+        EXPECT_TRUE(saw_root);
+        EXPECT_TRUE(saw_child);
+        // Make another draw happen after doing the copy request.
+        host_impl->SetNeedsRedrawRect(gfx::Rect(1, 1));
+        break;
+      case 3:
+        // If LayerTreeHostImpl does the wrong thing, it will try to draw the
+        // layer which had a copy request. But only the root should draw.
+        EXPECT_TRUE(saw_root);
+        EXPECT_FALSE(saw_child);
+
+        // End the test! Don't race with copy request callbacks, so post the end
+        // to the main thread.
+        EXPECT_TRUE(copy_happened_);
+        PostEndTestToMainThread();
+        break;
+    }
+    return draw_result;
+  }
+
+  void CopyOutputCallback(scoped_ptr<CopyOutputResult> result) {
+    EXPECT_FALSE(TestEnded());
+    copy_happened_ = true;
+  }
+
+  void AfterTest() override {}
+
+  scoped_refptr<FakeContentLayer> child_;
+  FakeContentLayerClient client_;
+  int num_draws_;
+  bool copy_happened_;
+};
+
+SINGLE_AND_MULTI_THREAD_DIRECT_RENDERER_NOIMPL_TEST_F(
+    LayerTreeHostCopyRequestTestMultipleDrawsWithHiddenCopyRequest);
+
 }  // namespace
 }  // namespace cc