Use FrameIsAd to decide whether to isolate a frame in TopDocumentIsolation mode.

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 1006 matching lines @@
     : existing_site_instance(nullptr), relation(relation_to_current) {
   new_site_url = SiteInstance::GetSiteForURL(browser_context, dest_url);
 }
 
 void RenderFrameHostManager::RenderProcessGone(SiteInstanceImpl* instance) {
   GetRenderFrameProxyHost(instance)->set_render_frame_proxy_created(false);
 }
 
 void RenderFrameHostManager::CancelPendingIfNecessary(
     RenderFrameHostImpl* render_frame_host) {
-  if (render_frame_host == pending_render_frame_host_.get())
+  if (render_frame_host == pending_render_frame_host_.get()) {
     CancelPending();
-  else if (render_frame_host == speculative_render_frame_host_.get()) {
+  } else if (render_frame_host == speculative_render_frame_host_.get()) {
     // TODO(nasko, clamy): This should just clean up the speculative RFH
     // without canceling the request.  See https://crbug.com/636119.
     if (frame_tree_node_->navigation_request() &&
         frame_tree_node_->navigation_request()->navigation_handle()) {
       frame_tree_node_->navigation_request()
           ->navigation_handle()
           ->set_net_error_code(net::ERR_ABORTED);
     }
     frame_tree_node_->ResetNavigationRequest(false, true);
   }
@@ skipping 438 matching lines @@
           frame_tree_node_->opener()->current_frame_host();
       if (IsCurrentlySameSite(opener_frame, dest_url))
         return SiteInstanceDescriptor(opener_frame->GetSiteInstance());
     }
   }
 
   if (!frame_tree_node_->IsMainFrame() &&
       SiteIsolationPolicy::IsTopDocumentIsolationEnabled() &&
       !SiteInstanceImpl::DoesSiteRequireDedicatedProcess(browser_context,
                                                          dest_url)) {
-    if (GetContentClient()
-            ->browser()
-            ->ShouldFrameShareParentSiteInstanceDespiteTopDocumentIsolation(
-                dest_url, current_instance)) {
+    RenderFrameHostImpl* main_frame =
+        frame_tree_node_->frame_tree()->root()->current_frame_host();
+    if (GetContentClient()->browser()->ShouldIsolateFrameFromMainContent(

[Charlie Reis, 2017/06/30 23:28:45]

Hmm.  It feels a little odd to have the embedder make the ultimate decision
about using TDI even thought content/ knows about IsTopDocumentIsolationEnabled.
 I suppose that's why it was phrased as an override before.

I guess I see the logic, though, since the ad-specific mode is easier to express
that way than as an override for the rest of cross-site iframes.  I'm ok with
it.

[Łukasz Anforowicz, 2017/07/01 00:10:53]

Do you think that maybe the base::Feature for TopDocumentIsolation should also
move to the //chrome layer?  This is doable, although we would have to change
how //content-layer browser tests opt into TDI (they would have to override
ContentBrowserClient rather than tweak command line flags).

[Charlie Reis, 2017/07/06 20:02:29]

I think I'd rather keep it in content.  Among other things, there's a fair
amount of DefaultSubframeProcess machinery that feels like it belongs within
RenderProcessHostImpl.  Also, if it proves to be a useful mode down the line,
this would make it easier for other embedders to use it.

+            frame_tree_node_->current_frame_host(), dest_url,
+            main_frame->GetSiteInstance())) {
+      // This is a cross-site subframe of a non-isolated origin, so place this
+      // frame in the default subframe site instance.
+      return SiteInstanceDescriptor(
+          browser_context, dest_url,
+          SiteInstanceRelation::RELATED_DEFAULT_SUBFRAME);
+    } else {
       return SiteInstanceDescriptor(render_frame_host_->GetSiteInstance());
     }
-
-    // This is a cross-site subframe of a non-isolated origin, so place this
-    // frame in the default subframe site instance.
-    return SiteInstanceDescriptor(
-        browser_context, dest_url,
-        SiteInstanceRelation::RELATED_DEFAULT_SUBFRAME);
   }
 
   // Keep subframes in the parent's SiteInstance unless a dedicated process is
   // required for either the parent or the subframe's destination URL.  This
   // isn't a strict invariant but rather a heuristic to avoid unnecessary
   // OOPIFs; see https://crbug.com/711006.  Note that this shouldn't apply to
   // TopDocumentIsolation, so do this after TDI checks above.
   if (!frame_tree_node_->IsMainFrame()) {
     RenderFrameHostImpl* parent =
         frame_tree_node_->parent()->current_frame_host();
@@ skipping 1319 matching lines @@
                       ->is_hidden() != delegate_->IsHidden()) {
     if (delegate_->IsHidden()) {
       view->Hide();
     } else {
       view->Show();
     }
   }
 }
 
 }  // namespace content