OOPIF: Fix window.open to work from frames with remote parent.

content/browser/renderer_host/render_view_host_impl.cc

Index: content/browser/renderer_host/render_view_host_impl.cc
diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc
index 08e220bde7322812df0dad094a7d10990a28fc75..9917dc49a09ac45d05b05fbcb0f7c39dab5a3e12 100644
--- a/content/browser/renderer_host/render_view_host_impl.cc
+++ b/content/browser/renderer_host/render_view_host_impl.cc
@@ -985,9 +985,8 @@ void RenderViewHostImpl::CreateNewWindow(
   GetProcess()->FilterURL(false, &validated_params.opener_url);
   GetProcess()->FilterURL(true, &validated_params.opener_security_origin);
 
-  delegate_->CreateNewWindow(
-      GetProcess()->GetID(), route_id, main_frame_route_id, validated_params,
-      session_storage_namespace);
+  delegate_->CreateNewWindow(GetSiteInstance(), route_id, main_frame_route_id,
+                             validated_params, session_storage_namespace);
 }
 
 void RenderViewHostImpl::CreateNewWidget(int route_id,
@@ -1003,23 +1002,19 @@ void RenderViewHostImpl::OnShowView(int route_id,
                                     WindowOpenDisposition disposition,
                                     const gfx::Rect& initial_rect,
                                     bool user_gesture) {
-  if (is_active_) {

[alexmos, 2015/07/09 22:14:06]

Is it ok to remove these?  I saw you introduced them way back in
https://codereview.chromium.org/6319001/, but it seems creating a popup with a
remote parent is a valid scenario where this should go through despite
is_active_ being false, since this gets sent to the opener's (swapped-out) RV. 
The two other IPCs below are also routed to the opener's RV, so I updated them
too.

[Charlie Reis, 2015/07/09 23:42:51]

I agree with your CL description that these should really move to
RenderFrameHost, where they can work whether the main frame is local or not.

In the meantime, it's not ideal to remove these checks, since they're intended
for racy situations with the main frame, where a swapped out page might be able
to show a popup.  I suppose that's not a huge problem if it happens, though.  We
can probably live with it if you have plans to move the IPCs over soon.

[alexmos, 2015/07/10 18:35:00]

I took a look at it, and it should be relatively straightforward at least for
the ShowView IPC.  It is only sent for renderer-created popups, and we should be
able to save/get the opener frame (which should always be local for this case)
for sending this IPC.  I can try this in a follow-up CL.

I'm less certain how much work is needed for the other two IPCs below since I'm
not really familiar in how they're used.  Looks like ShowWidget is involved in
things like DateTimeChooseImpl/ColorChooserPopupUIController/PopupMenuImpl, and
ShowFullscreenWidget is sent from CreatePepperFullscreenContainer, but I don't
know anything about those.  Maybe I should leave the is_active_ check alone for
those two for now, since they aren't involved in the "new popup" flow?

[Charlie Reis, 2015/07/10 21:44:49]

Yeah, let's leave these two checks until we're ready to move those cases to
RenderFrameHost.

-    delegate_->ShowCreatedWindow(
-        route_id, disposition, initial_rect, user_gesture);
-  }
+  delegate_->ShowCreatedWindow(route_id, disposition, initial_rect,
+                               user_gesture);
   Send(new ViewMsg_Move_ACK(route_id));
 }
 
 void RenderViewHostImpl::OnShowWidget(int route_id,
                                       const gfx::Rect& initial_rect) {
-  if (is_active_)
-    delegate_->ShowCreatedWidget(route_id, initial_rect);
+  delegate_->ShowCreatedWidget(route_id, initial_rect);
   Send(new ViewMsg_Move_ACK(route_id));
 }
 
 void RenderViewHostImpl::OnShowFullscreenWidget(int route_id) {
-  if (is_active_)
-    delegate_->ShowCreatedFullscreenWidget(route_id);
+  delegate_->ShowCreatedFullscreenWidget(route_id);
   Send(new ViewMsg_Move_ACK(route_id));
 }