Remove the locks in shared renderer state.

We don't actually need the locks because Android framework will block
the UI thread when RT thread is drawing

BUG=358889
NOTRY=true

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

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

4 files changed, 33 insertions, 113 deletions

diff --git a/android_webview/browser/browser_view_renderer.cc b/android_webview/browser/browser_view_renderer.cc
index 5bbc1ab..c3d9ecb 100644
--- a/android_webview/browser/browser_view_renderer.cc
+++ b/android_webview/browser/browser_view_renderer.cc
@@ -330,7 +330,8 @@ void BrowserViewRenderer::ScrollTo(gfx::Vector2d scroll_offset) {
   }
 
   if (has_compositor_)
-    shared_renderer_state_->CompositorDidChangeRootLayerScrollOffset();
+    shared_renderer_state_->GetCompositor()->
+        DidChangeRootLayerScrollOffset();
 }
 
 void BrowserViewRenderer::DidUpdateContent() {
@@ -549,7 +550,8 @@ void BrowserViewRenderer::ForceFakeCompositeSW() {
 
 bool BrowserViewRenderer::CompositeSW(SkCanvas* canvas) {
   DCHECK(has_compositor_);
-  bool result = shared_renderer_state_->CompositorDemandDrawSw(canvas);
+  bool result = shared_renderer_state_->GetCompositor()->
+      DemandDrawSw(canvas);
   DidComposite(false);
   return result;
 }
diff --git a/android_webview/browser/hardware_renderer.cc b/android_webview/browser/hardware_renderer.cc
index 1352825..8042d9d 100644
--- a/android_webview/browser/hardware_renderer.cc
+++ b/android_webview/browser/hardware_renderer.cc
@@ -50,7 +50,8 @@ HardwareRenderer::HardwareRenderer(SharedRendererState* state)
 
   gl_surface_ = new AwGLSurface;
   bool success =
-      shared_renderer_state_->CompositorInitializeHwDraw(gl_surface_);
+      shared_renderer_state_->GetCompositor()->
+          InitializeHwDraw(gl_surface_);
   DCHECK(success);
 }
 
@@ -62,7 +63,7 @@ HardwareRenderer::~HardwareRenderer() {
       ScopedAppGLStateRestore::MODE_RESOURCE_MANAGEMENT);
   internal::ScopedAllowGL allow_gl;
 
-  shared_renderer_state_->CompositorReleaseHwDraw();
+  shared_renderer_state_->GetCompositor()->ReleaseHwDraw();
   gl_surface_ = NULL;
 }
 
@@ -112,12 +113,13 @@ bool HardwareRenderer::DrawGL(AwDrawGLInfo* draw_info, DrawGLResult* result) {
                       draw_info->clip_right - draw_info->clip_left,
                       draw_info->clip_bottom - draw_info->clip_top);
 
