Flag INV_OP during BeginQuery if id is not obtained from GenQueriesEXT.

BUG=none

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@265360 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 44 insertions, 26 deletions

diff --git a/gpu/command_buffer/client/gles2_implementation_unittest.cc b/gpu/command_buffer/client/gles2_implementation_unittest.cc
index afb31bd..cb7f6bd 100644
--- a/gpu/command_buffer/client/gles2_implementation_unittest.cc
+++ b/gpu/command_buffer/client/gles2_implementation_unittest.cc
@@ -2906,9 +2906,6 @@ TEST_F(GLES2ImplementationTest, BeginEndQueryEXT) {
   EXPECT_TRUE(NoCommandsWritten());
   EXPECT_EQ(GL_INVALID_OPERATION, CheckError());
 
-  // Test BeginQueryEXT fails if id not GENed.
-  // TODO(gman):
-
   // Test BeginQueryEXT inserts command.
   struct BeginCmds {
     cmds::BeginQueryEXT begin_query;
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index 07d34d7..fdd08f4 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -9467,7 +9467,7 @@ bool GLES2DecoderImpl::GenQueriesEXTHelper(
       return false;
     }
   }
-  // NOTE: We don't generate Query objects here. Only in BeginQueryEXT
+  query_manager_->GenQueries(n, client_ids);
   return true;
 }
 
@@ -9482,8 +9482,8 @@ void GLES2DecoderImpl::DeleteQueriesEXTHelper(
         state_.current_queries.erase(it);
 
       query->Destroy(true);
-      query_manager_->RemoveQuery(client_ids[ii]);
     }
+    query_manager_->RemoveQuery(client_ids[ii]);
   }
 }
 
@@ -9577,24 +9577,12 @@ error::Error GLES2DecoderImpl::HandleBeginQueryEXT(
 
   QueryManager::Query* query = query_manager_->GetQuery(client_id);
   if (!query) {
-    // TODO(gman): Decide if we need this check.
-    //
-    // Checks id was made by glGenQueries
-    //
-    // From the POV of OpenGL ES 2.0 you need to call glGenQueriesEXT
-    // for all Query ids but from the POV of the command buffer service maybe
-    // you don't.
-    //
-    // The client can enforce this. I don't think the service cares.
-    //
-    // IdAllocatorInterface* id_allocator =
-    //     group_->GetIdAllocator(id_namespaces::kQueries);
-    // if (!id_allocator->InUse(client_id)) {
-    //   LOCAL_SET_GL_ERROR(
-    //       GL_INVALID_OPERATION,
-    //       "glBeginQueryEXT", "id not made by glGenQueriesEXT");
-    //   return error::kNoError;
-    // }
+    if (!query_manager_->IsValidQuery(client_id)) {
+      LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION,
+                         "glBeginQueryEXT",
+                         "id not made by glGenQueriesEXT");
+      return error::kNoError;
+    }
     query = query_manager_->CreateQuery(
         target, client_id, sync_shm_id, sync_shm_offset);
   }
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc b/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc
index eece250..8691dd1 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc
@@ -7066,15 +7066,29 @@ TEST_F(GLES2DecoderManualInitTest, BeginEndQueryEXT) {
   EXPECT_CALL(*gl_, BeginQueryARB(GL_ANY_SAMPLES_PASSED_EXT, kNewServiceId))
       .Times(1)
       .RetiresOnSaturation();
+
+  // Query object should not be created untill BeginQueriesEXT.
+  QueryManager* query_manager = decoder_->GetQueryManager();
+  ASSERT_TRUE(query_manager != NULL);
+  QueryManager::Query* query = query_manager->GetQuery(kNewClientId);
+  EXPECT_TRUE(query == NULL);
+
+  // BeginQueryEXT should fail  if id is not generated from GenQueriesEXT.
+  begin_cmd.Init(GL_ANY_SAMPLES_PASSED_EXT,
+                 kInvalidClientId,
+                 kSharedMemoryId,
+                 kSharedMemoryOffset);
+  EXPECT_EQ(error::kNoError, ExecuteCmd(begin_cmd));
+  EXPECT_EQ(GL_INVALID_OPERATION, GetGLError());
+
   begin_cmd.Init(
       GL_ANY_SAMPLES_PASSED_EXT, kNewClientId,
       kSharedMemoryId, kSharedMemoryOffset);
   EXPECT_EQ(error::kNoError, ExecuteCmd(begin_cmd));
   EXPECT_EQ(GL_NO_ERROR, GetGLError());
 
-  QueryManager* query_manager = decoder_->GetQueryManager();
-  ASSERT_TRUE(query_manager != NULL);
-  QueryManager::Query* query = query_manager->GetQuery(kNewClientId);
+  // After BeginQueriesEXT id name should have query object associated with it.
+  query = query_manager->GetQuery(kNewClientId);
   ASSERT_TRUE(query != NULL);
   EXPECT_FALSE(query->pending());
 
diff --git a/gpu/command_buffer/service/query_manager.cc b/gpu/command_buffer/service/query_manager.cc
index 30c036e..ffb54cc 100644
--- a/gpu/command_buffer/service/query_manager.cc
+++ b/gpu/command_buffer/service/query_manager.cc
@@ -512,6 +512,18 @@ QueryManager::Query* QueryManager::CreateQuery(
   return query.get();
 }
 
+void QueryManager::GenQueries(GLsizei n, const GLuint* queries) {
+  DCHECK_GE(n, 0);
+  for (GLsizei i = 0; i < n; ++i) {
+    generated_query_ids_.insert(queries[i]);
+  }
+}
+
+bool QueryManager::IsValidQuery(GLuint id) {
+  GeneratedQueryIds::iterator it = generated_query_ids_.find(id);
+  return it != generated_query_ids_.end();
+}
+
 QueryManager::Query* QueryManager::GetQuery(
     GLuint client_id) {
   QueryMap::iterator it = queries_.find(client_id);
@@ -526,6 +538,7 @@ void QueryManager::RemoveQuery(GLuint client_id) {
     query->MarkAsDeleted();
     queries_.erase(it);
   }
+  generated_query_ids_.erase(client_id);
 }
 
 void QueryManager::StartTracking(QueryManager::Query* /* query */) {
diff --git a/gpu/command_buffer/service/query_manager.h b/gpu/command_buffer/service/query_manager.h
index cbc7931..36d424c 100644
--- a/gpu/command_buffer/service/query_manager.h
+++ b/gpu/command_buffer/service/query_manager.h
@@ -188,6 +188,9 @@ class GPU_EXPORT QueryManager {
     return decoder_;
   }
 
+  void GenQueries(GLsizei n, const GLuint* queries);
+  bool IsValidQuery(GLuint id);
+
  private:
   void StartTracking(Query* query);
   void StopTracking(Query* query);
@@ -228,6 +231,9 @@ class GPU_EXPORT QueryManager {
   typedef base::hash_map<GLuint, scoped_refptr<Query> > QueryMap;
   QueryMap queries_;
 
+  typedef base::hash_set<GLuint> GeneratedQueryIds;
+  GeneratedQueryIds generated_query_ids_;
+
   // Queries waiting for completion.
   typedef std::deque<scoped_refptr<Query> > QueryQueue;
   QueryQueue pending_queries_;