aw: Fix hardware init/tear down in pop up flow

Previously we did not call onDetached on old native AwContents,
which now no longer cleans up hardware resources correctly.

This also had the side effect that the following onAttach with the new
native AwContents is ignored in java code due to attach/detach mismatch.

As make onDetached more strict with respect to ordering. And lock to
protect variable accessed on multiple threads.

Also need to ensure we never requestDrawGL on the blank native
AwContents that's destroyed during pop up.

BUG=376622

Review URL: https://codereview.chromium.org/288983007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@272500 0039d316-1c4b-4281-b951-d872f2087c98

android_webview/browser/gl_view_renderer_manager.cc

@@ -25,6 +25,11 @@ GLViewRendererManager::GLViewRendererManager() {}
 
 GLViewRendererManager::~GLViewRendererManager() {}
 
+GLViewRendererManager::Key GLViewRendererManager::NullKey() {
+  AutoLock auto_lock(lock_);
+  return mru_list_.end();
+}
+
 GLViewRendererManager::Key GLViewRendererManager::PushBack(RendererType view) {
   AutoLock auto_lock(lock_);
   DCHECK(mru_list_.end() ==

android_webview/browser/gl_view_renderer_manager.h

@@ -28,9 +28,7 @@ class GLViewRendererManager {
 
   static GLViewRendererManager* GetInstance();
 
-  Key NullKey() {
-    return mru_list_.end();
-  }
+  Key NullKey();
 
   Key PushBack(RendererType view);
 

android_webview/browser/shared_renderer_state.cc

@@ -23,6 +23,7 @@ SharedRendererState::SharedRendererState(
       ui_thread_weak_ptr_(weak_factory_on_ui_thread_.GetWeakPtr()),
       compositor_(NULL),
       memory_policy_dirty_(false),
+      hardware_allowed_(false),
       hardware_initialized_(false) {
   DCHECK(ui_loop_->BelongsToCurrentThread());
   DCHECK(client_on_ui_);
@@ -86,27 +87,14 @@ DrawGLInput SharedRendererState::GetDrawGLInput() const {
   return draw_gl_input_;
 }
 
-void SharedRendererState::ClearClosureQueue() {
+void SharedRendererState::SetHardwareAllowed(bool allowed) {
   base::AutoLock lock(lock_);
-  std::queue<base::Closure> empty;
-  std::swap(closure_queue_, empty);
+  hardware_allowed_ = allowed;
 }
 
-void SharedRendererState::AppendClosure(const base::Closure& closure) {
+bool SharedRendererState::IsHardwareAllowed() const {
   base::AutoLock lock(lock_);
-  closure_queue_.push(closure);
-}
-
-base::Closure SharedRendererState::PopFrontClosure() {
-  base::Closure closure;
-
-  base::AutoLock lock(lock_);
-  if (!closure_queue_.empty()) {
-    closure = closure_queue_.front();
-    closure_queue_.pop();
-  }
-
-  return closure;
+  return hardware_allowed_;
 }
 
 void SharedRendererState::SetHardwareInitialized(bool initialized) {

android_webview/browser/shared_renderer_state.h

@@ -5,9 +5,6 @@
 #ifndef ANDROID_WEBVIEW_BROWSER_SHARED_RENDERER_STATE_H_
 #define ANDROID_WEBVIEW_BROWSER_SHARED_RENDERER_STATE_H_
 
-#include <queue>
-
-#include "base/callback.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/synchronization/lock.h"
 #include "content/public/browser/android/synchronous_compositor.h"
@@ -64,11 +61,11 @@ class SharedRendererState {
   void SetDrawGLInput(const DrawGLInput& input);
   DrawGLInput GetDrawGLInput() const;
 
-  void ClearClosureQueue();
-  void AppendClosure(const base::Closure& closure);
-  // Will return empty closure if queue empty.
-  base::Closure PopFrontClosure();
+  // Set by UI and read by RT.
+  void SetHardwareAllowed(bool allowed);
+  bool IsHardwareAllowed() const;
 
+  // Set by RT and read by UI.
   void SetHardwareInitialized(bool initialized);
   bool IsHardwareInitialized() const;
 
@@ -89,7 +86,7 @@ class SharedRendererState {
   // Set to false when memory policy is read and enforced to compositor.
   bool memory_policy_dirty_;
   DrawGLInput draw_gl_input_;
-  std::queue<base::Closure> closure_queue_;
+  bool hardware_allowed_;
   bool hardware_initialized_;
 };
 

android_webview/java/src/org/chromium/android_webview/AwContents.java

@@ -621,16 +621,17 @@ public class AwContents {
         if (wasWindowFocused) onWindowFocusChanged(false);
         if (wasViewVisible) setViewVisibilityInternal(false);
         if (wasWindowVisible) setWindowVisibilityInternal(false);
+        if (wasAttached) onDetachedFromWindow();
         if (!wasPaused) onPause();
-        // Not calling onDetachedFromWindow here because native code requires GL context to release
-        // GL resources. This case is properly handled when destroy is called while still attached
-        // to window.
 
         setNewAwContents(popupNativeAwContents);
 
         // Finally refresh all view state for mContentViewCore and mNativeAwContents.
         if (!wasPaused) onResume();
-        if (wasAttached) onAttachedToWindow();
+        if (wasAttached) {
+            onAttachedToWindow();
+            postInvalidateOnAnimation();
+        }
         onSizeChanged(mContainerView.getWidth(), mContainerView.getHeight(), 0, 0);
         if (wasWindowVisible) setWindowVisibilityInternal(true);
         if (wasViewVisible) setViewVisibilityInternal(true);

android_webview/native/aw_contents.cc

@@ -326,7 +326,13 @@ jlong AwContents::GetAwDrawGLViewContext(JNIEnv* env, jobject obj) {
 }
 
 void AwContents::DrawGL(AwDrawGLInfo* draw_info) {
-  GLViewRendererManager::GetInstance()->DidDrawGL(renderer_manager_key_);
+  {
+    GLViewRendererManager* manager = GLViewRendererManager::GetInstance();
+    base::AutoLock lock(render_thread_lock_);
+    if (renderer_manager_key_ != manager->NullKey()) {
+      manager->DidDrawGL(renderer_manager_key_);
+    }
+  }
 
   ScopedAppGLStateRestore state_restore(
       draw_info->mode == AwDrawGLInfo::kModeDraw
@@ -334,15 +340,21 @@ void AwContents::DrawGL(AwDrawGLInfo* draw_info) {
           : ScopedAppGLStateRestore::MODE_RESOURCE_MANAGEMENT);
   ScopedAllowGL allow_gl;
 
-  for (base::Closure c = shared_renderer_state_.PopFrontClosure(); !c.is_null();
-       c = shared_renderer_state_.PopFrontClosure()) {
-    c.Run();
+  if (!shared_renderer_state_.IsHardwareAllowed()) {
+    hardware_renderer_.reset();
+    shared_renderer_state_.SetHardwareInitialized(false);
+    return;
   }
 
-  if (!hardware_renderer_)
+  if (draw_info->mode != AwDrawGLInfo::kModeDraw)
     return;
 
-  // TODO(boliu): Make this a task as well.
+  if (!hardware_renderer_) {
+    DCHECK(!shared_renderer_state_.IsHardwareInitialized());
+    hardware_renderer_.reset(new HardwareRenderer(&shared_renderer_state_));
+    shared_renderer_state_.SetHardwareInitialized(true);
+  }
+
   DrawGLResult result;
   if (hardware_renderer_->DrawGL(state_restore.stencil_enabled(),
                                  state_restore.framebuffer_binding_ext(),
@@ -777,64 +789,53 @@ void AwContents::SetIsPaused(JNIEnv* env, jobject obj, bool paused) {
 
 void AwContents::OnAttachedToWindow(JNIEnv* env, jobject obj, int w, int h) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  shared_renderer_state_.SetHardwareAllowed(true);
   browser_view_renderer_.OnAttachedToWindow(w, h);
 }
 
 void AwContents::InitializeHardwareDrawIfNeeded() {
   GLViewRendererManager* manager = GLViewRendererManager::GetInstance();
+
+  base::AutoLock lock(render_thread_lock_);
   if (renderer_manager_key_ == manager->NullKey()) {
-    // Add task but don't schedule it. It will run when DrawGL is called for
-    // the first time.
-    shared_renderer_state_.AppendClosure(
-        base::Bind(&AwContents::InitializeHardwareDrawOnRenderThread,
-                   base::Unretained(this)));
     renderer_manager_key_ = manager->PushBack(&shared_renderer_state_);
     DeferredGpuCommandService::SetInstance();
   }
 }
 
-void AwContents::InitializeHardwareDrawOnRenderThread() {
-  DCHECK(!hardware_renderer_);
-  DCHECK(!shared_renderer_state_.IsHardwareInitialized());
-  hardware_renderer_.reset(new HardwareRenderer(&shared_renderer_state_));
-  shared_renderer_state_.SetHardwareInitialized(true);
-}
-
 void AwContents::OnDetachedFromWindow(JNIEnv* env, jobject obj) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
-  shared_renderer_state_.ClearClosureQueue();
-  shared_renderer_state_.AppendClosure(base::Bind(
-      &AwContents::ReleaseHardwareDrawOnRenderThread, base::Unretained(this)));
-  bool draw_functor_succeeded = RequestDrawGL(NULL, true);
-  if (!draw_functor_succeeded &&
-      shared_renderer_state_.IsHardwareInitialized()) {
-    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);
-  } else {
-    shared_renderer_state_.ClearClosureQueue();
+  shared_renderer_state_.SetHardwareAllowed(false);
+
+  bool hardware_initialized = shared_renderer_state_.IsHardwareInitialized();
+  if (hardware_initialized) {
+    bool draw_functor_succeeded = RequestDrawGL(NULL, true);
+    if (!draw_functor_succeeded && hardware_initialized) {
+      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);
+    }
   }
 
+  DCHECK(!hardware_renderer_);
   browser_view_renderer_.OnDetachedFromWindow();
 
   GLViewRendererManager* manager = GLViewRendererManager::GetInstance();
-  if (renderer_manager_key_ != manager->NullKey()) {
-    manager->Remove(renderer_manager_key_);
-    renderer_manager_key_ = manager->NullKey();
-  }
-}
 
-void AwContents::ReleaseHardwareDrawOnRenderThread() {
-  // No point in running any other commands if we released hardware already.
-  shared_renderer_state_.ClearClosureQueue();
-  if (!shared_renderer_state_.IsHardwareInitialized())
-    return;
+  {
+    base::AutoLock lock(render_thread_lock_);
+    if (renderer_manager_key_ != manager->NullKey()) {
+      manager->Remove(renderer_manager_key_);
+      renderer_manager_key_ = manager->NullKey();
+    }
+  }
 
-  hardware_renderer_.reset();
-  shared_renderer_state_.SetHardwareInitialized(false);
+  if (hardware_initialized) {
+    // Flush any invoke functors that's caused by OnDetachedFromWindow.
+    RequestDrawGL(NULL, true);
+  }
 }
 
 base::android::ScopedJavaLocalRef<jbyteArray>

android_webview/native/aw_contents.h

@@ -216,8 +216,6 @@ class AwContents : public FindHelper::Listener,
   void InitAutofillIfNecessary(bool enabled);
 
   void InitializeHardwareDrawIfNeeded();
-  void InitializeHardwareDrawOnRenderThread();
-  void ReleaseHardwareDrawOnRenderThread();
 
   JavaObjectWeakGlobalRef java_ref_;
   scoped_ptr<content::WebContents> web_contents_;
@@ -240,6 +238,7 @@ class AwContents : public FindHelper::Listener,
   // The first element in the list is always the currently pending request.
   std::list<OriginCallback> pending_geolocation_prompts_;
 
+  base::Lock render_thread_lock_;
   GLViewRendererManager::Key renderer_manager_key_;
 
   DISALLOW_COPY_AND_ASSIGN(AwContents);