Reduce internal Flush() in GL resource glGen/Delete APIs.

gpu/command_buffer/client/gles2_implementation_unittest.cc

Index: gpu/command_buffer/client/gles2_implementation_unittest.cc
diff --git a/gpu/command_buffer/client/gles2_implementation_unittest.cc b/gpu/command_buffer/client/gles2_implementation_unittest.cc
index 61033094ccddddab51e80cbbe7d71939e197904e..2a24b4a658ee38b300e8f3ae2bb7ed2e66788985 100644
--- a/gpu/command_buffer/client/gles2_implementation_unittest.cc
+++ b/gpu/command_buffer/client/gles2_implementation_unittest.cc
@@ -2444,6 +2444,90 @@ TEST_F(GLES2ImplementationStrictSharedTest, BindsNotCached) {
   }
 }
 
+// glGen* Ids must not be reused until glDelete* commands have been
+// flushed by glFlush.
+TEST_F(GLES2ImplementationStrictSharedTest, FlushGenerationGenTextures) {
+  GLuint id1, id2, id3;
+
+  // Generate valid id.
+  gl_->GenTextures(1, &id1);
+  EXPECT_NE(id1, 0u);
+
+  // Delete id1 and generate id2.  id1 should not be reused.
+  gl_->DeleteTextures(1, &id1);
+  gl_->GenTextures(1, &id2);
+  EXPECT_NE(id2, 0u);
+  EXPECT_NE(id2, id1);
+
+  // Expect id1 reuse after Flush.
+  gl_->Flush();
+  gl_->GenTextures(1, &id3);
+  EXPECT_EQ(id3, id1);
+}
+
+// glGen* Ids must not be reused until glDelete* commands have been
+// flushed by glFlush.
+TEST_F(GLES2ImplementationStrictSharedTest, FlushGenerationGenBuffers) {
+  GLuint id1, id2, id3;
+
+  // Generate valid id.
+  gl_->GenBuffers(1, &id1);
+  EXPECT_NE(id1, 0u);
+
+  // Delete id1 and generate id2.  id1 should not be reused.
+  gl_->DeleteBuffers(1, &id1);
+  gl_->GenBuffers(1, &id2);
+  EXPECT_NE(id2, 0u);
+  EXPECT_NE(id2, id1);
+
+  // Expect id1 reuse after Flush.
+  gl_->Flush();
+  gl_->GenBuffers(1, &id3);
+  EXPECT_EQ(id3, id1);
+}
+
+// glGen* Ids must not be reused until glDelete* commands have been
+// flushed by glFlush.
+TEST_F(GLES2ImplementationStrictSharedTest, FlushGenerationGenFramebuffers) {
+  GLuint id1, id2, id3;
+
+  // Generate valid id.
+  gl_->GenFramebuffers(1, &id1);
+  EXPECT_NE(id1, 0u);
+
+  // Delete id1 and generate id2.  id1 should not be reused.
+  gl_->DeleteFramebuffers(1, &id1);
+  gl_->GenFramebuffers(1, &id2);
+  EXPECT_NE(id2, 0u);
+  EXPECT_NE(id2, id1);
+
+  // Expect id1 reuse after Flush.
+  gl_->Flush();
+  gl_->GenFramebuffers(1, &id3);
+  EXPECT_EQ(id3, id1);
+}
+
+// glGen* Ids must not be reused until glDelete* commands have been
+// flushed by glFlush.
+TEST_F(GLES2ImplementationStrictSharedTest, FlushGenerationGenRenderbuffers) {
+  GLuint id1, id2, id3;
+
+  // Generate valid id.
+  gl_->GenRenderbuffers(1, &id1);
+  EXPECT_NE(id1, 0u);
+
+  // Delete id1 and generate id2.  id1 should not be reused.
+  gl_->DeleteRenderbuffers(1, &id1);
+  gl_->GenRenderbuffers(1, &id2);
+  EXPECT_NE(id2, 0u);
+  EXPECT_NE(id2, id1);
+
+  // Expect id1 reuse after Flush.
+  gl_->Flush();
+  gl_->GenRenderbuffers(1, &id3);
+  EXPECT_EQ(id3, id1);
+}

[piman, 2014/02/12 23:52:34]

It would be good to add tests for multiple contexts that share.

For example, I think something like:

gl1_->GenTextures(1, &id1);
gl1_->DeleteTextures(1, &id1);
gl2_->Flush();   // Flush gl2, not gl1
gl2_->GenTextures(1, &id2);  // gl1 wasn't flushed, we shouldn't reuse the same
id, whose Delete (or even Gen) hasn't made it to the service side.
EXPECT_NE(id1, id2);

would fail with the current code (see comments I left in share_group.cc), but
should succeed.


It'd be good to test a variety of such cases.

[vmiura, 2014/02/14 00:11:11]

Added.  Please let me know if you think other cases would be useful.

+
 TEST_F(GLES2ImplementationTest, GetString) {
   const uint32 kBucketId = GLES2Implementation::kResultBucketId;
   const Str7 kString = {"foobar"};