OOPIF: Specify previous sibling frames when creating new RenderFrames.

content/renderer/render_frame_impl.cc

 // Copyright 2013 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_frame_impl.h"
 
 #include <map>
 #include <string>
 
 #include "base/auto_reset.h"
@@ skipping 546 matching lines @@
       g_routing_id_frame_map.Get().find(routing_id);
   if (iter != g_routing_id_frame_map.Get().end())
     return iter->second;
   return NULL;
 }
 
 // static
 void RenderFrameImpl::CreateFrame(
     int routing_id,
     int parent_routing_id,
+    int previous_sibling_routing_id,
     int proxy_routing_id,
     const FrameReplicationState& replicated_state,
     CompositorDependencies* compositor_deps,
     const FrameMsg_NewFrame_WidgetParams& widget_params) {
   // TODO(nasko): For now, this message is only sent for subframes, as the
   // top level frame is created when the RenderView is created through the
   // ViewMsg_New IPC.
   CHECK_NE(MSG_ROUTING_NONE, parent_routing_id);
 
   blink::WebLocalFrame* web_frame;
   RenderFrameImpl* render_frame;
   if (proxy_routing_id == MSG_ROUTING_NONE) {
     RenderFrameProxy* parent_proxy =
         RenderFrameProxy::FromRoutingID(parent_routing_id);
     // If the browser is sending a valid parent routing id, it should already
     // be created and registered.
     CHECK(parent_proxy);
     blink::WebRemoteFrame* parent_web_frame = parent_proxy->web_frame();
 
+    blink::WebFrame* previous_sibling_web_frame = nullptr;

[nasko, 2015/05/04 17:16:56]

Wouldn't RFP::FromRoutingID return nullptr if not found?

[alexmos, 2015/05/04 21:18:04]

It will, but that ends up in the previous_sibling_proxy, and we still need to
ensure the web_frame also ends up with nullptr in that case, right?

[nasko, 2015/05/04 22:02:13]

Acknowledged.

+    if (RenderFrameProxy* previous_sibling_proxy =
+            RenderFrameProxy::FromRoutingID(previous_sibling_routing_id)) {

[nasko, 2015/05/04 17:16:56]

nit: Separate the variable declaration and function call into a separate line
from the if statement testing the condition of the variable.

[alexmos, 2015/05/04 21:18:04]

Done.

+      previous_sibling_web_frame = previous_sibling_proxy->web_frame();
+    }
+

[nasko, 2015/05/04 17:16:56]

nit: What about (D)CHECKing the condition of MSG_ROUTING_NONE means no other
child frames exist in the parent frame?

[alexmos, 2015/05/04 21:18:04]

That's not necessarily the case, I think -- there could be other children, and
the NONE just means that the new frame should be inserted before all of them as
the leftmost child.

[nasko, 2015/05/04 22:02:13]

Acknowledged.

     // Create the RenderFrame and WebLocalFrame, linking the two.
     render_frame =
         RenderFrameImpl::Create(parent_proxy->render_view(), routing_id);
     web_frame = parent_web_frame->createLocalChild(
         WebString::fromUTF8(replicated_state.name),
-        ContentToWebSandboxFlags(replicated_state.sandbox_flags), render_frame);
+        ContentToWebSandboxFlags(replicated_state.sandbox_flags), render_frame,
+        previous_sibling_web_frame);
   } else {
     RenderFrameProxy* proxy =
         RenderFrameProxy::FromRoutingID(proxy_routing_id);
     CHECK(proxy);
     render_frame = RenderFrameImpl::Create(proxy->render_view(), routing_id);
     web_frame = blink::WebLocalFrame::create(render_frame);
     render_frame->proxy_routing_id_ = proxy_routing_id;
     web_frame->initializeToReplaceRemoteFrame(
         proxy->web_frame(), WebString::fromUTF8(replicated_state.name),
         ContentToWebSandboxFlags(replicated_state.sandbox_flags));
@@ skipping 4236 matching lines @@
 #elif defined(ENABLE_BROWSER_CDMS)
         cdm_manager_,
 #endif
         this);
   }
 
   return cdm_factory_;
 }
 
 }  // namespace content