TextureLayer: clear texture id when clearing client

cc/layers/texture_layer_unittest.cc

Index: cc/layers/texture_layer_unittest.cc
diff --git a/cc/layers/texture_layer_unittest.cc b/cc/layers/texture_layer_unittest.cc
index 6d2098800ac017e9c7ddbce6fe6c72da75400c10..a2a7b960ad333170fab9541e26460fbf0da4a586 100644
--- a/cc/layers/texture_layer_unittest.cc
+++ b/cc/layers/texture_layer_unittest.cc
@@ -8,6 +8,7 @@
 
 #include "base/callback.h"
 #include "cc/base/thread.h"
+#include "cc/layers/texture_layer_client.h"
 #include "cc/layers/texture_layer_impl.h"
 #include "cc/test/fake_impl_proxy.h"
 #include "cc/test/fake_layer_tree_host_client.h"
@@ -474,5 +475,99 @@ TEST_F(TextureLayerImplWithMailboxTest, TestCallbackOnInUseResource) {
   provider->ReceiveFromParent(list);
 }
 
+// Check that ClearClient correctly clears the state so that the impl side
+// doesn't try to use a texture that was destroyed.
+class TextureLayerClientTest :
+    public LayerTreeTest,
+    public TextureLayerClient {
+ public:
+  TextureLayerClientTest()
+      : context_(NULL),
+        texture_(0),
+        commit_count_(0),
+        delete_texture_on_next_commit_(0) {}
+
+  virtual scoped_ptr<OutputSurface> CreateOutputSurface() OVERRIDE {
+    scoped_ptr<WebKit::WebGraphicsContext3D> context(
+        TestWebGraphicsContext3D::Create());
+    context_ = context.get();
+    texture_ = context->createTexture();
+    return FakeOutputSurface::Create3d(context.Pass()).PassAs<OutputSurface>();
+  }
+
+  virtual unsigned PrepareTexture(ResourceUpdateQueue* queue) {
+    return texture_;
+  }
+
+  virtual WebKit::WebGraphicsContext3D* Context3d() {
+    return context_;
+  }
+
+  virtual void SetupTree() OVERRIDE {
+    scoped_refptr<Layer> root = Layer::Create();
+    root->SetBounds(gfx::Size(10, 10));
+    root->SetAnchorPoint(gfx::PointF());
+    root->SetIsDrawable(true);
+
+    texture_layer_ = TextureLayer::Create(this);
+    texture_layer_->SetBounds(gfx::Size(10, 10));
+    texture_layer_->SetAnchorPoint(gfx::PointF());
+    texture_layer_->SetIsDrawable(true);
+    root->AddChild(texture_layer_);
+
+    layer_tree_host()->SetRootLayer(root);
+    LayerTreeTest::SetupTree();
+  }
+
+  virtual void BeginTest() OVERRIDE {
+    PostSetNeedsCommitToMainThread();
+  }
+
+  virtual void DidCommitAndDrawFrame() OVERRIDE {
+    ++commit_count_;
+    switch (commit_count_) {
+      case 1:
+        texture_layer_->ClearClient();
+        {
+          // We want to delete the texture, so that if the layer still uses it

[danakj, 2013/03/28 01:44:44]

The test looks good, but would it be more straightforward to check the number of
textures used by drawing? ie TestWGC3D::NumUsedTextures() == 0. Counting on a
dcheck without testing that the dcheck exists (of course not possible) seems a
bit less reliable.

[piman, 2013/03/28 03:11:35]

Fair enough, I tried it on the new patch. It requires a bit more plumbing to
have the correct count passed through the commit/draw flow, but it seems fairly
reliable this way.

+          // we'll trigger the assert in TestWebGraphicsContext3D.
+          // However at this point, we can only safely access the context on the
+          // thread, so we'll do that on the next commit.
+          base::AutoLock lock(lock_);
+          delete_texture_on_next_commit_ = texture_;
+        }
+        texture_ = 0;
+        break;
+      case 2:
+        EndTest();
+        break;
+      default:
+        NOTREACHED();
+        break;
+    }
+  }
+
+  virtual void BeginCommitOnThread(LayerTreeHostImpl* host_impl) {
+    base::AutoLock lock(lock_);
+    if (delete_texture_on_next_commit_) {
+      context_->deleteTexture(delete_texture_on_next_commit_);
+      delete_texture_on_next_commit_ = 0;
+    }
+  }
+
+  virtual void AfterTest() OVERRIDE {}
+
+ private:
+  scoped_refptr<TextureLayer> texture_layer_;
+  WebKit::WebGraphicsContext3D* context_;
+  unsigned texture_;
+  int commit_count_;
+
+  base::Lock lock_;
+  unsigned delete_texture_on_next_commit_;
+};
+
+SINGLE_AND_MULTI_THREAD_TEST_F(TextureLayerClientTest);
+
 }  // namespace
 }  // namespace cc