aw: Fix race between first frame draw and view detach causing deadlock.

android_webview/browser/render_thread_manager.cc

Index: android_webview/browser/render_thread_manager.cc
diff --git a/android_webview/browser/render_thread_manager.cc b/android_webview/browser/render_thread_manager.cc
index 9a14dda0d25cca1b0c4b51fa9346d13167e76335..0d8045eaba572498f24157afee2fd604efc78eca 100644
--- a/android_webview/browser/render_thread_manager.cc
+++ b/android_webview/browser/render_thread_manager.cc
@@ -98,8 +98,8 @@ RenderThreadManager::RenderThreadManager(
     : ui_loop_(ui_loop),
       client_(client),
       compositor_frame_producer_(nullptr),
+      has_received_frame_(false),
       renderer_manager_key_(GLViewRendererManager::GetInstance()->NullKey()),
-      hardware_renderer_has_frame_(false),
       sync_on_draw_hardware_(base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kSyncOnDrawHardware)),
       inside_hardware_release_(false),
@@ -183,6 +183,9 @@ std::unique_ptr<ChildFrame> RenderThreadManager::SetFrameOnUI(
     std::unique_ptr<ChildFrame> new_frame) {
   DCHECK(new_frame);
   base::AutoLock lock(lock_);
+
+  has_received_frame_ = true;

[boliu, 2016/12/06 16:15:35]

maybe add a TODO here, this doesn't work if we switch to full async draw, ie we
only get a future here and don't actually know if the frame is empty or not,
maybe update TODO in BVR.h as well to point to this issue

[Tobias Sargeant, 2016/12/06 18:11:26]

Done.

+
   if (child_frames_.empty()) {
     child_frames_.emplace_back(std::move(new_frame));
     return nullptr;
@@ -212,8 +215,6 @@ std::unique_ptr<ChildFrame> RenderThreadManager::SetFrameOnUI(
 
 ChildFrameQueue RenderThreadManager::PassFramesOnRT() {
   base::AutoLock lock(lock_);
-  hardware_renderer_has_frame_ =
-      hardware_renderer_has_frame_ || !child_frames_.empty();
   ChildFrameQueue returned_frames;
   returned_frames.swap(child_frames_);
   return returned_frames;
@@ -334,8 +335,11 @@ void RenderThreadManager::DrawGL(AwDrawGLInfo* draw_info) {
   }
 
   if (IsInsideHardwareRelease()) {
-    hardware_renderer_has_frame_ = false;
     hardware_renderer_.reset();
+    // If there are no frames ready to pass to a new
+    // hardware_renderer_ then consider that equivalent to never
+    // having received a frame.
+    has_received_frame_ = HasFrameForHardwareRendererOnRT();

[boliu, 2016/12/06 16:15:35]

I don't understand this

also this means has_received_frame_ is accessed on RT as well

[Tobias Sargeant, 2016/12/06 16:29:27]

Good point about has_received_frame_. The thing I wanted to avoid was this
sequence:

* has_received_frame_ is true
* detach causes hr_ destroy but has_received_frame_ is not reset.
* first frame after attach (in sync mode) is empty, causing BVR::OnDrawHardware
to return true, even though there's nothing to draw.

[boliu, 2016/12/06 16:34:10]

can you unset has_received_frame_ at end of DeleteHardwareRendererOnUI, on UI
thread?

[Tobias Sargeant, 2016/12/06 16:36:38]

Yes. That's a much better solution.

[Tobias Sargeant, 2016/12/06 18:11:26]

Done.

     // Flush the idle queue in tear down.
     DeferredGpuCommandService::GetInstance()->PerformAllIdleWork();
     return;
@@ -358,14 +362,14 @@ void RenderThreadManager::DeleteHardwareRendererOnUI() {
 
   InsideHardwareReleaseReset auto_inside_hardware_release_reset(this);
 
+  client_->DetachFunctorFromView();
+
   // If the WebView gets onTrimMemory >= MODERATE twice in a row, the 2nd
   // onTrimMemory will result in an unnecessary Render Thread InvokeGL call.
-  bool hardware_initialized = HasFrameOnUI();
-  if (hardware_initialized) {
-    // The functor has only been attached to the view hierarchy if a compositor
-    // frame has been generated. Thus, it should only be detached in this case.
-    client_->DetachFunctorFromView();
-
+  if (has_received_frame_) {
+    // Receiving at least one frame is a precondition for
+    // initialization (such as looing up GL bindings and constructing
+    // hardware_renderer_).
     bool draw_functor_succeeded = client_->RequestInvokeGL(true);
     if (!draw_functor_succeeded) {
       LOG(ERROR) << "Unable to free GL resources. Has the Window leaked?";
@@ -386,7 +390,7 @@ void RenderThreadManager::DeleteHardwareRendererOnUI() {
     }
   }
 
-  if (hardware_initialized) {
+  if (has_received_frame_) {
     // Flush any invoke functors that's caused by ReleaseHardware.
     client_->RequestInvokeGL(true);
   }
@@ -402,7 +406,7 @@ void RenderThreadManager::SetCompositorFrameProducer(
 
 bool RenderThreadManager::HasFrameOnUI() const {
   base::AutoLock lock(lock_);
-  return hardware_renderer_has_frame_ || !child_frames_.empty();
+  return has_received_frame_;
 }
 
 bool RenderThreadManager::HasFrameForHardwareRendererOnRT() const {