Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1416)

Unified Diff: chrome/browser/chrome_content_browser_client.cc

Issue 2946113002: Use FrameIsAd to decide whether to isolate a frame in TopDocumentIsolation mode. (Closed)
Patch Set: Use FOR_EACH_TDI_MODE(V) macro. Created 3 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/chrome_content_browser_client.cc
diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc
index 6461d76826efa245ffa8394dd215028ea1f5938e..d7221ad034a948d4f8667087c231bc284d236d51 100644
--- a/chrome/browser/chrome_content_browser_client.cc
+++ b/chrome/browser/chrome_content_browser_client.cc
@@ -19,6 +19,7 @@
#include "base/lazy_instance.h"
#include "base/macros.h"
#include "base/memory/ptr_util.h"
+#include "base/metrics/field_trial_params.h"
#include "base/metrics/histogram_macros.h"
#include "base/path_service.h"
#include "base/strings/string_number_conversions.h"
@@ -57,6 +58,7 @@
#include "chrome/browser/notifications/platform_notification_service_impl.h"
#include "chrome/browser/page_load_metrics/experiments/delay_navigation_throttle.h"
#include "chrome/browser/page_load_metrics/metrics_navigation_throttle.h"
+#include "chrome/browser/page_load_metrics/observers/ads_detection.h"
#include "chrome/browser/password_manager/chrome_password_manager_client.h"
#include "chrome/browser/permissions/permission_context_base.h"
#include "chrome/browser/platform_util.h"
@@ -1338,20 +1340,36 @@ void ChromeContentBrowserClient::OverrideNavigationParams(
#endif
}
-bool ChromeContentBrowserClient::
- ShouldFrameShareParentSiteInstanceDespiteTopDocumentIsolation(
- const GURL& url,
- content::SiteInstance* parent_site_instance) {
- if (IsNTPSiteInstance(parent_site_instance))
- return true;
+bool ChromeContentBrowserClient::ShouldIsolateFrameFromMainContent(
+ content::RenderFrameHost* frame,
+ const GURL& dest_url,
+ content::SiteInstance* main_frame_site_instance) {
+ if (IsNTPSiteInstance(main_frame_site_instance))
+ return false;
#if BUILDFLAG(ENABLE_EXTENSIONS)
- return ChromeContentBrowserClientExtensionsPart::
- ShouldFrameShareParentSiteInstanceDespiteTopDocumentIsolation(
- url, parent_site_instance);
-#else
- return false;
+ if (ChromeContentBrowserClientExtensionsPart::
+ IsMainFrameSiteInstanceExcludedFromTopDocumentIsolation(
+ main_frame_site_instance))
+ return false;
#endif
+
+ features::TopDocumentIsolationMode tdiMode =
+ static_cast<features::TopDocumentIsolationMode>(
+ base::GetFieldTrialParamByFeatureAsInt(
+ ::features::kTopDocumentIsolation,
+ ::features::kTopDocumentIsolationModeParam,
+ static_cast<int>(features::TopDocumentIsolationMode::Default)));
+ switch (tdiMode) {
+ case features::TopDocumentIsolationMode::Default:
Charlie Reis 2017/06/30 23:28:45 I thought "Default" meant disabled? Shouldn't we
Łukasz Anforowicz 2017/07/01 00:10:53 TDI state has 2 parts: part1: is the feature enabl
Charlie Reis 2017/07/06 20:02:29 Thanks, I understand now. Yes, we should try to m
+ case features::TopDocumentIsolationMode::Ads:
+ return IsAdFrame(frame->GetFrameName(), dest_url);
+
+ case features::TopDocumentIsolationMode::Xsite:
+ return true;
+ }
+ NOTREACHED();
+ return false;
}
bool ChromeContentBrowserClient::IsSuitableHost(

Powered by Google App Engine
This is Rietveld 408576698