Port GLContextCGL backpresure fence workarounds to EGL.

With the passthrough command decoder on Mac, backpressure fence and flush
workarounds need to be done in the GLContextEGL as well.  Move them to the
base GLContext class so they are usable by both context types.

Fixes flickering issues on UI elements on Mac when using the passthrough
command decoder.

BUG=1011017

Change-Id: Iba226caada24341abc58053e7d033e114d16df3d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1838371
Commit-Queue: Geoff Lang <geofflang@chromium.org>
Reviewed-by: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703699}

gpu/command_buffer/service/gles2_cmd_decoder_passthrough.cc

@@ -699,6 +699,15 @@ GLES2Decoder::Error GLES2DecoderPassthroughImpl::DoCommandsImpl(
   if (entries_processed)
     *entries_processed = process_pos;
 
+#if defined(OS_MACOSX)
+  // Aggressively call glFlush on macOS. This is the only fix that has been
+  // found so far to avoid crashes on Intel drivers. The workaround
+  // isn't needed for WebGL contexts, though.
+  // https://crbug.com/863817
+  if (!feature_info_->IsWebGLContext())
+    context_->FlushForDriverCrashWorkaround();
+#endif
+
   return result;
 }
 

ui/gl/gl_context.cc

@@ -14,8 +14,10 @@
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_util.h"
 #include "base/threading/thread_local.h"
+#include "base/trace_event/trace_event.h"
 #include "build/build_config.h"
 #include "ui/gl/gl_bindings.h"
+#include "ui/gl/gl_fence.h"
 #include "ui/gl/gl_gl_api_implementation.h"
 #include "ui/gl/gl_implementation.h"
 #include "ui/gl/gl_surface.h"
