Android: Second attempt at freeing GLHelper ashmem

content/browser/renderer_host/render_widget_host_view_android.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/renderer_host/render_widget_host_view_android.h"
 
 #include <android/bitmap.h>
 
 #include <utility>
 
 #include "base/android/application_status_listener.h"
 #include "base/android/build_info.h"
 #include "base/android/context_utils.h"
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/macros.h"
-#include "base/memory/memory_pressure_listener.h"
+#include "base/memory/ref_counted.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/sys_info.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/threading/worker_pool.h"
 #include "cc/layers/layer.h"
 #include "cc/layers/surface_layer.h"
 #include "cc/output/compositor_frame.h"
 #include "cc/output/copy_output_request.h"
@@ skipping 64 matching lines @@
 #include "ui/touch_selection/touch_selection_controller.h"
 
 namespace content {
 
 namespace {
 
 const int kUndefinedCompositorFrameSinkId = -1;
 
 static const char kAsyncReadBackString[] = "Compositing.CopyFromSurfaceTime";
 
+class PendingReadbackLock;
+
+PendingReadbackLock* g_pending_readback_lock = nullptr;
+
+class PendingReadbackLock : public base::RefCounted<PendingReadbackLock> {

[danakj, 2016/10/03 20:47:43]

This is fine, but another way to do this would be

int g_num_readbacks_pending = 0;

class PendingReadbackLock() {
  PendingReadbackLock() { ++g_num_readbacks_pending; }
  ~PendingReadbackLock() { --g_num_readbacks_pending; }
};

Then we don't need any refcounted objects, the global is simply the refcount. Up
to you.

+ public:
+  PendingReadbackLock() {
+    DCHECK_EQ(g_pending_readback_lock, nullptr);
+    g_pending_readback_lock = this;
+  }
+
+  ~PendingReadbackLock() {

[danakj, 2016/10/03 20:47:43]

Destructor should be private with friend of RefCounted.

[no sievers, 2016/10/05 00:38:40]

Done.

+    DCHECK_EQ(g_pending_readback_lock, this);
+    g_pending_readback_lock = nullptr;
+  }
+};
+
 using base::android::ApplicationState;
 using base::android::ApplicationStatusListener;
 using base::MemoryPressureListener;
 
 class GLHelperHolder {
  public:
   static GLHelperHolder* Create();
 
   display_compositor::GLHelper* gl_helper() { return gl_helper_.get(); }
   bool IsLost() {
     if (!gl_helper_)
       return true;
     return provider_->ContextGL()->GetGraphicsResetStatusKHR() != GL_NO_ERROR;
   }
 
-  void FreeUnusedSharedMemoryIfNeeded();
+  void ReleaseIfPossible();
 
  private:
   GLHelperHolder();
   void Initialize();
   void OnContextLost();
   void OnApplicationStatusChanged(ApplicationState new_state);
-  void OnMemoryPressure(MemoryPressureListener::MemoryPressureLevel level);
 
   scoped_refptr<ContextProviderCommandBuffer> provider_;
   std::unique_ptr<display_compositor::GLHelper> gl_helper_;
 
   // Set to |false| if there are only stopped activities (or none).
   bool has_running_or_paused_activities_;
-  // Whether we recently were signaled a low memory condition.
-  bool did_signal_memory_pressure_;
 
   std::unique_ptr<ApplicationStatusListener> app_status_listener_;
-  std::unique_ptr<MemoryPressureListener> mem_pressure_listener_;
 
   DISALLOW_COPY_AND_ASSIGN(GLHelperHolder);
 };
 
 GLHelperHolder::GLHelperHolder()
     : has_running_or_paused_activities_(
           (ApplicationStatusListener::GetState() ==
            base::android::APPLICATION_STATE_HAS_RUNNING_ACTIVITIES) ||
           (ApplicationStatusListener::GetState() ==
-           base::android::APPLICATION_STATE_HAS_PAUSED_ACTIVITIES)),
-      did_signal_memory_pressure_(false) {}
+           base::android::APPLICATION_STATE_HAS_PAUSED_ACTIVITIES)) {}
 
 GLHelperHolder* GLHelperHolder::Create() {
   GLHelperHolder* holder = new GLHelperHolder;
   holder->Initialize();
   return holder;
 }
 
 void GLHelperHolder::Initialize() {
   auto* factory = BrowserGpuChannelHostFactory::instance();
   scoped_refptr<gpu::GpuChannelHost> gpu_channel_host(factory->GetGpuChannel());
@@ skipping 47 matching lines @@
       base::StringPrintf("CmdBufferImageTransportFactory-%p", provider_.get())
           .c_str());
   provider_->SetLostContextCallback(
       base::Bind(&GLHelperHolder::OnContextLost, base::Unretained(this)));
   gl_helper_.reset(new display_compositor::GLHelper(
       provider_->ContextGL(), provider_->ContextSupport()));
 
   // Unretained() is safe because |this| owns the following two callbacks.
   app_status_listener_.reset(new ApplicationStatusListener(base::Bind(
       &GLHelperHolder::OnApplicationStatusChanged, base::Unretained(this))));
