[ MimeHandlerView ] Use Associated Binding

Mojo calls from browser to renderer are currently on a non-associated
channel. Essentially, the SetInternalId API on
MimeHandlerViewContianerManager must be received before the document
is loaded. Right now this is not guaranteed and there is room for race
bugs.

Right now SetInternalId seems to be the only IPC which requires to be
on an associated channel. Eventually this IPC could be singled out into
its own interface and have a separate binding in MHCVCM for it.

TBR=wjmaclean@chromium.org

Bug: 659750
Change-Id: I2b337c4876f3eb3e4bc6b1e6f72033ccd4edda03
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1617016Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#662511}

chrome/renderer/chrome_content_renderer_client.cc

@@ -462,11 +462,6 @@ void ChromeContentRendererClient::RenderFrameCreated(
 #if BUILDFLAG(ENABLE_EXTENSIONS)
   ChromeExtensionsRendererClient::GetInstance()->RenderFrameCreated(
       render_frame, registry);
-  if (content::MimeHandlerViewMode::UsesCrossProcessFrame()) {
-    registry->AddInterface(base::BindRepeating(
-        &extensions::MimeHandlerViewContainerManager::BindRequest,
-        render_frame->GetRoutingID()));
-  }
 #endif
 
 #if BUILDFLAG(ENABLE_PLUGINS)
@@ -533,6 +528,14 @@ void ChromeContentRendererClient::RenderFrameCreated(
                                               associated_interfaces);
   }
 
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+  if (content::MimeHandlerViewMode::UsesCrossProcessFrame()) {
+    associated_interfaces->AddInterface(base::BindRepeating(
+        &extensions::MimeHandlerViewContainerManager::BindRequest,
+        render_frame->GetRoutingID()));
+  }
+#endif
+
   // Owned by |render_frame|.
   page_load_metrics::MetricsRenderFrameObserver* metrics_render_frame_observer =
       new page_load_metrics::MetricsRenderFrameObserver(render_frame);

extensions/browser/guest_view/mime_handler_view/mime_handler_view_attach_helper.cc

@@ -23,7 +23,7 @@
 #include "extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.h"
 #include "extensions/common/guest_view/extensions_guest_view_messages.h"
 #include "ppapi/buildflags/buildflags.h"
-#include "services/service_manager/public/cpp/interface_provider.h"
+#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
 #include "third_party/skia/include/core/SkColor.h"
 
 #if BUILDFLAG(ENABLE_PLUGINS)
