Pass origin information for remote frame creation.

content/browser/frame_host/navigator_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/browser/frame_host/navigator_impl.h"
 
 #include "base/command_line.h"
 #include "base/metrics/histogram.h"
 #include "base/time/time.h"
 #include "content/browser/frame_host/frame_tree.h"
@@ skipping 502 matching lines @@
       // Run tasks that must execute just before the commit.
       bool is_navigation_within_page = controller_->IsURLInPageNavigation(
           params.url, params.was_within_same_page, render_frame_host);
       delegate_->DidNavigateMainFramePreCommit(is_navigation_within_page);
     }
 
     if (!use_site_per_process)
       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) {
     FrameTreeNode* frame = render_frame_host->frame_tree_node();
     frame->render_manager()->DidNavigateFrame(render_frame_host);
   }
 
   // Update the site of the SiteInstance if it doesn't have one yet, unless
   // assigning a site is not necessary for this URL.  In that case, the
   // SiteInstance can still be considered unused until a navigation to a real
@@ skipping 383 matching lines @@
         "Navigation.TimeToCommit_ExistingRenderer_BeforeUnloadDiscounted",
         time_to_commit);
     UMA_HISTOGRAM_TIMES(
         "Navigation.TimeToURLJobStart_ExistingRenderer_BeforeUnloadDiscounted",
         time_to_network);
   }
   navigation_data_.reset();
 }
 
 }  // namespace content