Mojoify the RequestThumbnailForContextNode IPC message and reply

chrome/browser/ui/tab_contents/core_tab_helper.cc

Index: chrome/browser/ui/tab_contents/core_tab_helper.cc
diff --git a/chrome/browser/ui/tab_contents/core_tab_helper.cc b/chrome/browser/ui/tab_contents/core_tab_helper.cc
index 8c5cddcd6025fd07f65afe80fdec4cc3b76a362e..929b2bc699467c742c85c4792fc359a4b21266a2 100644
--- a/chrome/browser/ui/tab_contents/core_tab_helper.cc
+++ b/chrome/browser/ui/tab_contents/core_tab_helper.cc
@@ -20,6 +20,7 @@
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/render_messages.h"
+#include "chrome/common/thumbnail_capturer.mojom.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/guest_view/browser/guest_view_manager.h"
 #include "components/search_engines/template_url.h"
@@ -36,6 +37,7 @@
 #include "content/public/common/context_menu_params.h"
 #include "net/base/load_states.h"
 #include "net/http/http_request_headers.h"
+#include "services/service_manager/public/cpp/interface_provider.h"
 #include "ui/base/l10n/l10n_util.h"
 
 #if !defined(OS_ANDROID)
@@ -105,31 +107,39 @@ void CoreTabHelper::UpdateContentRestrictions(int content_restrictions) {
 }
 
 void CoreTabHelper::SearchByImageInNewTab(
-    content::RenderFrameHost* render_frame_host, const GURL& src_url) {
+    content::RenderFrameHost* render_frame_host,
+    const GURL& src_url) {
   RequestThumbnailForContextNode(
-      render_frame_host,
-      kImageSearchThumbnailMinSize,
-      gfx::Size(kImageSearchThumbnailMaxWidth,
-                kImageSearchThumbnailMaxHeight),
+      render_frame_host, kImageSearchThumbnailMinSize,
+      gfx::Size(kImageSearchThumbnailMaxWidth, kImageSearchThumbnailMaxHeight),
       base::Bind(&CoreTabHelper::DoSearchByImageInNewTab,
-                 base::Unretained(this),
-                 src_url));
+                 base::Unretained(this), src_url));
 }
 
 void CoreTabHelper::RequestThumbnailForContextNode(
     content::RenderFrameHost* render_frame_host,
-    int minimum_size,
-    gfx::Size maximum_size,
+    int thumbnail_min_area_pixels,
+    gfx::Size thumbnail_max_size_pixels,
     const ContextNodeThumbnailCallback& callback) {
-  int callback_id = thumbnail_callbacks_.Add(
-      base::MakeUnique<ContextNodeThumbnailCallback>(callback));
+  chrome::mojom::ThumbnailCapturerPtr* thumbnail_capturer =
+      new chrome::mojom::ThumbnailCapturerPtr();
+  int interface_id =
+      thumbnail_capturers_.Add(base::WrapUnique(thumbnail_capturer));

[watk, 2017/03/08 03:48:51]

WDYT about this? I don't like that it can leak InterfacePtrs because they're
only removed when the response comes back. On the other hand the old code could
leak callbacks, so I don't think it's worse.

I think the ideal API would be to pass ownership of the InterfacePtr to the
callback. And use a weak_ptr to allow the callback to outlive CoreTabHelper.

But it's maybe more subtle.

The other advantage is that we could remove this function and have the two
callsites use the mojo interface directly.

[tibell, 2017/03/08 04:19:49]

As discussed offline. Lets try the weak ptr approach.

+  render_frame_host->GetRemoteInterfaces()->GetInterface(thumbnail_capturer);
+  (*thumbnail_capturer)
+      ->RequestThumbnailForContextNode(
+          thumbnail_min_area_pixels, thumbnail_max_size_pixels,
+          base::Bind(&CoreTabHelper::OnThumbnailForContextNodeReceived,
+                     base::Unretained(this), interface_id, callback));
+}
 
-  render_frame_host->Send(
-      new ChromeViewMsg_RequestThumbnailForContextNode(
-          render_frame_host->GetRoutingID(),
-          minimum_size,
-          maximum_size,
-          callback_id));
+void CoreTabHelper::OnThumbnailForContextNodeReceived(
+    int interface_id,
+    const ContextNodeThumbnailCallback& callback,
+    const std::string& thumbnail_data,
+    const gfx::Size& original_size) {
+  thumbnail_capturers_.Remove(interface_id);
+  callback.Run(thumbnail_data, original_size);
 }
 
 // static
@@ -301,30 +311,6 @@ void CoreTabHelper::BeforeUnloadDialogCancelled() {
   OnCloseCanceled();
 }
 
-bool CoreTabHelper::OnMessageReceived(
-    const IPC::Message& message,
-    content::RenderFrameHost* render_frame_host) {
-  bool handled = true;
-  IPC_BEGIN_MESSAGE_MAP(CoreTabHelper, message)
-    IPC_MESSAGE_HANDLER(ChromeViewHostMsg_RequestThumbnailForContextNode_ACK,
-                        OnRequestThumbnailForContextNodeACK)
-    IPC_MESSAGE_UNHANDLED(handled = false)
-  IPC_END_MESSAGE_MAP()
-  return handled;
-}
-
-void CoreTabHelper::OnRequestThumbnailForContextNodeACK(
-    const std::string& thumbnail_data,
-    const gfx::Size& original_size,
-    int callback_id) {
-  ContextNodeThumbnailCallback* callback =
-      thumbnail_callbacks_.Lookup(callback_id);
-  if (!callback)
-    return;
-  callback->Run(thumbnail_data, original_size);
-  thumbnail_callbacks_.Remove(callback_id);
-}
-
 // Handles the image thumbnail for the context node, composes a image search
 // request based on the received thumbnail and opens the request in a new tab.
 void CoreTabHelper::DoSearchByImageInNewTab(const GURL& src_url,