@@ -60,6 +62,9 @@ GLContext::GLContext(GLShareGroup* share_group) : share_group_(share_group) {
 }
 
 GLContext::~GLContext() {
+#if defined(OS_MACOSX)
+  DCHECK(!HasBackpressureFences());
+#endif
   share_group_->RemoveContext(this);
   if (GetCurrent() == this) {
     SetCurrent(nullptr);
@@ -193,13 +198,82 @@ void GLContext::DirtyVirtualContextState() {
 }
 
 #if defined(OS_MACOSX)
+constexpr uint64_t kInvalidFenceId = 0;
+
 uint64_t GLContext::BackpressureFenceCreate() {
-  return 0;
+  TRACE_EVENT0("gpu", "GLContextEGL::BackpressureFenceCreate");
+
+  // This flush will trigger a crash if FlushForDriverCrashWorkaround is not
+  // called sufficiently frequently.
+  glFlush();
+
+  if (gl::GLFence::IsSupported()) {
+    next_backpressure_fence_ += 1;
+    backpressure_fences_[next_backpressure_fence_] = GLFence::Create();
+    return next_backpressure_fence_;
+  }
+  glFinish();
+  return kInvalidFenceId;
 }
 
-void GLContext::BackpressureFenceWait(uint64_t fence) {}
+void GLContext::BackpressureFenceWait(uint64_t fence_id) {
+  TRACE_EVENT0("gpu", "GLContext::BackpressureFenceWait");
+  if (fence_id == kInvalidFenceId) {
+    return;
+  }
 
-void GLContext::FlushForDriverCrashWorkaround() {}
+  // If a fence is not found, then it has already been waited on.
+  auto found = backpressure_fences_.find(fence_id);
+  if (found == backpressure_fences_.end())
+    return;
+  std::unique_ptr<GLFence> fence = std::move(found->second);
+  backpressure_fences_.erase(found);
+
+  // While we could call GLFence::ClientWait, this performs a busy wait on
+  // Mac, leading to high CPU usage. Instead we poll with a 1ms delay. This
+  // should have minimal impact, as we will only hit this path when we are
+  // more than one frame (16ms) behind.
+  //
+  // Note that on some platforms (10.9), fences appear to sometimes get
+  // lost and will never pass. Add a 32ms timeout to prevent these
+  // situations from causing a GPU process hang.
+  // https://crbug.com/618075
+  bool fence_completed = false;
+  for (int poll_iter = 0; !fence_completed && poll_iter < 32; ++poll_iter) {
+    if (poll_iter > 0) {
+      base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(1));
+    }
+    {
+      TRACE_EVENT0("gpu", "GLFence::HasCompleted");
+      fence_completed = fence->HasCompleted();
+    }
+  }
+  if (!fence_completed) {
+    TRACE_EVENT0("gpu", "Finish");
+    // We timed out waiting for the above fence, just issue a glFinish.
+    glFinish();
+  }
+  fence.reset();
+
+  // Waiting on |fence_id| has implicitly waited on all previous fences, so
+  // remove them.
+  while (backpressure_fences_.begin()->first < fence_id)
+    backpressure_fences_.erase(backpressure_fences_.begin());
+}
+
+bool GLContext::HasBackpressureFences() const {
+  return !backpressure_fences_.empty();
+}
+
+void GLContext::DestroyBackpressureFences() {
+  backpressure_fences_.clear();
+}
+
+void GLContext::FlushForDriverCrashWorkaround() {
+  if (!IsCurrent(nullptr))
+    return;
+  glFlush();
+}
 #endif
 
 bool GLContext::HasExtension(const char* name) {

ui/gl/gl_context.h

@@ -5,6 +5,7 @@
 #ifndef UI_GL_GL_CONTEXT_H_
 #define UI_GL_GL_CONTEXT_H_
 
+#include <map>
 #include <memory>
 #include <string>
 
@@ -39,6 +40,7 @@ struct CurrentGL;
 class DebugGLApi;
 struct DriverGL;
 class GLApi;
+class GLFence;
 class GLSurface;
 class GPUTiming;
 class GPUTimingClient;
@@ -256,6 +258,13 @@ class GL_EXPORT GLContext : public base::RefCounted<GLContext> {
 
   GLApi* gl_api() { return gl_api_.get(); }
 
+#if defined(OS_MACOSX)
+  // Child classes are responsible for calling DestroyBackpressureFences during
+  // their destruction while a context is current.
+  bool HasBackpressureFences() const;
+  void DestroyBackpressureFences();
+#endif
+
  private:
   friend class base::RefCounted<GLContext>;
 
@@ -290,6 +299,11 @@ class GL_EXPORT GLContext : public base::RefCounted<GLContext> {
   std::unique_ptr<GLStateRestorer> state_restorer_;
   std::unique_ptr<GLVersionInfo> version_info_;
 
+#if defined(OS_MACOSX)
+  std::map<uint64_t, std::unique_ptr<GLFence>> backpressure_fences_;
+  uint64_t next_backpressure_fence_ = 0;
+#endif
+
   DISALLOW_COPY_AND_ASSIGN(GLContext);
 };
 

ui/gl/gl_context_cgl.cc

@@ -19,7 +19,6 @@
 #include "base/trace_event/trace_event.h"
 #include "ui/gl/dual_gpu_state_mac.h"
 #include "ui/gl/gl_bindings.h"
-#include "ui/gl/gl_fence.h"
 #include "ui/gl/gl_gl_api_implementation.h"
 #include "ui/gl/gl_implementation.h"
 #include "ui/gl/gl_surface.h"
@@ -131,7 +130,7 @@ bool GLContextCGL::Initialize(GLSurface* compatible_surface,
 }
 
 void GLContextCGL::Destroy() {
-  if (!yuv_to_rgb_converters_.empty() || !backpressure_fences_.empty()) {
+  if (!yuv_to_rgb_converters_.empty() || HasBackpressureFences()) {
     // If this context is not current, bind this context's API so that the YUV
     // converter and GLFences can safely destruct
     GLContext* current_context = GetRealCurrent();
@@ -142,7 +141,7 @@ void GLContextCGL::Destroy() {
     ScopedCGLSetCurrentContext scoped_set_current(
         static_cast<CGLContextObj>(context_));
     yuv_to_rgb_converters_.clear();
-    backpressure_fences_.clear();
+    DestroyBackpressureFences();
 
     // Rebind the current context's API if needed.
     if (current_context && current_context != this) {
@@ -211,75 +210,6 @@ YUVToRGBConverter* GLContextCGL::GetYUVToRGBConverter(
   return yuv_to_rgb_converter.get();
 }
 
-constexpr uint64_t kInvalidFenceId = 0;
-
-uint64_t GLContextCGL::BackpressureFenceCreate() {
-  TRACE_EVENT0("gpu", "GLContextCGL::BackpressureFenceCreate");
-
-  // This flush will trigger a crash if FlushForDriverCrashWorkaround is not
-  // called sufficiently frequently.
-  glFlush();
-
-  if (gl::GLFence::IsSupported()) {
-    next_backpressure_fence_ += 1;
-    backpressure_fences_[next_backpressure_fence_] = GLFence::Create();
-    return next_backpressure_fence_;
-  }
-  glFinish();
-  return kInvalidFenceId;
-}
-
-void GLContextCGL::BackpressureFenceWait(uint64_t fence_id) {
-  TRACE_EVENT0("gpu", "GLContextCGL::BackpressureFenceWait");
-  if (fence_id == kInvalidFenceId) {
-    return;
-  }
-
-  // If a fence is not found, then it has already been waited on.
-  auto found = backpressure_fences_.find(fence_id);
-  if (found == backpressure_fences_.end())
-    return;
-  std::unique_ptr<GLFence> fence = std::move(found->second);
-  backpressure_fences_.erase(found);
-
-  // While we could call GLFence::ClientWait, this performs a busy wait on
-  // Mac, leading to high CPU usage. Instead we poll with a 1ms delay. This
-  // should have minimal impact, as we will only hit this path when we are
-  // more than one frame (16ms) behind.
-  //
-  // Note that on some platforms (10.9), fences appear to sometimes get
-  // lost and will never pass. Add a 32ms timout to prevent these
-  // situations from causing a GPU process hang.
-  // https://crbug.com/618075
-  bool fence_completed = false;
-  for (int poll_iter = 0; !fence_completed && poll_iter < 32; ++poll_iter) {
-    if (poll_iter > 0) {
-      base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(1));
-    }
-    {
-      TRACE_EVENT0("gpu", "GLFence::HasCompleted");
-      fence_completed = fence->HasCompleted();
-    }
-  }
-  if (!fence_completed) {
-    TRACE_EVENT0("gpu", "Finish");
-    // We timed out waiting for the above fence, just issue a glFinish.
-    glFinish();
-  }
-  fence.reset();
-
-  // Waiting on |fence_id| has implicitly waited on all previous fences, so
-  // remove them.
-  while (backpressure_fences_.begin()->first < fence_id)
-    backpressure_fences_.erase(backpressure_fences_.begin());
-}
-
-void GLContextCGL::FlushForDriverCrashWorkaround() {
-  if (!context_ || CGLGetCurrentContext() != context_)
-    return;
-  glFlush();
-}
-
 bool GLContextCGL::MakeCurrent(GLSurface* surface) {
   DCHECK(context_);
 

ui/gl/gl_context_cgl.h

@@ -7,7 +7,6 @@
 
 #include <OpenGL/CGLTypes.h>
 
-#include <map>
 #include <memory>
 
 #include "base/macros.h"
@@ -17,7 +16,6 @@
 
 namespace gl {
 
-class GLFence;
 class GLSurface;
 
 // Encapsulates a CGL OpenGL context.
@@ -36,9 +34,6 @@ class GL_EXPORT GLContextCGL : public GLContextReal {
   bool ForceGpuSwitchIfNeeded() override;
   YUVToRGBConverter* GetYUVToRGBConverter(
       const gfx::ColorSpace& color_space) override;
-  uint64_t BackpressureFenceCreate() override;
-  void BackpressureFenceWait(uint64_t fence) override;
-  void FlushForDriverCrashWorkaround() override;
   void SetVisibility(bool visibility) override;
 
  protected:
@@ -53,9 +48,6 @@ class GL_EXPORT GLContextCGL : public GLContextReal {
   std::map<gfx::ColorSpace, std::unique_ptr<YUVToRGBConverter>>
       yuv_to_rgb_converters_;
 
-  std::map<uint64_t, std::unique_ptr<GLFence>> backpressure_fences_;
-  uint64_t next_backpressure_fence_ = 0;
-
   int screen_ = -1;
   int renderer_id_ = -1;
   bool safe_to_force_gpu_switch_ = true;

ui/gl/gl_context_egl.cc

@@ -21,6 +21,7 @@
 #include "third_party/khronos/EGL/eglext.h"
 #include "ui/gl/egl_util.h"
 #include "ui/gl/gl_bindings.h"
+#include "ui/gl/gl_fence.h"
 #include "ui/gl/gl_gl_api_implementation.h"
 #include "ui/gl/gl_surface_egl.h"
 #include "ui/gl/yuv_to_rgb_converter.h"
@@ -214,7 +215,7 @@ bool GLContextEGL::Initialize(GLSurface* compatible_surface,
 }
 
 void GLContextEGL::Destroy() {
-  ReleaseYUVToRGBConverters();
+  ReleaseYUVToRGBConvertersAndBackpressureFences();
   if (context_) {
     if (!eglDestroyContext(display_, context_)) {
       LOG(ERROR) << "eglDestroyContext failed with error "
@@ -230,7 +231,7 @@ YUVToRGBConverter* GLContextEGL::GetYUVToRGBConverter(
   // Make sure YUVToRGBConverter objects never get created when surfaceless EGL
   // contexts aren't supported since support for surfaceless EGL contexts is
   // required in order to properly release YUVToRGBConverter objects (see
-  // GLContextEGL::ReleaseYUVToRGBConverters())
+  // GLContextEGL::ReleaseYUVToRGBConvertersAndBackpressureFences())
   if (!GLSurfaceEGL::IsEGLSurfacelessContextSupported()) {
     return nullptr;
   }
@@ -244,8 +245,14 @@ YUVToRGBConverter* GLContextEGL::GetYUVToRGBConverter(
   return yuv_to_rgb_converter.get();
 }
 
-void GLContextEGL::ReleaseYUVToRGBConverters() {
-  if (!yuv_to_rgb_converters_.empty()) {
+void GLContextEGL::ReleaseYUVToRGBConvertersAndBackpressureFences() {
+#if defined(OS_MACOSX)
+  bool has_backpressure_fences = HasBackpressureFences();
+#else
+  bool has_backpressure_fences = false;
+#endif
+
+  if (!yuv_to_rgb_converters_.empty() || has_backpressure_fences) {
     // If this context is not current, bind this context's API so that the YUV
     // converter can safely destruct
     GLContext* current_context = GetRealCurrent();
@@ -269,6 +276,9 @@ void GLContextEGL::ReleaseYUVToRGBConverters() {
     }
 
     yuv_to_rgb_converters_.clear();
+#if defined(OS_MACOSX)
+    DestroyBackpressureFences();
+#endif
 
     // Rebind the current context's API if needed.
     if (current_context && current_context != this) {

ui/gl/gl_context_egl.h

@@ -43,7 +43,7 @@ class GL_EXPORT GLContextEGL : public GLContextReal {
 
  private:
   void Destroy();
-  void ReleaseYUVToRGBConverters();
+  void ReleaseYUVToRGBConvertersAndBackpressureFences();
 
   EGLContext context_ = nullptr;
   EGLDisplay display_ = nullptr;

ui/gl/gl_context_stub.cc

@@ -75,6 +75,10 @@ bool GLContextStub::HasRobustness() {
          HasExtension("GL_KHR_robustness") || HasExtension("GL_EXT_robustness");
 }
 
+#if defined(OS_MACOSX)
+void GLContextStub::FlushForDriverCrashWorkaround() {}
+#endif
+
 GLContextStub::~GLContextStub() {}
 
 GLApi* GLContextStub::CreateGLApi(DriverGL* driver) {

ui/gl/gl_context_stub.h

@@ -6,6 +6,7 @@
 #define UI_GL_GL_CONTEXT_STUB_H_
 
 #include "base/macros.h"
+#include "build/build_config.h"
 #include "ui/gl/gl_context.h"
 #include "ui/gl/gl_export.h"
 
@@ -35,6 +36,10 @@ class GL_EXPORT GLContextStub : public GLContextReal {
   void SetGLVersionString(const char* version_str);
   bool HasRobustness();
 
+#if defined(OS_MACOSX)
+  void FlushForDriverCrashWorkaround() override;
+#endif
+
  protected:
   ~GLContextStub() override;