Fix an early return and the error status in glLoseContextCHROMIUM impl

The implementation of glLoseContextCHROMIUM did not return early
in gl error conditions of invalid enums passed as arguments. Add
the early returns and tests for the invalid enums.

Upon successful glLoseContextCHROMIUM, the subsequent calls
such as glGetError would also fail, because the implementation
would set the decoder error variable as well as return the error
value. Fix this by just setting the decoder error variable. Add
a test for this (LoseContextCHROMIUMValidArgs)

BUG=373159

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

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

8 files changed, 64 insertions, 26 deletions

diff --git a/gpu/command_buffer/build_gles2_cmd_buffer.py b/gpu/command_buffer/build_gles2_cmd_buffer.py
index 470ff29..d918b73 100755
--- a/gpu/command_buffer/build_gles2_cmd_buffer.py
+++ b/gpu/command_buffer/build_gles2_cmd_buffer.py
@@ -2540,8 +2540,8 @@ _FUNCTION_INFO = {
     'extension_flag': 'ext_discard_framebuffer',
   },
   'LoseContextCHROMIUM': {
-    'type': 'Manual',
-    'impl_func': True,
+    'decoder_func': 'DoLoseContextCHROMIUM',
+    'unit_test': False,
     'extension': True,
     'chromium': True,
   },
diff --git a/gpu/command_buffer/client/gles2_implementation_impl_autogen.h b/gpu/command_buffer/client/gles2_implementation_impl_autogen.h
index 6ec777c..4f721a8 100644
--- a/gpu/command_buffer/client/gles2_implementation_impl_autogen.h
+++ b/gpu/command_buffer/client/gles2_implementation_impl_autogen.h
@@ -2064,8 +2064,8 @@ void GLES2Implementation::DiscardFramebufferEXT(GLenum target,
 void GLES2Implementation::LoseContextCHROMIUM(GLenum current, GLenum other) {
   GPU_CLIENT_SINGLE_THREAD_CHECK();
   GPU_CLIENT_LOG("[" << GetLogPrefix() << "] glLoseContextCHROMIUM("
-                     << GLES2Util::GetStringEnum(current) << ", "
-                     << GLES2Util::GetStringEnum(other) << ")");
+                     << GLES2Util::GetStringResetStatus(current) << ", "
+                     << GLES2Util::GetStringResetStatus(other) << ")");
   helper_->LoseContextCHROMIUM(current, other);
   CheckGLError();
 }
diff --git a/gpu/command_buffer/client/gles2_implementation_unittest_autogen.h b/gpu/command_buffer/client/gles2_implementation_unittest_autogen.h
index 7b6d70b..d75697e 100644
--- a/gpu/command_buffer/client/gles2_implementation_unittest_autogen.h
+++ b/gpu/command_buffer/client/gles2_implementation_unittest_autogen.h
@@ -1852,9 +1852,10 @@ TEST_F(GLES2ImplementationTest, LoseContextCHROMIUM) {
     cmds::LoseContextCHROMIUM cmd;
   };
   Cmds expected;
-  expected.cmd.Init(1, 2);
+  expected.cmd.Init(GL_GUILTY_CONTEXT_RESET_ARB, GL_GUILTY_CONTEXT_RESET_ARB);
 
-  gl_->LoseContextCHROMIUM(1, 2);
+  gl_->LoseContextCHROMIUM(GL_GUILTY_CONTEXT_RESET_ARB,
+                           GL_GUILTY_CONTEXT_RESET_ARB);
   EXPECT_EQ(0, memcmp(&expected, commands_, sizeof(expected)));
 }
 // TODO: Implement unit test for InsertSyncPointCHROMIUM