@@ -166,9 +166,10 @@ void MimeHandlerViewAttachHelper::ResumeAttachOrDestroy(
   auto* guest_view = pending_guests_[element_instance_id];
   pending_guests_.erase(element_instance_id);
   if (!plugin_rfh) {
-    mojom::MimeHandlerViewContainerManagerPtr container_manager;
-    guest_view->GetEmbedderFrame()->GetRemoteInterfaces()->GetInterface(
-        &container_manager);
+    mojom::MimeHandlerViewContainerManagerAssociatedPtr container_manager;
+    guest_view->GetEmbedderFrame()
+        ->GetRemoteAssociatedInterfaces()
+        ->GetInterface(&container_manager);
     container_manager->DestroyFrameContainer(element_instance_id);
     guest_view->Destroy(true);
     return;

extensions/browser/guest_view/mime_handler_view/mime_handler_view_browsertest.cc

@@ -42,7 +42,7 @@
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "net/test/embedded_test_server/http_request.h"
 #include "services/network/public/cpp/features.h"
-#include "services/service_manager/public/cpp/interface_provider.h"
+#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
 #include "ui/base/ui_base_features.h"
 #include "url/url_constants.h"
 
@@ -327,9 +327,11 @@ IN_PROC_BROWSER_TEST_P(MimeHandlerViewCrossProcessTest,
   render_frame_observer.WaitUntilDeleted();
   // Send the IPC. During destruction MHVFC would cause a UaF since it was not
   // removed from the global map.
-  extensions::mojom::MimeHandlerViewContainerManagerPtr container_manager;
-  embedder_web_contents->GetMainFrame()->GetRemoteInterfaces()->GetInterface(
-      &container_manager);
+  extensions::mojom::MimeHandlerViewContainerManagerAssociatedPtr
+      container_manager;
+  embedder_web_contents->GetMainFrame()
+      ->GetRemoteAssociatedInterfaces()
+      ->GetInterface(&container_manager);
   container_manager->DestroyFrameContainer(element_instance_id);
   // Running the following JS code fails if the renderer has crashed.
   ASSERT_TRUE(content::ExecJs(embedder_web_contents, "window.name = 'foo'"));

extensions/browser/guest_view/mime_handler_view/mime_handler_view_embedder.cc

@@ -18,7 +18,7 @@
 #include "extensions/browser/guest_view/mime_handler_view/mime_handler_view_constants.h"
 #include "extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.h"
 #include "extensions/common/mojo/guest_view.mojom.h"
-#include "services/service_manager/public/cpp/interface_provider.h"
+#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
 #include "third_party/blink/public/common/frame/frame_owner_element_type.h"
 
 namespace extensions {
@@ -180,7 +180,7 @@ void MimeHandlerViewEmbedder::DidCreateMimeHandlerViewGuest(
 mojom::MimeHandlerViewContainerManager*
 MimeHandlerViewEmbedder::GetContainerManager() {
   if (!container_manager_) {
-    render_frame_host_->GetRemoteInterfaces()->GetInterface(
+    render_frame_host_->GetRemoteAssociatedInterfaces()->GetInterface(
         &container_manager_);
   }
   return container_manager_.get();

extensions/browser/guest_view/mime_handler_view/mime_handler_view_embedder.h

@@ -78,7 +78,7 @@ class MimeHandlerViewEmbedder : public content::WebContentsObserver {
   // to after it is created.
   mime_handler::BeforeUnloadControlPtrInfo pending_before_unload_control_;
 
-  mojom::MimeHandlerViewContainerManagerPtr container_manager_;
+  mojom::MimeHandlerViewContainerManagerAssociatedPtr container_manager_;
 
   const std::string internal_id_;
 

extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc

@@ -36,8 +36,9 @@
 #include "extensions/common/mojo/guest_view.mojom.h"
 #include "extensions/strings/grit/extensions_strings.h"
 #include "services/service_manager/public/cpp/binder_registry.h"
-#include "services/service_manager/public/cpp/interface_provider.h"
+#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
 #include "third_party/blink/public/platform/web_gesture_event.h"
+
 using content::WebContents;
 using guest_view::GuestViewBase;
 
@@ -114,9 +115,11 @@ MimeHandlerViewGuest::~MimeHandlerViewGuest() {
     if (GetEmbedderFrame() && GetEmbedderFrame()->GetParent()) {
       // TODO(ekaramad): This should only be needed if the embedder frame is in
       // a plugin element (https://crbug.com/957373).
-      mojom::MimeHandlerViewContainerManagerPtr container_manager;
-      GetEmbedderFrame()->GetParent()->GetRemoteInterfaces()->GetInterface(
-          &container_manager);
+      mojom::MimeHandlerViewContainerManagerAssociatedPtr container_manager;
+      GetEmbedderFrame()
+          ->GetParent()
+          ->GetRemoteAssociatedInterfaces()
+          ->GetInterface(&container_manager);
       container_manager->DestroyFrameContainer(element_instance_id());
     }
   }
@@ -448,8 +451,8 @@ void MimeHandlerViewGuest::DocumentOnLoadCompletedInMainFrame() {
     // just send the upadte to the embedder (full page  MHV).
     auto* rfh = maybe_has_frame_container_ ? GetEmbedderFrame()->GetParent()
                                            : GetEmbedderFrame();
-    mojom::MimeHandlerViewContainerManagerPtr container_manager;
-    rfh->GetRemoteInterfaces()->GetInterface(&container_manager);
+    mojom::MimeHandlerViewContainerManagerAssociatedPtr container_manager;
+    rfh->GetRemoteAssociatedInterfaces()->GetInterface(&container_manager);
     container_manager->DidLoad(element_instance_id(), original_resource_url_);
     return;
   }

extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container_manager.cc

@@ -42,7 +42,7 @@ RenderFrameMap* GetRenderFrameMap() {
 // static
 void MimeHandlerViewContainerManager::BindRequest(
     int32_t routing_id,
-    mojom::MimeHandlerViewContainerManagerRequest request) {
+    mojom::MimeHandlerViewContainerManagerAssociatedRequest request) {
   CHECK(content::MimeHandlerViewMode::UsesCrossProcessFrame());
   auto* render_frame = content::RenderFrame::FromRoutingID(routing_id);
   if (!render_frame)

extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container_manager.h

@@ -14,7 +14,7 @@
 #include "extensions/common/guest_view/mime_handler_view_uma_types.h"
 #include "extensions/common/mojo/guest_view.mojom.h"
 #include "extensions/renderer/guest_view/mime_handler_view/post_message_support.h"
-#include "mojo/public/cpp/bindings/binding_set.h"
+#include "mojo/public/cpp/bindings/associated_binding_set.h"
 #include "third_party/blink/public/web/web_element.h"
 #include "url/gurl.h"
 
@@ -49,7 +49,7 @@ class MimeHandlerViewContainerManager
  public:
   static void BindRequest(
       int32_t routing_id,
-      mojom::MimeHandlerViewContainerManagerRequest request);
+      mojom::MimeHandlerViewContainerManagerAssociatedRequest request);
   // Returns the container manager associated with |render_frame|. If none
   // exists and |create_if_does_not_exist| is set true, creates and returns a
   // new instance for |render_frame|.
@@ -134,7 +134,7 @@ class MimeHandlerViewContainerManager
   // The plugin element that is managed by MimeHandlerViewContainerManager.
   blink::WebElement plugin_element_;
 
-  mojo::BindingSet<mojom::MimeHandlerViewContainerManager> bindings_;
+  mojo::AssociatedBindingSet<mojom::MimeHandlerViewContainerManager> bindings_;
   mojo::Binding<mime_handler::BeforeUnloadControl>
       before_unload_control_binding_;