Move browser-to-renderer opener plumbing to frame routing IDs

content/renderer/render_view_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/renderer/render_view_impl.h"
 
 #include <algorithm>
 #include <cmath>
 
 #include "base/auto_reset.h"
@@ skipping 593 matching lines @@
       &blink_settings);
   for (const std::string& setting : blink_settings) {
     size_t pos = setting.find('=');
     settings->setFromStrings(
         blink::WebString::fromLatin1(setting.substr(0, pos)),
         blink::WebString::fromLatin1(
             pos == std::string::npos ? "" : setting.substr(pos + 1)));
   }
 }
 
+// Looks up and returns the WebFrame corresponding to a given opener frame
+// routing ID.  Also stores the opener's RenderView routing ID into
+// |opener_view_routing_id|.
+WebFrame* ResolveOpener(int opener_frame_routing_id,
+                        int* opener_view_routing_id) {
+  *opener_view_routing_id = MSG_ROUTING_NONE;
+  if (opener_frame_routing_id == MSG_ROUTING_NONE)
+    return nullptr;
+
+  // Opener routing ID could refer to either a RenderFrameProxy or a
+  // RenderFrame, so need to check both.
+  RenderFrameProxy* opener_proxy =
+      RenderFrameProxy::FromRoutingID(opener_frame_routing_id);
+  if (opener_proxy) {
+    *opener_view_routing_id = opener_proxy->render_view()->GetRoutingID();
+
+    // TODO(nasko,alexmos): This check won't be needed once swapped-out:// is
+    // gone.
+    if (opener_proxy->IsMainFrameDetachedFromTree())

[nasko, 2015/07/07 16:26:21]

Can we DCHECK the IsSwappedOutForbidden() value? This way we ensure we don't
regress somehow.

[alexmos, 2015/07/08 04:42:18]

Done.  Added a check for IsSwappedOutForbidden() being false if
IsMainFrameDetachedFromTree() is true.

+      return opener_proxy->render_view()->webview()->mainFrame();
+    else
+      return opener_proxy->web_frame();
+  }
+
+  RenderFrameImpl* opener_frame =
+      RenderFrameImpl::FromRoutingID(opener_frame_routing_id);
+  if (opener_frame) {
+    *opener_view_routing_id = opener_frame->render_view()->GetRoutingID();
+    return opener_frame->GetWebFrame();
+  }
+
+  return nullptr;
+}
+
 }  // namespace
 
 RenderViewImpl::RenderViewImpl(const ViewMsg_New_Params& params)
     : RenderWidget(blink::WebPopupTypeNone,
                    params.initial_size.screen_info,
                    params.swapped_out,
                    params.hidden,
                    params.never_visible),
       webkit_preferences_(params.web_preferences),
       send_content_state_immediately_(false),
