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

chrome/browser/page_load_metrics/observers/ads_detection.cc

+// Copyright 2017 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 "chrome/browser/page_load_metrics/observers/ads_detection.h"
+
+#include <memory>
+
+#include "base/strings/string_util.h"
+#include "base/supports_user_data.h"
+#include "content/public/browser/navigation_handle.h"
+#include "content/public/browser/render_frame_host.h"
+#include "url/gurl.h"
+
+namespace {
+
+class NavigationHandleAdsData : public base::SupportsUserData::Data {
+ public:
+  static NavigationHandleAdsData* From(
+      content::NavigationHandle* navigation_handle) {
+    DCHECK(navigation_handle);
+    return static_cast<NavigationHandleAdsData*>(
+        navigation_handle->GetUserData(kUserDataKey));
+  }
+
+  static NavigationHandleAdsData* Create(
+      content::NavigationHandle* navigation_handle) {
+    DCHECK(navigation_handle);
+    DCHECK(!From(navigation_handle));
+
+    NavigationHandleAdsData* ads_data = new NavigationHandleAdsData;
+    navigation_handle->SetUserData(
+        kUserDataKey, std::unique_ptr<NavigationHandleAdsData>(ads_data));
+    return ads_data;  // Safe, because navigation_handle will keep ads_data
+                      // alive until the navigation_handle is destroyed.
+  }
+
+  ~NavigationHandleAdsData() override {}
+
+  AdTypes& ad_types() { return ad_types_; }
+  const AdTypes& ad_types() const { return ad_types_; }
+
+ private:
+  NavigationHandleAdsData() = default;
+
+  AdTypes ad_types_;
+
+  static const char kUserDataKey[];
+};
+
+const char NavigationHandleAdsData::kUserDataKey[] = "AdsData";
+
+bool IsGoogleAd(content::NavigationHandle* navigation_handle) {
+  // Because sub-resource filtering isn't always enabled, and doesn't work
+  // well in monitoring mode (no CSS enforcement), it's difficult to identify
+  // ads. Google ads are prevalent and easy to track, so we'll start by
+  // tracking those. Note that the frame name can be very large, so be careful
+  // to avoid full string searches if possible.
+  // TODO(jkarlin): Track other ad networks that are easy to identify.
+
+  content::RenderFrameHost* current_frame_host =
+      navigation_handle->GetRenderFrameHost();
+  if (current_frame_host) {
+    const std::string& frame_name = current_frame_host->GetFrameName();
+    if (base::StartsWith(frame_name, "google_ads_iframe",
+                         base::CompareCase::SENSITIVE) ||
+        base::StartsWith(frame_name, "google_ads_frame",
+                         base::CompareCase::SENSITIVE)) {
+      return true;
+    }
+  }
+
+  const GURL& frame_url = navigation_handle->GetURL();
+  if (frame_url.host_piece() == "tpc.googlesyndication.com" &&
+      base::StartsWith(frame_url.path_piece(), "/safeframe",
+                       base::CompareCase::SENSITIVE)) {
+    return true;
+  }
+
+  return false;
+}
+
+}  // namespace
+
+AdTypes GetAdDetectionHeuristics(content::NavigationHandle* navigation_handle) {
+  AdTypes ad_types;
+
+  if (IsGoogleAd(navigation_handle))

[jkarlin, 2017/07/18 17:34:44]

Perhaps:

AdTypes GetAdDetectionHeuristics(content::NavigationHandle* navigation_handle) {
  const NavigationHandleAdsData* ads_data =
         NavigationHandleAdsData::From(navigation_handle);

  if (!ads_data->ad_types().test(AD_TYPE_GOOGLE) &&
IsGoogleAd(navigation_handle)) {
    ads_data->SetDetectedAdType(AD_TYPE_GOOGLE);
  }

  return ads_data->ad_types();
}

That way we save the Google state once it's discovered and all of the detected
types are stored in the NavigationHandleAdsData structure.

[Łukasz Anforowicz, 2017/07/18 19:06:11]

Good point.  This probably means that NavigationHandleAdsData::From should
create NavigationHandleAdsData if it doesn't already exist.  This does seem
cleaner / avoiding this allocation doesn't seem desirable (especially given that
subresource filter will in most cases create NavigationHandleAdsData instance
anyway).

FWIW, I am checking IsGoogleAd inside NavigationHandleAdsData::From, in case
when it needs to create a new NavigationHandleAdsData instance from scratch.

The suggested change also allows returning |const AdTypes&| rather than
|AdTypes| (i.e. avoids constructing a new AdTypes and possibly copying [although
maybe RVO / copy elision is expected here]).

Done.

+    ad_types.set(AD_TYPE_GOOGLE);
+
+  const NavigationHandleAdsData* ads_data =
+      NavigationHandleAdsData::From(navigation_handle);
+  if (ads_data)
+    ad_types |= ads_data->ad_types();
+
+  return ad_types;
+}
+
+void SetAdDetectionHeuristics(content::NavigationHandle* navigation_handle,
+                              AdType type) {
+  DCHECK(navigation_handle);
+  NavigationHandleAdsData* ads_data =
+      NavigationHandleAdsData::Create(navigation_handle);
+  ads_data->ad_types() |= type;
+}