Remove redundant code from Android context menu native.

The two methods that fetch the currently selected image can
be collapsed into one.

BUG=655359

Review-Url: https://codereview.chromium.org/2804913003
Cr-Commit-Position: refs/heads/master@{#462923}

base/android/callback_android.cc

@@ -4,6 +4,8 @@
 
 #include "base/android/callback_android.h"
 
+#include "base/android/jni_array.h"
+#include "base/android/scoped_java_ref.h"
 #include "jni/Callback_jni.h"
 
 namespace base {
@@ -25,5 +27,13 @@ void RunCallbackAndroid(const JavaRef<jobject>& callback, int arg) {
                                       callback, arg);
 }
 
+void RunCallbackAndroid(const JavaRef<jobject>& callback,
+                        const std::vector<uint8_t>& arg) {
+  JNIEnv* env = base::android::AttachCurrentThread();
+  base::android::ScopedJavaLocalRef<jbyteArray> j_bytes =
+      base::android::ToJavaByteArray(env, arg);
+  Java_Callback_onResultFromNativeV_AB(env, callback, j_bytes);
+}
+
 }  // namespace android
 }  // namespace base

base/android/callback_android.h

@@ -6,24 +6,27 @@
 #define BASE_ANDROID_CALLBACK_ANDROID_H_
 
 #include <jni.h>
+#include <vector>
 
 #include "base/android/scoped_java_ref.h"
 #include "base/base_export.h"
 
+// Provides helper utility methods that run the given callback with the
+// specified argument.
 namespace base {
 namespace android {
 
-// Runs the given |callback| with the specified |arg|.
 void BASE_EXPORT RunCallbackAndroid(const JavaRef<jobject>& callback,
                                     const JavaRef<jobject>& arg);
 
-// Runs the given |callback| with the specified |arg|.
 void BASE_EXPORT RunCallbackAndroid(const JavaRef<jobject>& callback,
                                     bool arg);
 
-// Runs the given |callback| with the specified |arg|.
 void BASE_EXPORT RunCallbackAndroid(const JavaRef<jobject>& callback, int arg);
 
+void BASE_EXPORT RunCallbackAndroid(const JavaRef<jobject>& callback,
+                                    const std::vector<uint8_t>& arg);
+
 }  // namespace android
 }  // namespace base
 

base/android/java/src/org/chromium/base/Callback.java

@@ -34,4 +34,10 @@ public abstract class Callback<T> {
     private void onResultFromNative(int result) {
         onResult((T) Integer.valueOf(result));
     }
+
+    @SuppressWarnings("unchecked")
+    @CalledByNative
+    private void onResultFromNative(byte[] result) {
+        onResult((T) result);
+    }
 }

chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ContextMenuHelper.java

@@ -28,10 +28,14 @@ import org.chromium.ui.base.WindowAndroid.OnCloseContextMenuListener;
 
 import java.util.List;
 
