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

7 files changed, 67 insertions, 78 deletions

diff --git a/android_webview/browser/gl_view_renderer_manager.cc b/android_webview/browser/gl_view_renderer_manager.cc
index b9d1a83..204239c 100644
--- a/android_webview/browser/gl_view_renderer_manager.cc
+++ b/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() ==
diff --git a/android_webview/browser/gl_view_renderer_manager.h b/android_webview/browser/gl_view_renderer_manager.h
index 5d6cb8d..f34d9cb 100644
--- a/android_webview/browser/gl_view_renderer_manager.h
+++ b/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);
 
diff --git a/android_webview/browser/shared_renderer_state.cc b/android_webview/browser/shared_renderer_state.cc
index 3483d63..193eca0 100644
--- a/android_webview/browser/shared_renderer_state.cc
+++ b/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) {
diff --git a/android_webview/browser/shared_renderer_state.h b/android_webview/browser/shared_renderer_state.h
index 9c90731..7959541 100644
--- a/android_webview/browser/shared_renderer_state.h
+++ b/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_;
 };
 
diff --git a/android_webview/java/src/org/chromium/android_webview/AwContents.java b/android_webview/java/src/org/chromium/android_webview/AwContents.java
index 4027eeb..900ac95 100644
--- a/android_webview/java/src/org/chromium/android_webview/AwContents.java
+++ b/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);
diff --git a/android_webview/native/aw_contents.cc b/android_webview/native/aw_contents.cc
index c0f7b1b..91443a0 100644
--- a/android_webview/native/aw_contents.cc
+++ b/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>
diff --git a/android_webview/native/aw_contents.h b/android_webview/native/aw_contents.h
index a74d928..3227edf 100644
--- a/android_webview/native/aw_contents.h
+++ b/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);