diff --git a/gpu/command_buffer/cmd_buffer_functions.txt b/gpu/command_buffer/cmd_buffer_functions.txt
index 0c20a5b..3e38cbd 100644
--- a/gpu/command_buffer/cmd_buffer_functions.txt
+++ b/gpu/command_buffer/cmd_buffer_functions.txt
@@ -215,7 +215,7 @@ GL_APICALL void         GL_APIENTRY glAsyncTexImage2DCHROMIUM (GLenumTextureTarg
 GL_APICALL void         GL_APIENTRY glWaitAsyncTexImage2DCHROMIUM (GLenumTextureTarget target);
 GL_APICALL void         GL_APIENTRY glWaitAllAsyncTexImage2DCHROMIUM (void);
 GL_APICALL void         GL_APIENTRY glDiscardFramebufferEXT (GLenum target, GLsizei count, const GLenum* attachments);
-GL_APICALL void         GL_APIENTRY glLoseContextCHROMIUM (GLenum current, GLenum other);
+GL_APICALL void         GL_APIENTRY glLoseContextCHROMIUM (GLenumResetStatus current, GLenumResetStatus other);
 GL_APICALL GLuint       GL_APIENTRY glInsertSyncPointCHROMIUM (void);
 GL_APICALL void         GL_APIENTRY glWaitSyncPointCHROMIUM (GLuint sync_point);
 GL_APICALL void         GL_APIENTRY glDrawBuffersEXT (GLsizei count, const GLenum* bufs);
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index 8d50037..6deb837 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -948,6 +948,8 @@ class GLES2DecoderImpl : public GLES2Decoder,
 
   void DoDrawBuffersEXT(GLsizei count, const GLenum* bufs);
 
+  void DoLoseContextCHROMIUM(GLenum current, GLenum other);
+
   // Creates a Program for the given program.
   Program* CreateProgram(
       GLuint client_id, GLuint service_id) {
@@ -1716,7 +1718,9 @@ class GLES2DecoderImpl : public GLES2Decoder,
   // Backbuffer attachments that are currently undefined.
   uint32 backbuffer_needs_clear_bits_;
 
-  // The current decoder error.
+  // The current decoder error communicates the decoder error through command
+  // processing functions that do not return the error value. Should be set only
+  // if not returning an error.
   error::Error current_decoder_error_;
 
   bool use_shader_translator_;
@@ -9444,23 +9448,6 @@ void GLES2DecoderImpl::LoseContext(uint32 reset_status) {
   current_decoder_error_ = error::kLostContext;
 }
 
-error::Error GLES2DecoderImpl::HandleLoseContextCHROMIUM(
-    uint32 immediate_data_size, const cmds::LoseContextCHROMIUM& c) {
-  GLenum current = static_cast<GLenum>(c.current);
-  GLenum other = static_cast<GLenum>(c.other);
-  if (!validators_->reset_status.IsValid(current)) {
-    LOCAL_SET_GL_ERROR_INVALID_ENUM(
-        "glLoseContextCHROMIUM", current, "current");
-  }
-  if (!validators_->reset_status.IsValid(other)) {
-    LOCAL_SET_GL_ERROR_INVALID_ENUM("glLoseContextCHROMIUM", other, "other");
-  }
-  group_->LoseContexts(other);
-  reset_status_ = current;
-  current_decoder_error_ = error::kLostContext;
-  return error::kLostContext;
-}
-
 error::Error GLES2DecoderImpl::HandleInsertSyncPointCHROMIUM(
     uint32 immediate_data_size, const cmds::InsertSyncPointCHROMIUM& c) {
   return error::kUnknownCommand;
@@ -10458,6 +10445,12 @@ void GLES2DecoderImpl::DoDrawBuffersEXT(
   }
 }
 
+void GLES2DecoderImpl::DoLoseContextCHROMIUM(GLenum current, GLenum other) {
+  group_->LoseContexts(other);
+  reset_status_ = current;
+  current_decoder_error_ = error::kLostContext;
+}
+
 bool GLES2DecoderImpl::ValidateAsyncTransfer(
     const char* function_name,
     TextureRef* texture_ref,
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_autogen.h b/gpu/command_buffer/service/gles2_cmd_decoder_autogen.h
index 17ce8e3..b9a007a 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder_autogen.h
+++ b/gpu/command_buffer/service/gles2_cmd_decoder_autogen.h
@@ -3398,6 +3398,24 @@ error::Error GLES2DecoderImpl::HandleDiscardFramebufferEXTImmediate(
   return error::kNoError;
 }
 
+error::Error GLES2DecoderImpl::HandleLoseContextCHROMIUM(
+    uint32_t immediate_data_size,
+    const gles2::cmds::LoseContextCHROMIUM& c) {
+  GLenum current = static_cast<GLenum>(c.current);
+  GLenum other = static_cast<GLenum>(c.other);
+  if (!validators_->reset_status.IsValid(current)) {
+    LOCAL_SET_GL_ERROR_INVALID_ENUM(
+        "glLoseContextCHROMIUM", current, "current");
+    return error::kNoError;
+  }
+  if (!validators_->reset_status.IsValid(other)) {
+    LOCAL_SET_GL_ERROR_INVALID_ENUM("glLoseContextCHROMIUM", other, "other");
+    return error::kNoError;
+  }
+  DoLoseContextCHROMIUM(current, other);
+  return error::kNoError;
+}
+
 error::Error GLES2DecoderImpl::HandleDrawBuffersEXT(
     uint32_t immediate_data_size,
     const gles2::cmds::DrawBuffersEXT& c) {
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc b/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc
index 002e142..f97a6dc5b 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc
@@ -1257,6 +1257,33 @@ TEST_P(GLES2DecoderManualInitTest, ImmutableCopyTexImage2D) {
   EXPECT_EQ(GL_INVALID_OPERATION, GetGLError());
 }
 
+TEST_P(GLES2DecoderTest, LoseContextCHROMIUMValidArgs) {
+  EXPECT_CALL(*mock_decoder_, LoseContext(GL_GUILTY_CONTEXT_RESET_ARB))
+      .Times(1);
+  cmds::LoseContextCHROMIUM cmd;
+  cmd.Init(GL_GUILTY_CONTEXT_RESET_ARB, GL_GUILTY_CONTEXT_RESET_ARB);
+  EXPECT_EQ(error::kLostContext, ExecuteCmd(cmd));
+  EXPECT_EQ(GL_NO_ERROR, GetGLError());
+}
+
+TEST_P(GLES2DecoderTest, LoseContextCHROMIUMInvalidArgs0_0) {
+  EXPECT_CALL(*mock_decoder_, LoseContext(_))
+      .Times(0);
+  cmds::LoseContextCHROMIUM cmd;
+  cmd.Init(GL_NONE, GL_GUILTY_CONTEXT_RESET_ARB);
+  EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
+  EXPECT_EQ(GL_INVALID_ENUM, GetGLError());
+}
+
+TEST_P(GLES2DecoderTest, LoseContextCHROMIUMInvalidArgs1_0) {
+  EXPECT_CALL(*mock_decoder_, LoseContext(_))
+      .Times(0);
+  cmds::LoseContextCHROMIUM cmd;
+  cmd.Init(GL_GUILTY_CONTEXT_RESET_ARB, GL_NONE);
+  EXPECT_EQ(error::kNoError, ExecuteCmd(cmd));
+  EXPECT_EQ(GL_INVALID_ENUM, GetGLError());
+}
+
 INSTANTIATE_TEST_CASE_P(Service, GLES2DecoderTest, ::testing::Bool());
 
 INSTANTIATE_TEST_CASE_P(Service, GLES2DecoderWithShaderTest, ::testing::Bool());
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_3_autogen.h b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_3_autogen.h
index 1fc4e8f..7d83956 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_3_autogen.h
+++ b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_3_autogen.h
@@ -59,7 +59,6 @@
 // TODO(gman): DiscardFramebufferEXT
 // TODO(gman): DiscardFramebufferEXTImmediate
 // TODO(gman): LoseContextCHROMIUM
-
 // TODO(gman): InsertSyncPointCHROMIUM
 
 // TODO(gman): WaitSyncPointCHROMIUM