@@ skipping 31 matching lines @@
       enumeration_completion_id_(0),
       session_storage_namespace_id_(params.session_storage_namespace_id),
       page_scale_factor_is_one_(true) {
 }
 
 void RenderViewImpl::Initialize(const ViewMsg_New_Params& params,
                                 CompositorDependencies* compositor_deps,
                                 bool was_created_by_renderer) {
   routing_id_ = params.view_id;
   surface_id_ = params.surface_id;
-  if (params.opener_route_id != MSG_ROUTING_NONE && was_created_by_renderer)
-    opener_id_ = params.opener_route_id;

[alexmos, 2015/07/06 23:56:42]

Unfortunately, I had to keep this around as a view routing ID, as this is
entangled with a bunch of non-opener stuff (such as ViewHostMsg_Show* IPCs).  I
didn't want to pass in two IDs (for view and frame) around though, so I now look
up the view ID from the opener frame (I ran tryjobs with DCHECKs to verify that
this is equivalent).

+
+  int opener_view_routing_id;
+  WebFrame* opener_frame =
+      ResolveOpener(params.opener_frame_route_id, &opener_view_routing_id);
+  if (opener_view_routing_id != MSG_ROUTING_NONE && was_created_by_renderer)
+    opener_id_ = opener_view_routing_id;
+
   display_mode_= params.initial_size.display_mode;
 
   // Ensure we start with a valid next_page_id_ from the browser.
   DCHECK_GE(next_page_id_, 0);
 
   if (params.main_frame_routing_id != MSG_ROUTING_NONE) {
     main_render_frame_ = RenderFrameImpl::Create(
         this, params.main_frame_routing_id);
     // The main frame WebLocalFrame object is closed by
     // RenderFrameImpl::frameDetached().
@@ skipping 134 matching lines @@
 
   new IdleUserDetector(this);
 
   if (command_line.HasSwitch(switches::kDomAutomationController))
     enabled_bindings_ |= BINDINGS_POLICY_DOM_AUTOMATION;
   if (command_line.HasSwitch(switches::kStatsCollectionController))
     enabled_bindings_ |= BINDINGS_POLICY_STATS_COLLECTION;
 
   GetContentClient()->renderer()->RenderViewCreated(this);
 
-  // If we have an opener_id but we weren't created by a renderer, then
-  // it's the browser asking us to set our opener to another RenderView.
-  if (params.opener_route_id != MSG_ROUTING_NONE && !was_created_by_renderer) {
-    RenderViewImpl* opener_view = FromRoutingID(params.opener_route_id);
-    if (opener_view)
-      webview()->mainFrame()->setOpener(opener_view->webview()->mainFrame());
-  }
+  // If we have an opener_frame but we weren't created by a renderer, then it's
+  // the browser asking us to set our opener to another frame.
+  if (opener_frame && !was_created_by_renderer)
+    webview()->mainFrame()->setOpener(opener_frame);

[alexmos, 2015/07/06 23:56:42]

Note that previously, if this RenderView was opened from a subframe on a
cross-process RenderView, its opener was set to the main frame of the opener
RenderView rather than the subframe.  This is incorrect and inconsistent with
same-process behavior.  Now, if the subframe's proxy doesn't exist in the new
RenderView (as would be the case without --site-per-process), the opener won't
be set and is left as null.  This is consistent with how we handled a similar
case for postMessage (issue 485520).  Trybots seem happy with this as well.

[nasko, 2015/07/07 16:26:21]

Acknowledged.

 
   // If we are initially swapped out, navigate to kSwappedOutURL.
   // This ensures we are in a unique origin that others cannot script.
   if (is_swapped_out_ && webview()->mainFrame()->isWebLocalFrame())
     main_render_frame_->NavigateToSwappedOutURL();
 }
 
 RenderViewImpl::~RenderViewImpl() {
   for (BitmapMap::iterator it = disambiguation_bitmaps_.begin();
        it != disambiguation_bitmaps_.end();
@@ skipping 765 matching lines @@
 
   ViewMsg_Resize_Params initial_size = ViewMsg_Resize_Params();
   initial_size.screen_info = screen_info_;
 
   // The initial hidden state for the RenderViewImpl here has to match what the
   // browser will eventually decide for the given disposition. Since we have to
   // return from this call synchronously, we just have to make our best guess
   // and rely on the browser sending a WasHidden / WasShown message if it
   // disagrees.
   ViewMsg_New_Params view_params;
-  view_params.opener_route_id = routing_id_;
+
+  RenderFrameImpl* creator_frame = RenderFrameImpl::FromWebFrame(creator);
+  view_params.opener_frame_route_id = creator_frame->GetRoutingID();
+  DCHECK_EQ(routing_id_, creator_frame->render_view()->GetRoutingID());
+
   view_params.window_was_created_with_opener = true;
   view_params.renderer_preferences = renderer_preferences_;
   view_params.web_preferences = webkit_preferences_;
   view_params.view_id = routing_id;
   view_params.main_frame_routing_id = main_frame_routing_id;
   view_params.surface_id = surface_id;
   view_params.session_storage_namespace_id =
       cloned_session_storage_namespace_id;
   view_params.swapped_out = false;
   // WebCore will take care of setting the correct name.
@@ skipping 2172 matching lines @@
     std::vector<gfx::Size> sizes;
     ConvertToFaviconSizes(icon_urls[i].sizes(), &sizes);
     if (!url.isEmpty())
       urls.push_back(
           FaviconURL(url, ToFaviconType(icon_urls[i].iconType()), sizes));
   }
   SendUpdateFaviconURL(urls);
 }
 
 }  // namespace content