-  mem_pressure_listener_.reset(new MemoryPressureListener(base::Bind(
-      &GLHelperHolder::OnMemoryPressure, base::Unretained(this))));
 }
 
-void GLHelperHolder::FreeUnusedSharedMemoryIfNeeded() {
-  if (!has_running_or_paused_activities_ || did_signal_memory_pressure_)
-    provider_->FreeUnusedSharedMemory();
+void GLHelperHolder::ReleaseIfPossible() {
+  if (!has_running_or_paused_activities_ && !g_pending_readback_lock) {
+    gl_helper_.reset();
+    provider_ = nullptr;
+  }
 }
 
 void GLHelperHolder::OnContextLost() {
   app_status_listener_.reset();
-  mem_pressure_listener_.reset();
   gl_helper_.reset();
   // Need to post a task because the command buffer client cannot be deleted
   // from within this callback.
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE, base::Bind(&RenderWidgetHostViewAndroid::OnContextLost));
 }
 
 void GLHelperHolder::OnApplicationStatusChanged(ApplicationState new_state) {
-  bool new_has_running_or_paused_activities =
+  has_running_or_paused_activities_ =
       new_state == base::android::APPLICATION_STATE_HAS_RUNNING_ACTIVITIES ||
       new_state == base::android::APPLICATION_STATE_HAS_PAUSED_ACTIVITIES;
-
-  if (!has_running_or_paused_activities_ &&
-      new_has_running_or_paused_activities) {
-    // Reset at this time since on Android there is no 'back to normal'
-    // notification (but only onTrim/onLowMemory).
-    did_signal_memory_pressure_ = false;
-  } else if (!new_has_running_or_paused_activities) {
-    provider_->FreeUnusedSharedMemory();
-  }
-
-  has_running_or_paused_activities_ = new_has_running_or_paused_activities;
-}
-
-void GLHelperHolder::OnMemoryPressure(
-    MemoryPressureListener::MemoryPressureLevel level) {
-  if (level == MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE ||
-      level == MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL) {
-    did_signal_memory_pressure_ = true;
-    provider_->FreeUnusedSharedMemory();
-  }
+  ReleaseIfPossible();
 }
 
 GLHelperHolder* GetPostReadbackGLHelperHolder(bool create_if_necessary) {
   static GLHelperHolder* g_readback_helper_holder = nullptr;
 
   if (!create_if_necessary && !g_readback_helper_holder)
     return nullptr;
 
   if (g_readback_helper_holder && g_readback_helper_holder->IsLost()) {
     delete g_readback_helper_holder;
     g_readback_helper_holder = nullptr;
   }
 
   if (!g_readback_helper_holder)
     g_readback_helper_holder = GLHelperHolder::Create();
 
   return g_readback_helper_holder;
 }
 
 display_compositor::GLHelper* GetPostReadbackGLHelper() {
   bool create_if_necessary = true;
   return GetPostReadbackGLHelperHolder(create_if_necessary)->gl_helper();
 }
 
