Don't swap processes for chrome:// subframes.

content/browser/frame_host/render_frame_host_manager.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/render_frame_host_manager.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <string>
@@ skipping 1302 matching lines @@
   //
   // NOTE: This can be removed once we have a way to transition between
   //       RenderViews in response to a link click.
   //
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kProcessPerSite) &&
       ui::PageTransitionCoreTypeIs(transition, ui::PAGE_TRANSITION_GENERATED)) {
     return SiteInstanceDescriptor(current_instance_impl);
   }
 
+  // In the site-per-process model, process changes are skipped for chrome://
+  // subframes for now, since e.g. chrome://settings has multiple "cross-site"

[Charlie Reis, 2017/01/20 23:32:11]

Let's rephrase this a bit to more closely match what we're doing, since the code
isn't limited to --site-per-process, and simply skipping a swap for a chrome://
subframe isn't sufficient (e.g., it would be bad if the subframe started on the
web and then somehow went to chrome://).  Something like:

If the parent frame is a chrome:// page and the subframe is as well, keep the
subframe in the parent's process even if they would be considered different
sites.  This avoids unnecessary OOPIFs on pages like chrome://settings, which
currently has multiple "cross-site" subframes that don't need isolation.

[nasko, 2017/01/20 23:51:58]

Done.

+  // chrome:// frames, and that doesn't yet work cross-process.
+  if (SiteIsolationPolicy::AreCrossProcessFramesPossible() &&
+      !frame_tree_node_->IsMainFrame()) {
+    GURL parent_site_url = frame_tree_node_->parent()

[Charlie Reis, 2017/01/20 23:32:11]

nit: const ref

[nasko, 2017/01/20 23:51:57]

No longer present.

+                               ->current_frame_host()
+                               ->GetSiteInstance()

[Charlie Reis, 2017/01/20 23:32:11]

Let's define parent_site_instance as a local variable to make this and the line
below more legible.

[nasko, 2017/01/20 23:51:57]

Done.

+                               ->GetSiteURL();
+    if (parent_site_url.SchemeIs(kChromeUIScheme) &&
+        dest_url.SchemeIs(kChromeUIScheme)) {
+      return SiteInstanceDescriptor(
+          frame_tree_node_->parent()->current_frame_host()->GetSiteInstance());
+    }
+  }
+
   // If we haven't used our SiteInstance (and thus RVH) yet, then we can use it
   // for this entry.  We won't commit the SiteInstance to this site until the
   // navigation commits (in DidNavigate), unless the navigation entry was
   // restored or it's a Web UI as described below.
   if (!current_instance_impl->HasSite()) {
     // If we've already created a SiteInstance for our destination, we don't
     // want to use this unused SiteInstance; use the existing one.  (We don't
     // do this check if the current_instance has a site, because for now, we
     // want to compare against the current URL and not the SiteInstance's site.
     // In this case, there is no current URL, so comparing against the site is
@@ skipping 1430 matching lines @@
                                              resolved_url)) {
     DCHECK(!dest_instance ||
            dest_instance == render_frame_host_->GetSiteInstance());
     return false;
   }
 
   return true;
 }
 
 }  // namespace content