gpu: Fix some context lost marking glitches+leaks and add UMA stats

gpu/command_buffer/service/gles2_cmd_decoder.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "gpu/command_buffer/service/gles2_cmd_decoder.h"
 
 #include <stdio.h>
 
 #include <algorithm>
 #include <list>
@@ skipping 742 matching lines @@
   base::Closure AsyncUploadTokenCompletionClosure(uint32 async_upload_token,
                                                   uint32 sync_data_shm_id,
                                                   uint32 sync_data_shm_offset);
 
 
 
   // Workarounds
   void OnFboChanged() const;
   void OnUseFramebuffer() const;
 
+  error::ContextLostReason GetContextLostReasonFromResetStatus(
+      GLenum reset_status) const;
+
   // TODO(gman): Cache these pointers?
   BufferManager* buffer_manager() {
     return group_->buffer_manager();
   }
 
   RenderbufferManager* renderbuffer_manager() {
     return group_->renderbuffer_manager();
   }
 
   FramebufferManager* framebuffer_manager() {
@@ skipping 875 matching lines @@
                        uint32 shm_id,
                        uint32 shm_offset,
                        error::Error* error,
                        GLint* real_location,
                        GLuint* service_id,
                        void** result,
                        GLenum* result_type,
                        GLsizei* result_size);
 
   void MaybeExitOnContextLost();
