Move ownership of source SiteInstance to the FrameNavigationEntry.

content/browser/frame_host/navigation_controller_impl.cc

Index: content/browser/frame_host/navigation_controller_impl.cc
diff --git a/content/browser/frame_host/navigation_controller_impl.cc b/content/browser/frame_host/navigation_controller_impl.cc
index 7446d5153b845270ae510ff358d4c2fb960cf9e4..7681d26561ec8ea9c488c927b24b50d371bc1570 100644
--- a/content/browser/frame_host/navigation_controller_impl.cc
+++ b/content/browser/frame_host/navigation_controller_impl.cc
@@ -736,8 +736,10 @@ void NavigationControllerImpl::LoadURLWithParams(const LoadURLParams& params) {
       if (SiteIsolationPolicy::UseSubframeNavigationEntries()) {
         entry = GetLastCommittedEntry()->Clone();
         entry->SetPageID(-1);
-        entry->AddOrUpdateFrameEntry(node, -1, -1, nullptr, params.url,
-                                     params.referrer, PageState(), "GET", -1);
+        entry->AddOrUpdateFrameEntry(
+            node, -1, -1,
+            static_cast<SiteInstanceImpl*>(params.source_site_instance.get()),
+            nullptr, params.url, params.referrer, PageState(), "GET", -1);
       }
     }
   }
@@ -747,11 +749,12 @@ void NavigationControllerImpl::LoadURLWithParams(const LoadURLParams& params) {
     entry = NavigationEntryImpl::FromNavigationEntry(CreateNavigationEntry(
         params.url, params.referrer, params.transition_type,
         params.is_renderer_initiated, params.extra_headers, browser_context_));
+    entry->root_node()->frame_entry->set_source_site_instance(

[Charlie Reis, 2016/05/04 23:03:01]

Hmm.  I think I'd prefer to have a method on NavigationEntryImpl for this after
all, because it's unfortunate to expose TreeNode outside NavigationEntry.  I
kind of want to make that a private detail of NavigationEntry-- it looks like
root_node() is only used in tests these days.

[nasko, 2016/05/05 17:51:33]

Done.

+        static_cast<SiteInstanceImpl*>(params.source_site_instance.get()));

[Charlie Reis, 2016/05/04 23:03:01]

Just curious-- was there a reason not to move this parameter into
CreateNavigationEntry (and the FrameNavigationEntry constructor)?  Maybe that
had too many call sites?

[nasko, 2016/05/05 17:51:33]

It is content/public/ API. I don't think it is worthwhile adding at this point,
since I haven't seen anyone outside of content creating such a NavigationEntry.
There is usage of source_site_instance outside of content/, but it is passed
through the various params structs.

[Charlie Reis, 2016/05/05 22:20:00]

Acknowledged.

   }
+
   // Set the FTN ID (only used in non-site-per-process, for tests).
   entry->set_frame_tree_node_id(frame_tree_node_id);
-  entry->set_source_site_instance(
-      static_cast<SiteInstanceImpl*>(params.source_site_instance.get()));
   if (params.redirect_chain.size() > 0)
     entry->SetRedirectChain(params.redirect_chain);
   // Don't allow an entry replacement if there is no entry to replace.
@@ -888,10 +891,11 @@ bool NavigationControllerImpl::RendererDidNavigate(
   NavigationEntryImpl* active_entry = GetLastCommittedEntry();
   active_entry->SetTimestamp(timestamp);
   active_entry->SetHttpStatusCode(params.http_status_code);
+
+  // Update the frame-specific PageState.

[Charlie Reis, 2016/05/04 23:03:01]

This comment belongs inside the conditional, since the else branch does a
full-page PageState rather than a frame-specifc one.

[nasko, 2016/05/05 17:51:33]

Done.

+  FrameNavigationEntry* frame_entry =
+      active_entry->GetFrameEntry(rfh->frame_tree_node());
   if (SiteIsolationPolicy::UseSubframeNavigationEntries()) {
-    // Update the frame-specific PageState.
-    FrameNavigationEntry* frame_entry =
-        active_entry->GetFrameEntry(rfh->frame_tree_node());
     // We may not find a frame_entry in some cases; ignore the PageState if so.
     // TODO(creis): Remove the "if" once https://crbug.com/522193 is fixed.
     if (frame_entry)
@@ -911,7 +915,7 @@ bool NavigationControllerImpl::RendererDidNavigate(
 
   // Once it is committed, we no longer need to track several pieces of state on
   // the entry.
-  active_entry->ResetForCommit();
+  active_entry->ResetForCommit(frame_entry);
 
   // The active entry's SiteInstance should match our SiteInstance.
   // TODO(creis): This check won't pass for subframes until we create entries
@@ -1339,8 +1343,9 @@ bool NavigationControllerImpl::RendererDidNavigateAutoSubframe(
     NavigationEntryImpl* last_committed = GetLastCommittedEntry();
     last_committed->AddOrUpdateFrameEntry(
         rfh->frame_tree_node(), params.item_sequence_number,
-        params.document_sequence_number, rfh->GetSiteInstance(), params.url,
-        params.referrer, params.page_state, params.method, params.post_id);
+        params.document_sequence_number, nullptr, rfh->GetSiteInstance(),
+        params.url, params.referrer, params.page_state, params.method,
+        params.post_id);
 
     // Cross-process subframe navigations may leave a pending entry around.
     // Clear it if it's actually for the subframe.