+import javax.annotation.Nullable;
+
 /**
  * A helper class that handles generating context menus for {@link ContentViewCore}s.
  */
 public class ContextMenuHelper implements OnCreateContextMenuListener {
+    private static final int MAX_SHARE_DIMEN_PX = 2048;
+
     private long mNativeContextMenuHelper;
 
     private ContextMenuPopulator mPopulator;
@@ -40,8 +44,6 @@ public class ContextMenuHelper implements OnCreateContextMenuListener {
     private Callback<Integer> mCallback;
     private Runnable mOnMenuShown;
     private Runnable mOnMenuClosed;
-    private Callback<Bitmap> mOnThumbnailReceivedCallback;
-    private ComponentName mComponentName;
 
     private ContextMenuHelper(long nativeContextMenuHelper) {
         mNativeContextMenuHelper = nativeContextMenuHelper;
@@ -169,50 +171,47 @@ public class ContextMenuHelper implements OnCreateContextMenuListener {
      * Share the image that triggered the current context menu.
      */
     public void shareImage() {
-        if (mNativeContextMenuHelper == 0) return;
-        nativeShareImage(mNativeContextMenuHelper);
-    }
-
-    @CalledByNative
-    private void onShareImageReceived(
-            WindowAndroid windowAndroid, byte[] jpegImageData) {
-        Activity activity = windowAndroid.getActivity().get();
-        if (activity == null) return;
-
-        ShareHelper.shareImage(activity, jpegImageData, mComponentName);
-        // This needs to be reset to null after a share. This way the next time a user shares an
-        // image it won't share with the last shared app unless explicitly told.
-        mComponentName = null;
+        shareImageDirectly(null);
     }
 
     /**
      * Share image triggered with the current context menu directly with a specific app.
      * @param name The {@link ComponentName} of the app to share the image directly with.
      */
-    public void shareImageDirectly(ComponentName name) {
-        mComponentName = name;
-        shareImage();
+    public void shareImageDirectly(@Nullable final ComponentName name) {
+        if (mNativeContextMenuHelper == 0) return;
+        Callback<byte[]> callback = new Callback<byte[]>() {
+            @Override
+            public void onResult(byte[] result) {
+                WebContents webContents = nativeGetJavaWebContents(mNativeContextMenuHelper);
+                WindowAndroid windowAndroid = webContents.getTopLevelNativeWindow();
+
+                Activity activity = windowAndroid.getActivity().get();
+                if (activity == null) return;
+
+                ShareHelper.shareImage(activity, result, name);
+            }
+        };
+        nativeRetrieveImage(mNativeContextMenuHelper, callback, MAX_SHARE_DIMEN_PX);
     }
 
     /**
      * Gets the thumbnail of the current image that triggered the context menu.
      * @param callback Called once the the thumbnail is received.
      */
-    private void getThumbnail(Callback<Bitmap> callback) {
-        mOnThumbnailReceivedCallback = callback;
+    private void getThumbnail(final Callback<Bitmap> callback) {
         if (mNativeContextMenuHelper == 0) return;
         int maxSizePx = mActivity.getResources().getDimensionPixelSize(
                 R.dimen.context_menu_header_image_max_size);
-        nativeRetrieveHeaderThumbnail(mNativeContextMenuHelper, maxSizePx);
-    }
-
-    @CalledByNative
-    private void onHeaderThumbnailReceived(WindowAndroid windowAndroid, byte[] jpegImageData) {
-        // TODO(tedchoc): Decode in separate process before launch.
-        Bitmap bitmap = BitmapFactory.decodeByteArray(jpegImageData, 0, jpegImageData.length);
-        if (mOnThumbnailReceivedCallback != null) {
-            mOnThumbnailReceivedCallback.onResult(bitmap);
-        }
+        Callback<byte[]> rawDataCallback = new Callback<byte[]>() {
+            @Override
+            public void onResult(byte[] result) {
+                // TODO(tedchoc): Decode in separate process before launch.
+                Bitmap bitmap = BitmapFactory.decodeByteArray(result, 0, result.length);
+                callback.onResult(bitmap);
+            }
+        };
+        nativeRetrieveImage(mNativeContextMenuHelper, rawDataCallback, maxSizePx);
     }
 
     @Override
@@ -232,10 +231,11 @@ public class ContextMenuHelper implements OnCreateContextMenuListener {
         return mPopulator;
     }
 
+    private native WebContents nativeGetJavaWebContents(long nativeContextMenuHelper);
     private native void nativeOnStartDownload(
             long nativeContextMenuHelper, boolean isLink, boolean isDataReductionProxyEnabled);
     private native void nativeSearchForImage(long nativeContextMenuHelper);
-    private native void nativeShareImage(long nativeContextMenuHelper);
+    private native void nativeRetrieveImage(
+            long nativeContextMenuHelper, Callback<byte[]> callback, int maxSizePx);
     private native void nativeOnContextMenuClosed(long nativeContextMenuHelper);
-    private native void nativeRetrieveHeaderThumbnail(long nativeContextMenuHelper, int maxSizePx);
 }

chrome/browser/ui/android/context_menu_helper.cc

@@ -8,10 +8,10 @@
 
 #include <vector>
 
-#include "base/android/jni_android.h"
-#include "base/android/jni_array.h"
+#include "base/android/callback_android.h"
 #include "base/android/jni_string.h"
 #include "base/bind_helpers.h"
+#include "base/callback.h"
 #include "chrome/browser/android/download/download_controller_base.h"
 #include "chrome/browser/ui/tab_contents/core_tab_helper.h"
 #include "chrome/common/thumbnail_capturer.mojom.h"
@@ -24,8 +24,8 @@
 #include "jni/ContextMenuParams_jni.h"
 #include "services/service_manager/public/cpp/interface_provider.h"
 #include "third_party/WebKit/public/web/WebContextMenuData.h"
-#include "ui/android/window_android.h"
 #include "ui/gfx/geometry/point.h"
+#include "ui/gfx/geometry/size.h"
 
 using base::android::ConvertJavaStringToUTF8;
 using base::android::ConvertUTF8ToJavaString;
@@ -34,14 +34,22 @@ using base::android::JavaParamRef;
 
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(ContextMenuHelper);
 
-const int kShareImageMaxWidth = 2048;
-const int kShareImageMaxHeight = 2048;
-
 const char kDataReductionProxyPassthroughHeader[] =
     "Chrome-Proxy: pass-through\r\n";
 
+namespace {
+
+void OnRetrieveImage(chrome::mojom::ThumbnailCapturerPtr thumbnail_capturer,
+                     const base::android::JavaRef<jobject>& jcallback,
+                     const std::vector<uint8_t>& thumbnail_data,
+                     const gfx::Size& original_size) {
+  base::android::RunCallbackAndroid(jcallback, thumbnail_data);
+}
+
+}  // namespace
+
 ContextMenuHelper::ContextMenuHelper(content::WebContents* web_contents)
-    : web_contents_(web_contents), weak_factory_(this) {
+    : web_contents_(web_contents) {
   JNIEnv* env = base::android::AttachCurrentThread();
   java_obj_.Reset(
       env,
@@ -120,6 +128,12 @@ ContextMenuHelper::CreateJavaContextMenuParams(
   return jmenu_info;
 }
 
+base::android::ScopedJavaLocalRef<jobject>
+ContextMenuHelper::GetJavaWebContents(JNIEnv* env,
+                                      const JavaParamRef<jobject>& obj) {
+  return web_contents_->GetJavaWebContents();
+}
+
 void ContextMenuHelper::OnStartDownload(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,
@@ -147,55 +161,12 @@ void ContextMenuHelper::SearchForImage(JNIEnv* env,
       render_frame_host, context_menu_params_.src_url);
 }
 
-void ContextMenuHelper::ShareImage(JNIEnv* env,
-                                   const JavaParamRef<jobject>& obj) {
-  content::RenderFrameHost* render_frame_host =
-      content::RenderFrameHost::FromID(render_process_id_, render_frame_id_);
-  if (!render_frame_host)
-    return;
-
-  chrome::mojom::ThumbnailCapturerPtr thumbnail_capturer;
-  render_frame_host->GetRemoteInterfaces()->GetInterface(&thumbnail_capturer);
-  // Bind the InterfacePtr into the callback so that it's kept alive until
-  // there's either a connection error or a response.
-  auto* thumbnail_capturer_proxy = thumbnail_capturer.get();
-  thumbnail_capturer_proxy->RequestThumbnailForContextNode(
-      0, gfx::Size(kShareImageMaxWidth, kShareImageMaxHeight),
-      base::Bind(&ContextMenuHelper::OnShareImage, weak_factory_.GetWeakPtr(),
-                 base::Passed(&thumbnail_capturer)));
-}
-
-void ContextMenuHelper::OnShareImage(
-    chrome::mojom::ThumbnailCapturerPtr thumbnail_capturer,
-    const std::vector<uint8_t>& thumbnail_data,
-    const gfx::Size& original_size) {
-  content::ContentViewCore* content_view_core =
-      content::ContentViewCore::FromWebContents(web_contents_);
-  if (!content_view_core)
-    return;
-
-  base::android::ScopedJavaLocalRef<jobject> jwindow_android(
-      content_view_core->GetWindowAndroid()->GetJavaObject());
-
-  if (jwindow_android.is_null())
-    return;
-
-  JNIEnv* env = base::android::AttachCurrentThread();
-  base::android::ScopedJavaLocalRef<jbyteArray> j_bytes =
-      base::android::ToJavaByteArray(env, thumbnail_data);
-
-  Java_ContextMenuHelper_onShareImageReceived(env, java_obj_, jwindow_android,
-                                              j_bytes);
-}
-
-// TODO(tedchoc): Unify RetrieveHeaderThumbnail and ShareImage.
-void ContextMenuHelper::RetrieveHeaderThumbnail(
-    JNIEnv* env,
-    const JavaParamRef<jobject>& obj,
-    jint j_max_size_px) {
+void ContextMenuHelper::RetrieveImage(JNIEnv* env,
+                                      const JavaParamRef<jobject>& obj,
+                                      const JavaParamRef<jobject>& jcallback,
+                                      jint max_dimen_px) {
   content::RenderFrameHost* render_frame_host =
       content::RenderFrameHost::FromID(render_process_id_, render_frame_id_);
-
   if (!render_frame_host)
     return;
 
@@ -205,33 +176,9 @@ void ContextMenuHelper::RetrieveHeaderThumbnail(
   // there's either a connection error or a response.
   auto* thumbnail_capturer_proxy = thumbnail_capturer.get();
   thumbnail_capturer_proxy->RequestThumbnailForContextNode(
-      0, gfx::Size(j_max_size_px, j_max_size_px),
-      base::Bind(&ContextMenuHelper::OnHeaderThumbnailReceived,
-                 weak_factory_.GetWeakPtr(),
-                 base::Passed(&thumbnail_capturer)));
-}
-
-void ContextMenuHelper::OnHeaderThumbnailReceived(
-    chrome::mojom::ThumbnailCapturerPtr thumbnail_capturer,
-    const std::vector<uint8_t>& thumbnail_data,
-    const gfx::Size& original_size) {
-  content::ContentViewCore* content_view_core =
-      content::ContentViewCore::FromWebContents(web_contents_);
-  if (!content_view_core)
-    return;
-
-  base::android::ScopedJavaLocalRef<jobject> jwindow_android(
-      content_view_core->GetWindowAndroid()->GetJavaObject());
-
-  if (jwindow_android.is_null())
-    return;
-
-  JNIEnv* env = base::android::AttachCurrentThread();
-  base::android::ScopedJavaLocalRef<jbyteArray> j_bytes =
-      base::android::ToJavaByteArray(env, thumbnail_data);
-
-  Java_ContextMenuHelper_onHeaderThumbnailReceived(env, java_obj_,
-                                                   jwindow_android, j_bytes);
+      0, gfx::Size(max_dimen_px, max_dimen_px),
+      base::Bind(&OnRetrieveImage, base::Passed(&thumbnail_capturer),
+                 base::android::ScopedJavaGlobalRef<jobject>(env, jcallback)));
 }
 
 bool RegisterContextMenuHelper(JNIEnv* env) {

chrome/browser/ui/android/context_menu_helper.h

@@ -10,12 +10,10 @@
 #include <vector>
 
 #include "base/android/jni_android.h"
-#include "base/callback.h"
+#include "base/android/scoped_java_ref.h"
 #include "base/macros.h"
-#include "chrome/common/thumbnail_capturer.mojom.h"
 #include "content/public/browser/web_contents_user_data.h"
 #include "content/public/common/context_menu_params.h"
-#include "ui/gfx/geometry/size.h"
 
 namespace content {
 struct ContextMenuParams;
@@ -37,17 +35,19 @@ class ContextMenuHelper
   void SetPopulator(jobject jpopulator);
 
   // Methods called from Java via JNI ------------------------------------------
+  base::android::ScopedJavaLocalRef<jobject> GetJavaWebContents(
+      JNIEnv* env,
+      const base::android::JavaParamRef<jobject>& obj);
   void OnStartDownload(JNIEnv* env,
                        const base::android::JavaParamRef<jobject>& obj,
                        jboolean jis_link,
                        jboolean jis_data_reduction_proxy_enabled);
+  void RetrieveImage(JNIEnv* env,
+                     const base::android::JavaParamRef<jobject>& obj,
+                     const base::android::JavaParamRef<jobject>& jcallback,
+                     jint max_dimen_px);
   void SearchForImage(JNIEnv* env,
                       const base::android::JavaParamRef<jobject>& obj);
-  void ShareImage(JNIEnv* env, const base::android::JavaParamRef<jobject>& obj);
-
-  void RetrieveHeaderThumbnail(JNIEnv* env,
-                               const base::android::JavaParamRef<jobject>& obj,
-                               jint j_max_size_px);
 
  private:
   explicit ContextMenuHelper(content::WebContents* web_contents);
@@ -56,15 +56,6 @@ class ContextMenuHelper
   static base::android::ScopedJavaLocalRef<jobject> CreateJavaContextMenuParams(
       const content::ContextMenuParams& params);
 
-  void OnShareImage(chrome::mojom::ThumbnailCapturerPtr thumbnail_capturer,
-                    const std::vector<uint8_t>& thumbnail_data,
-                    const gfx::Size& original_size);
-
-  void OnHeaderThumbnailReceived(
-      chrome::mojom::ThumbnailCapturerPtr thumbnail_capturer,
-      const std::vector<uint8_t>& thumbnail_data,
-      const gfx::Size& original_size);
-
   base::android::ScopedJavaGlobalRef<jobject> java_obj_;
   content::WebContents* web_contents_;
 
@@ -72,8 +63,6 @@ class ContextMenuHelper
   int render_frame_id_;
   int render_process_id_;
 
-  base::WeakPtrFactory<ContextMenuHelper> weak_factory_;
-
   DISALLOW_COPY_AND_ASSIGN(ContextMenuHelper);
 };