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

> 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

TBR=boliu@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/branches/1985/src@273487 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 4f14219..5b0a427 100644
--- a/android_webview/java/src/org/chromium/android_webview/AwContents.java
+++ b/android_webview/java/src/org/chromium/android_webview/AwContents.java
@@ -619,16 +619,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 594f508..526ba1b 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 73c1036..90ced58 100644
--- a/android_webview/native/aw_contents.h
+++ b/android_webview/native/aw_contents.h
@@ -218,8 +218,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_;
@@ -242,6 +240,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);