Convert ManifestManager IPCs to Mojo.

content/browser/manifest/manifest_manager_host.cc

Index: content/browser/manifest/manifest_manager_host.cc
diff --git a/content/browser/manifest/manifest_manager_host.cc b/content/browser/manifest/manifest_manager_host.cc
index e6c70b5f87708eef81e962dc3b4415679a82377e..6234088b68bc3106eedf12932ad6feb3eb438bd1 100644
--- a/content/browser/manifest/manifest_manager_host.cc
+++ b/content/browser/manifest/manifest_manager_host.cc
@@ -4,230 +4,120 @@
 
 #include "content/browser/manifest/manifest_manager_host.h"
 
-#include <stdint.h>
-
-#include "base/stl_util.h"
-#include "content/common/manifest_manager_messages.h"
+#include "base/id_map.h"
+#include "content/common/manifest_type_converters.h"
 #include "content/public/browser/render_frame_host.h"
-#include "content/public/browser/render_process_host.h"
 #include "content/public/common/manifest.h"
-#include "content/public/common/result_codes.h"
+#include "content/public/common/service_registry.h"
 
 namespace content {
 
-namespace {
+class ManifestManagerHost::FrameManifestManagerHost {
+ public:
+  FrameManifestManagerHost(ManifestManagerHost* parent,
+                           RenderFrameHost* render_frame_host)
+      : parent_(parent), render_frame_host_(render_frame_host) {
+    render_frame_host->GetServiceRegistry()->ConnectToRemoteService(
+        mojo::GetProxy(&manifest_manager_));
+    manifest_manager_.set_connection_error_handler(base::Bind(
+        &ManifestManagerHost::FrameManifestManagerHost::OnConnectionError,
+        base::Unretained(this)));
+  }
 
-void KillRenderer(RenderFrameHost* render_frame_host) {
-  base::ProcessHandle process_handle =
-      render_frame_host->GetProcess()->GetHandle();
-  if (process_handle == base::kNullProcessHandle)
-    return;
-  render_frame_host->GetProcess()->Shutdown(RESULT_CODE_KILLED_BAD_MESSAGE,
-                                            false);
-}
+  ~FrameManifestManagerHost() {
+    for (GetCallbackMap::const_iterator it(&get_callbacks_); !it.IsAtEnd();
+         it.Advance()) {
+      it.GetCurrentValue()->Run(Manifest());
+    }
+    for (HasCallbackMap::const_iterator it(&has_callbacks_); !it.IsAtEnd();
+         it.Advance()) {
+      it.GetCurrentValue()->Run(false);
+    }
+  }
 
-} // anonymous namespace
+  // Calls the given callback with the manifest associated with the
+  // given RenderFrameHost. If the frame has no manifest or if getting it failed
+  // the callback will have an empty manifest.
+  void GetManifest(const GetManifestCallback& callback) {
+    int request_id = get_callbacks_.Add(new GetManifestCallback(callback));
 
-ManifestManagerHost::ManifestManagerHost(WebContents* web_contents)
-  : WebContentsObserver(web_contents) {
-}
+    manifest_manager_->RequestManifest(
+        base::Bind(&ManifestManagerHost::FrameManifestManagerHost::
+                       OnRequestManifestResponse,
+                   base::Unretained(this), request_id));
+  }
 
-ManifestManagerHost::~ManifestManagerHost() {
-  STLDeleteValues(&pending_get_callbacks_);
-  STLDeleteValues(&pending_has_callbacks_);
-}
+  // Calls the given callback with a bool indicating whether or not the document
+  // associated with the given RenderFrameHost has a manifest.
+  void HasManifest(const HasManifestCallback& callback) {
+    int request_id = has_callbacks_.Add(new HasManifestCallback(callback));
 
-ManifestManagerHost::GetCallbackMap*
-ManifestManagerHost::GetCallbackMapForFrame(
-    RenderFrameHost* render_frame_host) {
-  FrameGetCallbackMap::iterator it =
-      pending_get_callbacks_.find(render_frame_host);
-  return it != pending_get_callbacks_.end() ? it->second : nullptr;
-}
+    manifest_manager_->HasManifest(base::Bind(
+        &ManifestManagerHost::FrameManifestManagerHost::OnHasManifestResponse,
+        base::Unretained(this), request_id));
+  }
 
-ManifestManagerHost::HasCallbackMap*
-ManifestManagerHost::HasCallbackMapForFrame(
-    RenderFrameHost* render_frame_host) {
-  FrameHasCallbackMap::iterator it =
-      pending_has_callbacks_.find(render_frame_host);
-  return it != pending_has_callbacks_.end() ? it->second : nullptr;
-}
+ private:
+  using GetCallbackMap = IDMap<GetManifestCallback, IDMapOwnPointer>;
+  using HasCallbackMap = IDMap<HasManifestCallback, IDMapOwnPointer>;
 
-void ManifestManagerHost::RenderFrameDeleted(
-    RenderFrameHost* render_frame_host) {
-  {
-    GetCallbackMap* callbacks = GetCallbackMapForFrame(render_frame_host);
-    if (!callbacks)
-      return;
-
-    // Call the callbacks with a failure state before deleting them. Do this in
-    // a block so the iterator is destroyed before |callbacks|.
-    {
-      GetCallbackMap::const_iterator it(callbacks);
-      for (; !it.IsAtEnd(); it.Advance())
-        it.GetCurrentValue()->Run(Manifest());
-    }
+  void OnConnectionError() { parent_->RenderFrameDeleted(render_frame_host_); }
 
-    delete callbacks;
-    pending_get_callbacks_.erase(render_frame_host);
+  void OnRequestManifestResponse(int request_id,
+                                 blink::mojom::ManifestPtr manifest) {
+    get_callbacks_.Lookup(request_id)->Run(manifest.To<Manifest>());
+    get_callbacks_.Remove(request_id);
+    DeleteIfIdle();
   }
 
-  {
-    HasCallbackMap* callbacks = HasCallbackMapForFrame(render_frame_host);
-    if (!callbacks)
-      return;
-
-    // Call the callbacks with a failure state before deleting them. Do this in
-    // a block so the iterator is destroyed before |callbacks|.
-    {
-      HasCallbackMap::const_iterator it(callbacks);
-      for (; !it.IsAtEnd(); it.Advance())
-        it.GetCurrentValue()->Run(false);
-    }
-
-    delete callbacks;
-    pending_has_callbacks_.erase(render_frame_host);
+  void OnHasManifestResponse(int request_id, bool has_manifest) {
+    has_callbacks_.Lookup(request_id)->Run(has_manifest);
+    has_callbacks_.Remove(request_id);
+    DeleteIfIdle();
   }
-}
 
-void ManifestManagerHost::GetManifest(RenderFrameHost* render_frame_host,
-                                      const GetManifestCallback& callback) {
-  GetCallbackMap* callbacks = GetCallbackMapForFrame(render_frame_host);
-  if (!callbacks) {
-    callbacks = new GetCallbackMap();
-    pending_get_callbacks_[render_frame_host] = callbacks;
+  void DeleteIfIdle() {
+    if (get_callbacks_.IsEmpty() && has_callbacks_.IsEmpty())
+      OnConnectionError();

[mlamouri (slow - plz ping), 2016/04/27 13:38:08]

OnConnectionError is for deleting? That sounds odd but I guess you are the mojo
specialist here :)

[Sam McNally, 2016/04/28 08:25:27]

Renamed it to not be specific to connection errors.

   }
 
-  int request_id = callbacks->Add(new GetManifestCallback(callback));
+  ManifestManagerHost* parent_;
+  RenderFrameHost* render_frame_host_;
+  blink::mojom::ManifestManagerPtr manifest_manager_;
+  GetCallbackMap get_callbacks_;
+  HasCallbackMap has_callbacks_;
 
-  render_frame_host->Send(new ManifestManagerMsg_RequestManifest(
-      render_frame_host->GetRoutingID(), request_id));
-}
+  DISALLOW_COPY_AND_ASSIGN(FrameManifestManagerHost);
+};
 
-void ManifestManagerHost::HasManifest(RenderFrameHost* render_frame_host,
-                                      const HasManifestCallback& callback) {
-  HasCallbackMap* callbacks = HasCallbackMapForFrame(render_frame_host);
-  if (!callbacks) {
-    callbacks = new HasCallbackMap();
-    pending_has_callbacks_[render_frame_host] = callbacks;
-  }
+ManifestManagerHost::ManifestManagerHost(WebContents* web_contents)
+    : WebContentsObserver(web_contents) {}
 
-  int request_id = callbacks->Add(new HasManifestCallback(callback));
+ManifestManagerHost::~ManifestManagerHost() = default;
 
-  render_frame_host->Send(new ManifestManagerMsg_HasManifest(
-      render_frame_host->GetRoutingID(), request_id));
+void ManifestManagerHost::RenderFrameDeleted(
+    RenderFrameHost* render_frame_host) {
+  hosts_.erase(render_frame_host);
 }
 
-bool ManifestManagerHost::OnMessageReceived(
-    const IPC::Message& message, RenderFrameHost* render_frame_host) {
-  bool handled = true;
-
-  IPC_BEGIN_MESSAGE_MAP_WITH_PARAM(ManifestManagerHost, message,
-                                   render_frame_host)
-    IPC_MESSAGE_HANDLER(ManifestManagerHostMsg_RequestManifestResponse,
-                        OnRequestManifestResponse)
-    IPC_MESSAGE_HANDLER(ManifestManagerHostMsg_HasManifestResponse,
-                        OnHasManifestResponse)
-    IPC_MESSAGE_UNHANDLED(handled = false)
-  IPC_END_MESSAGE_MAP()
-
-  return handled;
+void ManifestManagerHost::GetManifest(RenderFrameHost* render_frame_host,
+                                      const GetManifestCallback& callback) {
+  GetOrCreateHostForFrame(render_frame_host).GetManifest(callback);
 }
 
-void ManifestManagerHost::OnRequestManifestResponse(
-    RenderFrameHost* render_frame_host,
-    int request_id,
-    const Manifest& insecure_manifest) {
-  GetCallbackMap* callbacks = GetCallbackMapForFrame(render_frame_host);
-  if (!callbacks) {
-    DVLOG(1) << "Unexpected RequestManifestResponse to from renderer. "
-                "Killing renderer.";
-    KillRenderer(render_frame_host);
-    return;
-  }
-
-  GetManifestCallback* callback = callbacks->Lookup(request_id);
-  if (!callback) {
-    DVLOG(1) << "Received a request_id (" << request_id << ") from renderer "
-                "with no associated callback. Killing renderer.";
-    KillRenderer(render_frame_host);
-    return;
-  }
-
-  // When receiving a Manifest, the browser process can't trust that it is
-  // coming from a known and secure source. It must be processed accordingly.
-  Manifest manifest = insecure_manifest;
-  manifest.name = base::NullableString16(
-      manifest.name.string().substr(0, Manifest::kMaxIPCStringLength),
-      manifest.name.is_null());
-  manifest.short_name = base::NullableString16(
-        manifest.short_name.string().substr(0, Manifest::kMaxIPCStringLength),
-        manifest.short_name.is_null());
-  if (!manifest.start_url.is_valid())
-    manifest.start_url = GURL();
-  for (auto& icon : manifest.icons) {
-    if (!icon.src.is_valid())
-      icon.src = GURL();
-    icon.type = base::NullableString16(
-        icon.type.string().substr(0, Manifest::kMaxIPCStringLength),
-        icon.type.is_null());
-  }
-  manifest.gcm_sender_id = base::NullableString16(
-        manifest.gcm_sender_id.string().substr(
-            0, Manifest::kMaxIPCStringLength),
-        manifest.gcm_sender_id.is_null());
-  for (auto& related_application : manifest.related_applications) {
-    if (!related_application.url.is_valid())
-      related_application.url = GURL();
-    related_application.id =
-        base::NullableString16(related_application.id.string().substr(
-                                   0, Manifest::kMaxIPCStringLength),
-                               related_application.id.is_null());
-  }
-  // theme_color and background_color are 32 bit unsigned integers with 64 bit
-  // integers simply being used to encode the occurence of an error. Therefore,
-  // any value outside the range of a 32 bit integer is invalid.
-  if (manifest.theme_color < std::numeric_limits<int32_t>::min() ||
-      manifest.theme_color > std::numeric_limits<int32_t>::max())
-    manifest.theme_color = Manifest::kInvalidOrMissingColor;
-  if (manifest.background_color < std::numeric_limits<int32_t>::min() ||
-      manifest.background_color > std::numeric_limits<int32_t>::max())
-    manifest.background_color = Manifest::kInvalidOrMissingColor;
-
-  callback->Run(manifest);
-  callbacks->Remove(request_id);
-  if (callbacks->IsEmpty()) {
-    delete callbacks;
-    pending_get_callbacks_.erase(render_frame_host);
-  }
+void ManifestManagerHost::HasManifest(RenderFrameHost* render_frame_host,
+                                      const HasManifestCallback& callback) {
+  GetOrCreateHostForFrame(render_frame_host).HasManifest(callback);
 }
 
-void ManifestManagerHost::OnHasManifestResponse(
-    RenderFrameHost* render_frame_host,
-    int request_id,
-    bool has_manifest) {
-  HasCallbackMap* callbacks = HasCallbackMapForFrame(render_frame_host);
-  if (!callbacks) {
-    DVLOG(1) << "Unexpected HasManifestResponse from renderer. "
-                "Killing renderer.";
-    KillRenderer(render_frame_host);
-    return;
-  }
-
-  HasManifestCallback* callback = callbacks->Lookup(request_id);
-  if (!callback) {
-    DVLOG(1) << "Received a request_id (" << request_id << ") from renderer "
-                "with no associated callback. Killing renderer.";
-    KillRenderer(render_frame_host);
-    return;
-  }
-
-  callback->Run(has_manifest);
-  callbacks->Remove(request_id);
-  if (callbacks->IsEmpty()) {
-    delete callbacks;
-    pending_has_callbacks_.erase(render_frame_host);
-  }
+ManifestManagerHost::FrameManifestManagerHost&
+ManifestManagerHost::GetOrCreateHostForFrame(
+    RenderFrameHost* render_frame_host) {
+  std::unique_ptr<ManifestManagerHost::FrameManifestManagerHost>& host =
+      hosts_[render_frame_host];
+  if (!host)
+    host.reset(new FrameManifestManagerHost(this, render_frame_host));
+  return *host;
 }
 
-} // namespace content
+}  // namespace content