cc: Don't claim ownership of resources in the pending tree.

DelegatedRendererLayerImpl will claim ownership of resources in a frame,
and this makes unused resources from the previous frame be returned to
the child.

However, this was being done on the pending tree, but the previous frame
was still in the active tree and its resources would be returned early.
Instead, delay telling the ResourceProvider about the frame's resources
until the frame replaces the resources on the active tree.

R=enne, [email protected]
BUG=475894

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

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

cc/layers/delegated_renderer_layer_impl.cc

@@ -80,6 +80,9 @@ void DelegatedRendererLayerImpl::PushPropertiesTo(LayerImpl* layer) {
   if (have_render_passes_to_push_) {
     // This passes ownership of the render passes to the active tree.
     delegated_layer->SetRenderPasses(&render_passes_in_draw_order_);
+    // Once resources are on the active tree, give them to the ResourceProvider
+    // and release unused resources from the old frame.
+    delegated_layer->TakeOwnershipOfResourcesIfOnActiveTree(resources_);
     DCHECK(render_passes_in_draw_order_.empty());
     have_render_passes_to_push_ = false;
   }
@@ -130,14 +133,17 @@ void DelegatedRendererLayerImpl::SetFrameData(
   }
 
   if (invalid_frame) {
-    // Declare we are still using the last frame's resources.
+    // Declare we are still using the last frame's resources. Drops ownership of
+    // any invalid resources, keeping only those in use by the active tree.
     resource_provider->DeclareUsedResourcesFromChild(child_id_, resources_);
     return;
   }
 
-  // Declare we are using the new frame's resources.
+  // Save the new frame's resources, but don't give them to the ResourceProvider
+  // until they are active, since the resources on the active tree will still be
+  // used and we don't want to return them early.
   resources_.swap(resources_in_frame);
-  resource_provider->DeclareUsedResourcesFromChild(child_id_, resources_);
+  TakeOwnershipOfResourcesIfOnActiveTree(resources_);
 
   inverse_device_scale_factor_ = 1.0f / frame_data->device_scale_factor;
   // Display size is already set so we can compute what the damage rect
@@ -155,6 +161,15 @@ void DelegatedRendererLayerImpl::SetFrameData(
   have_render_passes_to_push_ = true;
 }
 
+void DelegatedRendererLayerImpl::TakeOwnershipOfResourcesIfOnActiveTree(
+    const ResourceProvider::ResourceIdSet& resources) {
+  DCHECK(child_id_);
+  if (!layer_tree_impl()->IsActiveTree())
+    return;
+  layer_tree_impl()->resource_provider()->DeclareUsedResourcesFromChild(
+      child_id_, resources);
+}
+
 void DelegatedRendererLayerImpl::SetRenderPasses(
     RenderPassList* render_passes_in_draw_order) {
   ClearRenderPasses();

cc/layers/delegated_renderer_layer_impl.h

@@ -67,6 +67,8 @@ class CC_EXPORT DelegatedRendererLayerImpl : public LayerImpl {
 
   void AppendRainbowDebugBorder(RenderPass* render_pass);
 
+  void TakeOwnershipOfResourcesIfOnActiveTree(
+      const ResourceProvider::ResourceIdSet& resources);
   void SetRenderPasses(RenderPassList* render_passes_in_draw_order);
   void ClearRenderPasses();
 

cc/trees/layer_tree_host_unittest_delegated.cc

@@ -243,6 +243,24 @@ class LayerTreeHostDelegatedTest : public LayerTreeTest {
       output_surface()->ReturnResource(resources_to_return[i], &ack);
     host_impl->ReclaimResources(&ack);
   }
+
+  void ReturnAllResourcesFromParent(LayerTreeHostImpl* host_impl) {
+    DelegatedFrameData* delegated_frame_data =
+        output_surface()->last_sent_frame().delegated_frame_data.get();
+    if (!delegated_frame_data)
+      return;
+
+    const TransferableResourceArray& resources_held_by_parent =
+        output_surface()->resources_held_by_parent();
+
+    if (resources_held_by_parent.empty())
+      return;
+
+    CompositorFrameAck ack;
+    for (size_t i = 0; i < resources_held_by_parent.size(); ++i)
+      output_surface()->ReturnResource(resources_held_by_parent[i].id, &ack);
+    host_impl->ReclaimResources(&ack);
+  }
 };
 
 class LayerTreeHostDelegatedTestCaseSingleDelegatedLayer
@@ -1225,7 +1243,7 @@ class LayerTreeHostDelegatedTestBadFrame
 
     switch (host_impl->active_tree()->source_frame_number()) {
       case 1: {
-        // We have the first good frame with just 990 and 555 in it.
+        // We have the first good frame with just 999 and 555 in it.
         // layer.
         EXPECT_EQ(2u, map.size());
         EXPECT_EQ(1u, map.count(999));
@@ -1293,7 +1311,7 @@ class LayerTreeHostDelegatedTestUnnamedResource
  public:
   void BeginTest() override { PostSetNeedsCommitToMainThread(); }
 
-  void DidCommit() override {
+  void DidCommitAndDrawFrame() override {
     scoped_ptr<DelegatedFrameData> frame;
     ReturnedResourceArray resources;
 
@@ -2165,5 +2183,81 @@ class LayerTreeHostDelegatedTestRemoveAndChangeResources
 SINGLE_AND_MULTI_THREAD_TEST_F(
     LayerTreeHostDelegatedTestRemoveAndChangeResources);
 
+class LayerTreeHostDelegatedTestActiveFrameIsValid
+    : public LayerTreeHostDelegatedTestCaseSingleDelegatedLayer {
+ public:
+  LayerTreeHostDelegatedTestActiveFrameIsValid()
+      : drew_with_pending_tree_(false) {}
+
+  void DidCommitAndDrawFrame() override {
+    scoped_ptr<DelegatedFrameData> frame;
+    switch (layer_tree_host()->source_frame_number()) {
+      case 1:
+        // This frame goes to the active tree.
+        frame = CreateFrameData(gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1));
+        AddTextureQuad(frame.get(), 999);
+        AddTransferableResource(frame.get(), 999);
+        SetFrameData(frame.Pass());
+        break;
+      case 2:
+        // This frame stops in the pending tree while we redraw the active tree.
+        frame = CreateFrameData(gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1));
+        AddTextureQuad(frame.get(), 555);
+        AddTransferableResource(frame.get(), 555);
+        SetFrameData(frame.Pass());
+        break;
+    }
+  }
+
+  void DrawLayersOnThread(LayerTreeHostImpl* host_impl) override {
+    if (host_impl->active_tree()->source_frame_number() < 1)
+      return;
+
+    LayerImpl* root_impl = host_impl->active_tree()->root_layer();
+    FakeDelegatedRendererLayerImpl* delegated_impl =
+        static_cast<FakeDelegatedRendererLayerImpl*>(root_impl->children()[0]);
+    const ResourceProvider::ResourceIdMap& map =
+        host_impl->resource_provider()->GetChildToParentMap(
+            delegated_impl->ChildId());
+
+    switch (host_impl->active_tree()->source_frame_number()) {
+      case 1:
+        if (!host_impl->pending_tree()) {
+          // Frame 2 is blocked from activating until another draw happens with
+          // Frame 1. This ensures we draw a different active frame from
+          // what's in the pending tree.
+          host_impl->BlockNotifyReadyToActivateForTesting(true);
+          host_impl->SetNeedsRedrawRect(gfx::Rect(1, 1));
+          break;
+        }
+
+        // The resources in the active tree should be valid.
+        EXPECT_EQ(1u, map.count(999));
+
+        host_impl->BlockNotifyReadyToActivateForTesting(false);
+        drew_with_pending_tree_ = true;
+        break;
+      case 2:
+        EXPECT_TRUE(drew_with_pending_tree_);
+
+        // The resources in the active tree should be valid.
+        EXPECT_EQ(1u, map.count(555));
+        EndTest();
+        break;
+    }
+  }
+
+  void SwapBuffersOnThread(LayerTreeHostImpl* host_impl, bool result) override {
+    // Return everything so that we can reliably delete resources that lose
+    // their references. This would happen if the tab was backgrounded or
+    // the parent decided to drop all resources for some reason.
+    ReturnAllResourcesFromParent(host_impl);
+  }
+
+  bool drew_with_pending_tree_;
+};
+
+MULTI_THREAD_IMPL_TEST_F(LayerTreeHostDelegatedTestActiveFrameIsValid);
+
 }  // namespace
 }  // namespace cc