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

content/public/browser/content_browser_client.h

Index: content/public/browser/content_browser_client.h
diff --git a/content/public/browser/content_browser_client.h b/content/public/browser/content_browser_client.h
index 571f9937a8fea459440b36d9bed1576e880e0d55..9bc7d4a1875e8486d3d4624dfd39687367bbcda3 100644
--- a/content/public/browser/content_browser_client.h
+++ b/content/public/browser/content_browser_client.h
@@ -9,6 +9,7 @@
 
 #include <map>
 #include <memory>
+#include <set>
 #include <string>
 #include <utility>
 #include <vector>
@@ -256,13 +257,13 @@ class CONTENT_EXPORT ContentBrowserClient {
                                         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|.