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

content/browser/android/content_view_core_impl.cc

 // Copyright 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/android/content_view_core_impl.h"
 
 #include "base/android/jni_android.h"
 #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"
 #include "base/metrics/histogram.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "cc/layers/layer.h"
 #include "cc/output/begin_frame_args.h"
 #include "content/browser/android/content_video_view.h"
 #include "content/browser/android/interstitial_page_delegate_android.h"
@@ skipping 563 matching lines @@
 void ContentViewCoreImpl::ShowPastePopup(int x_dip, int y_dip) {
   JNIEnv* env = AttachCurrentThread();
   ScopedJavaLocalRef<jobject> obj = java_ref_.get(env);
   if (obj.is_null())
     return;
   Java_ContentViewCore_showPastePopup(env, obj.obj(),
                                       static_cast<jint>(x_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();
 
-  return view->GetScaledContentTexture(scale, out_size);
+  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;
+  }
+
+  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) {
   JNIEnv* env = AttachCurrentThread();
   ScopedJavaLocalRef<jobject> j_obj = java_ref_.get(env);
   if (j_obj.is_null())
     return;
   ScopedJavaLocalRef<jstring> jcontent_url =
       ConvertUTF8ToJavaString(env, content_url.spec());
   Java_ContentViewCore_startContentIntent(env,
@@ skipping 1093 matching lines @@
       reinterpret_cast<ui::ViewAndroid*>(view_android),
       reinterpret_cast<ui::WindowAndroid*>(window_android));
   return reinterpret_cast<intptr_t>(view);
 }
 
 bool RegisterContentViewCore(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
 
 }  // namespace content