Allow MimeHandlerViewGuest be embedded inside OOPIFs

components/guest_view/browser/guest_view_manager.cc

Index: components/guest_view/browser/guest_view_manager.cc
diff --git a/components/guest_view/browser/guest_view_manager.cc b/components/guest_view/browser/guest_view_manager.cc
index 1734c762657e9ae169d5d621fc0c84cd8fbe7d26..3824952840c81cd158aafbc85ab1dec1f6a79d21 100644
--- a/components/guest_view/browser/guest_view_manager.cc
+++ b/components/guest_view/browser/guest_view_manager.cc
@@ -1,7 +1,7 @@
 // Copyright 2014 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 "base/debug/stack_trace.h"

[Charlie Reis, 2016/10/18 22:52:20]

This belongs below.

[EhsanK, 2016/10/20 21:41:17]

Sorry for leaving this behind. Will remove it.

 #include "components/guest_view/browser/guest_view_manager.h"
 
 #include <tuple>
@@ -71,6 +71,7 @@ GuestViewManager::GuestViewManager(
       last_instance_id_removed_(0),
       context_(context),
       delegate_(std::move(delegate)),
+      attaching_guest_embedder_routing_id_(MSG_ROUTING_NONE),
       weak_ptr_factory_(this) {}
 
 GuestViewManager::~GuestViewManager() {}
@@ -110,9 +111,13 @@ content::WebContents* GuestViewManager::GetGuestByInstanceIDSafely(
 }
 
 void GuestViewManager::AttachGuest(int embedder_process_id,
+                                   int embedder_routing_id,
                                    int element_instance_id,
                                    int guest_instance_id,
                                    const base::DictionaryValue& attach_params) {
+  DCHECK_EQ(attaching_guest_embedder_routing_id_, MSG_ROUTING_NONE);
+  attaching_guest_embedder_routing_id_ = embedder_routing_id;
+
   auto* guest_view =
       GuestViewBase::From(embedder_process_id, guest_instance_id);
   if (!guest_view)
@@ -463,8 +468,25 @@ bool GuestViewManager::CanEmbedderAccessInstanceID(
   if (!guest_view)
     return false;
 
-  return embedder_render_process_id ==
-      guest_view->owner_web_contents()->GetRenderProcessHost()->GetID();
+  int render_frame_routing_id = attaching_guest_embedder_routing_id_;
+  if (render_frame_routing_id == MSG_ROUTING_NONE) {
+    render_frame_routing_id =
+        guest_view->owner_web_contents()->GetMainFrame()->GetRoutingID();
+  }
+
+  if (embedder_render_process_id !=
+          guest_view->owner_web_contents()->GetRenderProcessHost()->GetID() &&
+      !guest_view->IsViewType("mimehandler")) {
+    // Only MimeHandlerViewGuest can be embedded in a cross origin frame.
+    return false;
+  }
+
+  content::WebContents* web_contents = guest_view->host()->RegisterEmbedderID(
+      embedder_render_process_id, render_frame_routing_id);

[Charlie Reis, 2016/10/18 22:52:20]

This doesn't seem obviously safe to me-- we're mixing a routing ID stored as
state with a process ID passed in by the caller.  It would be very easy to have
bugs where these don't correspond to the same RenderFrameHost.

[EhsanK, 2016/10/20 21:41:17]

This variable and logic is removed form here. I now use
MimeHandlerViewGuestCreatedCallback() to set these values as both embedder
process and frame id are provided there.

+  attaching_guest_embedder_routing_id_ = MSG_ROUTING_NONE;
+
+  return web_contents->GetRenderProcessHost()->GetID() ==
+         guest_view->owner_web_contents()->GetRenderProcessHost()->GetID();

[Charlie Reis, 2016/10/18 22:52:20]

nit: No need for the GetID() calls, since you can just compare the RPHs
directly.

Also, it seems like it's worth including a comment on what this block (from 484
down) is trying to do.

[EhsanK, 2016/10/20 21:41:17]

Acknowledged.

 }
 
 GuestViewManager::ElementInstanceKey::ElementInstanceKey()