aw: Improve idle task scheduling

Three related changes:
* Only run idle tasks in the queue, not newly added ones.
* Move g_request_pending logic to SharedRendererState. This
  allows pending tasks to be cancelled correctly.
* Wait for kModeProcess to happen first before trying to
  request another to avoid a queue of pending tasks.

BUG=401712

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

Cr-Commit-Position: refs/heads/master@{#288285}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288285 0039d316-1c4b-4281-b951-d872f2087c98

android_webview/browser/deferred_gpu_command_service.cc

@@ -7,6 +7,7 @@
 #include "android_webview/browser/gl_view_renderer_manager.h"
 #include "android_webview/browser/shared_renderer_state.h"
 #include "base/debug/trace_event.h"
+#include "base/lazy_instance.h"
 #include "base/synchronization/lock.h"
 #include "content/public/browser/android/synchronous_compositor.h"
 #include "gpu/command_buffer/service/shader_translator_cache.h"
@@ -14,49 +15,6 @@
 namespace android_webview {
 
 namespace {
-
-// TODO(boliu): Consider using base/atomicops.h.
-class ThreadSafeBool {
- public:
-  ThreadSafeBool();
-  void Set(bool boolean);
-  bool Get();
-  bool GetAndSet();
-
- private:
-  base::Lock lock_;
-  bool boolean_;
-  DISALLOW_COPY_AND_ASSIGN(ThreadSafeBool);
-};
-
-ThreadSafeBool::ThreadSafeBool() : boolean_(false) {
-}
-
-void ThreadSafeBool::Set(bool boolean) {
-  base::AutoLock lock(lock_);
-  boolean_ = boolean;
-}
-
-bool ThreadSafeBool::GetAndSet() {
-  base::AutoLock lock(lock_);
-  bool rv = boolean_;
-  boolean_ = true;
-  return rv;
-}
-
-bool ThreadSafeBool::Get() {
-  base::AutoLock lock(lock_);
-  return boolean_;
-}
-
-base::LazyInstance<ThreadSafeBool> g_request_pending =
-    LAZY_INSTANCE_INITIALIZER;
-
-// Because request is posted to UI thread, have to treat requests on UI thread
-// specifically because UI can immediately block waiting for the request.
-base::LazyInstance<ThreadSafeBool> g_request_pending_on_ui =
-    LAZY_INSTANCE_INITIALIZER;
-
 base::LazyInstance<scoped_refptr<DeferredGpuCommandService> >
     g_service = LAZY_INSTANCE_INITIALIZER;
 }  // namespace
@@ -78,13 +36,11 @@ ScopedAllowGL::ScopedAllowGL() {
 
 ScopedAllowGL::~ScopedAllowGL() {
   allow_gl.Get().Set(false);
-  g_request_pending.Get().Set(false);
-  g_request_pending_on_ui.Get().Set(false);
 
   DeferredGpuCommandService* service = g_service.Get();
   if (service) {
     service->RunTasks();
-    if (service->HasIdleWork()) {
+    if (service->IdleQueueSize()) {
       service->RequestProcessGL();
     }
   }
@@ -95,10 +51,6 @@ void DeferredGpuCommandService::SetInstance() {
   if (!g_service.Get()) {
     g_service.Get() = new DeferredGpuCommandService;
     content::SynchronousCompositor::SetGpuService(g_service.Get());
-
-    // Initialize global booleans.
-    g_request_pending.Get().Set(false);
-    g_request_pending_on_ui.Get().Set(false);
   }
 }
 
@@ -124,14 +76,7 @@ void DeferredGpuCommandService::RequestProcessGL() {
     LOG(ERROR) << "No hardware renderer. Deadlock likely";
     return;
   }
-
-  bool on_ui_thread = renderer_state->CurrentlyOnUIThread();
-  bool need_request = on_ui_thread ? !g_request_pending_on_ui.Get().GetAndSet()
-                                   : !g_request_pending.Get().GetAndSet();
-  if (need_request) {
-    g_request_pending.Get().Set(true);
-    renderer_state->ClientRequestDrawGL();
-  }
+  renderer_state->ClientRequestDrawGL();
 }
 
 // Called from different threads!
@@ -147,9 +92,9 @@ void DeferredGpuCommandService::ScheduleTask(const base::Closure& task) {
   }
 }
 
-bool DeferredGpuCommandService::HasIdleWork() {
+size_t DeferredGpuCommandService::IdleQueueSize() {
   base::AutoLock lock(tasks_lock_);
-  return idle_tasks_.size() > 0;
+  return idle_tasks_.size();
 }
 
 void DeferredGpuCommandService::ScheduleIdleWork(
@@ -171,7 +116,8 @@ void DeferredGpuCommandService::PerformIdleWork(bool is_idle) {
       base::TimeDelta::FromMilliseconds(16);
 
   const base::Time now = base::Time::Now();
-  while (HasIdleWork()) {
+  size_t queue_size = IdleQueueSize();
+  while (queue_size--) {
     base::Closure task;
     {
       base::AutoLock lock(tasks_lock_);
@@ -188,6 +134,14 @@ void DeferredGpuCommandService::PerformIdleWork(bool is_idle) {
   }
 }
 
+void DeferredGpuCommandService::PerformAllIdleWork() {
+  TRACE_EVENT0("android_webview",
+               "DeferredGpuCommandService::PerformAllIdleWork");
+  while (IdleQueueSize()) {
+    PerformIdleWork(true);
+  }
+}
+
 bool DeferredGpuCommandService::UseVirtualizedGLContexts() { return true; }
 
 scoped_refptr<gpu::gles2::ShaderTranslatorCache>

android_webview/browser/deferred_gpu_command_service.h

@@ -45,7 +45,10 @@ class DeferredGpuCommandService
   void RunTasks();
   // If |is_idle| is false, this will only run older idle tasks.
   void PerformIdleWork(bool is_idle);
-  bool HasIdleWork();
+  // Flush the idle queue until it is empty. This is different from
+  // PerformIdleWork(is_idle = true), which does not run any newly scheduled
+  // idle tasks during the idle run.
+  void PerformAllIdleWork();
 
   virtual void AddRef() const OVERRIDE;
   virtual void Release() const OVERRIDE;
@@ -59,6 +62,7 @@ class DeferredGpuCommandService
   static void RequestProcessGL();
 
   DeferredGpuCommandService();
+  size_t IdleQueueSize();
 
   base::Lock tasks_lock_;
   std::queue<base::Closure> tasks_;

android_webview/browser/shared_renderer_state.cc

@@ -6,10 +6,67 @@
 
 #include "android_webview/browser/browser_view_renderer_client.h"
 #include "base/bind.h"
+#include "base/lazy_instance.h"
 #include "base/location.h"
 
 namespace android_webview {
 
+namespace internal {
+
+class RequestDrawGLTracker {
+ public:
+  RequestDrawGLTracker();
+  bool ShouldRequestOnNoneUiThread(SharedRendererState* state);
+  bool ShouldRequestOnUiThread(SharedRendererState* state);
+  void DidRequestOnUiThread();
+  void ResetPending();
+
+ private:
+  base::Lock lock_;
+  SharedRendererState* pending_ui_;
+  SharedRendererState* pending_non_ui_;
+};
+
+RequestDrawGLTracker::RequestDrawGLTracker()
+    : pending_ui_(NULL), pending_non_ui_(NULL) {
+}
+
+bool RequestDrawGLTracker::ShouldRequestOnNoneUiThread(
+    SharedRendererState* state) {
+  base::AutoLock lock(lock_);
+  if (pending_ui_ || pending_non_ui_)
+    return false;
+  pending_non_ui_ = state;
+  return true;
+}
+
+bool RequestDrawGLTracker::ShouldRequestOnUiThread(SharedRendererState* state) {
+  base::AutoLock lock(lock_);
+  if (pending_non_ui_) {
+    pending_non_ui_->ResetRequestDrawGLCallback();
+    pending_non_ui_ = NULL;
+  }
+  if (pending_ui_)
+    return false;
+  pending_ui_ = state;
+  return true;
+}
+
+void RequestDrawGLTracker::ResetPending() {
+  base::AutoLock lock(lock_);
+  pending_non_ui_ = NULL;
+  pending_ui_ = NULL;
+}
+
+}  // namespace internal
+
+namespace {
+
+base::LazyInstance<internal::RequestDrawGLTracker> g_request_draw_gl_tracker =
+    LAZY_INSTANCE_INITIALIZER;
+
+}
+
 DrawGLInput::DrawGLInput() : width(0), height(0) {
 }
 
@@ -27,30 +84,48 @@ SharedRendererState::SharedRendererState(
       share_context_(NULL) {
   DCHECK(ui_loop_->BelongsToCurrentThread());
   DCHECK(client_on_ui_);
+  ResetRequestDrawGLCallback();
 }
 
 SharedRendererState::~SharedRendererState() {
   DCHECK(ui_loop_->BelongsToCurrentThread());
 }
 
-bool SharedRendererState::CurrentlyOnUIThread() {
-  return ui_loop_->BelongsToCurrentThread();
-}
-
 void SharedRendererState::ClientRequestDrawGL() {
   if (ui_loop_->BelongsToCurrentThread()) {
+    if (!g_request_draw_gl_tracker.Get().ShouldRequestOnUiThread(this))
+      return;
     ClientRequestDrawGLOnUIThread();
   } else {
-    ui_loop_->PostTask(
-        FROM_HERE,
-        base::Bind(&SharedRendererState::ClientRequestDrawGLOnUIThread,
-                   ui_thread_weak_ptr_));
+    if (!g_request_draw_gl_tracker.Get().ShouldRequestOnNoneUiThread(this))
+      return;
+    base::Closure callback;
+    {
+      base::AutoLock lock(lock_);
+      callback = request_draw_gl_closure_;
+    }
+    ui_loop_->PostTask(FROM_HERE, callback);
   }
 }
 
+void SharedRendererState::DidDrawGLProcess() {
+  g_request_draw_gl_tracker.Get().ResetPending();
+}
+
+void SharedRendererState::ResetRequestDrawGLCallback() {
+  DCHECK(ui_loop_->BelongsToCurrentThread());
+  base::AutoLock lock(lock_);
+  request_draw_gl_cancelable_closure_.Reset(
+      base::Bind(&SharedRendererState::ClientRequestDrawGLOnUIThread,
+                 base::Unretained(this)));
+  request_draw_gl_closure_ = request_draw_gl_cancelable_closure_.callback();
+}
+
 void SharedRendererState::ClientRequestDrawGLOnUIThread() {
   DCHECK(ui_loop_->BelongsToCurrentThread());
+  ResetRequestDrawGLCallback();
   if (!client_on_ui_->RequestDrawGL(NULL, false)) {
+    g_request_draw_gl_tracker.Get().ResetPending();
     LOG(ERROR) << "Failed to request GL process. Deadlock likely";
   }
 }

android_webview/browser/shared_renderer_state.h

@@ -6,6 +6,7 @@
 #define ANDROID_WEBVIEW_BROWSER_SHARED_RENDERER_STATE_H_
 
 #include "android_webview/browser/parent_compositor_draw_constraints.h"
+#include "base/cancelable_callback.h"
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/synchronization/lock.h"
@@ -24,6 +25,10 @@ class GLInProcessContext;
 
 namespace android_webview {
 
+namespace internal {
+class RequestDrawGLTracker;
+}
+
 class BrowserViewRendererClient;
 class InsideHardwareReleaseReset;
 
@@ -45,8 +50,8 @@ class SharedRendererState {
                       BrowserViewRendererClient* client);
   ~SharedRendererState();
 
-  bool CurrentlyOnUIThread();
   void ClientRequestDrawGL();
+  void DidDrawGLProcess();
 
   void SetDrawGLInput(scoped_ptr<DrawGLInput> input);
   scoped_ptr<DrawGLInput> PassDrawGLInput();
@@ -66,7 +71,9 @@ class SharedRendererState {
 
  private:
   friend class InsideHardwareReleaseReset;
+  friend class internal::RequestDrawGLTracker;
 
+  void ResetRequestDrawGLCallback();
   void ClientRequestDrawGLOnUIThread();
   void UpdateParentDrawConstraintsOnUIThread();
   void SetInsideHardwareRelease(bool inside);
@@ -75,6 +82,7 @@ class SharedRendererState {
   BrowserViewRendererClient* client_on_ui_;
   base::WeakPtrFactory<SharedRendererState> weak_factory_on_ui_thread_;
   base::WeakPtr<SharedRendererState> ui_thread_weak_ptr_;
+  base::CancelableClosure request_draw_gl_cancelable_closure_;
 
   // Accessed by both UI and RT thread.
   mutable base::Lock lock_;
@@ -83,6 +91,7 @@ class SharedRendererState {
   ParentCompositorDrawConstraints parent_draw_constraints_;
   gpu::GLInProcessContext* share_context_;
   cc::ReturnedResourceArray returned_resources_;
+  base::Closure request_draw_gl_closure_;
 
   DISALLOW_COPY_AND_ASSIGN(SharedRendererState);
 };

android_webview/native/aw_contents.cc

@@ -362,11 +362,24 @@ void AwContents::DrawGL(AwDrawGLInfo* draw_info) {
           : ScopedAppGLStateRestore::MODE_RESOURCE_MANAGEMENT);
   ScopedAllowGL allow_gl;
 
+  if (draw_info->mode == AwDrawGLInfo::kModeProcessNoContext) {
+    LOG(ERROR) << "Received unexpected kModeProcessNoContext";
+  }
+
+  // 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) {
+    shared_renderer_state_.DidDrawGLProcess();
+  }
+
   if (shared_renderer_state_.IsInsideHardwareRelease()) {
     hardware_renderer_.reset();
     // Flush the idle queue in tear down.
-    DeferredGpuCommandService::GetInstance()->PerformIdleWork(true);
-    DCHECK(!DeferredGpuCommandService::GetInstance()->HasIdleWork());
+    DeferredGpuCommandService::GetInstance()->PerformAllIdleWork();
     return;
   }