OOPIF: Data URLs are now rendered in the renderer that initiated the navigation.

content/browser/frame_host/render_frame_host_manager.cc

Index: content/browser/frame_host/render_frame_host_manager.cc
diff --git a/content/browser/frame_host/render_frame_host_manager.cc b/content/browser/frame_host/render_frame_host_manager.cc
index 06ab825123e4b1fd294dd577e6aee18ab9ff94fe..1a32ba77f74035809b071f83b5484e6d4b15576b 100644
--- a/content/browser/frame_host/render_frame_host_manager.cc
+++ b/content/browser/frame_host/render_frame_host_manager.cc
@@ -161,12 +161,10 @@ RenderFrameHostImpl* RenderFrameHostManager::Navigate(
                "FrameTreeNode id", frame_tree_node_->frame_tree_node_id());
   // Create a pending RenderFrameHost to use for the navigation.
   RenderFrameHostImpl* dest_render_frame_host = UpdateStateForNavigate(
-      entry.GetURL(),
-      entry.site_instance(),
+      entry.GetURL(), entry.source_site_instance(), entry.site_instance(),
       entry.GetTransitionType(),
       entry.restore_type() != NavigationEntryImpl::RESTORE_NONE,
-      entry.IsViewSourceMode(),
-      entry.transferred_global_request_id(),
+      entry.IsViewSourceMode(), entry.transferred_global_request_id(),
       entry.bindings());
   if (!dest_render_frame_host)
     return NULL;  // We weren't able to create a pending render frame host.
@@ -378,15 +376,9 @@ void RenderFrameHostManager::OnCrossSiteResponse(
   // However, since we force the navigation to be in the current tab, it
   // doesn't matter.
   pending_render_frame_host->frame_tree_node()->navigator()->RequestTransferURL(
-      pending_render_frame_host,
-      transfer_url,
-      rest_of_chain,
-      referrer,
-      page_transition,
-      CURRENT_TAB,
-      global_request_id,
-      should_replace_current_entry,
-      true);
+      pending_render_frame_host, transfer_url, nullptr, rest_of_chain, referrer,

[Charlie Reis, 2014/12/06 00:18:50]

Darn.  We don't have the source SiteInstance at this point in time (after server
redirects).  It might still be on the pending entry but that's not guaranteed.

The good news is, that won't break our current use case, since a server redirect
can't go to about:blank, at least in a way that allows you to script the
resulting page.

I suppose nullptr is ok in this case, requiring us to document that it may be
null when we don't know it.  I still think it's worth setting it in the common
case.

[lfg, 2014/12/08 20:45:33]

Done.

+      page_transition, CURRENT_TAB, global_request_id,
+      should_replace_current_entry, true);
 
   // The transferring request was only needed during the RequestTransferURL
   // call, so it is safe to clear at this point.
@@ -631,7 +623,7 @@ RenderFrameHostImpl* RenderFrameHostManager::GetFrameHostForNavigation(
   // Pick the right RenderFrameHost to commit the navigation.
   // TODO(clamy): Replace the default values by the right ones.
   RenderFrameHostImpl* render_frame_host = UpdateStateForNavigate(
-      url, NULL, transition, false, false, GlobalRequestID(),
+      url, nullptr, nullptr, transition, false, false, GlobalRequestID(),
       NavigationEntryImpl::kInvalidBindings);
 
   // If the renderer that needs to navigate is not live (it was just created or
@@ -644,7 +636,7 @@ RenderFrameHostImpl* RenderFrameHostManager::GetFrameHostForNavigation(
                         opener_route_id,
                         MSG_ROUTING_NONE,
                         frame_tree_node_->IsMainFrame())) {
-      return NULL;
+      return nullptr;
     }
   }
   return render_frame_host;
