android: add resource state machine to RenderWidgetHostViewAndroid

We need to ensure freeing resources via AcceleratedSurfaceRelease() does not
compete with async readback and rapid switching between Hide() and Show().  This
change adds a state machine for to abstract out the case handling logic.

Also added public API functions LockResources() and UnlockResources(), which
are intended for future async readback uses.

BUG=326363

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

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

11 files changed, 191 insertions, 49 deletions

diff --git a/content/browser/renderer_host/delegated_frame_evictor.cc b/content/browser/renderer_host/delegated_frame_evictor.cc
index 8993d11..33d6b2c 100644
--- a/content/browser/renderer_host/delegated_frame_evictor.cc
+++ b/content/browser/renderer_host/delegated_frame_evictor.cc
@@ -4,6 +4,8 @@
 
 #include "content/browser/renderer_host/delegated_frame_evictor.h"
 
+#include "base/logging.h"
+
 namespace content {
 
 DelegatedFrameEvictor::DelegatedFrameEvictor(
@@ -23,8 +25,23 @@ void DelegatedFrameEvictor::DiscardedFrame() {
 }
 
 void DelegatedFrameEvictor::SetVisible(bool visible) {
-  if (has_frame_)
-    RendererFrameManager::GetInstance()->SetFrameVisibility(this, visible);
+  if (has_frame_) {
+    if (visible) {
+      RendererFrameManager::GetInstance()->LockFrame(this);
+    } else {
+      RendererFrameManager::GetInstance()->UnlockFrame(this);
+    }
+  }
+}
+
+void DelegatedFrameEvictor::LockFrame() {
+  DCHECK(has_frame_);
+  RendererFrameManager::GetInstance()->LockFrame(this);
+}
+
+void DelegatedFrameEvictor::UnlockFrame() {
+  DCHECK(has_frame_);
+  RendererFrameManager::GetInstance()->UnlockFrame(this);
 }
 
 void DelegatedFrameEvictor::EvictCurrentFrame() {
diff --git a/content/browser/renderer_host/delegated_frame_evictor.h b/content/browser/renderer_host/delegated_frame_evictor.h
index c54f2c4..d52b2d3 100644
--- a/content/browser/renderer_host/delegated_frame_evictor.h
+++ b/content/browser/renderer_host/delegated_frame_evictor.h
@@ -25,6 +25,8 @@ class CONTENT_EXPORT DelegatedFrameEvictor : public RendererFrameManagerClient {
   void SwappedFrame(bool visible);
   void DiscardedFrame();
   void SetVisible(bool visible);
+  void LockFrame();
+  void UnlockFrame();
 
  private:
   // RendererFrameManagerClient implementation.
diff --git a/content/browser/renderer_host/render_widget_host_view_android.cc b/content/browser/renderer_host/render_widget_host_view_android.cc
index 3df0b11..c8afbd6 100644
--- a/content/browser/renderer_host/render_widget_host_view_android.cc
+++ b/content/browser/renderer_host/render_widget_host_view_android.cc
@@ -124,7 +124,8 @@ RenderWidgetHostViewAndroid::RenderWidgetHostViewAndroid(
       accelerated_surface_route_id_(0),
       using_synchronous_compositor_(SynchronousCompositorImpl::FromID(
                                         widget_host->GetProcess()->GetID(),
-                                        widget_host->GetRoutingID()) != NULL) {
+                                        widget_host->GetRoutingID()) != NULL),
+      frame_evictor_(new DelegatedFrameEvictor(this)) {
   if (!UsingDelegatedRenderer()) {
     texture_layer_ = cc::TextureLayer::Create(NULL);
     layer_ = texture_layer_;
@@ -359,6 +360,7 @@ void RenderWidgetHostViewAndroid::Show() {
   are_layers_attached_ = true;
   AttachLayers();
 
+  frame_evictor_->SetVisible(true);
   WasShown();
 }
 
@@ -369,6 +371,7 @@ void RenderWidgetHostViewAndroid::Hide() {
   are_layers_attached_ = false;
   RemoveLayers();
 
+  frame_evictor_->SetVisible(false);
   WasHidden();
 }
 
@@ -379,6 +382,18 @@ bool RenderWidgetHostViewAndroid::IsShowing() {
   return are_layers_attached_ && content_view_core_;
 }
 
+void RenderWidgetHostViewAndroid::LockResources() {
+  DCHECK(HasValidFrame());
+  DCHECK(host_);
+  DCHECK(!host_->is_hidden());
+  frame_evictor_->LockFrame();
+}
+
+void RenderWidgetHostViewAndroid::UnlockResources() {
+  DCHECK(HasValidFrame());
+  frame_evictor_->UnlockFrame();
+}
+
 gfx::Rect RenderWidgetHostViewAndroid::GetViewBounds() const {
   if (!content_view_core_)
     return gfx::Rect(default_size_);
@@ -788,6 +803,7 @@ void RenderWidgetHostViewAndroid::OnSwapCompositorFrame(
   }
 
   BuffersSwapped(frame->gl_frame_data->mailbox, output_surface_id, callback);
+  frame_evictor_->SwappedFrame(!host_->is_hidden());
 }
 
 void RenderWidgetHostViewAndroid::SynchronousFrameMetadata(
@@ -940,7 +956,10 @@ void RenderWidgetHostViewAndroid::AcceleratedSurfaceSuspend() {
 }
 
 void RenderWidgetHostViewAndroid::AcceleratedSurfaceRelease() {
-  // This tells us we should free the frontbuffer.
+  NOTREACHED();
+}
+
+void RenderWidgetHostViewAndroid::EvictDelegatedFrame() {
   if (texture_id_in_layer_) {
     texture_layer_->SetTextureId(0);
     texture_layer_->SetIsDrawable(false);
@@ -952,6 +971,7 @@ void RenderWidgetHostViewAndroid::AcceleratedSurfaceRelease() {
   }
   if (delegated_renderer_layer_.get())
     DestroyDelegatedContent();
+  frame_evictor_->DiscardedFrame();
 }
 
 bool RenderWidgetHostViewAndroid::HasAcceleratedSurface(
diff --git a/content/browser/renderer_host/render_widget_host_view_android.h b/content/browser/renderer_host/render_widget_host_view_android.h
index db1bc55..42b455a6 100644
--- a/content/browser/renderer_host/render_widget_host_view_android.h
+++ b/content/browser/renderer_host/render_widget_host_view_android.h
@@ -18,6 +18,7 @@
 #include "cc/layers/texture_layer_client.h"
 #include "cc/output/begin_frame_args.h"
 #include "content/browser/accessibility/browser_accessibility_manager.h"
+#include "content/browser/renderer_host/delegated_frame_evictor.h"
 #include "content/browser/renderer_host/image_transport_factory_android.h"
 #include "content/browser/renderer_host/ime_adapter_android.h"
 #include "content/browser/renderer_host/render_widget_host_view_base.h"
@@ -63,7 +64,8 @@ class RenderWidgetHostViewAndroid
       public BrowserAccessibilityDelegate,
       public cc::DelegatedFrameResourceCollectionClient,
       public ImageTransportFactoryAndroidObserver,
-      public ui::WindowAndroidObserver {
+      public ui::WindowAndroidObserver,
+      public DelegatedFrameEvictorClient {
  public:
   RenderWidgetHostViewAndroid(RenderWidgetHostImpl* widget,
                               ContentViewCoreImpl* content_view_core);
@@ -196,6 +198,9 @@ class RenderWidgetHostViewAndroid
   // ImageTransportFactoryAndroidObserver implementation.
   virtual void OnLostResources() OVERRIDE;
 
+  // DelegatedFrameEvictor implementation
+  virtual void EvictDelegatedFrame() OVERRIDE;
+
   // Non-virtual methods
   void SetContentViewCore(ContentViewCoreImpl* content_view_core);
   SkColor GetCachedBackgroundColor() const;
@@ -211,6 +216,9 @@ class RenderWidgetHostViewAndroid
   void OnStartContentIntent(const GURL& content_url);
   void OnSetNeedsBeginFrame(bool enabled);
 
+  void LockResources();
+  void UnlockResources();
+
   int GetNativeImeAdapter();
 
   void WasResized();
@@ -340,6 +348,8 @@ class RenderWidgetHostViewAndroid
 
   const bool using_synchronous_compositor_;
 
+  scoped_ptr<DelegatedFrameEvictor> frame_evictor_;
+
   DISALLOW_COPY_AND_ASSIGN(RenderWidgetHostViewAndroid);
 };
 
diff --git a/content/browser/renderer_host/render_widget_host_view_aura.cc b/content/browser/renderer_host/render_widget_host_view_aura.cc
index 04e259d..fc1bb4b 100644
--- a/content/browser/renderer_host/render_widget_host_view_aura.cc
+++ b/content/browser/renderer_host/render_widget_host_view_aura.cc
@@ -3471,6 +3471,16 @@ void RenderWidgetHostViewAura::DetachFromInputMethod() {
     input_method->SetFocusedTextInputClient(NULL);
 }
 
+void RenderWidgetHostViewAura::LockResources() {
+  DCHECK(frame_provider_);
+  delegated_frame_evictor_->LockFrame();
+}
+
+void RenderWidgetHostViewAura::UnlockResources() {
+  DCHECK(frame_provider_);
+  delegated_frame_evictor_->UnlockFrame();
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // RenderWidgetHostView, public:
 
diff --git a/content/browser/renderer_host/render_widget_host_view_aura.h b/content/browser/renderer_host/render_widget_host_view_aura.h
index 86db864..749d320 100644
--- a/content/browser/renderer_host/render_widget_host_view_aura.h
+++ b/content/browser/renderer_host/render_widget_host_view_aura.h
@@ -373,6 +373,8 @@ class CONTENT_EXPORT RenderWidgetHostViewAura
   // Exposed for tests.
   aura::Window* window() { return window_; }
   gfx::Size current_frame_size() const { return current_frame_size_; }
+  void LockResources();
+  void UnlockResources();
 
   // Overridden from ui::CompositorObserver:
   virtual void OnCompositingDidCommit(ui::Compositor* compositor) OVERRIDE;
@@ -396,6 +398,8 @@ class CONTENT_EXPORT RenderWidgetHostViewAura
   FRIEND_TEST_ALL_PREFIXES(RenderWidgetHostViewAuraTest, OutputSurfaceIdChange);
   FRIEND_TEST_ALL_PREFIXES(RenderWidgetHostViewAuraTest,
                            DiscardDelegatedFrames);
+  FRIEND_TEST_ALL_PREFIXES(RenderWidgetHostViewAuraTest,
+                           DiscardDelegatedFramesWithLocking);
   FRIEND_TEST_ALL_PREFIXES(RenderWidgetHostViewAuraTest, SoftwareDPIChange);
 
   class WindowObserver;
diff --git a/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc b/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
index 8063bbc..7ad6ecb 100644
--- a/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
+++ b/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
@@ -1080,6 +1080,67 @@ TEST_F(RenderWidgetHostViewAuraTest, DiscardDelegatedFrames) {
   }
 }
 
+TEST_F(RenderWidgetHostViewAuraTest, DiscardDelegatedFramesWithLocking) {
+  size_t max_renderer_frames =
+      RendererFrameManager::GetInstance()->max_number_of_saved_frames();
+  ASSERT_LE(2u, max_renderer_frames);
+  size_t renderer_count = max_renderer_frames + 1;
+  gfx::Rect view_rect(100, 100);
+  gfx::Size frame_size = view_rect.size();
+
+  scoped_ptr<RenderWidgetHostImpl * []> hosts(
+      new RenderWidgetHostImpl* [renderer_count]);
+  scoped_ptr<FakeRenderWidgetHostViewAura * []> views(
+      new FakeRenderWidgetHostViewAura* [renderer_count]);
+
+  // Create a bunch of renderers.
+  for (size_t i = 0; i < renderer_count; ++i) {
+    hosts[i] = new RenderWidgetHostImpl(
+        &delegate_, process_host_, MSG_ROUTING_NONE, false);
+    hosts[i]->Init();
+    hosts[i]->OnMessageReceived(
+        ViewHostMsg_DidActivateAcceleratedCompositing(0, true));
+    views[i] = new FakeRenderWidgetHostViewAura(hosts[i]);
+    views[i]->InitAsChild(NULL);
+    aura::client::ParentWindowWithContext(
+        views[i]->GetNativeView(),
+        parent_view_->GetNativeView()->GetRootWindow(),
+        gfx::Rect());
+    views[i]->SetSize(view_rect.size());
+  }
+
+  // Make each renderer visible and swap a frame on it. No eviction should
+  // occur because all frames are visible.
+  for (size_t i = 0; i < renderer_count; ++i) {
+    views[i]->WasShown();
+    views[i]->OnSwapCompositorFrame(
+        1, MakeDelegatedFrame(1.f, frame_size, view_rect));
+    EXPECT_TRUE(views[i]->frame_provider_);
+  }
+
+  // If we hide [0], then [0] should be evicted.
+  views[0]->WasHidden();
+  EXPECT_FALSE(views[0]->frame_provider_);
+
+  // If we lock [0] before hiding it, then [0] should not be evicted.
+  views[0]->WasShown();
+  views[0]->OnSwapCompositorFrame(
+        1, MakeDelegatedFrame(1.f, frame_size, view_rect));
+  EXPECT_TRUE(views[0]->frame_provider_);
+  views[0]->LockResources();
+  views[0]->WasHidden();
+  EXPECT_TRUE(views[0]->frame_provider_);
+
+  // If we unlock [0] now, then [0] should be evicted.
+  views[0]->UnlockResources();
+  EXPECT_FALSE(views[0]->frame_provider_);
+
+  for (size_t i = 0; i < renderer_count; ++i) {
+    views[i]->Destroy();
+    delete hosts[i];
+  }
+}
+
 TEST_F(RenderWidgetHostViewAuraTest, SoftwareDPIChange) {
   gfx::Rect view_rect(100, 100);
   gfx::Size frame_size(100, 100);
diff --git a/content/browser/renderer_host/renderer_frame_manager.cc b/content/browser/renderer_host/renderer_frame_manager.cc
index 3d16361..15c9814 100644
--- a/content/browser/renderer_host/renderer_frame_manager.cc
+++ b/content/browser/renderer_host/renderer_frame_manager.cc
@@ -4,6 +4,8 @@
 
 #include "content/browser/renderer_host/renderer_frame_manager.h"
 
+#include <algorithm>
+
 #include "base/logging.h"
 #include "base/sys_info.h"
 
@@ -14,46 +16,68 @@ RendererFrameManager* RendererFrameManager::GetInstance() {
 }
 
 void RendererFrameManager::AddFrame(RendererFrameManagerClient* frame,
-                                    bool visible) {
+                                    bool locked) {
   RemoveFrame(frame);
-  if (visible)
-    visible_frames_.insert(frame);
+  if (locked)
+    locked_frames_[frame] = 1;
   else
-    hidden_frames_.push_front(frame);
-  CullHiddenFrames();
+    unlocked_frames_.push_front(frame);
+  CullUnlockedFrames();
 }
 
 void RendererFrameManager::RemoveFrame(RendererFrameManagerClient* frame) {
-  visible_frames_.erase(frame);
-  hidden_frames_.remove(frame);
+  std::map<RendererFrameManagerClient*, size_t>::iterator locked_iter =
+      locked_frames_.find(frame);
+  if (locked_iter != locked_frames_.end())
+    locked_frames_.erase(locked_iter);
+  unlocked_frames_.remove(frame);
+}
+
+void RendererFrameManager::LockFrame(RendererFrameManagerClient* frame) {
+  std::list<RendererFrameManagerClient*>::iterator unlocked_iter =
+    std::find(unlocked_frames_.begin(), unlocked_frames_.end(), frame);
+  if (unlocked_iter != unlocked_frames_.end()) {
+    DCHECK(locked_frames_.find(frame) == locked_frames_.end());
+    unlocked_frames_.remove(frame);
+    locked_frames_[frame] = 1;
+  } else {
+    DCHECK(locked_frames_.find(frame) != locked_frames_.end());
+    locked_frames_[frame]++;
+  }
 }
 
-void RendererFrameManager::SetFrameVisibility(RendererFrameManagerClient* frame,
-                                              bool visible) {
-  if (visible) {
-    hidden_frames_.remove(frame);
-    visible_frames_.insert(frame);
+void RendererFrameManager::UnlockFrame(RendererFrameManagerClient* frame) {
+  DCHECK(locked_frames_.find(frame) != locked_frames_.end());
+  size_t locked_count = locked_frames_[frame];
+  DCHECK(locked_count);
+  if (locked_count > 1) {
+    locked_frames_[frame]--;
   } else {
-    visible_frames_.erase(frame);
-    hidden_frames_.push_front(frame);
-    CullHiddenFrames();
+    RemoveFrame(frame);
+    unlocked_frames_.push_front(frame);
+    CullUnlockedFrames();
   }
 }
 
-RendererFrameManager::RendererFrameManager()
-    : max_number_of_saved_frames_(
-          std::min(5, 2 + (base::SysInfo::AmountOfPhysicalMemoryMB() / 256))) {}
+RendererFrameManager::RendererFrameManager() {
+  max_number_of_saved_frames_ =
+#if defined(OS_ANDROID)
+      1;
+#else
+      std::min(5, 2 + (base::SysInfo::AmountOfPhysicalMemoryMB() / 256));
+#endif
+}
 
 RendererFrameManager::~RendererFrameManager() {}
 
-void RendererFrameManager::CullHiddenFrames() {
-  while (!hidden_frames_.empty() &&
-         hidden_frames_.size() + visible_frames_.size() >
+void RendererFrameManager::CullUnlockedFrames() {
+  while (!unlocked_frames_.empty() &&
+         unlocked_frames_.size() + locked_frames_.size() >
              max_number_of_saved_frames()) {
-    size_t old_size = hidden_frames_.size();
+    size_t old_size = unlocked_frames_.size();
     // Should remove self from list.
-    hidden_frames_.back()->EvictCurrentFrame();
-    DCHECK_EQ(hidden_frames_.size() + 1, old_size);
+    unlocked_frames_.back()->EvictCurrentFrame();
+    DCHECK_EQ(unlocked_frames_.size() + 1, old_size);
   }
 }
 
diff --git a/content/browser/renderer_host/renderer_frame_manager.h b/content/browser/renderer_host/renderer_frame_manager.h
index 134e1ef..8b72a65 100644
--- a/content/browser/renderer_host/renderer_frame_manager.h
+++ b/content/browser/renderer_host/renderer_frame_manager.h
@@ -6,7 +6,7 @@
 #define CONTENT_BROWSER_RENDERER_HOST_RENDERER_FRAME_MANAGER_H_
 
 #include <list>
-#include <set>
+#include <map>
 
 #include "base/basictypes.h"
 #include "base/memory/singleton.h"
@@ -24,9 +24,10 @@ class CONTENT_EXPORT RendererFrameManager {
  public:
   static RendererFrameManager* GetInstance();
 
-  void AddFrame(RendererFrameManagerClient*, bool visible);
+  void AddFrame(RendererFrameManagerClient*, bool locked);
   void RemoveFrame(RendererFrameManagerClient*);
-  void SetFrameVisibility(RendererFrameManagerClient*, bool visible);
+  void LockFrame(RendererFrameManagerClient*);
+  void UnlockFrame(RendererFrameManagerClient*);
 
   size_t max_number_of_saved_frames() const {
     return max_number_of_saved_frames_;
@@ -35,11 +36,12 @@ class CONTENT_EXPORT RendererFrameManager {
  private:
   RendererFrameManager();
   ~RendererFrameManager();
-  void CullHiddenFrames();
+  void CullUnlockedFrames();
+
   friend struct DefaultSingletonTraits<RendererFrameManager>;
 
-  std::set<RendererFrameManagerClient*> visible_frames_;
-  std::list<RendererFrameManagerClient*> hidden_frames_;
+  std::map<RendererFrameManagerClient*, size_t> locked_frames_;
+  std::list<RendererFrameManagerClient*> unlocked_frames_;
   size_t max_number_of_saved_frames_;
 
   DISALLOW_COPY_AND_ASSIGN(RendererFrameManager);
diff --git a/content/browser/renderer_host/software_frame_manager.cc b/content/browser/renderer_host/software_frame_manager.cc
index 147c21e..ce5909e 100644
--- a/content/browser/renderer_host/software_frame_manager.cc
+++ b/content/browser/renderer_host/software_frame_manager.cc
@@ -150,7 +150,11 @@ void SoftwareFrameManager::SwapToNewFrameComplete(bool visible) {
 
 void SoftwareFrameManager::SetVisibility(bool visible) {
   if (HasCurrentFrame()) {
-    RendererFrameManager::GetInstance()->SetFrameVisibility(this, visible);
+    if (visible) {
+      RendererFrameManager::GetInstance()->LockFrame(this);
+    } else {
+      RendererFrameManager::GetInstance()->UnlockFrame(this);
+    }
   }
 }
 
diff --git a/content/common/gpu/image_transport_surface_android.cc b/content/common/gpu/image_transport_surface_android.cc
index 5f45469..7e12bda 100644
--- a/content/common/gpu/image_transport_surface_android.cc
+++ b/content/common/gpu/image_transport_surface_android.cc
@@ -44,7 +44,6 @@ class ImageTransportSurfaceAndroid
   virtual bool SwapBuffers() OVERRIDE;
   virtual std::string GetExtensions() OVERRIDE;
   virtual bool OnMakeCurrent(gfx::GLContext* context) OVERRIDE;
-  virtual void SetFrontbufferAllocation(bool allocated) OVERRIDE;
   virtual void WakeUpGpu() OVERRIDE;
 
  protected:
@@ -55,7 +54,6 @@ class ImageTransportSurfaceAndroid
   void DoWakeUpGpu();
 
   uint32 parent_client_id_;
-  bool frontbuffer_suggested_allocation_;
   base::TimeTicks begin_wake_up_time_;
 };
 
@@ -82,8 +80,7 @@ ImageTransportSurfaceAndroid::ImageTransportSurfaceAndroid(
     gfx::GLSurface* surface,
     uint32 parent_client_id)
     : PassThroughImageTransportSurface(manager, stub, surface, true),
-      parent_client_id_(parent_client_id),
-      frontbuffer_suggested_allocation_(true) {}
+      parent_client_id_(parent_client_id) {}
 
 ImageTransportSurfaceAndroid::~ImageTransportSurfaceAndroid() {}
 
@@ -112,15 +109,6 @@ std::string ImageTransportSurfaceAndroid::GetExtensions() {
   return extensions;
 }
 
-void ImageTransportSurfaceAndroid::SetFrontbufferAllocation(bool allocation) {
-  if (frontbuffer_suggested_allocation_ == allocation)
-    return;
-  frontbuffer_suggested_allocation_ = allocation;
-  // TODO(sievers): This races with CompositorFrame messages.
-  if (!allocation)
-    GetHelper()->SendAcceleratedSurfaceRelease();
-}
-
 bool ImageTransportSurfaceAndroid::OnMakeCurrent(gfx::GLContext* context) {
   DidAccessGpu();
   return PassThroughImageTransportSurface::OnMakeCurrent(context);