Temporarily remove DCHECK that triggers during hardware teardown.

android_webview/browser/shared_renderer_state.cc

Index: android_webview/browser/shared_renderer_state.cc
diff --git a/android_webview/browser/shared_renderer_state.cc b/android_webview/browser/shared_renderer_state.cc
index 184c86dfff14da71eb9e2cd889f2d8097a075250..0a9e75faea2fd453394c96d528c038f4152613dd 100644
--- a/android_webview/browser/shared_renderer_state.cc
+++ b/android_webview/browser/shared_renderer_state.cc
@@ -69,7 +69,7 @@ void RequestDrawGLTracker::ResetPending() {
 void RequestDrawGLTracker::SetQueuedFunctorOnUi(SharedRendererState* state) {
   base::AutoLock lock(lock_);
   DCHECK(state);
-  DCHECK(pending_ui_ == state || pending_non_ui_ == state);
+  // DCHECK(pending_ui_ == state || pending_non_ui_ == state);

[boliu, 2015/02/05 17:10:22]

Just remove it.

We usually don't comment things out in hopes of uncommenting them in the future,
because it usually never happens and it's just cruft left in the source.

   pending_ui_ = state;
   pending_non_ui_ = NULL;
 }
@@ -279,16 +279,6 @@ void SharedRendererState::DrawGL(AwDrawGLInfo* draw_info) {
     return;
   }
 
-  // kModeProcessNoContext should never happen because we tear down hardware
-  // in onTrimMemory. However that guarantee is maintained outside of chromium
-  // code. Not notifying shared state in kModeProcessNoContext can lead to
-  // immediate deadlock, which is slightly more catastrophic than leaks or
-  // corruption.
-  if (draw_info->mode == AwDrawGLInfo::kModeProcess ||
-      draw_info->mode == AwDrawGLInfo::kModeProcessNoContext) {
-    DidDrawGLProcess();
-  }
-
   {
     GLViewRendererManager* manager = GLViewRendererManager::GetInstance();
     base::AutoLock lock(lock_);
@@ -303,51 +293,69 @@ void SharedRendererState::DrawGL(AwDrawGLInfo* draw_info) {
           : ScopedAppGLStateRestore::MODE_RESOURCE_MANAGEMENT);
   ScopedAllowGL allow_gl;
 
-  if (draw_info->mode == AwDrawGLInfo::kModeProcessNoContext) {
+  switch (draw_info->mode) {
+  case AwDrawGLInfo::kModeProcessNoContext: {

[boliu, 2015/02/05 17:10:22]

Indent here is wrong.

See
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Loops_and_Swi...

In general, don't really have to worry about style issues and just learn to use
clang-format: https://code.google.com/p/chromium/wiki/ClangFormat

[Tobias Sargeant, 2015/02/05 17:29:39]

Done.

     LOG(ERROR) << "Received unexpected kModeProcessNoContext";
+    DidDrawGLProcess();
+    break;
   }
 
-  if (IsInsideHardwareRelease()) {
-    hardware_renderer_.reset();
-    // Flush the idle queue in tear down.
-    DeferredGpuCommandService::GetInstance()->PerformAllIdleWork();
-    return;
-  }
+  case AwDrawGLInfo::kModeProcess: {
+    // kModeProcessNoContext should never happen because we tear down hardware

[boliu, 2015/02/05 17:10:22]

Move this comment up.

[Tobias Sargeant, 2015/02/05 17:29:39]

Done.

+    // in onTrimMemory. However that guarantee is maintained outside of chromium
+    // code. Not notifying shared state in kModeProcessNoContext can lead to
+    // immediate deadlock, which is slightly more catastrophic than leaks or
+    // corruption.
+    DidDrawGLProcess();
 
-  if (draw_info->mode != AwDrawGLInfo::kModeDraw) {
-    if (draw_info->mode == AwDrawGLInfo::kModeProcess) {
+    if (IsInsideHardwareRelease()) {
+      hardware_renderer_.reset();
+      // Flush the idle queue in tear down.
+      DeferredGpuCommandService::GetInstance()->PerformAllIdleWork();
+    } else {
       DeferredGpuCommandService::GetInstance()->PerformIdleWork(true);
     }
-    return;
+    break;
   }
 
-  if (!hardware_renderer_) {
-    hardware_renderer_.reset(new HardwareRenderer(this));
-    hardware_renderer_->CommitFrame();
+  case AwDrawGLInfo::kModeDraw: {
+    if (browser_view_renderer_->IsVisible()) {

[boliu, 2015/02/05 17:10:22]

Not thread safe. BVR is a UI thread only object, and DrawGL is called on render
thread.

What's wrong with using !IsInsideHardwareRelease() here?

[Tobias Sargeant, 2015/02/05 17:29:39]

The problem is that it seems possible to get draw calls that happen after the
teardown, when IsInsideHardwareRelease() is no longer true, but before the
shared_renderer_state destructor is called.

[boliu, 2015/02/05 17:36:17]

And that call is kModeDraw (not kModeProcess)? The answer matters a lot to where
the actual bug is.

The goal here is DrawGL should not be called after
ReleaseHardwareDrawIfNeededOnUI (at least until the next time this BVR is
hardware initialized again)

+      if (!hardware_renderer_) {
+        hardware_renderer_.reset(new HardwareRenderer(this));
+        hardware_renderer_->CommitFrame();
+      }
+
+      hardware_renderer_->DrawGL(state_restore.stencil_enabled(),
+                                 state_restore.framebuffer_binding_ext(),
+                                 draw_info);
+      DeferredGpuCommandService::GetInstance()->PerformIdleWork(false);
+    }
+    break;
   }
 
-  hardware_renderer_->DrawGL(state_restore.stencil_enabled(),
-                             state_restore.framebuffer_binding_ext(),
-                             draw_info);
-  DeferredGpuCommandService::GetInstance()->PerformIdleWork(false);
+  default: {
+    break;
+  }
+  }
 }
 
 void SharedRendererState::ReleaseHardwareDrawIfNeededOnUI() {
   DCHECK(ui_loop_->BelongsToCurrentThread());
-  InsideHardwareReleaseReset auto_inside_hardware_release_reset(this);
-
   browser_view_renderer_->InvalidateOnFunctorDestroy();
   bool hardware_initialized = browser_view_renderer_->hardware_enabled();
   if (hardware_initialized) {
-    bool draw_functor_succeeded = browser_view_renderer_->RequestDrawGL(true);
-    if (!draw_functor_succeeded) {
-      LOG(ERROR) << "Unable to free GL resources. Has the Window leaked?";
-      // Calling release on wrong thread intentionally.
-      AwDrawGLInfo info;
-      info.mode = AwDrawGLInfo::kModeProcess;
-      DrawGL(&info);
+    {
+      InsideHardwareReleaseReset auto_inside_hardware_release_reset(this);

[boliu, 2015/02/05 17:10:22]

This used to wrap BVR::ReleaseHardware too.

+
+      bool draw_functor_succeeded = browser_view_renderer_->RequestDrawGL(true);
+      if (!draw_functor_succeeded) {
+        LOG(ERROR) << "Unable to free GL resources. Has the Window leaked?";
+        // Calling release on wrong thread intentionally.
+        AwDrawGLInfo info;
+        info.mode = AwDrawGLInfo::kModeProcess;
+        DrawGL(&info);
+      }
     }
-
     browser_view_renderer_->ReleaseHardware();
   }