-  bool WasContextLost() override;
-  bool WasContextLostByRobustnessExtension() override;
-  void LoseContext(uint32 reset_status) override;
+  bool WasContextLost() const override;
+  bool WasContextLostByRobustnessExtension() const override;
+  void MarkContextLost(error::ContextLostReason reason) override;
+  bool CheckResetStatus();
 
 #if defined(OS_MACOSX)
   void ReleaseIOSurfaceForTexture(GLuint texture_id);
 #endif
 
   bool ValidateCompressedTexDimensions(
       const char* function_name,
       GLint level, GLsizei width, GLsizei height, GLenum format);
   bool ValidateCompressedTexFuncData(
       const char* function_name,
@@ skipping 182 matching lines @@
   // Cached from ContextGroup
   const Validators* validators_;
   scoped_refptr<FeatureInfo> feature_info_;
 
   int frame_number_;
 
   // Number of commands remaining to be processed in DoCommands().
   int commands_to_process_;
 
   bool has_robustness_extension_;
-  GLenum reset_status_;
+  error::ContextLostReason context_lost_reason_;
+  bool context_was_lost_;
   bool reset_by_robustness_extension_;
   bool supports_post_sub_buffer_;
 
   // These flags are used to override the state of the shared feature_info_
   // member.  Because the same FeatureInfo instance may be shared among many
   // contexts, the assumptions on the availablity of extensions in WebGL
   // contexts may be broken.  These flags override the shared state to preserve
   // WebGL semantics.
   bool force_webgl_glsl_validation_;
   bool derivatives_explicitly_enabled_;
@@ skipping 521 matching lines @@
       back_buffer_has_depth_(false),
       back_buffer_has_stencil_(false),
       surfaceless_(false),
       backbuffer_needs_clear_bits_(0),
       current_decoder_error_(error::kNoError),
       use_shader_translator_(true),
       validators_(group_->feature_info()->validators()),
       feature_info_(group_->feature_info()),
       frame_number_(0),
       has_robustness_extension_(false),
-      reset_status_(GL_NO_ERROR),
+      context_lost_reason_(error::kUnknown),
+      context_was_lost_(false),
       reset_by_robustness_extension_(false),
       supports_post_sub_buffer_(false),
       force_webgl_glsl_validation_(false),
       derivatives_explicitly_enabled_(false),
       frag_depth_explicitly_enabled_(false),
       draw_buffers_explicitly_enabled_(false),
       shader_texture_lod_explicitly_enabled_(false),
       compile_shader_always_succeeds_(false),
       lose_context_when_out_of_memory_(false),
       service_logging_(base::CommandLine::ForCurrentProcess()->HasSwitch(
@@ skipping 901 matching lines @@
     }
   }
 }
 
 // }  // anonymous namespace
 
 bool GLES2DecoderImpl::MakeCurrent() {
   if (!context_.get())
     return false;
 
-  if (!context_->MakeCurrent(surface_.get()) || WasContextLost()) {
-    LOG(ERROR) << "  GLES2DecoderImpl: Context lost during MakeCurrent.";
-
-    MaybeExitOnContextLost();
-
+  if (WasContextLost()) {
+    LOG(ERROR) << "  GLES2DecoderImpl: Trying to make lost context current.";
     return false;
   }
 
+  if (!context_->MakeCurrent(surface_.get())) {
+    LOG(ERROR) << "  GLES2DecoderImpl: Context lost during MakeCurrent.";
+    MarkContextLost(error::kMakeCurrentFailed);
+    MaybeExitOnContextLost();
+    return false;
+  }
+
+  if (CheckResetStatus()) {

[piman, 2015/04/18 01:03:18]

Do we really want to check this on every MakeCurrent?

[Ken Russell (switch to Gerrit), 2015/04/24 02:39:41]

Actually, I think it was already being done in WasContextLost(). It's supposed
to be inexpensive.

[no sievers, 2015/04/24 18:52:47]

We might also want to check it proactively here for EXT_robustness, because
unlike with KHR_robustness we wouldn't get the 'context lost' GL error and never
reach OnContextLostError().

+    LOG(ERROR)
+        << "  GLES2DecoderImpl: Context reset detected after MakeCurrent.";
+    return false;
+  }
+
   ProcessFinishedAsyncTransfers();
 
   // Rebind the FBO if it was unbound by the context.
   if (workarounds().unbind_fbo_on_context_switch)
     RestoreFramebufferBindings();
 
   framebuffer_state_.clear_state_dirty = true;
 
   return true;
 }
@@ skipping 6944 matching lines @@
       offscreen_saved_color_texture_->AllocateStorage(
           offscreen_size_, offscreen_saved_color_format_, false);
 
       offscreen_saved_frame_buffer_->AttachRenderTexture(
           offscreen_saved_color_texture_.get());
       if (offscreen_size_.width() != 0 && offscreen_size_.height() != 0) {
         if (offscreen_saved_frame_buffer_->CheckStatus() !=
             GL_FRAMEBUFFER_COMPLETE) {
           LOG(ERROR) << "GLES2DecoderImpl::ResizeOffscreenFrameBuffer failed "
                      << "because offscreen saved FBO was incomplete.";
-          LoseContext(GL_UNKNOWN_CONTEXT_RESET_ARB);
+          MarkContextLost(error::kUnknown);

[piman, 2015/04/18 01:03:18]

Here, and other places we call MarkContextLost, we should actually call
group_->LoseContexts() too. This is a requirement from the spec, and actually
otherwise may cause some leaks (context1 is lost, we can't destroy resources on
it. If unrelated-but-shared context2 is not lost, then we will not tear down the
context group and never release resources of context2.).

I think this has been missing for a while and causing some problems.

           return;
         }
 
         // Clear the offscreen color texture.
         // TODO(piman): Is this still necessary?
         {
           ScopedFrameBufferBinder binder(this,
                                          offscreen_saved_frame_buffer_->id());
           glClearColor(0, 0, 0, 0);
           state_.SetDeviceColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE);
@@ skipping 36 matching lines @@
 
       // Ensure the side effects of the copy are visible to the parent
       // context. There is no need to do this for ANGLE because it uses a
       // single D3D device for all contexts.
       if (!feature_info_->gl_version_info().is_angle)
         glFlush();
     }
   } else {
     if (!surface_->SwapBuffers()) {
       LOG(ERROR) << "Context lost because SwapBuffers failed.";
-      LoseContext(GL_UNKNOWN_CONTEXT_RESET_ARB);
+      if (!CheckResetStatus())
+        MarkContextLost(error::kUnknown);
     }
   }
 
   // This may be a slow command.  Exit command processing to allow for
   // context preemption and GPU watchdog checks.
   ExitCommandProcessingEarly();
 }
 
 void GLES2DecoderImpl::DoSwapInterval(int interval) {
   context_->SetSwapInterval(interval);
@@ skipping 239 matching lines @@
   Program* program = NULL;
   program = GetProgram(program_id);
   if (!program || !program->IsValid()) {
     return error::kNoError;
   }
   program->GetTransformFeedbackVaryings(bucket);
   return error::kNoError;
 }
 
 error::ContextLostReason GLES2DecoderImpl::GetContextLostReason() {
-  switch (reset_status_) {
+  return context_lost_reason_;
+}
+
+error::ContextLostReason GLES2DecoderImpl::GetContextLostReasonFromResetStatus(
+    GLenum reset_status) const {
+  switch (reset_status) {
     case GL_NO_ERROR:
       // TODO(kbr): improve the precision of the error code in this case.
       // Consider delegating to context for error code if MakeCurrent fails.
       return error::kUnknown;
     case GL_GUILTY_CONTEXT_RESET_ARB:
       return error::kGuilty;
     case GL_INNOCENT_CONTEXT_RESET_ARB:
       return error::kInnocent;
     case GL_UNKNOWN_CONTEXT_RESET_ARB:
       return error::kUnknown;
   }
 
   NOTREACHED();
   return error::kUnknown;
 }
 
 void GLES2DecoderImpl::MaybeExitOnContextLost() {
   // Some D3D drivers cannot recover from device lost in the GPU process
   // sandbox. Allow a new GPU process to launch.
   if (workarounds().exit_on_context_lost) {
     LOG(ERROR) << "Exiting GPU process because some drivers cannot reset"
                << " a D3D device in the Chrome GPU process sandbox.";
 #if defined(OS_WIN)
     base::win::SetShouldCrashOnProcessDetach(false);
 #endif
     exit(0);
   }
 }
 
-bool GLES2DecoderImpl::WasContextLost() {
-  if (reset_status_ != GL_NO_ERROR) {
-    MaybeExitOnContextLost();
+bool GLES2DecoderImpl::WasContextLost() const {
+  return context_was_lost_;
+}
+
+bool GLES2DecoderImpl::WasContextLostByRobustnessExtension() const {
+  return WasContextLost() && reset_by_robustness_extension_;
+}
+
+void GLES2DecoderImpl::MarkContextLost(error::ContextLostReason reason) {
+  // Only lose the context once.
+  if (WasContextLost())
+    return;
+
+  // Don't make GL calls in here, the context might not be current.
+  context_lost_reason_ = reason;
+  current_decoder_error_ = error::kLostContext;
+  context_was_lost_ = true;
+  MaybeExitOnContextLost();
+}
+
+bool GLES2DecoderImpl::CheckResetStatus() {
+  DCHECK(!WasContextLost());
+  DCHECK(context_->IsCurrent(NULL));
+
+  if (IsRobustnessSupported()) {
+    // If the reason for the call was a GL error, we can try to determine the
+    // reset status more accurately.
+    GLenum driver_status = glGetGraphicsResetStatusARB();
+    if (driver_status == GL_NO_ERROR)
+      return false;
+
+    LOG(ERROR) << (surface_->IsOffscreen() ? "Offscreen" : "Onscreen")
+               << " context lost via ARB/EXT_robustness. Reset status = "
+               << GLES2Util::GetStringEnum(driver_status);
+
+    // Don't pretend we know which client was responsible.
+    if (workarounds().use_virtualized_gl_contexts)
+      driver_status = GL_UNKNOWN_CONTEXT_RESET_ARB;
+
+    switch (driver_status) {
+      case GL_GUILTY_CONTEXT_RESET_ARB:
+        MarkContextLost(error::kGuilty);
+        break;
+      case GL_INNOCENT_CONTEXT_RESET_ARB:
+        MarkContextLost(error::kInnocent);
+        break;
+      case GL_UNKNOWN_CONTEXT_RESET_ARB:
+        MarkContextLost(error::kUnknown);
+        break;
+      default:
+        NOTREACHED();
+        return false;
+    }
+    reset_by_robustness_extension_ = true;
     return true;
   }
-  if (IsRobustnessSupported()) {
-    GLenum status = glGetGraphicsResetStatusARB();
-    if (status != GL_NO_ERROR) {
-      // The graphics card was reset. Signal a lost context to the application.
-      reset_status_ = status;
-      reset_by_robustness_extension_ = true;
-      LOG(ERROR) << (surface_->IsOffscreen() ? "Offscreen" : "Onscreen")
-                 << " context lost via ARB/EXT_robustness. Reset status = "
-                 << GLES2Util::GetStringEnum(status);
-      MaybeExitOnContextLost();
-      return true;
-    }
-  }
   return false;
 }
 
-bool GLES2DecoderImpl::WasContextLostByRobustnessExtension() {
-  return WasContextLost() && reset_by_robustness_extension_;
-}
-
-void GLES2DecoderImpl::LoseContext(uint32 reset_status) {
-  // Only loses the context once.
-  if (reset_status_ != GL_NO_ERROR) {
-    return;
-  }
-
-  if (workarounds().use_virtualized_gl_contexts) {
-    // If the context is virtual, the real context being guilty does not ensure
-    // that the virtual context is guilty.
-    if (reset_status == GL_GUILTY_CONTEXT_RESET_ARB) {
-      reset_status = GL_UNKNOWN_CONTEXT_RESET_ARB;
-    }
-  } else if (reset_status == GL_UNKNOWN_CONTEXT_RESET_ARB &&
-             IsRobustnessSupported()) {
-    // If the reason for the call was a GL error, we can try to determine the
-    // reset status more accurately.
-    GLenum driver_status = glGetGraphicsResetStatusARB();
-    if (driver_status == GL_GUILTY_CONTEXT_RESET_ARB ||
-        driver_status == GL_INNOCENT_CONTEXT_RESET_ARB) {
-      reset_status = driver_status;
-    }
-  }
-
-  // Marks this context as lost.
-  reset_status_ = reset_status;
-  current_decoder_error_ = error::kLostContext;
-}
-
 error::Error GLES2DecoderImpl::HandleInsertSyncPointCHROMIUM(
     uint32 immediate_data_size,
     const void* cmd_data) {
   return error::kUnknownCommand;
 }
 
 error::Error GLES2DecoderImpl::HandleWaitSyncPointCHROMIUM(
     uint32 immediate_data_size,
     const void* cmd_data) {
   const gles2::cmds::WaitSyncPointCHROMIUM& c =
@@ skipping 1353 matching lines @@
     if (GetBackbufferServiceId() != 0 &&  // emulated backbuffer
         bufs[0] == GL_BACK) {
       mapped_buf = GL_COLOR_ATTACHMENT0;
     }
     glDrawBuffersARB(count, &mapped_buf);
     group_->set_draw_buffer(bufs[0]);
   }
 }
 
 void GLES2DecoderImpl::DoLoseContextCHROMIUM(GLenum current, GLenum other) {
-  group_->LoseContexts(other);
-  reset_status_ = current;
-  current_decoder_error_ = error::kLostContext;
+  MarkContextLost(GetContextLostReasonFromResetStatus(current));
+  group_->LoseContexts(GetContextLostReasonFromResetStatus(other));
+  reset_by_robustness_extension_ = true;
 }
 
 void GLES2DecoderImpl::DoMatrixLoadfCHROMIUM(GLenum matrix_mode,
                                              const GLfloat* matrix) {
   DCHECK(matrix_mode == GL_PATH_PROJECTION_CHROMIUM ||
          matrix_mode == GL_PATH_MODELVIEW_CHROMIUM);
   if (!features().chromium_path_rendering) {
     LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION,
                        "glMatrixLoadfCHROMIUM",
                        "function not available");
@@ skipping 497 matching lines @@
   }
   buffer->RemoveMappedRange();
   GLboolean rt = glUnmapBuffer(target);
   if (rt == GL_FALSE) {
     // At this point, we have already done the necessary validation, so
     // GL_FALSE indicates data corruption.
     // TODO(zmo): We could redo the map / copy data / unmap to recover, but
     // the second unmap could still return GL_FALSE. For now, we simply lose
     // the contexts in the share group.
     LOG(ERROR) << "glUnmapBuffer unexpectedly returned GL_FALSE";
-    group_->LoseContexts(GL_INNOCENT_CONTEXT_RESET_ARB);
-    reset_status_ = GL_GUILTY_CONTEXT_RESET_ARB;
+    // Need to lose current context before broadcasting!
+    MarkContextLost(error::kGuilty);
+    group_->LoseContexts(error::kInnocent);
     return error::kLostContext;
   }
   return error::kNoError;
 }
 
 void GLES2DecoderImpl::OnTextureRefDetachedFromFramebuffer(
     TextureRef* texture_ref) {
   Texture* texture = texture_ref->texture();
   DoDidUseTexImageIfNeeded(texture, texture->target());
 }
 
 void GLES2DecoderImpl::OnContextLostError() {
-  group_->LoseContexts(GL_UNKNOWN_CONTEXT_RESET_ARB);
+  if (!WasContextLost()) {
+    // Need to lose current context before broadcasting!
+    CheckResetStatus();
+    group_->LoseContexts(error::kUnknown);
+    reset_by_robustness_extension_ = true;
+  }
 }
 
 void GLES2DecoderImpl::OnOutOfMemoryError() {
-  if (lose_context_when_out_of_memory_) {
-    group_->LoseContexts(GL_UNKNOWN_CONTEXT_RESET_ARB);
+  if (lose_context_when_out_of_memory_ && !WasContextLost()) {
+    error::ContextLostReason other = error::kOutOfMemory;
+    if (CheckResetStatus()) {
+      other = error::kUnknown;
+    } else {
+      // Need to lose current context before broadcasting!
+      MarkContextLost(error::kOutOfMemory);
+    }
+    group_->LoseContexts(other);
   }
 }
 
 // Include the auto-generated part of this file. We split this because it means
 // we can easily edit the non-auto generated parts right here in this file
 // instead of having to edit some template or the code generator.
 #include "gpu/command_buffer/service/gles2_cmd_decoder_autogen.h"
 
 }  // namespace gles2
 }  // namespace gpu