-void FreeUnusedGLHelperMemory() {
+void ReleaseGLHelper() {
   bool create_if_necessary = false;
   GLHelperHolder* holder = GetPostReadbackGLHelperHolder(create_if_necessary);
 
   if (holder) {
-    holder->FreeUnusedSharedMemoryIfNeeded();
+    holder->ReleaseIfPossible();
   }
 }
 
 void CopyFromCompositingSurfaceFinished(
     const ReadbackRequestCallback& callback,
     std::unique_ptr<cc::SingleReleaseCallback> release_callback,
     std::unique_ptr<SkBitmap> bitmap,
     const base::TimeTicks& start_time,
     std::unique_ptr<SkAutoLockPixels> bitmap_pixels_lock,
+    scoped_refptr<PendingReadbackLock> readback_lock,
     bool result) {
   TRACE_EVENT0(
       "cc", "RenderWidgetHostViewAndroid::CopyFromCompositingSurfaceFinished");
   bitmap_pixels_lock.reset();
   gpu::SyncToken sync_token;
   if (result) {
     display_compositor::GLHelper* gl_helper = GetPostReadbackGLHelper();
     if (gl_helper)
       gl_helper->GenerateSyncToken(&sync_token);
   }
 
-  FreeUnusedGLHelperMemory();
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,

[danakj, 2016/10/03 20:47:44]

I'd like a comment explaining why this is posting the task instead of doing it
immediately, cuz that's not clear to me

[no sievers, 2016/10/05 00:38:40]

Done.

+                          base::Bind(&ReleaseGLHelper));
 
   const bool lost_resource = !sync_token.HasData();
   release_callback->Run(sync_token, lost_resource);
   UMA_HISTOGRAM_TIMES(kAsyncReadBackString,
                       base::TimeTicks::Now() - start_time);
   ReadbackResponse response = result ? READBACK_SUCCESS : READBACK_FAILED;
   callback.Run(*bitmap, response);
 }
 
 std::unique_ptr<ui::TouchSelectionController> CreateSelectionController(
@@ skipping 24 matching lines @@
 gfx::RectF GetSelectionRect(const ui::TouchSelectionController& controller) {
   gfx::RectF rect = controller.GetRectBetweenBounds();
   if (rect.IsEmpty())
     return rect;
 
   rect.Union(controller.GetStartHandleRect());
   rect.Union(controller.GetEndHandleRect());
   return rect;
 }
 
+void PrepareTextureCopyOutputResult(
+    const gfx::Size& dst_size_in_pixel,
+    SkColorType color_type,
+    const base::TimeTicks& start_time,
+    const ReadbackRequestCallback& callback,
+    scoped_refptr<PendingReadbackLock> readback_lock,
+    std::unique_ptr<cc::CopyOutputResult> result);
+
 }  // anonymous namespace
 
 RenderWidgetHostViewAndroid::LastFrameInfo::LastFrameInfo(
     uint32_t compositor_frame_sink_id,
     cc::CompositorFrame output_frame)
     : compositor_frame_sink_id(compositor_frame_sink_id),
       frame(std::move(output_frame)) {}
 
 RenderWidgetHostViewAndroid::LastFrameInfo::~LastFrameInfo() {}
 
@@ skipping 576 matching lines @@
                             preferred_color_type);
     UMA_HISTOGRAM_TIMES("Compositing.CopyFromSurfaceTimeSynchronous",
                         base::TimeTicks::Now() - start_time);
     return;
   }
 
   ui::WindowAndroidCompositor* compositor =
       content_view_core_->GetWindowAndroid()->GetCompositor();
   DCHECK(compositor);
   DCHECK(delegated_frame_host_);
