OOPIF: Specify previous sibling frames when creating RenderFrames.

When initializing a new renderer for an OOP frame, the current
behavior is to first create all the RenderFrameProxies, and then to
create the new RenderFrame, appending it as its parent's last child in
the frame tree.  This disregards the order of sibling frames and thus
may break indexed window access (e.g., window.frames[2]).

This CL passes the previous sibling's routing ID in the
FrameMsg_NewFrame message, so that the new frame can be inserted in
the correct place in the frame tree.  Note that we don't need to do
this for RenderFrameProxies, as those are already created in the
correct order (by CreateProxiesForSiteInstance) when initializing a
new renderer process.

Corresponding Blink CL: https://codereview.chromium.org/1119823003/

BUG=478792

Review URL: https://codereview.chromium.org/1113393004

Cr-Commit-Position: refs/heads/master@{#328384}

16 files changed, 275 insertions, 102 deletions

diff --git a/content/browser/frame_host/frame_tree_node.cc b/content/browser/frame_host/frame_tree_node.cc
index 3db5b6c..b87af21 100644
--- a/content/browser/frame_host/frame_tree_node.cc
+++ b/content/browser/frame_host/frame_tree_node.cc
@@ -162,6 +162,19 @@ bool FrameTreeNode::IsDescendantOf(FrameTreeNode* other) const {
   return false;
 }
 
+FrameTreeNode* FrameTreeNode::PreviousSibling() const {
+  if (!parent_)
+    return nullptr;
+
+  for (size_t i = 0; i < parent_->child_count(); ++i) {
+    if (parent_->child_at(i) == this)
+      return (i == 0) ? nullptr : parent_->child_at(i - 1);
+  }
+
+  NOTREACHED() << "FrameTreeNode not found in its parent's children.";
+  return nullptr;
+}
+
 bool FrameTreeNode::IsLoading() const {
   RenderFrameHostImpl* current_frame_host =
       render_manager_.current_frame_host();
diff --git a/content/browser/frame_host/frame_tree_node.h b/content/browser/frame_host/frame_tree_node.h
index 4825082..4e98559 100644
--- a/content/browser/frame_host/frame_tree_node.h
+++ b/content/browser/frame_host/frame_tree_node.h
@@ -124,6 +124,10 @@ class CONTENT_EXPORT FrameTreeNode {
 
   bool IsDescendantOf(FrameTreeNode* other) const;
 
+  // Return the node immediately preceding this node in its parent's
+  // |children_|, or nullptr if there is no such node.
+  FrameTreeNode* PreviousSibling() const;
+
   // Returns true if this node is in a loading state.
   bool IsLoading() const;
 
diff --git a/content/browser/frame_host/frame_tree_unittest.cc b/content/browser/frame_host/frame_tree_unittest.cc
index e4e8bed..96f6b4d 100644
--- a/content/browser/frame_host/frame_tree_unittest.cc
+++ b/content/browser/frame_host/frame_tree_unittest.cc
@@ -257,6 +257,30 @@ TEST_F(FrameTreeTest, FindFrames) {
   EXPECT_EQ(nullptr, frame_tree->FindByName("no such frame"));
 }
 
+// Check that PreviousSibling() is retrieved correctly.
+TEST_F(FrameTreeTest, PreviousSibling) {
+  // Add a few child frames to the main frame.
+  FrameTree* frame_tree = contents()->GetFrameTree();
+  FrameTreeNode* root = frame_tree->root();
+  main_test_rfh()->OnCreateChildFrame(22, "child0", SandboxFlags::NONE);
+  main_test_rfh()->OnCreateChildFrame(23, "child1", SandboxFlags::NONE);
+  main_test_rfh()->OnCreateChildFrame(24, "child2", SandboxFlags::NONE);
+  FrameTreeNode* child0 = root->child_at(0);
+  FrameTreeNode* child1 = root->child_at(1);
+  FrameTreeNode* child2 = root->child_at(2);
+
+  // Add one grandchild frame.
+  child1->current_frame_host()->OnCreateChildFrame(33, "grandchild",
+                                                   SandboxFlags::NONE);
+  FrameTreeNode* grandchild = child1->child_at(0);
+
+  EXPECT_EQ(nullptr, root->PreviousSibling());
+  EXPECT_EQ(nullptr, child0->PreviousSibling());
+  EXPECT_EQ(child0, child1->PreviousSibling());
+  EXPECT_EQ(child1, child2->PreviousSibling());
+  EXPECT_EQ(nullptr, grandchild->PreviousSibling());
+}
+
 // Do some simple manipulations of the frame tree, making sure that
 // WebContentsObservers see a consistent view of the tree as we go.
 TEST_F(FrameTreeTest, ObserverWalksTreeDuringFrameCreation) {
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index aeb9aa4..7a2ccc9 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -585,6 +585,7 @@ BrowserAccessibility* RenderFrameHostImpl::AccessibilityGetParentFrame() {
 }
 
 bool RenderFrameHostImpl::CreateRenderFrame(int parent_routing_id,
+                                            int previous_sibling_routing_id,
                                             int proxy_routing_id) {
   TRACE_EVENT0("navigation", "RenderFrameHostImpl::CreateRenderFrame");
   DCHECK(!IsRenderFrameLive()) << "Creating frame twice";
@@ -598,22 +599,26 @@ bool RenderFrameHostImpl::CreateRenderFrame(int parent_routing_id,
 
   DCHECK(GetProcess()->HasConnection());
 
-  FrameMsg_NewFrame_WidgetParams widget_params;
+  FrameMsg_NewFrame_Params params;
+  params.routing_id = routing_id_;
+  params.parent_routing_id = parent_routing_id;
+  params.proxy_routing_id = proxy_routing_id;
+  params.previous_sibling_routing_id = previous_sibling_routing_id;
+  params.replication_state = frame_tree_node()->current_replication_state();
+
   if (render_widget_host_) {
-    widget_params.routing_id = render_widget_host_->GetRoutingID();
-    widget_params.surface_id = render_widget_host_->surface_id();
-    widget_params.hidden = render_widget_host_->is_hidden();
+    params.widget_params.routing_id = render_widget_host_->GetRoutingID();
+    params.widget_params.surface_id = render_widget_host_->surface_id();
+    params.widget_params.hidden = render_widget_host_->is_hidden();
   } else {
     // MSG_ROUTING_NONE will prevent a new RenderWidget from being created in
     // the renderer process.
-    widget_params.routing_id = MSG_ROUTING_NONE;
-    widget_params.surface_id = 0;
-    widget_params.hidden = true;
+    params.widget_params.routing_id = MSG_ROUTING_NONE;
+    params.widget_params.surface_id = 0;
+    params.widget_params.hidden = true;
   }
 
-  Send(new FrameMsg_NewFrame(routing_id_, parent_routing_id, proxy_routing_id,
-                             frame_tree_node()->current_replication_state(),
-                             widget_params));
+  Send(new FrameMsg_NewFrame(params));
 
   // The RenderWidgetHost takes ownership of its view. It is tied to the
   // lifetime of the current RenderProcessHost for this RenderFrameHost.
diff --git a/content/browser/frame_host/render_frame_host_impl.h b/content/browser/frame_host/render_frame_host_impl.h
index 14b9937..8af7d7a 100644
--- a/content/browser/frame_host/render_frame_host_impl.h
+++ b/content/browser/frame_host/render_frame_host_impl.h
@@ -184,7 +184,9 @@ class CONTENT_EXPORT RenderFrameHostImpl
 
   // Creates a RenderFrame in the renderer process.  Only called for
   // cross-process subframe navigations in --site-per-process.
-  bool CreateRenderFrame(int parent_routing_id, int proxy_routing_id);
+  bool CreateRenderFrame(int parent_routing_id,
+                         int previous_sibling_routing_id,
+                         int proxy_routing_id);
 
   // Returns whether the RenderFrame in the renderer process has been created
   // and still has a connection.  This is valid for all frames.
diff --git a/content/browser/frame_host/render_frame_host_manager.cc b/content/browser/frame_host/render_frame_host_manager.cc
index e134c92..769bc4a 100644
--- a/content/browser/frame_host/render_frame_host_manager.cc
+++ b/content/browser/frame_host/render_frame_host_manager.cc
@@ -1651,12 +1651,28 @@ bool RenderFrameHostManager::InitRenderFrame(
     return true;
 
   int parent_routing_id = MSG_ROUTING_NONE;
+  int previous_sibling_routing_id = MSG_ROUTING_NONE;
   int proxy_routing_id = MSG_ROUTING_NONE;
+
   if (frame_tree_node_->parent()) {
     parent_routing_id = frame_tree_node_->parent()->render_manager()->
         GetRoutingIdForSiteInstance(render_frame_host->GetSiteInstance());
     CHECK_NE(parent_routing_id, MSG_ROUTING_NONE);
   }
+
+  // At this point, all RenderFrameProxies for sibling frames have already been
+  // created, including any proxies that come after this frame.  To preserve
+  // correct order for indexed window access (e.g., window.frames[1]), pass the
+  // previous sibling frame so that this frame is correctly inserted into the
+  // frame tree on the renderer side.
+  FrameTreeNode* previous_sibling = frame_tree_node_->PreviousSibling();
+  if (previous_sibling) {
+    previous_sibling_routing_id =
+        previous_sibling->render_manager()->GetRoutingIdForSiteInstance(
+            render_frame_host->GetSiteInstance());
+    CHECK_NE(previous_sibling_routing_id, MSG_ROUTING_NONE);
+  }
+
   // Check whether there is an existing proxy for this frame in this
   // SiteInstance. If there is, the new RenderFrame needs to be able to find
   // the proxy it is replacing, so that it can fully initialize itself.
@@ -1672,9 +1688,9 @@ bool RenderFrameHostManager::InitRenderFrame(
     if (!existing_proxy->is_render_frame_proxy_live())
       existing_proxy->InitRenderFrameProxy();
   }
-  return delegate_->CreateRenderFrameForRenderManager(render_frame_host,
-                                                      parent_routing_id,
-                                                      proxy_routing_id);
+  return delegate_->CreateRenderFrameForRenderManager(
+      render_frame_host, parent_routing_id, previous_sibling_routing_id,
+      proxy_routing_id);
 }
 
 int RenderFrameHostManager::GetRoutingIdForSiteInstance(
diff --git a/content/browser/frame_host/render_frame_host_manager.h b/content/browser/frame_host/render_frame_host_manager.h
index 086dea5..1c77b3e 100644
--- a/content/browser/frame_host/render_frame_host_manager.h
+++ b/content/browser/frame_host/render_frame_host_manager.h
@@ -125,6 +125,7 @@ class CONTENT_EXPORT RenderFrameHostManager : public NotificationObserver {
     virtual bool CreateRenderFrameForRenderManager(
         RenderFrameHost* render_frame_host,
         int parent_routing_id,
+        int previous_sibling_routing_id,
         int proxy_routing_id) = 0;
     virtual void BeforeUnloadFiredFromRenderManager(
         bool proceed, const base::TimeTicks& proceed_time,
diff --git a/content/browser/site_per_process_browsertest.cc b/content/browser/site_per_process_browsertest.cc
index ca32952..df9e55e 100644
--- a/content/browser/site_per_process_browsertest.cc
+++ b/content/browser/site_per_process_browsertest.cc
@@ -35,6 +35,32 @@
 
 namespace content {
 
+namespace {
+
+// Helper function to send a postMessage and wait for a reply message.  The
+// |post_message_script| is executed on the |sender_ftn| frame, and the sender
+// frame is expected to post |reply_status| from the DOMAutomationController
+// when it receives a reply.
+void PostMessageAndWaitForReply(FrameTreeNode* sender_ftn,
+                                const std::string& post_message_script,
+                                const std::string& reply_status) {
+  bool success = false;
+  EXPECT_TRUE(ExecuteScriptAndExtractBool(
+      sender_ftn->current_frame_host(),
+      "window.domAutomationController.send(" + post_message_script + ");",
+      &success));
+  EXPECT_TRUE(success);
+
+  content::DOMMessageQueue msg_queue;
+  std::string status;
+  while (msg_queue.WaitForMessage(&status)) {
+    if (status == reply_status)
+      break;
+  }
+}
+
+}  // anonymous namespace
+
 class RedirectNotificationObserver : public NotificationObserver {
  public:
   // Register to listen for notifications of the given type from either a
@@ -839,6 +865,8 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
   // Navigate the second subframe to b.com to recreate the b.com process.
   GURL b_url = embedded_test_server()->GetURL("b.com", "/post_message.html");
   NavigateFrameToURL(root->child_at(1), b_url);
+  // TODO(alexmos): This can be removed once TestFrameNavigationObserver is
+  // fixed to use DidFinishLoad.
   EXPECT_TRUE(
       WaitForRenderFrameReady(root->child_at(1)->current_frame_host()));
   EXPECT_TRUE(observer.last_navigation_succeeded());
@@ -855,23 +883,10 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
       DepictFrameTree(root));
 
   // Check that third subframe's proxy is available in the b.com process by
-  // sending it a postMessage from second subframe.
-  bool success = false;
-  EXPECT_TRUE(ExecuteScriptAndExtractBool(
-      root->child_at(1)->current_frame_host(),
-      "window.domAutomationController.send("
-      "    postToSibling('subframe-msg','frame3'));",
-      &success));
-  EXPECT_TRUE(success);
-
-  // Wait to receive a reply from third subframe.  Second subframe sends
-  // "done-frame2" from the DOMAutomationController when the reply arrives.
-  content::DOMMessageQueue msg_queue;
-  std::string status;
-  while (msg_queue.WaitForMessage(&status)) {
-    if (status == "\"done-frame2\"")
-      break;
-  }
+  // sending it a postMessage from second subframe, and waiting for a reply.
+  PostMessageAndWaitForReply(root->child_at(1),
+                             "postToSibling('subframe-msg','frame3')",
+                             "\"done-frame2\"");
 }
 
 // In A-embed-B-embed-C scenario, verify that killing process B clears proxies
@@ -2193,48 +2208,18 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, SubframePostMessage) {
 
   // Send a message from first, same-site frame to second, cross-site frame.
   // Expect the second frame to reply back to the first frame.
-  //
-  // TODO(alexmos): Also try sending from second to first frame.  Currently,
-  // this fails due to https://crbug.com/473518, which prevents
-  // parent.frames[x] from working when "parent" is a remote frame.
-  bool success = false;
-  EXPECT_TRUE(ExecuteScriptAndExtractBool(
-      root->child_at(0)->current_frame_host(),
-      "window.domAutomationController.send("
-      "    postToSibling('subframe-msg','subframe2'));",
-      &success));
-  EXPECT_TRUE(success);
-
-  // Wait for first frame to receive a reply from the second frame. It will
-  // send "done-subframe1" from the DOMAutomationController when the reply
-  // arrives.
-  content::DOMMessageQueue msg_queue;
-  std::string status;
-  while (msg_queue.WaitForMessage(&status)) {
-    if (status == "\"done-subframe1\"")
-      break;
-  }
+  PostMessageAndWaitForReply(root->child_at(0),
+                             "postToSibling('subframe-msg','subframe2')",
+                             "\"done-subframe1\"");
 
   // Send a postMessage from second, cross-site frame to its parent.  Expect
   // parent to send a reply to the frame.
   base::string16 expected_title(base::ASCIIToUTF16("subframe-msg"));
   TitleWatcher title_watcher(shell()->web_contents(), expected_title);
-  success = false;
-  EXPECT_TRUE(ExecuteScriptAndExtractBool(
-      root->child_at(1)->current_frame_host(),
-      "window.domAutomationController.send(postToParent('subframe-msg'));",
-      &success));
-  EXPECT_TRUE(success);
+  PostMessageAndWaitForReply(root->child_at(1), "postToParent('subframe-msg')",
+                             "\"done-subframe2\"");
   EXPECT_EQ(expected_title, title_watcher.WaitAndGetTitle());
 
-  // Wait for second frame to receive a reply from the parent. The frame will
-  // return "done-subframe2" from the DOMAutomationController when the reply
-  // arrives.
-  while (msg_queue.WaitForMessage(&status)) {
-    if (status == "\"done-subframe2\"")
-      break;
-  }
-
   // Verify the total number of received messages for each subframe.  First
   // frame should have one message (reply from second frame), and second frame
   // should have two messages (message from first frame and reply from parent).
@@ -2252,6 +2237,104 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, SubframePostMessage) {
   EXPECT_EQ(2, subframe2_received_messages);
 }
 
+// Check that parent.frames[num] references correct sibling frames when the
+// parent is remote.  See https://crbug.com/478792.
+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, IndexedFrameAccess) {
+  // Start on a page with three same-site subframes.
+  GURL main_url(
+      embedded_test_server()->GetURL("a.com", "/frame_tree/top.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), main_url));
+
+  // It is safe to obtain the root frame tree node here, as it doesn't change.
+  FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
+                            ->GetFrameTree()
+                            ->root();
+  ASSERT_EQ(3U, root->child_count());
+  FrameTreeNode* child0 = root->child_at(0);
+  FrameTreeNode* child1 = root->child_at(1);
+  FrameTreeNode* child2 = root->child_at(2);
+
+  // Send each of the frames to a different site.  Each new renderer will first
+  // create proxies for the parent and two sibling subframes and then create
+  // and insert the new RenderFrame into the frame tree.
+  GURL b_url(embedded_test_server()->GetURL("b.com", "/post_message.html"));
+  GURL c_url(embedded_test_server()->GetURL("c.com", "/post_message.html"));
+  GURL d_url(embedded_test_server()->GetURL("d.com", "/post_message.html"));
+  NavigateFrameToURL(child0, b_url);
+  // TODO(alexmos): The calls to WaitForRenderFrameReady can be removed once
+  // TestFrameNavigationObserver is fixed to use DidFinishLoad.
+  EXPECT_TRUE(WaitForRenderFrameReady(child0->current_frame_host()));
+  NavigateFrameToURL(child1, c_url);
+  EXPECT_TRUE(WaitForRenderFrameReady(child1->current_frame_host()));
+  NavigateFrameToURL(child2, d_url);
+  EXPECT_TRUE(WaitForRenderFrameReady(child2->current_frame_host()));
+
+  EXPECT_EQ(
+      " Site A ------------ proxies for B C D\n"
+      "   |--Site B ------- proxies for A C D\n"
+      "   |--Site C ------- proxies for A B D\n"
+      "   +--Site D ------- proxies for A B C\n"
+      "Where A = http://a.com/\n"
+      "      B = http://b.com/\n"
+      "      C = http://c.com/\n"
+      "      D = http://d.com/",
+      DepictFrameTree(root));
+
+  // Check that each subframe sees itself at correct index in parent.frames.
+  bool success = false;
+  EXPECT_TRUE(ExecuteScriptAndExtractBool(
+      child0->current_frame_host(),
+      "window.domAutomationController.send(window === parent.frames[0]);",
+      &success));
+  EXPECT_TRUE(success);
+
+  success = false;
+  EXPECT_TRUE(ExecuteScriptAndExtractBool(
+      child1->current_frame_host(),
+      "window.domAutomationController.send(window === parent.frames[1]);",
+      &success));
+  EXPECT_TRUE(success);
+
+  success = false;
+  EXPECT_TRUE(ExecuteScriptAndExtractBool(
+      child2->current_frame_host(),
+      "window.domAutomationController.send(window === parent.frames[2]);",
+      &success));
+  EXPECT_TRUE(success);
+
+  // Send a postMessage from B to parent.frames[1], which should go to C, and
+  // wait for reply.
+  PostMessageAndWaitForReply(child0, "postToSibling('subframe-msg', 1)",
+                             "\"done-1-1-name\"");
+
+  // Send a postMessage from C to parent.frames[2], which should go to D, and
+  // wait for reply.
+  PostMessageAndWaitForReply(child1, "postToSibling('subframe-msg', 2)",
+                             "\"done-1-2-name\"");
+
+  // Verify the total number of received messages for each subframe.
+  int child0_received_messages = 0;
+  EXPECT_TRUE(ExecuteScriptAndExtractInt(
+      child0->current_frame_host(),
+      "window.domAutomationController.send(window.receivedMessages);",
+      &child0_received_messages));
+  EXPECT_EQ(1, child0_received_messages);
+
+  int child1_received_messages = 0;
+  EXPECT_TRUE(ExecuteScriptAndExtractInt(
+      child1->current_frame_host(),
+      "window.domAutomationController.send(window.receivedMessages);",
+      &child1_received_messages));
+  EXPECT_EQ(2, child1_received_messages);
+
+  int child2_received_messages = 0;
+  EXPECT_TRUE(ExecuteScriptAndExtractInt(
+      child2->current_frame_host(),
+      "window.domAutomationController.send(window.receivedMessages);",
+      &child2_received_messages));
+  EXPECT_EQ(1, child2_received_messages);
+}
+
 IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, RFPHDestruction) {
   GURL main_url(embedded_test_server()->GetURL("/site_per_process_main.html"));
   NavigateToURL(shell(), main_url);
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index b310645..2e6a5f7 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -4221,13 +4221,15 @@ bool WebContentsImpl::CreateRenderViewForRenderManager(
 bool WebContentsImpl::CreateRenderFrameForRenderManager(
     RenderFrameHost* render_frame_host,
     int parent_routing_id,
+    int previous_sibling_routing_id,
     int proxy_routing_id) {
   TRACE_EVENT0("browser,navigation",
                "WebContentsImpl::CreateRenderFrameForRenderManager");
 
   RenderFrameHostImpl* rfh =
       static_cast<RenderFrameHostImpl*>(render_frame_host);
-  if (!rfh->CreateRenderFrame(parent_routing_id, proxy_routing_id))
+  if (!rfh->CreateRenderFrame(parent_routing_id, previous_sibling_routing_id,
+                              proxy_routing_id))
     return false;
 
   // TODO(nasko): When RenderWidgetHost is owned by RenderFrameHost, the passed
diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h
index 8ad8a8b..e5e473e 100644
--- a/content/browser/web_contents/web_contents_impl.h
+++ b/content/browser/web_contents/web_contents_impl.h
@@ -577,6 +577,7 @@ class CONTENT_EXPORT WebContentsImpl
       bool for_main_frame_navigation) override;
   bool CreateRenderFrameForRenderManager(RenderFrameHost* render_frame_host,
                                          int parent_routing_id,
+                                         int previous_sibling_routing_id,
                                          int proxy_routing_id) override;
   void BeforeUnloadFiredFromRenderManager(
       bool proceed,
diff --git a/content/common/frame_messages.h b/content/common/frame_messages.h
index 53f0fb4..69b56e3 100644
--- a/content/common/frame_messages.h
+++ b/content/common/frame_messages.h
@@ -298,6 +298,35 @@ IPC_STRUCT_BEGIN(FrameMsg_NewFrame_WidgetParams)
   IPC_STRUCT_MEMBER(bool, hidden)
 IPC_STRUCT_END()
 
+IPC_STRUCT_BEGIN(FrameMsg_NewFrame_Params)
+  // Specifies the routing ID of the new RenderFrame object.
+  IPC_STRUCT_MEMBER(int, routing_id)
+
+  // The new frame should be created as a child of the object
+  // identified by |parent_routing_id| or as top level if that is
+  // MSG_ROUTING_NONE.
+  IPC_STRUCT_MEMBER(int, parent_routing_id)
+
+  // Identifies the previous sibling of the new frame, so that the new frame is
+  // inserted into the correct place in the frame tree.  If this is
+  // MSG_ROUTING_NONE, the frame will be created as the leftmost child of its
+  // parent frame, in front of any other children.
+  IPC_STRUCT_MEMBER(int, previous_sibling_routing_id)
+
+  // If a valid |proxy_routing_id| is provided, the new frame will be
+  // configured to replace the proxy on commit.
+  IPC_STRUCT_MEMBER(int, proxy_routing_id)
+
+  // When the new frame has a parent, |replication_state| holds the new frame's
+  // properties replicated from the process rendering the parent frame, such as
+  // the new frame's sandbox flags.
+  IPC_STRUCT_MEMBER(content::FrameReplicationState, replication_state)
+
+  // Specifies properties for a new RenderWidget that will be attached to the
+  // new RenderFrame (if one is needed).
+  IPC_STRUCT_MEMBER(FrameMsg_NewFrame_WidgetParams, widget_params)
+IPC_STRUCT_END()
+
 IPC_STRUCT_BEGIN(FrameHostMsg_OpenURL_Params)
   IPC_STRUCT_MEMBER(GURL, url)
   IPC_STRUCT_MEMBER(content::Referrer, referrer)
@@ -389,19 +418,8 @@ IPC_MESSAGE_ROUTED0(FrameMsg_DisownOpener)
 // commit, activation and frame swap of the current DOM tree in blink.
 IPC_MESSAGE_ROUTED1(FrameMsg_VisualStateRequest, uint64 /* id */)
 
-// Instructs the renderer to create a new RenderFrame object with |routing_id|.
-// The new frame should be created as a child of the object identified by
-// |parent_routing_id| or as top level if that is MSG_ROUTING_NONE.
-// If a valid |proxy_routing_id| is provided, the new frame will be configured
-// to replace the proxy on commit.  When the new frame has a parent,
-// |replication_state| holds properties replicated from the process rendering
-// the parent frame, such as the new frame's sandbox flags.
-IPC_MESSAGE_CONTROL5(FrameMsg_NewFrame,
-                     int /* routing_id */,
-                     int /* parent_routing_id */,
-                     int /* proxy_routing_id */,
-                     content::FrameReplicationState /* replication_state */,
-                     FrameMsg_NewFrame_WidgetParams /* widget_params */)
+// Instructs the renderer to create a new RenderFrame object.
+IPC_MESSAGE_CONTROL1(FrameMsg_NewFrame, FrameMsg_NewFrame_Params /* params */);
 
 // Instructs the renderer to create a new RenderFrameProxy object with
 // |routing_id|. The new proxy should be created as a child of the object
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 043b33c..fba4b77 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -565,6 +565,7 @@ RenderFrameImpl* RenderFrameImpl::FromRoutingID(int32 routing_id) {
 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,
@@ -584,12 +585,19 @@ void RenderFrameImpl::CreateFrame(
     CHECK(parent_proxy);
     blink::WebRemoteFrame* parent_web_frame = parent_proxy->web_frame();
 
+    blink::WebFrame* previous_sibling_web_frame = nullptr;
+    RenderFrameProxy* previous_sibling_proxy =
+        RenderFrameProxy::FromRoutingID(previous_sibling_routing_id);
+    if (previous_sibling_proxy)
+      previous_sibling_web_frame = previous_sibling_proxy->web_frame();
+
     // 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);
diff --git a/content/renderer/render_frame_impl.h b/content/renderer/render_frame_impl.h
index d64a507..7d4eaa7 100644
--- a/content/renderer/render_frame_impl.h
+++ b/content/renderer/render_frame_impl.h
@@ -131,14 +131,16 @@ class CONTENT_EXPORT RenderFrameImpl
   // Creates a new RenderFrame with |routing_id| as a child of the RenderFrame
   // identified by |parent_routing_id| or as the top-level frame if the latter
   // is MSG_ROUTING_NONE. If |proxy_routing_id| is MSG_ROUTING_NONE, it creates
-  // the Blink WebLocalFrame and inserts it in the proper place in the frame
-  // tree. Otherwise, the frame is semi-orphaned until it commits, at which
-  // point it replaces the proxy identified by |proxy_routing_id|.
-  // Note: This is called only when RenderFrame is being created in response to
-  // IPC message from the browser process. All other frame creation is driven
-  // through Blink and Create.
+  // the Blink WebLocalFrame and inserts it into the frame tree after the frame
+  // identified by |previous_sibling_routing_id|, or as the first child if
+  // |previous_sibling_routing_id| is MSG_ROUTING_NONE. Otherwise, the frame is
+  // semi-orphaned until it commits, at which point it replaces the proxy
+  // identified by |proxy_routing_id|. Note: This is called only when
+  // RenderFrame is being created in response to IPC message from the browser
+  // process. All other frame creation is driven through Blink and Create.
   static void CreateFrame(int routing_id,
                           int parent_routing_id,
+                          int previous_sibling_routing_id,
                           int proxy_routing_id,
                           const FrameReplicationState& replicated_state,
                           CompositorDependencies* compositor_deps,
diff --git a/content/renderer/render_frame_impl_browsertest.cc b/content/renderer/render_frame_impl_browsertest.cc
index 1086b3b..7a5aa73 100644
--- a/content/renderer/render_frame_impl_browsertest.cc
+++ b/content/renderer/render_frame_impl_browsertest.cc
@@ -51,7 +51,8 @@ class RenderFrameImplTest : public RenderViewTest {
         ->OnSwapOut(kFrameProxyRouteId, false, FrameReplicationState());
 
     RenderFrameImpl::CreateFrame(kSubframeRouteId, kFrameProxyRouteId,
-                                 MSG_ROUTING_NONE, FrameReplicationState(),
+                                 MSG_ROUTING_NONE, MSG_ROUTING_NONE,
+                                 FrameReplicationState(),
                                  compositor_deps_.get(), widget_params);
 
     frame_ = RenderFrameImpl::FromRoutingID(kSubframeRouteId);
diff --git a/content/renderer/render_thread_impl.cc b/content/renderer/render_thread_impl.cc
index 896d5cb..207724b 100644
--- a/content/renderer/render_thread_impl.cc
+++ b/content/renderer/render_thread_impl.cc
@@ -1563,15 +1563,12 @@ bool RenderThreadImpl::OnControlMessageReceived(const IPC::Message& msg) {
   return handled;
 }
 
-void RenderThreadImpl::OnCreateNewFrame(
-    int routing_id,
-    int parent_routing_id,
-    int proxy_routing_id,
-    const FrameReplicationState& replicated_state,
-    FrameMsg_NewFrame_WidgetParams params) {
+void RenderThreadImpl::OnCreateNewFrame(FrameMsg_NewFrame_Params params) {
   CompositorDependencies* compositor_deps = this;
-  RenderFrameImpl::CreateFrame(routing_id, parent_routing_id, proxy_routing_id,
-                               replicated_state, compositor_deps, params);
+  RenderFrameImpl::CreateFrame(
+      params.routing_id, params.parent_routing_id,
+      params.previous_sibling_routing_id, params.proxy_routing_id,
+      params.replication_state, compositor_deps, params.widget_params);
 }
 
 void RenderThreadImpl::OnCreateNewFrameProxy(
diff --git a/content/renderer/render_thread_impl.h b/content/renderer/render_thread_impl.h
index c4f760b..82748e9 100644
--- a/content/renderer/render_thread_impl.h
+++ b/content/renderer/render_thread_impl.h
@@ -35,7 +35,7 @@
 
 class GrContext;
 class SkBitmap;
-struct FrameMsg_NewFrame_WidgetParams;
+struct FrameMsg_NewFrame_Params;
 struct ViewMsg_New_Params;
 struct WorkerProcessMsg_CreateWorker_Params;
 
@@ -438,11 +438,7 @@ class CONTENT_EXPORT RenderThreadImpl
 
   void Init();
 
-  void OnCreateNewFrame(int routing_id,
-                        int parent_routing_id,
-                        int proxy_routing_id,
-                        const FrameReplicationState& replicated_state,
-                        FrameMsg_NewFrame_WidgetParams params);
+  void OnCreateNewFrame(FrameMsg_NewFrame_Params params);
   void OnCreateNewFrameProxy(int routing_id,
                              int parent_routing_id,
                              int render_view_routing_id,