Make page popups work under OOPIF

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 4a8e86b36ef797495dfc657c799570b1bfd82d86..3a7bdce6f06e4c3fd2cb05fa612107225934d69c 100644
--- a/content/browser/renderer_host/render_view_host_impl.cc
+++ b/content/browser/renderer_host/render_view_host_impl.cc
@@ -979,7 +979,11 @@ void RenderViewHostImpl::OnShowView(int route_id,
 
 void RenderViewHostImpl::OnShowWidget(int route_id,
                                       const gfx::Rect& initial_rect) {
-  if (is_active_)
+  if (delegate_->GetFrameTree()
+          ->root()
+          ->render_manager()
+          ->current_host()
+          ->is_active())

[nasko, 2016/03/17 17:25:45]

Is this the correct check to make here? I think this will always evaluate to
true. Can you put a comment with what the intent is here?

[kenrb, 2016/03/17 18:59:48]

If the is_active_ bit is never false on a top-level RenderViewHost, then we can
remove this check. I wasn't sure about this, but it looks like the condition
might become redundant with the death of swapped out state. Can I safely remove
this now?

[nasko, 2016/03/17 19:10:46]

Oops, I've read it current_frame_host. I still think the comment applies though,
as the implementation of current_host() takes the current RFH and grabs its
RenderViewHost. It is considered active when the RFH is the current one. Once we
start on the path of swapping it out and the new RFH has been committed, the RVH
is considered inactive, but after commit we will get the new RFH's RVH.

I'm curious though what will be the effect of dropping this check? I think the
check was enforcing that this IPC is not received from a swapped out RVH, which
you just relaxed as a requirement. Is there any attack possible if a compromised
renderer for one of the oopifs ends up sending this IPC to the browser?

[kenrb, 2016/03/17 20:00:06]

Possibly it could be used for some kind of UI redressing attack, but we consider
those out of scope.

It sounds like this check can go, in any case.

     delegate_->ShowCreatedWidget(route_id, initial_rect);
   Send(new ViewMsg_Move_ACK(route_id));
 }