Abort copy requests from never-drawn trees

In rare-ish cases, the scheduler will commit/activate onto the active
tree without ever having drawn that tree.  This happens in cases where
the tree is !CanDraw, such as in cases of empty viewports or not being
visible or losing a context.

Preserving render surface creation on the main thread (along with clip
nodes and transform nodes in the future) makes handling this edge case
extra complicated, so I'd like to just abort these requests.

Callers to CopyOutputRequest already have to deal with failure, so they
can just re-request.

R=danakj@chromium.org,vollick@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

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

4 files changed, 30 insertions, 85 deletions

diff --git a/cc/layers/layer.cc b/cc/layers/layer.cc
index 406688f..a498736 100644
--- a/cc/layers/layer.cc
+++ b/cc/layers/layer.cc
@@ -1176,7 +1176,7 @@ void Layer::PushPropertiesTo(LayerImpl* layer) {
       draw_checkerboard_for_missing_tiles_);
   layer->SetDrawsContent(DrawsContent());
   layer->SetHideLayerAndSubtree(hide_layer_and_subtree_);
-  layer->SetHasRenderSurface(has_render_surface_ || layer->HasCopyRequest());
+  layer->SetHasRenderSurface(has_render_surface_);
   if (!layer->FilterIsAnimatingOnImplOnly() && !FilterIsAnimating())
     layer->SetFilters(filters_);
   DCHECK(!(FilterIsAnimating() && layer->FilterIsAnimatingOnImplOnly()));
diff --git a/cc/layers/layer_impl.cc b/cc/layers/layer_impl.cc
index 7e3a2c7..357b429 100644
--- a/cc/layers/layer_impl.cc
+++ b/cc/layers/layer_impl.cc
@@ -228,8 +228,19 @@ void LayerImpl::SetOpacityTreeIndex(int index) {
 }
 
 void LayerImpl::PassCopyRequests(ScopedPtrVector<CopyOutputRequest>* requests) {
+  // In the case that a layer still has a copy request, this means that there's
+  // a commit to the active tree without a draw.  This only happens in some
+  // edge cases during lost context or visibility changes, so don't try to
+  // handle preserving these output requests (and their surface).
+  if (!copy_requests_.empty()) {
+    layer_tree_impl()->RemoveLayerWithCopyOutputRequest(this);
+    // Destroying these will abort them.
+    copy_requests_.clear();
+  }
+
   if (requests->empty())
     return;
+
   DCHECK(render_surface());
   bool was_empty = copy_requests_.empty();
   copy_requests_.insert_and_take(copy_requests_.end(), requests);
@@ -533,7 +544,7 @@ void LayerImpl::PushPropertiesTo(LayerImpl* layer) {
       draw_checkerboard_for_missing_tiles_);
   layer->SetDrawsContent(DrawsContent());
   layer->SetHideLayerAndSubtree(hide_layer_and_subtree_);
-  layer->SetHasRenderSurface(!!render_surface() || layer->HasCopyRequest());
+  layer->SetHasRenderSurface(!!render_surface());
   layer->SetFilters(filters());
   layer->SetBackgroundFilters(background_filters());
   layer->SetMasksToBounds(masks_to_bounds_);
diff --git a/cc/trees/layer_tree_host_unittest.cc b/cc/trees/layer_tree_host_unittest.cc
index ffe090d..79c5f53 100644
--- a/cc/trees/layer_tree_host_unittest.cc
+++ b/cc/trees/layer_tree_host_unittest.cc
@@ -5750,85 +5750,6 @@ class LayerTreeHostTestNoTasksBetweenWillAndDidCommit
 
 SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestNoTasksBetweenWillAndDidCommit);
 