+  scoped_refptr<PendingReadbackLock> readback_lock(
+      g_pending_readback_lock ? g_pending_readback_lock
+                              : new PendingReadbackLock);
   delegated_frame_host_->RequestCopyOfSurface(
       compositor, src_subrect_in_pixel,
       base::Bind(&PrepareTextureCopyOutputResult, dst_size_in_pixel,
-                 preferred_color_type, start_time, callback));
+                 preferred_color_type, start_time, callback, readback_lock));
 }
 
 void RenderWidgetHostViewAndroid::CopyFromCompositingSurfaceToVideoFrame(
     const gfx::Rect& src_subrect,
     const scoped_refptr<media::VideoFrame>& target,
     const base::Callback<void(const gfx::Rect&, bool)>& callback) {
   NOTIMPLEMENTED();
   callback.Run(gfx::Rect(), false);
 }
 
@@ skipping 892 matching lines @@
   ReleaseLocksOnSurface();
   DestroyDelegatedContent();
   DCHECK(ack_callbacks_.empty());
 }
 
 // TODO(wjmaclean): There is significant overlap between
 // PrepareTextureCopyOutputResult and CopyFromCompositingSurfaceFinished in
 // this file, and the versions in surface_utils.cc. They should
 // be merged. See https://crbug.com/582955
 
-// static
-void RenderWidgetHostViewAndroid::PrepareTextureCopyOutputResult(
+namespace {

[danakj, 2016/10/03 20:47:44]

bonus points if this method can move up to the anon namespace at the top of the
file.

[no sievers, 2016/10/05 00:38:40]

Done.

+
+void PrepareTextureCopyOutputResult(
     const gfx::Size& dst_size_in_pixel,
     SkColorType color_type,
     const base::TimeTicks& start_time,
     const ReadbackRequestCallback& callback,
+    scoped_refptr<PendingReadbackLock> readback_lock,
     std::unique_ptr<cc::CopyOutputResult> result) {
   base::ScopedClosureRunner scoped_callback_runner(
       base::Bind(callback, SkBitmap(), READBACK_FAILED));
   TRACE_EVENT0("cc",
                "RenderWidgetHostViewAndroid::PrepareTextureCopyOutputResult");
   if (!result->HasTexture() || result->IsEmpty() || result->size().IsEmpty())
     return;
   cc::TextureMailbox texture_mailbox;
   std::unique_ptr<cc::SingleReleaseCallback> release_callback;
   result->TakeTexture(&texture_mailbox, &release_callback);
@@ skipping 26 matching lines @@
       new SkAutoLockPixels(*bitmap));
   uint8_t* pixels = static_cast<uint8_t*>(bitmap->getPixels());
 
   ignore_result(scoped_callback_runner.Release());
 
   gl_helper->CropScaleReadbackAndCleanMailbox(
       texture_mailbox.mailbox(), texture_mailbox.sync_token(), result->size(),
       gfx::Rect(result->size()), output_size_in_pixel, pixels, color_type,
       base::Bind(&CopyFromCompositingSurfaceFinished, callback,
                  base::Passed(&release_callback), base::Passed(&bitmap),
-                 start_time, base::Passed(&bitmap_pixels_lock)),
+                 start_time, base::Passed(&bitmap_pixels_lock), readback_lock),
       display_compositor::GLHelper::SCALER_QUALITY_GOOD);
 }
 
+}  // anonymous namespace

[danakj, 2016/10/03 20:47:44]

just "// namespace"

[no sievers, 2016/10/05 00:38:40]

Done.

+
 void RenderWidgetHostViewAndroid::OnStylusSelectBegin(float x0,
                                                       float y0,
                                                       float x1,
                                                       float y1) {
   SelectBetweenCoordinates(gfx::PointF(x0, y0), gfx::PointF(x1, y1));
 }
 
 void RenderWidgetHostViewAndroid::OnStylusSelectUpdate(float x, float y) {
   MoveRangeSelectionExtent(gfx::PointF(x, y));
 }
@@ skipping 31 matching lines @@
     case ui::MotionEvent::ACTION_UP:
     case ui::MotionEvent::ACTION_POINTER_UP:
       UMA_HISTOGRAM_CUSTOM_COUNTS("Event.Latency.OS.TOUCH_RELEASED",
                                   delta.InMicroseconds(), 1, 1000000, 50);
     default:
       return;
   }
 }
 
 }  // namespace content