Dump flushed GL commands to crash key

Instrument GLES2DecoderImpl::DoCommandsImpl to
- record in a crash key all of the GL commands that have been flushed
- flush at the end of every time that it is called

This will severely regress performance.

What we hope to learn:
- There's a good chance this will make the bug vanish (egregious glFlush
  calls have been known to do this), in which case we'll learn nothing.
  - But maybe we can add less egregious glFlush calls to avoid the issue
    in Chrome 69
- If elevated crash rates persist, hopefully this will point to a particular
  call or sequence of calls that is causing the crash

Bug: 863817
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I462fab8a6d86ee3896dcd9c0c3c4aa68f9de0a69
Reviewed-on: https://chromium-review.googlesource.com/1159831
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580213}

gpu/command_buffer/service/gl_context_virtual.cc

@@ -110,6 +110,10 @@ uint64_t GLContextVirtual::BackpressureFenceCreate() {
 void GLContextVirtual::BackpressureFenceWait(uint64_t fence) {
   shared_context_->BackpressureFenceWait(fence);
 }
+
+void GLContextVirtual::FlushForDebugging() {
+  shared_context_->FlushForDebugging();
+}
 #endif
 
 GLContextVirtual::~GLContextVirtual() {

gpu/command_buffer/service/gl_context_virtual.h

@@ -50,6 +50,7 @@ class GPU_GLES2_EXPORT GLContextVirtual : public gl::GLContext {
 #if defined(OS_MACOSX)
   uint64_t BackpressureFenceCreate() override;
   void BackpressureFenceWait(uint64_t fence) override;
+  void FlushForDebugging() override;
 #endif
 
  protected:

gpu/command_buffer/service/gles2_cmd_decoder.cc

@@ -24,6 +24,7 @@
 #include "base/containers/queue.h"
 #include "base/containers/span.h"
 #include "base/debug/alias.h"
+#include "base/debug/crash_logging.h"
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/numerics/ranges.h"
@@ -5617,6 +5618,14 @@ error::Error GLES2DecoderImpl::DoCommandsImpl(unsigned int num_commands,
   int process_pos = 0;
   unsigned int command = 0;
 
+// Instrumentation for https://crbug.com/863817. Keep a list of all commands
+// that are flushed, to see what calls may be the culprit.
+#if defined(OS_MACOSX)
+  static auto* crash_key = base::debug::AllocateCrashKeyString(
+      "mac_gl_commands_in_flush", base::debug::CrashKeySize::Size256);
+  std::string crash_info;
+#endif
+
   while (process_pos < num_entries && result == error::kNoError &&
          commands_to_process_--) {
     const unsigned int size = cmd_data->value_header.size;
@@ -5632,6 +5641,19 @@ error::Error GLES2DecoderImpl::DoCommandsImpl(unsigned int num_commands,
       break;
     }
 
+// Continued instrumentation for https://crbug.com/863817. If we run out of
+// buffer space then flush.
+#if defined(OS_MACOSX)
+    std::string command_name = GetCommandName(command);
+    if (!crash_info.empty() && crash_info.size() + command_name.size() > 256) {
+      TRACE_EVENT0("gpu", "Egregious Flush");
+      context_->FlushForDebugging();
+      crash_info = "";
+    }
+    crash_info += command_name + ",";
+    base::debug::SetCrashKeyString(crash_key, crash_info);
+#endif
+
     if (DebugImpl && log_commands()) {
       LOG(ERROR) << "[" << logger_.GetLogPrefix() << "]"
                  << "cmd: " << GetCommandName(command);
@@ -5690,6 +5712,15 @@ error::Error GLES2DecoderImpl::DoCommandsImpl(unsigned int num_commands,
     }
   }
 
+// Continued instrumentation for https://crbug.com/863817
+#if defined(OS_MACOSX)
+  {
+    TRACE_EVENT0("gpu", "Egregious Flush");
+    context_->FlushForDebugging();
+    base::debug::ClearCrashKeyString(crash_key);
+  }
+#endif
+
   *entries_processed = process_pos;
 
   if (error::IsError(result)) {

ui/gl/gl_context.cc

@@ -194,6 +194,8 @@ uint64_t GLContext::BackpressureFenceCreate() {
 }
 
 void GLContext::BackpressureFenceWait(uint64_t fence) {}
+
+void GLContext::FlushForDebugging() {}
 #endif
 
 bool GLContext::HasExtension(const char* name) {

ui/gl/gl_context.h

@@ -202,6 +202,8 @@ class GL_EXPORT GLContext : public base::RefCounted<GLContext> {
   virtual uint64_t BackpressureFenceCreate();
   // Perform a client-side wait on a previously-created fence.
   virtual void BackpressureFenceWait(uint64_t fence);
+  // Temporary instrumentation for https://crbug.com/863817
+  virtual void FlushForDebugging();
 #endif
 
  protected:

ui/gl/gl_context_cgl.cc

@@ -286,6 +286,12 @@ void GLContextCGL::BackpressureFenceWait(uint64_t fence_id) {
     backpressure_fences_.erase(backpressure_fences_.begin());
 }
 
+void GLContextCGL::FlushForDebugging() {
+  if (!context_ || CGLGetCurrentContext() != context_)
+    return;
+  glFlush();
+}
+
 bool GLContextCGL::MakeCurrent(GLSurface* surface) {
   DCHECK(context_);
 

ui/gl/gl_context_cgl.h

@@ -38,6 +38,7 @@ class GL_EXPORT GLContextCGL : public GLContextReal {
       const gfx::ColorSpace& color_space) override;
   uint64_t BackpressureFenceCreate() override;
   void BackpressureFenceWait(uint64_t fence) override;
+  void FlushForDebugging() override;
 
  protected:
   ~GLContextCGL() override;