Mojoify the RequestThumbnailForContextNode IPC message and reply

chrome/browser/ui/android/context_menu_helper.cc

Index: chrome/browser/ui/android/context_menu_helper.cc
diff --git a/chrome/browser/ui/android/context_menu_helper.cc b/chrome/browser/ui/android/context_menu_helper.cc
index ade834a6ac5d87bc4315a662615c2ca9148bf6ad..ee1a6237178e57f8cfe35c0d426214b602b289fa 100644
--- a/chrome/browser/ui/android/context_menu_helper.cc
+++ b/chrome/browser/ui/android/context_menu_helper.cc
@@ -9,8 +9,10 @@
 #include "base/android/jni_android.h"
 #include "base/android/jni_array.h"
 #include "base/android/jni_string.h"
+#include "base/bind_helpers.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"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_headers.h"
 #include "content/public/browser/android/content_view_core.h"
 #include "content/public/browser/render_frame_host.h"
@@ -18,6 +20,7 @@
 #include "content/public/common/context_menu_params.h"
 #include "jni/ContextMenuHelper_jni.h"
 #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"
@@ -36,7 +39,7 @@ const char kDataReductionProxyPassthroughHeader[] =
     "Chrome-Proxy: pass-through\r\n";
 
 ContextMenuHelper::ContextMenuHelper(content::WebContents* web_contents)
-    : web_contents_(web_contents) {
+    : web_contents_(web_contents), weak_factory_(this) {
   JNIEnv* env = base::android::AttachCurrentThread();
   java_obj_.Reset(
       env,
@@ -149,17 +152,20 @@ void ContextMenuHelper::ShareImage(JNIEnv* env,
   if (!render_frame_host)
     return;
 
-  CoreTabHelper::FromWebContents(web_contents_)->
-      RequestThumbnailForContextNode(
-          render_frame_host,
-          0,
-          gfx::Size(kShareImageMaxWidth, kShareImageMaxHeight),
-          base::Bind(&ContextMenuHelper::OnShareImage,
-                     base::Unretained(this)));
+  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.
+  thumbnail_capturer->RequestThumbnailForContextNode(
+      0, gfx::Size(kShareImageMaxWidth, kShareImageMaxHeight),
+      base::Bind(&ContextMenuHelper::OnShareImage, weak_factory_.GetWeakPtr(),
+                 base::Passed(&thumbnail_capturer)));

[tibell, 2017/03/09 02:21:49]

This move is probably undefined behavior since you're calling methods on
thumbnail_capturer after moving. Take a raw pointer to it first and use that for
the call and keep the move as is.

[watk, 2017/03/09 04:01:01]

Discussed offline. Looks like this is guaranteed to work by the standard. The
expression naming the function is sequenced before the arguments are evaluated.
But it relies on the move constructor not changing the address of the underlying
proxy object.

 }
 
-void ContextMenuHelper::OnShareImage(const std::string& thumbnail_data,
-                                     const gfx::Size& original_size) {
+void ContextMenuHelper::OnShareImage(
+    chrome::mojom::ThumbnailCapturerPtr thumbnail_capturer,
+    const std::string& thumbnail_data,
+    const gfx::Size& original_size) {
   content::ContentViewCore* content_view_core =
       content::ContentViewCore::FromWebContents(web_contents_);
   if (!content_view_core)