-  bool did_draw = shared_renderer_state_->CompositorDemandDrawHw(
-      gfx::Size(draw_info->width, draw_info->height),
-      transform,
-      clip_rect,  // viewport
-      clip_rect,
-      state_restore.stencil_enabled());
+  bool did_draw = shared_renderer_state_->GetCompositor()->
+      DemandDrawHw(
+          gfx::Size(draw_info->width, draw_info->height),
+          transform,
+          clip_rect,  // viewport
+          clip_rect,
+          state_restore.stencil_enabled());
   gl_surface_->ResetBackingFrameBufferObject();
 
   if (did_draw) {
@@ -176,7 +178,8 @@ void HardwareRenderer::SetMemoryPolicy(
     return;
 
   memory_policy_ = new_policy;
-  shared_renderer_state_->CompositorSetMemoryPolicy(memory_policy_);
+  shared_renderer_state_->GetCompositor()->
+      SetMemoryPolicy(memory_policy_);
 }
 
 // static
diff --git a/android_webview/browser/shared_renderer_state.cc b/android_webview/browser/shared_renderer_state.cc
index 596f994..ba198a3 100644
--- a/android_webview/browser/shared_renderer_state.cc
+++ b/android_webview/browser/shared_renderer_state.cc
@@ -8,20 +8,12 @@
 #include "base/bind.h"
 #include "base/location.h"
 
-using base::AutoLock;
-
 namespace android_webview {
 
 DrawGLInput::DrawGLInput() : frame_id(0) {}
 
 DrawGLResult::DrawGLResult() : frame_id(0), clip_contains_visible_rect(false) {}
 
-namespace internal {
-
-BothThreads::BothThreads() : compositor(NULL) {}
-
-}  // namespace internal
-
 SharedRendererState::SharedRendererState(
     scoped_refptr<base::MessageLoopProxy> ui_loop,
     BrowserViewRendererClient* client)
@@ -55,73 +47,21 @@ void SharedRendererState::ClientRequestDrawGLOnUIThread() {
 
 void SharedRendererState::SetCompositorOnUiThread(
     content::SynchronousCompositor* compositor) {
-  AutoLock lock(lock_);
   DCHECK(ui_loop_->BelongsToCurrentThread());
-  both().compositor = compositor;
-}
-
-bool SharedRendererState::CompositorInitializeHwDraw(
-    scoped_refptr<gfx::GLSurface> surface) {
-  AutoLock lock(lock_);
-  DCHECK(both().compositor);
-  return both().compositor->InitializeHwDraw(surface);
-}
-
-void SharedRendererState::CompositorReleaseHwDraw() {
-  AutoLock lock(lock_);
-  DCHECK(both().compositor);
-  both().compositor->ReleaseHwDraw();
-}
-
-bool SharedRendererState::CompositorDemandDrawHw(
-    gfx::Size surface_size,
-    const gfx::Transform& transform,
-    gfx::Rect viewport,
-    gfx::Rect clip,
-    bool stencil_enabled) {
-  AutoLock lock(lock_);
-  DCHECK(both().compositor);
-  return both().compositor->DemandDrawHw(
-      surface_size, transform, viewport, clip, stencil_enabled);
-}
-
-bool SharedRendererState::CompositorDemandDrawSw(SkCanvas* canvas) {
-  AutoLock lock(lock_);
-  DCHECK(both().compositor);
-  return both().compositor->DemandDrawSw(canvas);
-}
-
-void SharedRendererState::CompositorSetMemoryPolicy(
-    const content::SynchronousCompositorMemoryPolicy& policy) {
-  AutoLock lock(lock_);
-  DCHECK(both().compositor);
-  return both().compositor->SetMemoryPolicy(policy);
+  compositor_ = compositor;
 }
 
-void SharedRendererState::CompositorDidChangeRootLayerScrollOffset() {
-  AutoLock lock(lock_);
-  DCHECK(both().compositor);
-  both().compositor->DidChangeRootLayerScrollOffset();
+content::SynchronousCompositor* SharedRendererState::GetCompositor() {
+  DCHECK(compositor_);
+  return compositor_;
 }
 
 void SharedRendererState::SetDrawGLInput(const DrawGLInput& input) {
-  AutoLock lock(lock_);
-  both().draw_gl_input = input;
+  draw_gl_input_ = input;
 }
 
 DrawGLInput SharedRendererState::GetDrawGLInput() const {
-  AutoLock lock(lock_);
-  return both().draw_gl_input;
-}
-
-internal::BothThreads& SharedRendererState::both() {
-  lock_.AssertAcquired();
-  return both_threads_;
-}
-
-const internal::BothThreads& SharedRendererState::both() const {
-  lock_.AssertAcquired();
-  return both_threads_;
+  return draw_gl_input_;
 }
 
 }  // namespace android_webview
diff --git a/android_webview/browser/shared_renderer_state.h b/android_webview/browser/shared_renderer_state.h
index 9410e27..c85552d 100644
--- a/android_webview/browser/shared_renderer_state.h
+++ b/android_webview/browser/shared_renderer_state.h
@@ -6,7 +6,6 @@
 #define ANDROID_WEBVIEW_BROWSER_SHARED_RENDERER_STATE_H_
 
 #include "base/message_loop/message_loop_proxy.h"
-#include "base/synchronization/lock.h"
 #include "content/public/browser/android/synchronous_compositor.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/vector2d.h"
@@ -32,25 +31,11 @@ struct DrawGLResult {
   DrawGLResult();
 };
 
-namespace internal {
-
-// Holds variables that are accessed on both UI and RT threads and need to be
-// lock-protected when accessed. Separate struct purely for syntactic barrier
-// to implement the AssertAcquired check.
-struct BothThreads {
-  // TODO(boliu): Remove |compositor| from shared state.
-  content::SynchronousCompositor* compositor;
-  DrawGLInput draw_gl_input;
-
-  BothThreads();
-};
-
-};  // namespace internal
-
-// This class holds renderer state that is shared between UI and RT threads
-// and lock protects all access the state. In the interim, this class is
-// also responsible for thread hopping and lock protection beyond simple
-// state that should eventually be removed once RT support work is complete.
+// This class holds renderer state that is shared between UI and RT threads.
+// Android framework will block the UI thread when RT is drawing, so no locking
+// is needed in this class. In the interim, this class is also responsible for
+// thread hopping that should eventually be removed once RT support work is
+// complete.
 class SharedRendererState {
  public:
   SharedRendererState(scoped_refptr<base::MessageLoopProxy> ui_loop,
@@ -59,27 +44,16 @@ class SharedRendererState {
 
   void ClientRequestDrawGL();
 
-  // Holds the compositor and lock protects all calls into it.
+  // This function should only be called on UI thread.
   void SetCompositorOnUiThread(content::SynchronousCompositor* compositor);
-  bool CompositorInitializeHwDraw(scoped_refptr<gfx::GLSurface> surface);
-  void CompositorReleaseHwDraw();
-  bool CompositorDemandDrawHw(gfx::Size surface_size,
-                              const gfx::Transform& transform,
-                              gfx::Rect viewport,
-                              gfx::Rect clip,
-                              bool stencil_enabled);
-  bool CompositorDemandDrawSw(SkCanvas* canvas);
-  void CompositorSetMemoryPolicy(
-      const content::SynchronousCompositorMemoryPolicy& policy);
-  void CompositorDidChangeRootLayerScrollOffset();
 
+  // This function can be called on both UI and RT thread.
+  content::SynchronousCompositor* GetCompositor();
   void SetDrawGLInput(const DrawGLInput& input);
   DrawGLInput GetDrawGLInput() const;
 
  private:
   void ClientRequestDrawGLOnUIThread();
-  internal::BothThreads& both();
-  const internal::BothThreads& both() const;
 
   scoped_refptr<base::MessageLoopProxy> ui_loop_;
   // TODO(boliu): Remove |client_on_ui_| from shared state.
@@ -87,8 +61,9 @@ class SharedRendererState {
   base::WeakPtrFactory<SharedRendererState> weak_factory_on_ui_thread_;
   base::WeakPtr<SharedRendererState> ui_thread_weak_ptr_;
 
-  mutable base::Lock lock_;
-  internal::BothThreads both_threads_;
+  // Accessed by both UI and RT thread.
+  content::SynchronousCompositor* compositor_;
+  DrawGLInput draw_gl_input_;
 };
 
 }  // namespace android_webview