cc: Don't leak the texture returned in CopyOutputRequest ever.

cc/trees/layer_tree_host_unittest.cc

 // Copyright 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/trees/layer_tree_host.h"
 
 #include <algorithm>
 
 #include "base/auto_reset.h"
 #include "base/synchronization/lock.h"
@@ skipping 3019 matching lines @@
   int callback_count_;
   FakeContentLayerClient client_;
   scoped_refptr<FakeContentLayer> root_;
   scoped_refptr<FakeContentLayer> copy_layer_;
 };
 
 // No output to copy for delegated renderers.
 SINGLE_AND_MULTI_THREAD_DIRECT_RENDERER_TEST_F(
     LayerTreeHostTestAsyncTwoReadbacksWithoutDraw);
 
+class LayerTreeHostTestAsyncReadbackLostOutputSurface
+    : public LayerTreeHostTest {
+ protected:
+  virtual scoped_ptr<OutputSurface> CreateOutputSurface(bool fallback)
+      OVERRIDE {
+    if (!first_context_provider_.get()) {
+      first_context_provider_ = TestContextProvider::Create();
+      return FakeOutputSurface::Create3d(first_context_provider_)
+          .PassAs<OutputSurface>();
+    }
+
+    EXPECT_FALSE(second_context_provider_.get());
+    second_context_provider_ = TestContextProvider::Create();
+    return FakeOutputSurface::Create3d(second_context_provider_)
+        .PassAs<OutputSurface>();
+  }
+
+  virtual void SetupTree() OVERRIDE {
+    root_ = FakeContentLayer::Create(&client_);
+    root_->SetBounds(gfx::Size(20, 20));
+
+    copy_layer_ = FakeContentLayer::Create(&client_);
+    copy_layer_->SetBounds(gfx::Size(10, 10));
+    root_->AddChild(copy_layer_);
+
+    layer_tree_host()->SetRootLayer(root_);
+    LayerTreeHostTest::SetupTree();
+  }
+
+  virtual void BeginTest() OVERRIDE { PostSetNeedsCommitToMainThread(); }
+
+  void CopyOutputCallback(scoped_ptr<CopyOutputResult> result) {
+    EXPECT_TRUE(layer_tree_host()->proxy()->IsMainThread());
+    EXPECT_EQ(gfx::Size(10, 10).ToString(), result->size().ToString());
+    EXPECT_TRUE(result->HasTexture());
+
+    // Save the result for later.
+    EXPECT_FALSE(result_);
+    result_ = result.Pass();
+
+    // Post a commit to lose the output surface.
+    layer_tree_host()->SetNeedsCommit();
+  }
+
+  virtual void DidCommitAndDrawFrame() OVERRIDE {
+    switch (layer_tree_host()->source_frame_number()) {
+      case 1:
+        // The layers have been pushed to the impl side. The layer textures have
+        // been allocated.
+
+        // Request a copy of the layer. This will use another texture.
+        copy_layer_->RequestCopyOfOutput(
+            CopyOutputRequest::CreateRequest(base::Bind(
+                &LayerTreeHostTestAsyncReadbackLostOutputSurface::
+                CopyOutputCallback,
+                base::Unretained(this))));
+        break;
+    }
+  }
+
+  virtual void SwapBuffersOnThread(LayerTreeHostImpl *impl, bool result)
+      OVERRIDE {
+    switch (impl->active_tree()->source_frame_number()) {
+      case 0:
+        // The layers have been drawn, so their textures have been allocated.
+        EXPECT_FALSE(result_);
+        num_textures_without_readback_ =
+            first_context_provider_->TestContext3d()->NumTextures();
+        break;
+      case 1:
+        // We did a readback, so there will be a readback texture around now.
+        EXPECT_LT(num_textures_without_readback_,
+                  first_context_provider_->TestContext3d()->NumTextures());
+        break;
+      case 2:
+        // The readback texture is collected.
+        EXPECT_TRUE(result_);

[piman, 2013/08/16 03:32:27]

result_ is set on the main thread, don't we have a potential data race here?

[danakj, 2013/08/16 18:18:38]

I think it's fine, because we get here because of the SNC() in the result
callback.

+
+        // Lose the output surface.
+        first_context_provider_->TestContext3d()->loseContextCHROMIUM(
+            GL_GUILTY_CONTEXT_RESET_ARB,
+            GL_INNOCENT_CONTEXT_RESET_ARB);
+        break;
+      case 3:
+        // With SingleThreadProxy it takes two commits to finally swap after a
+        // context loss.
+      case 4:
+        // The output surface has been recreated.
+        EXPECT_TRUE(second_context_provider_.get());
+
+        num_textures_after_loss_ =
+            first_context_provider_->TestContext3d()->NumTextures();
+
+        // Now destroy the CopyOutputResult, releasing the texture inside back
+        // to the compositor.
+        EXPECT_TRUE(result_);
+        result_.reset();

[piman, 2013/08/16 03:32:27]

Is it ok to run the mailbox callback on the compositor thread?

[danakj, 2013/08/16 18:18:38]

oh, darn. I had done this better earlier. it can actually be called from any
thread and will post task to the impl. but we should verify it works on a
non-impl thread.

+
+        // Check that it is released.
+        base::MessageLoopProxy::current()->PostTask(
+            FROM_HERE,
+            base::Bind(&LayerTreeHostTestAsyncReadbackLostOutputSurface::
+                           CheckNumTextures,
+                       base::Unretained(this),
+                       num_textures_after_loss_ - 1));
+        break;
+    }
+  }
+
+  void CheckNumTextures(size_t expected_num_textures) {
+    EXPECT_EQ(expected_num_textures,
+              first_context_provider_->TestContext3d()->NumTextures());
+    EndTest();
+  }
+
+  virtual void AfterTest() OVERRIDE {}
+
+  scoped_refptr<TestContextProvider> first_context_provider_;
+  scoped_refptr<TestContextProvider> second_context_provider_;
+  size_t num_textures_without_readback_;
+  size_t num_textures_after_loss_;
+  FakeContentLayerClient client_;
+  scoped_refptr<FakeContentLayer> root_;
+  scoped_refptr<FakeContentLayer> copy_layer_;
+  scoped_ptr<CopyOutputResult> result_;
+};
+
+// No output to copy for delegated renderers.
+SINGLE_AND_MULTI_THREAD_DIRECT_RENDERER_TEST_F(
+    LayerTreeHostTestAsyncReadbackLostOutputSurface);
+
 class LayerTreeHostTestNumFramesPending : public LayerTreeHostTest {
  public:
   virtual void BeginTest() OVERRIDE {
     frame_ = 0;
     PostSetNeedsCommitToMainThread();
   }
 
   // Round 1: commit + draw
   // Round 2: commit only (no draw/swap)
   // Round 3: draw only (no commit)
@@ skipping 1296 matching lines @@
   int num_will_begin_frames_;
   int num_impl_commits_;
 };
 
 // Commits can only be aborted when using the thread proxy.
 MULTI_THREAD_TEST_F(LayerTreeHostTestAbortEvictedTextures);
 
 }  // namespace
 
 }  // namespace cc