Pass origin information for remote frame creation.

content/browser/frame_host/navigator_impl.cc

Index: content/browser/frame_host/navigator_impl.cc
diff --git a/content/browser/frame_host/navigator_impl.cc b/content/browser/frame_host/navigator_impl.cc
index e3b56215d4d26daf3218f1b52f0133bda53ffdc0..a35f5c4cd1e9170f6390aa00ad0d0583c18fe444 100644
--- a/content/browser/frame_host/navigator_impl.cc
+++ b/content/browser/frame_host/navigator_impl.cc
@@ -520,6 +520,13 @@ void NavigatorImpl::DidNavigate(
       frame_tree->root()->render_manager()->DidNavigateFrame(render_frame_host);
   }
 
+  // OOPIF: Save the origin of the new page here, so that it is available to

[Charlie Reis, 2014/11/13 18:00:57]

nit: Remove "OOPIF."  This looks like it happens in default mode as well, and we
may even find uses for it there (e.g., returning the proper origin in console
error messages rather than swappedout://).

[alexmos, 2014/11/18 18:25:32]

Done.

+  // send as part of FrameMsg_SwapOut, which will result in a new remote frame

[Charlie Reis, 2014/11/13 18:00:57]

Can we split this in two sentences, or rephrase it a bit?  This sentence is a
bit hard to follow.

[alexmos, 2014/11/18 18:25:32]

Rephrased.  Please let me know if this makes more sense now.

+  // that needs this to initialize its security context.  This origin will also
+  // be sent to remote frames created via ViewMsg_New and
+  // FrameMsg_NewFrameProxy.
+  render_frame_host->frame_tree_node()->SetOriginFromURL(params.url);

[Charlie Reis, 2014/11/13 18:00:57]

Would it be easier to just do this action inside the set_current_url call below?
 (Line 569.)

[alexmos, 2014/11/18 18:25:32]

That would be too late because we need the origin on the next line inside
DidNavigateFrame (it calls CommitPending, which sends the SwapOut message, which
needs the origin).

There is actually another issue here that I wanted to discuss.  For main frames
only, DidNavigateFrame eventually calls CommitPending ->
RenderViewHostImpl::AttachToFrameTree -> FrameTree::ResetForMainFrameSwap ->
FrameTreeNode::ResetForNewProcess.  ResetForNewProcess is supposed to clear the
state of FrameTreeNodes, and I wanted to also clear the replicated_state_ there
to avoid stale origins.  However, doing that clears out the origin before
CommitPending gets a chance to grab it for the SwapOut message.  I'm not sure
how to best handle this; after talking to Nasko, some possibilities are:
- plumb navigation params (which contain the origin) through DidNavigateFrame
and CommitPending (unlikely, since CommitPending is also called from
RFHM::UpdateStateForNavigate where we don't have the params)
- save the origin in a temporary field (replication_state_for_swap_out_) on the
FTN and don't clear it in ResetForNewProcess.
- don't clear the origin at all in ResetForNewProcess (current approach; should
be currently harmless but doesn't feel ideal).
- CommitPending can save the FTN origin before calling AttachToFrameTree and
pass it along to SwapOutOldFrame.

Thoughts?

[Charlie Reis, 2014/11/19 00:46:17]

Good question.  As we chatted about in person, I think it makes sense to keep
the frame replication state after a commit, and even after a crash.

(In fact, most of ResetForNewProcess doesn't even make sense during
CommitPending.  The current URL doesn't need to be cleared because it's about to
be set to a new value.  We *definitely* shouldn't call
set_render_frame_created(false) during CommitPending, but luckily that's a no-op
on the main frame.  The only thing we need is to clear the children.  The other
things only make sense during RenderProcessGone.)

+
   // When using --site-per-process, we notify the RFHM for all navigations,
   // not just main frame navigations.
   if (use_site_per_process) {