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

content/public/browser/content_browser_client.h

 // Copyright (c) 2012 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.
 
 #ifndef CONTENT_PUBLIC_BROWSER_CONTENT_BROWSER_CLIENT_H_
 #define CONTENT_PUBLIC_BROWSER_CONTENT_BROWSER_CLIENT_H_
 
 #include <stddef.h>
 
 #include <map>
 #include <memory>
+#include <set>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/callback.h"
 #include "base/task_scheduler/task_scheduler.h"
 #include "base/values.h"
 #include "build/build_config.h"
 #include "content/public/browser/certificate_request_result_type.h"
 #include "content/public/browser/navigation_throttle.h"
@@ skipping 227 matching lines @@
   // This also applies in cases where the new URL will open in another process.
   virtual bool ShouldAllowOpenURL(SiteInstance* site_instance, const GURL& url);
 
   // Allows the embedder to override parameters when navigating. Called for both
   // opening new URLs and when transferring URLs across processes.
   virtual void OverrideNavigationParams(SiteInstance* site_instance,
                                         ui::PageTransition* transition,
                                         bool* is_renderer_initiated,
                                         content::Referrer* referrer) {}
 
-  // Allows the embedder to override top document isolation for specific frames.
-  // |url| is the URL being loaded in the subframe, and |parent_site_instance|
-  // is the SiteInstance of the parent frame. Called only for subframes and only
-  // when top document isolation mode is enabled.
-  virtual bool ShouldFrameShareParentSiteInstanceDespiteTopDocumentIsolation(
-      const GURL& url,
-      SiteInstance* parent_site_instance);
+  // Called in TopDocumentIsolation mode to let the embedder decide whether the
+  // |frame| should be isolated from the main content (if the frame is navigated
+  // to |url| and the main content is in the |main_frame_site_instance|).

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

Let's clarify that (1) the dest_url is already assumed to be cross-site
(right?), and (2) the default should be to return true unless the embedder needs
to override TopDocumentIsolation in some cases.

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

Correct.  I've added this information to the comment.
I am not sure about this part.  Returning false for non-ads seems pretty
natural.

I think it is okay to leave the decision entirely up to the embedder (without
guiding toward selecting any specific set of frames to isolate).

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

Ok.  My goal was to convey that a vanilla TDI mode (of isolating all cross-site
iframes) would return true here rather than needing to do any kind of
interesting work like checking if it's cross-site.  I think the current wording
is sufficient for that.

+  virtual bool ShouldIsolateFrameFromMainContent(

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

I'm finding it confusing that the name doesn't reference either cross-site
frames or TDI, which makes it sound like it's called much more generally.

If you want to stick with the inversion from the previous behavior, maybe we
could call it ShouldIsolateFrameForTopDocumentIsolation and change dest_url to
cross_site_dest_url?

(I also thought about ShouldIsolateCrossSiteFrameFromMainContent, but couldn't
work TDI into that very well.)

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

Good point.  Done.  (and thanks for suggesting the name)

+      RenderFrameHost* frame,
+      const GURL& dest_url,
+      SiteInstance* main_frame_site_instance);
 
   // Returns whether a new view for a given |site_url| can be launched in a
   // given |process_host|.
   virtual bool IsSuitableHost(RenderProcessHost* process_host,
                               const GURL& site_url);
 
   // Returns whether a new view for a new site instance can be added to a
   // given |process_host|.
   virtual bool MayReuseHost(RenderProcessHost* process_host);
 
@@ skipping 528 matching lines @@
   // Allows the embedder to register one or more URLLoaderThrottles for a
   // URL request. This is used only when --enable-network-service is in effect.
   // This is called on the IO thread.
   virtual std::vector<std::unique_ptr<URLLoaderThrottle>>
   CreateURLLoaderThrottles(const base::Callback<WebContents*()>& wc_getter);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_PUBLIC_BROWSER_CONTENT_BROWSER_CLIENT_H_