Replace image_messages.h with Mojo service

content/browser/frame_host/render_frame_host_impl.cc

Index: content/browser/frame_host/render_frame_host_impl.cc
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index 8dc4395003b355fffabc072338aeeb9e7e51a4e3..5a8c32f4aeaf95e4290129e91b8a7bdd3ab2f43d 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -64,8 +64,13 @@
 #include "content/public/common/isolated_world_ids.h"
 #include "content/public/common/url_constants.h"
 #include "content/public/common/url_utils.h"
+#include "mojo/common/url_type_converters.h"
+#include "mojo/converters/geometry/geometry_type_converters.h"
+#include "skia/public/type_converters.h"
+#include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/accessibility/ax_tree.h"
 #include "ui/accessibility/ax_tree_update.h"
+#include "ui/gfx/geometry/size.h"
 #include "url/gurl.h"
 
 #if defined(OS_ANDROID)
@@ -113,6 +118,21 @@ base::i18n::TextDirection WebTextDirectionToChromeTextDirection(
   }
 }
 
+void DidDownloadImage(const WebContents::ImageDownloadCallback& callback,
+                      int id,
+                      const GURL& image_url,
+                      image_downloader::DownloadResultPtr result) {
+  DCHECK(result);
+
+  const std::vector<SkBitmap> images =
+      result->images.To<std::vector<SkBitmap>>();
+  const std::vector<gfx::Size> original_image_sizes =
+      result->original_image_sizes.To<std::vector<gfx::Size>>();
+
+  callback.Run(id, result->http_status_code, image_url, images,
+               original_image_sizes);
+}
+
 }  // namespace
 
 // static
@@ -1852,6 +1872,9 @@ void RenderFrameHostImpl::InvalidateMojoConnection() {
 #endif
 
   service_registry_.reset();
+
+  // Disconnect with ImageDownloader Mojo service in RenderFrame.
+  mojo_image_downloader_.reset();
 }
 
 bool RenderFrameHostImpl::IsFocused() {
@@ -1864,6 +1887,32 @@ bool RenderFrameHostImpl::IsFocused() {
           frame_tree_->GetFocusedFrame()->IsDescendantOf(frame_tree_node()));
 }
 
+int RenderFrameHostImpl::DownloadImage(

[nasko, 2015/07/02 08:41:07]

To avoid all the complexity of defining a callback in a common location, why not
allow access to the image download service by making it accessible from
RenderFrameHost? With that approach, the code of this method can be fully moved
to WebContentsImpl and there will be no layering violations.

[leonhsl(Using Gerrit), 2015/07/06 02:15:25]

Done.

+    const GURL& url,
+    bool is_favicon,
+    uint32_t max_bitmap_size,
+    bool bypass_cache,
+    const WebContents::ImageDownloadCallback& callback) {
+  static int next_image_download_id_ = 0;
+  image_downloader::DownloadRequestPtr req =
+      image_downloader::DownloadRequest::New();
+
+  if (!mojo_image_downloader_.get()) {
+    GetServiceRegistry()->ConnectToRemoteService(
+        mojo::GetProxy(&mojo_image_downloader_));

[nasko, 2015/07/02 08:41:07]

Can this fail? If not handled, this will cause a browser crash.

[Anand Mistry (off Chromium), 2015/07/02 08:56:51]

Not in a way that will cause a crash. This line will always succeed because the
connection is done asynchronously. If the connection ultimately fails (i.e. the
renderer crashes), then this will result in a hanging callback. But that problem
already exists in the current implementation.

+  }
+
+  req->url = mojo::String::From(url);
+  req->is_favicon = is_favicon;
+  req->max_bitmap_size = max_bitmap_size;
+  req->bypass_cache = bypass_cache;
+
+  mojo_image_downloader_->DownloadImage(
+      req.Pass(),
+      base::Bind(&DidDownloadImage, callback, ++next_image_download_id_, url));
+  return next_image_download_id_;
+}
+
 void RenderFrameHostImpl::UpdateCrossProcessIframeAccessibility(
     const std::map<int32, int>& node_to_frame_routing_id_map) {
   for (const auto& iter : node_to_frame_routing_id_map) {