Mojoify the RequestThumbnailForContextNode IPC message and reply

chrome/browser/ui/tab_contents/core_tab_helper.cc

 // Copyright (c) 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 "chrome/browser/ui/tab_contents/core_tab_helper.h"
 
 #include <string>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/profiler/scoped_tracker.h"
 #include "base/strings/stringprintf.h"
 #include "build/build_config.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/browser/ui/browser_command_controller.h"
 #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"
 #include "components/search_engines/template_url_service.h"
 #include "components/strings/grit/components_strings.h"
 #include "components/web_cache/browser/web_cache_manager.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_switches.h"
 #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)
 #include "chrome/browser/ui/browser.h"
 #endif
 
 using content::WebContents;
 
 namespace {
 
@@ skipping 49 matching lines @@
 #if !defined(OS_ANDROID)
   Browser* browser = chrome::FindBrowserWithWebContents(web_contents());
   if (!browser)
     return;
 
   browser->command_controller()->ContentRestrictionsChanged();
 #endif
 }
 
 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
 bool CoreTabHelper::GetStatusTextForWebContents(
     base::string16* status_text, content::WebContents* source) {
   // TODO(robliao): Remove ScopedTracker below once https://crbug.com/467185 is
   // fixed.
   tracked_objects::ScopedTracker tracking_profile1(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "467185 CoreTabHelper::GetStatusTextForWebContents1"));
@@ skipping 151 matching lines @@
 }
 
 void CoreTabHelper::BeforeUnloadFired(const base::TimeTicks& proceed_time) {
   before_unload_end_time_ = proceed_time;
 }
 
 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,
                                             const std::string& thumbnail_data,
                                             const gfx::Size& original_size) {
   if (thumbnail_data.empty())
     return;
 
   Profile* profile =
       Profile::FromBrowserContext(web_contents()->GetBrowserContext());
@@ skipping 27 matching lines @@
     DCHECK(!content_type.empty());
     open_url_params.uses_post = true;
     open_url_params.post_data = content::ResourceRequestBody::CreateFromBytes(
         post_data.data(), post_data.size());
     open_url_params.extra_headers += base::StringPrintf(
         "%s: %s\r\n", net::HttpRequestHeaders::kContentType,
         content_type.c_str());
   }
   web_contents()->OpenURL(open_url_params);
 }