android: Migrate old content readback to use async readback (and delegated renderer)

content/browser/android/content_view_core_impl.cc

Index: content/browser/android/content_view_core_impl.cc
diff --git a/content/browser/android/content_view_core_impl.cc b/content/browser/android/content_view_core_impl.cc
index 182c0a6f336ca2071a3303065f41072bb9fa2e4b..7201d10674caeb04331921ca3cf5e6da0a4bd6d4 100644
--- a/content/browser/android/content_view_core_impl.cc
+++ b/content/browser/android/content_view_core_impl.cc
@@ -8,6 +8,7 @@
 #include "base/android/jni_array.h"
 #include "base/android/jni_string.h"
 #include "base/android/scoped_java_ref.h"
+#include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "base/json/json_writer.h"
 #include "base/logging.h"
@@ -591,14 +592,41 @@ void ContentViewCoreImpl::ShowPastePopup(int x_dip, int y_dip) {
                                       static_cast<jint>(y_dip));
 }
 
-unsigned int ContentViewCoreImpl::GetScaledContentTexture(
-    float scale,
-    gfx::Size* out_size) {
+void ContentViewCoreImpl::OnFinishGetScaledContentBitmap(
+    const base::Callback<void(bool, const SkBitmap&)>& compositor_callback,
+    bool success,
+    const SkBitmap& bitmap) {
+  base::ScopedClosureRunner scoped_callback_runner(
+      base::Bind(compositor_callback, false, SkBitmap()));
+
   RenderWidgetHostViewAndroid* view = GetRenderWidgetHostViewAndroid();
   if (!view)
-    return 0;
+    return;
+  view->UnlockResources();
+
+  if (!success)
+    return;
+  ignore_result(scoped_callback_runner.Release());
+  compositor_callback.Run(success, bitmap);
+}
+
+void ContentViewCoreImpl::GetScaledContentBitmap(
+    float scale,
+    gfx::Size* out_size,
+    const base::Callback<void(bool, const SkBitmap&)>& compositor_callback) {
+  RenderWidgetHostViewAndroid* view = GetRenderWidgetHostViewAndroid();
+  if (!view || !view->IsSurfaceAvailableForCopy()) {
+    base::ScopedClosureRunner scoped_callback_runner(
+      base::Bind(compositor_callback, false, SkBitmap()));

[no sievers, 2014/01/28 20:47:21]

nit: you can just do compositor_callback.Run(false, SkBitmap());

[powei, 2014/01/29 13:53:17]

Done.

+    return;
+  }
 
-  return view->GetScaledContentTexture(scale, out_size);
+  base::Callback<void(bool, const SkBitmap&)> callback =
+      base::Bind(&ContentViewCoreImpl::OnFinishGetScaledContentBitmap,
+                 base::Unretained(this),

[no sievers, 2014/01/28 20:47:21]

The Unretained() is a problem, since the ContentViewCore might go away in the
meantime.

To get the code functionally equivalent to what you're doing here and avoid the
problem, you could just have RWHVAndroid imply the lock.

However, isn't the problem that releasing the live layer when the readback
callback is run is not good enough, because the application also wants to
ETC1-compress the bitmap first which is another async task?

It really only works if the live layer is replaced inside the callback with
something suitable. And it seems silly to reupload the same texture again that
we just read back if you could as well just keep the live layer around instead
for another moment.

In which case you want to move the lock/unlock further up (instead of further
down) to be exposed to the app.

[powei, 2014/01/29 13:53:17]

I took out the locks here, but I'm not sure how to add it in RWHVA since don't
we have the same problem with RWHVA's lifetime when the callback comes back?

[no sievers, 2014/01/29 21:13:47]

You lock the live layer in CopyFromCompositingSurface() and then pass a weakptr
(weak_ptr_factory_.GetWeakPtr()) to PrepareTextureCopyOutputResult as an
argument. And in the latter you do:
if (rwhv.get())
  rwhv->UnlockResources();


But it might be better to push the responsiblity to the app sooner rather then
later. It probably doesn't get us much in the meantime and if we start locking
and unlocking the live layer from multiple places, things will likely get out of
whack.

+                 compositor_callback);
+  view->LockResources();
+  view->GetScaledContentBitmap(scale, out_size, callback);
 }
 
 void ContentViewCoreImpl::StartContentIntent(const GURL& content_url) {