Make page popups work under OOPIF

content/browser/renderer_host/render_view_host_impl.cc

 // Copyright (c) 2012 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 "content/browser/renderer_host/render_view_host_impl.h"
 
 #include <set>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 961 matching lines @@
                                     WindowOpenDisposition disposition,
                                     const gfx::Rect& initial_rect,
                                     bool 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_)
+  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));
 }
 
 void RenderViewHostImpl::OnShowFullscreenWidget(int route_id) {
   if (is_active_)
     delegate_->ShowCreatedFullscreenWidget(route_id);
   Send(new ViewMsg_Move_ACK(route_id));
 }
 
@@ skipping 361 matching lines @@
   } else {
     render_view_ready_on_process_launch_ = true;
   }
 }
 
 void RenderViewHostImpl::RenderViewReady() {
   delegate_->RenderViewReady(this);
 }
 
 }  // namespace content