Align CheckResetStatus and MarkContextLost

CheckResetStatus need to three different things and in specific order if
it detects problem:

1. Set `device_needs_reset_` to true. This needs to happen first as it's
   used in MarkContextLost.
2. Call MarkContextLost
3. Return true. This must happen only if context was lost.

Before this CL there was couple of problems:
* default case in switch would return true, but never lose context.
* OutOfMemory case would lost context before setting device_needs_reset_

This CL extracts logic for checking errors from CheckResetStatus into
GetResetStatus to make sure we do all the actions above in one place and
make code less error prone.

Bug: 1126490

Change-Id: I1a86ad2cc783ee2fd2dcf828612f2b657aba23ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2401400
Reviewed-by: Jonathan Backer <backer@chromium.org>
Commit-Queue: Vasiliy Telezhnikov <vasilyt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805765}

gpu/command_buffer/service/shared_context_state.cc

@@ -734,45 +734,40 @@ QueryManager* SharedContextState::GetQueryManager() {
   return nullptr;
 }
 
-bool SharedContextState::CheckResetStatus(bool needs_gl) {
+base::Optional<error::ContextLostReason> SharedContextState::GetResetStatus(
+    bool needs_gl) {
   DCHECK(!context_lost());
 
-  if (device_needs_reset_)
-    return true;
-
   if (gr_context_) {
     // Maybe Skia detected VK_ERROR_DEVICE_LOST.
     if (gr_context_->abandoned()) {
       LOG(ERROR) << "SharedContextState context lost via Skia.";
-      device_needs_reset_ = true;
+      return error::kUnknown;
-      MarkContextLost(error::kUnknown);
-      return true;
     }
 
     if (gr_context_->oomed()) {
       LOG(ERROR) << "SharedContextState context lost via Skia OOM.";
-      device_needs_reset_ = true;
+      return error::kOutOfMemory;
-      MarkContextLost(error::kOutOfMemory);
-      return true;
     }
   }
 
   // Not using GL.
   if (!GrContextIsGL() && !needs_gl)
-    return false;
+    return base::nullopt;
 
   // GL is not initialized.
   if (!context_state_)
-    return false;
+    return base::nullopt;
 
-  GLenum error = context_state_->api()->glGetErrorFn();
+  GLenum error;
-  if (error == GL_OUT_OF_MEMORY) {
+  while ((error = context_state_->api()->glGetErrorFn()) != GL_NO_ERROR) {
-    LOG(ERROR) << "SharedContextState lost due to GL_OUT_OF_MEMORY";
+    if (error == GL_OUT_OF_MEMORY) {
-    MarkContextLost(error::kOutOfMemory);
+      LOG(ERROR) << "SharedContextState lost due to GL_OUT_OF_MEMORY";
-    device_needs_reset_ = true;
+      return error::kOutOfMemory;
-    return true;
+    }
+    if (error == GL_CONTEXT_LOST_KHR)
+      break;
   }
-
   // Checking the reset status is expensive on some OS/drivers
   // (https://crbug.com/1090232). Rate limit it.
   constexpr base::TimeDelta kMinCheckDelay =
@@ -780,33 +775,42 @@ bool SharedContextState::CheckResetStatus(bool needs_gl) {
   base::Time now = base::Time::Now();
   if (!disable_check_reset_status_throttling_for_test_ &&
       now < last_gl_check_graphics_reset_status_ + kMinCheckDelay) {
-    return false;
+    return base::nullopt;
   }
   last_gl_check_graphics_reset_status_ = now;
 
   GLenum driver_status = context()->CheckStickyGraphicsResetStatus();
   if (driver_status == GL_NO_ERROR)
-    return false;
+    return base::nullopt;
   LOG(ERROR) << "SharedContextState context lost via ARB/EXT_robustness. Reset "
                 "status = "
              << gles2::GLES2Util::GetStringEnum(driver_status);
 
-  device_needs_reset_ = true;
   switch (driver_status) {
     case GL_GUILTY_CONTEXT_RESET_ARB:
-      MarkContextLost(error::kGuilty);
+      return error::kGuilty;
-      break;
     case GL_INNOCENT_CONTEXT_RESET_ARB:
-      MarkContextLost(error::kInnocent);
+      return error::kInnocent;
-      break;
     case GL_UNKNOWN_CONTEXT_RESET_ARB:
-      MarkContextLost(error::kUnknown);
+      return error::kUnknown;
-      break;
     default:
       NOTREACHED();
       break;
   }
-  return true;
+  return base::nullopt;
+}
+
+bool SharedContextState::CheckResetStatus(bool need_gl) {
+  DCHECK(!context_lost());
+  DCHECK(!device_needs_reset_);
+
+  auto status = GetResetStatus(need_gl);
+  if (status.has_value()) {
+    device_needs_reset_ = true;
+    MarkContextLost(status.value());
+    return true;
+  }
+  return false;
 }
 
 }  // namespace gpu

gpu/command_buffer/service/shared_context_state.h

@@ -273,6 +273,8 @@ class GPU_GLES2_EXPORT SharedContextState
 
   ~SharedContextState() override;
 
+  base::Optional<error::ContextLostReason> GetResetStatus(bool needs_gl);
+
   // gpu::GLContextVirtualDelegate implementation.
   bool initialized() const override;
   const gles2::ContextState* GetContextState() override;