-// Verify that if a LayerImpl holds onto a copy request for multiple
-// frames that it will continue to have a render surface through
-// multiple commits, even though the Layer itself has no reason
-// to have a render surface.
-class LayerPreserveRenderSurfaceFromOutputRequests : public LayerTreeHostTest {
- protected:
-  void SetupTree() override {
-    scoped_refptr<Layer> root = Layer::Create(layer_settings());
-    root->CreateRenderSurface();
-    root->SetBounds(gfx::Size(10, 10));
-    child_ = Layer::Create(layer_settings());
-    child_->SetBounds(gfx::Size(20, 20));
-    root->AddChild(child_);
-
-    layer_tree_host()->SetRootLayer(root);
-    LayerTreeHostTest::SetupTree();
-  }
-
-  static void CopyOutputCallback(scoped_ptr<CopyOutputResult> result) {}
-
-  void BeginTest() override {
-    child_->RequestCopyOfOutput(
-        CopyOutputRequest::CreateBitmapRequest(base::Bind(CopyOutputCallback)));
-    EXPECT_TRUE(child_->HasCopyRequest());
-    PostSetNeedsCommitToMainThread();
-  }
-
-  void DidCommit() override { EXPECT_FALSE(child_->HasCopyRequest()); }
-
-  void CommitCompleteOnThread(LayerTreeHostImpl* host_impl) override {
-    LayerImpl* child_impl = host_impl->sync_tree()->LayerById(child_->id());
-
-    switch (host_impl->sync_tree()->source_frame_number()) {
-      case 0:
-        EXPECT_TRUE(child_impl->HasCopyRequest());
-        EXPECT_TRUE(child_impl->render_surface());
-        break;
-      case 1:
-        if (host_impl->proxy()->CommitToActiveTree()) {
-          EXPECT_TRUE(child_impl->HasCopyRequest());
-          EXPECT_TRUE(child_impl->render_surface());
-        } else {
-          EXPECT_FALSE(child_impl->HasCopyRequest());
-          EXPECT_FALSE(child_impl->render_surface());
-        }
-        break;
-      default:
-        NOTREACHED();
-        break;
-    }
-  }
-
-  void DidActivateTreeOnThread(LayerTreeHostImpl* host_impl) override {
-    LayerImpl* child_impl = host_impl->active_tree()->LayerById(child_->id());
-    EXPECT_TRUE(child_impl->HasCopyRequest());
-    EXPECT_TRUE(child_impl->render_surface());
-
-    switch (host_impl->active_tree()->source_frame_number()) {
-      case 0:
-        // Lose output surface to prevent drawing and cause another commit.
-        host_impl->DidLoseOutputSurface();
-        break;
-      case 1:
-        EndTest();
-        break;
-      default:
-        NOTREACHED();
-        break;
-    }
-  }
-
-  void AfterTest() override {}
-
- private:
-  scoped_refptr<Layer> child_;
-};
-
-SINGLE_AND_MULTI_THREAD_TEST_F(LayerPreserveRenderSurfaceFromOutputRequests);
-
 class LayerTreeHostTestUpdateCopyRequests : public LayerTreeHostTest {
  protected:
   void SetupTree() override {
diff --git a/cc/trees/layer_tree_host_unittest_copyrequest.cc b/cc/trees/layer_tree_host_unittest_copyrequest.cc
index 70e0c79..d92bd9c 100644
--- a/cc/trees/layer_tree_host_unittest_copyrequest.cc
+++ b/cc/trees/layer_tree_host_unittest_copyrequest.cc
@@ -513,11 +513,24 @@ class LayerTreeHostTestAsyncTwoReadbacksWithoutDraw
 
   void CopyOutputCallback(scoped_ptr<CopyOutputResult> result) {
     EXPECT_TRUE(layer_tree_host()->proxy()->IsMainThread());
-    EXPECT_EQ(copy_layer_->bounds().ToString(), result->size().ToString());
-    ++callback_count_;
 
-    if (callback_count_ == 2)
-      EndTest();
+    // The first frame can't be drawn.
+    switch (callback_count_) {
+      case 0:
+        EXPECT_TRUE(result->IsEmpty());
+        EXPECT_EQ(gfx::Size(), result->size());
+        break;
+      case 1:
+        EXPECT_FALSE(result->IsEmpty());
+        EXPECT_EQ(copy_layer_->bounds().ToString(), result->size().ToString());
+        EndTest();
+        break;
+      default:
+        NOTREACHED();
+        break;
+    }
+
+    ++callback_count_;
   }
 
   void AfterTest() override { EXPECT_TRUE(saw_copy_request_); }