@@ -778,6 +770,7 @@ bool RenderFrameHostManager::ShouldReuseWebUI(
 
 SiteInstance* RenderFrameHostManager::GetSiteInstanceForNavigation(
     const GURL& dest_url,
+    SiteInstance* source_instance,
     SiteInstance* dest_instance,
     ui::PageTransition dest_transition,
     bool dest_is_restore,
@@ -813,12 +806,8 @@ SiteInstance* RenderFrameHostManager::GetSiteInstanceForNavigation(
       dest_is_view_source_mode);
   if (ShouldTransitionCrossSite() || force_swap) {
     new_instance = GetSiteInstanceForURL(
-        dest_url,
-        dest_instance,
-        dest_transition,
-        dest_is_restore,
-        dest_is_view_source_mode,
-        current_instance,
+        dest_url, source_instance, dest_instance, dest_transition,
+        dest_is_restore, dest_is_view_source_mode, current_instance,
         force_swap);
   }
 
@@ -832,6 +821,7 @@ SiteInstance* RenderFrameHostManager::GetSiteInstanceForNavigation(
 
 SiteInstance* RenderFrameHostManager::GetSiteInstanceForURL(
     const GURL& dest_url,
+    SiteInstance* source_instance,
     SiteInstance* dest_instance,
     ui::PageTransition dest_transition,
     bool dest_is_restore,
@@ -978,7 +968,7 @@ SiteInstance* RenderFrameHostManager::GetSiteInstanceForURL(
       GetCurrentURLForSiteInstance(current_instance, current_entry);
   if (SiteInstance::IsSameWebSite(browser_context, current_url, dest_url) &&
       !current_site_instance->HasWrongProcessForURL(dest_url)) {
-    return current_instance;
+    return source_instance ? source_instance : current_instance;

[Charlie Reis, 2014/12/06 00:18:50]

Ooh, this is wrong, and if we don't have any tests that would catch it, we
should add one.

Suppose a window on site A is telling another window to go from B1 to B2.  This
logic would load B2 in A's SiteInstance.

We only want to use source_site_instance if the dest_url is about:blank or a
data URL, right?  Oh, and I guess we should test iframe srcdoc as well.  I don't
know that feature as well, but I think it behaves similarly to data URLs.

[lfg, 2014/12/08 20:45:33]

I switched to a specific case instead of using IsSameWebSite(). I looked iframe
srcdoc, and it seems to navigate to about:blank first, which causes the correct
process to be selected, however, after navigating to about:blank it doesn't
render the correct html. We should create a new bug to fix that.

   }
 
   // Start the new renderer in a new SiteInstance, but in the current
@@ -1437,11 +1427,12 @@ void RenderFrameHostManager::ShutdownRenderFrameProxyHostsInSiteInstance(
 }
 
 RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate(
-    const GURL& url,
-    SiteInstance* instance,
-    ui::PageTransition transition,
-    bool is_restore,
-    bool is_view_source_mode,
+    const GURL& dest_url,
+    SiteInstance* source_instance,
+    SiteInstance* dest_instance,
+    ui::PageTransition dest_transition,
+    bool dest_is_restore,
+    bool dest_is_view_source_mode,
     const GlobalRequestID& transferred_request_id,
     int bindings) {
   // If we are currently navigating cross-process, we want to get back to normal
@@ -1454,7 +1445,8 @@ RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate(
 
   SiteInstance* current_instance = render_frame_host_->GetSiteInstance();
   scoped_refptr<SiteInstance> new_instance = GetSiteInstanceForNavigation(
-      url, instance, transition, is_restore, is_view_source_mode);
+      dest_url, source_instance, dest_instance, dest_transition,
+      dest_is_restore, dest_is_view_source_mode);
 
   const NavigationEntry* current_entry =
       delegate_->GetLastCommittedNavigationEntryForRenderManager();
@@ -1476,7 +1468,7 @@ RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate(
     // It must also happen after the above conditional call to CancelPending(),
     // otherwise CancelPending may clear the pending_web_ui_ and the page will
     // not have its bindings set appropriately.
-    SetPendingWebUI(url, bindings);
+    SetPendingWebUI(dest_url, bindings);
     CreatePendingRenderFrameHost(current_instance, new_instance.get(),
                                  frame_tree_node_->IsMainFrame());
     if (!pending_render_frame_host_.get()) {
@@ -1554,11 +1546,11 @@ RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate(
   // delete the proxy.
   DeleteRenderFrameProxyHost(new_instance.get());
 
-  if (ShouldReuseWebUI(current_entry, url)) {
+  if (ShouldReuseWebUI(current_entry, dest_url)) {
     pending_web_ui_.reset();
     pending_and_current_web_ui_ = web_ui_->AsWeakPtr();
   } else {
-    SetPendingWebUI(url, bindings);
+    SetPendingWebUI(dest_url, bindings);
     // Make sure the new RenderViewHost has the right bindings.
     if (pending_web_ui() &&
         !render_frame_host_->GetProcess()->IsIsolatedGuest()) {
@@ -1574,7 +1566,7 @@ RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate(
 
   // The renderer can exit view source mode when any error or cancellation
   // happen. We must overwrite to recover the mode.
-  if (is_view_source_mode) {
+  if (dest_is_view_source_mode) {
     render_frame_host_->render_view_host()->Send(
         new ViewMsg_EnableViewSourceMode(
             render_frame_host_->render_view_host()->GetRoutingID()));