Collect RIND reports for google ads that cause blocked popups.
The trigger is notified when a popup is blocked. If the frame causing the blocked popup is a google ad frame we send a RIND report.
For more details see: go/extending-chrind-q2-2019-1
Bug: 965587
Change-Id: I8f079bd3b254a3664abd4855d9c9cb393ce2ec4c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1628173
Commit-Queue: Archana Simha <archanasimha@google.com>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Carlos IL <carlosil@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Reviewed-by: Luke Z <lpz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#670272}
diff --git a/chrome/browser/interstitials/enterprise_util.cc b/chrome/browser/interstitials/enterprise_util.cc
index edc03d35..f441208a 100644
--- a/chrome/browser/interstitials/enterprise_util.cc
+++ b/chrome/browser/interstitials/enterprise_util.cc
@@ -83,6 +83,7 @@
DEPRECATED_SB_THREAT_TYPE_URL_PASSWORD_PROTECTION_PHISHING:
case safe_browsing::SB_THREAT_TYPE_SIGN_IN_PASSWORD_REUSE:
case safe_browsing::SB_THREAT_TYPE_AD_SAMPLE:
+ case safe_browsing::SB_THREAT_TYPE_BLOCKED_AD_POPUP:
case safe_browsing::SB_THREAT_TYPE_SUSPICIOUS_SITE:
case safe_browsing::SB_THREAT_TYPE_ENTERPRISE_PASSWORD_REUSE:
case safe_browsing::SB_THREAT_TYPE_APK_DOWNLOAD:
diff --git a/chrome/browser/safe_browsing/BUILD.gn b/chrome/browser/safe_browsing/BUILD.gn
index 22604a5..b03d7a1 100644
--- a/chrome/browser/safe_browsing/BUILD.gn
+++ b/chrome/browser/safe_browsing/BUILD.gn
@@ -102,6 +102,7 @@
"//components/safe_browsing/db:whitelist_checker_client",
"//components/safe_browsing/password_protection",
"//components/safe_browsing/triggers",
+ "//components/safe_browsing/triggers:ad_popup_trigger",
"//components/safe_browsing/triggers:ad_sampler_trigger",
"//components/safe_browsing/triggers:suspicious_site_trigger",
"//components/safe_browsing/triggers:trigger_throttler",
diff --git a/chrome/browser/safe_browsing/trigger_creator.cc b/chrome/browser/safe_browsing/trigger_creator.cc
index 94765c2..3208dc2 100644
--- a/chrome/browser/safe_browsing/trigger_creator.cc
+++ b/chrome/browser/safe_browsing/trigger_creator.cc
@@ -9,6 +9,8 @@
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/safe_browsing/safe_browsing_service.h"
#include "components/prefs/pref_service.h"
+#include "components/safe_browsing/features.h"
+#include "components/safe_browsing/triggers/ad_popup_trigger.h"
#include "components/safe_browsing/triggers/ad_sampler_trigger.h"
#include "components/safe_browsing/triggers/suspicious_site_trigger.h"
#include "components/safe_browsing/triggers/trigger_manager.h"
@@ -57,6 +59,13 @@
HistoryServiceFactory::GetForProfile(
profile, ServiceAccessType::EXPLICIT_ACCESS));
}
+ if (base::FeatureList::IsEnabled(kAdPopupTriggerFeature) &&
+ trigger_manager->CanStartDataCollection(options, TriggerType::AD_POPUP)) {
+ safe_browsing::AdPopupTrigger::CreateForWebContents(
+ web_contents, trigger_manager, profile->GetPrefs(), url_loader_factory,
+ HistoryServiceFactory::GetForProfile(
+ profile, ServiceAccessType::EXPLICIT_ACCESS));
+ }
TriggerManagerReason reason;
if (trigger_manager->CanStartDataCollectionWithReason(
options, TriggerType::SUSPICIOUS_SITE, &reason) ||
diff --git a/chrome/browser/ssl/security_state_tab_helper.cc b/chrome/browser/ssl/security_state_tab_helper.cc
index 83bfd27..fc8ce66 100644
--- a/chrome/browser/ssl/security_state_tab_helper.cc
+++ b/chrome/browser/ssl/security_state_tab_helper.cc
@@ -209,6 +209,7 @@
case safe_browsing::SB_THREAT_TYPE_SUBRESOURCE_FILTER:
case safe_browsing::SB_THREAT_TYPE_CSD_WHITELIST:
case safe_browsing::SB_THREAT_TYPE_AD_SAMPLE:
+ case safe_browsing::SB_THREAT_TYPE_BLOCKED_AD_POPUP:
case safe_browsing::SB_THREAT_TYPE_SUSPICIOUS_SITE:
case safe_browsing::SB_THREAT_TYPE_APK_DOWNLOAD:
// These threat types are not currently associated with
diff --git a/chrome/browser/ui/BUILD.gn b/chrome/browser/ui/BUILD.gn
index 70d4d12..61a0558 100644
--- a/chrome/browser/ui/BUILD.gn
+++ b/chrome/browser/ui/BUILD.gn
@@ -476,6 +476,7 @@
"//components/safe_browsing/db:database_manager",
"//components/safe_browsing/db:util",
"//components/safe_browsing/password_protection",
+ "//components/safe_browsing/triggers:ad_popup_trigger",
"//components/safe_browsing/web_ui",
"//components/search",
"//components/search_engines",
diff --git a/chrome/browser/ui/blocked_content/popup_blocker.cc b/chrome/browser/ui/blocked_content/popup_blocker.cc
index 826490b..950bc0db 100644
--- a/chrome/browser/ui/blocked_content/popup_blocker.cc
+++ b/chrome/browser/ui/blocked_content/popup_blocker.cc
@@ -14,6 +14,7 @@
#include "chrome/common/chrome_switches.h"
#include "components/content_settings/core/browser/host_content_settings_map.h"
#include "components/content_settings/core/common/content_settings.h"
+#include "components/safe_browsing/triggers/ad_popup_trigger.h"
#include "content/public/browser/page_navigator.h"
#include "content/public/browser/web_contents.h"
@@ -91,7 +92,33 @@
auto* popup_blocker = PopupBlockerTabHelper::FromWebContents(web_contents);
if (popup_blocker && block_type != PopupBlockType::kNotBlocked) {
popup_blocker->AddBlockedPopup(params, window_features, block_type);
+ if (safe_browsing::AdPopupTrigger::FromWebContents(web_contents)) {
+ content::RenderFrameHost* source_frame =
+ GetSourceFrameForPopup(params, open_url_params, web_contents);
+ safe_browsing::AdPopupTrigger::FromWebContents(web_contents)
+ ->PopupWasBlocked(source_frame);
+ }
return true;
}
return false;
}
+
+content::RenderFrameHost* GetSourceFrameForPopup(
+ NavigateParams* params,
+ const content::OpenURLParams* open_url_params,
+ content::WebContents* web_contents) {
+ if (params->opener)
+ return params->opener;
+ // Make sure the source render frame host is alive before we attempt to
+ // retrieve it from |open_url_params|.
+ if (open_url_params) {
+ content::RenderFrameHost* source = content::RenderFrameHost::FromID(
+ open_url_params->source_render_frame_id,
+ open_url_params->source_render_process_id);
+ if (source)
+ return source;
+ }
+ // The focused frame is not always the frame initiating the popup navigation
+ // and is used as a fallback in case opener information is not available.
+ return web_contents->GetFocusedFrame();
+}
diff --git a/chrome/browser/ui/blocked_content/popup_blocker.h b/chrome/browser/ui/blocked_content/popup_blocker.h
index 1cb99b9..c926be6 100644
--- a/chrome/browser/ui/blocked_content/popup_blocker.h
+++ b/chrome/browser/ui/blocked_content/popup_blocker.h
@@ -6,6 +6,7 @@
#define CHROME_BROWSER_UI_BLOCKED_CONTENT_POPUP_BLOCKER_H_
#include "base/optional.h"
+#include "content/public/browser/render_frame_host.h"
#include "third_party/blink/public/mojom/window_features/window_features.mojom.h"
#include "ui/base/window_open_disposition.h"
#include "url/gurl.h"
@@ -48,4 +49,11 @@
const content::OpenURLParams* open_url_params,
const blink::mojom::WindowFeatures& window_features);
+// Tries to get the opener from either the |params| or |open_url_params|,
+// otherwise uses the focused frame from |web_contents| as a proxy.
+content::RenderFrameHost* GetSourceFrameForPopup(
+ NavigateParams* params,
+ const content::OpenURLParams* open_url_params,
+ content::WebContents* web_contents);
+
#endif // CHROME_BROWSER_UI_BLOCKED_CONTENT_POPUP_BLOCKER_H_
diff --git a/components/safe_browsing/browser/threat_details.cc b/components/safe_browsing/browser/threat_details.cc
index dbc8385..d238196 100644
--- a/components/safe_browsing/browser/threat_details.cc
+++ b/components/safe_browsing/browser/threat_details.cc
@@ -93,6 +93,8 @@
return ClientSafeBrowsingReportRequest::URL_CLIENT_SIDE_PHISHING;
case SB_THREAT_TYPE_URL_CLIENT_SIDE_MALWARE:
return ClientSafeBrowsingReportRequest::URL_CLIENT_SIDE_MALWARE;
+ case SB_THREAT_TYPE_BLOCKED_AD_POPUP:
+ return ClientSafeBrowsingReportRequest::BLOCKED_AD_POPUP;
case SB_THREAT_TYPE_AD_SAMPLE:
return ClientSafeBrowsingReportRequest::AD_SAMPLE;
case SB_THREAT_TYPE_SIGN_IN_PASSWORD_REUSE:
diff --git a/components/safe_browsing/db/v4_protocol_manager_util.h b/components/safe_browsing/db/v4_protocol_manager_util.h
index 10dffda..6ac6526 100644
--- a/components/safe_browsing/db/v4_protocol_manager_util.h
+++ b/components/safe_browsing/db/v4_protocol_manager_util.h
@@ -149,6 +149,9 @@
// A sample of an ad was collected
SB_THREAT_TYPE_AD_SAMPLE,
+ // A report of Google ad that caused a blocked popup was collected.
+ SB_THREAT_TYPE_BLOCKED_AD_POPUP,
+
// The page loaded a resource from the Suspicious Site list.
SB_THREAT_TYPE_SUSPICIOUS_SITE,
diff --git a/components/safe_browsing/features.cc b/components/safe_browsing/features.cc
index 98e0073..66e157a 100644
--- a/components/safe_browsing/features.cc
+++ b/components/safe_browsing/features.cc
@@ -17,6 +17,8 @@
// Please define any new SafeBrowsing related features in this file, and add
// them to the ExperimentalFeaturesList below to start displaying their status
// on the chrome://safe-browsing page.
+const base::Feature kAdPopupTriggerFeature{"SafeBrowsingAdPopupTrigger",
+ base::FEATURE_DISABLED_BY_DEFAULT};
// Controls various parameters related to occasionally collecting ad samples,
// for example to control how often collection should occur.
@@ -77,6 +79,7 @@
// True if the feature's state should be listed on chrome://safe-browsing.
bool show_state;
} kExperimentalFeatures[]{
+ {&kAdPopupTriggerFeature, true},
{&kAdSamplerTriggerFeature, false},
{&kCaptureInlineJavascriptForGoogleAds, true},
{&kCaptureSafetyNetId, true},
diff --git a/components/safe_browsing/features.h b/components/safe_browsing/features.h
index e9c8ba67..ed61fed 100644
--- a/components/safe_browsing/features.h
+++ b/components/safe_browsing/features.h
@@ -19,6 +19,10 @@
namespace safe_browsing {
// Features list
+// Controls whether we send RIND reports when a popup originating from a Google
+// ad is blocked.
+extern const base::Feature kAdPopupTriggerFeature;
+
extern const base::Feature kAdSamplerTriggerFeature;
// Controls whether we sample inline JavaScript for ads in RIND
diff --git a/components/safe_browsing/proto/csd.proto b/components/safe_browsing/proto/csd.proto
index 1dfe8e2..9e164c3 100644
--- a/components/safe_browsing/proto/csd.proto
+++ b/components/safe_browsing/proto/csd.proto
@@ -1155,6 +1155,7 @@
URL_SUSPICIOUS = 15;
BILLING = 16;
APK_DOWNLOAD = 17;
+ BLOCKED_AD_POPUP = 20;
}
message HTTPHeader {
diff --git a/components/safe_browsing/triggers/BUILD.gn b/components/safe_browsing/triggers/BUILD.gn
index 94e713b5..ea0d45ecb 100644
--- a/components/safe_browsing/triggers/BUILD.gn
+++ b/components/safe_browsing/triggers/BUILD.gn
@@ -65,6 +65,21 @@
]
}
+source_set("ad_popup_trigger") {
+ sources = [
+ "ad_popup_trigger.cc",
+ "ad_popup_trigger.h",
+ ]
+ deps = [
+ ":trigger_throttler",
+ ":trigger_util",
+ ":triggers",
+ "//base:base",
+ "//components/safe_browsing:features",
+ "//content/public/browser",
+ ]
+}
+
source_set("suspicious_site_trigger") {
sources = [
"suspicious_site_trigger.cc",
@@ -85,6 +100,7 @@
source_set("unit_tests") {
testonly = true
sources = [
+ "ad_popup_trigger_unittest.cc",
"ad_sampler_trigger_unittest.cc",
"mock_trigger_manager.cc",
"mock_trigger_manager.h",
@@ -93,6 +109,7 @@
"trigger_throttler_unittest.cc",
]
deps = [
+ ":ad_popup_trigger",
":ad_sampler_trigger",
":suspicious_site_trigger",
":trigger_throttler",
@@ -102,6 +119,7 @@
"//components/prefs:test_support",
"//components/safe_browsing:features",
"//components/safe_browsing/browser:browser",
+ "//components/subresource_filter/content/browser:test_support",
"//content/test:test_support",
"//testing/gmock",
"//testing/gtest",
diff --git a/components/safe_browsing/triggers/ad_popup_trigger.cc b/components/safe_browsing/triggers/ad_popup_trigger.cc
new file mode 100644
index 0000000..df1f0b8
--- /dev/null
+++ b/components/safe_browsing/triggers/ad_popup_trigger.cc
@@ -0,0 +1,148 @@
+// Copyright 2019 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 "components/safe_browsing/triggers/ad_popup_trigger.h"
+
+#include <string>
+
+#include "base/bind.h"
+#include "base/feature_list.h"
+#include "base/metrics/field_trial_params.h"
+#include "base/metrics/histogram_macros.h"
+#include "base/rand_util.h"
+#include "base/single_thread_task_runner.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/task/post_task.h"
+#include "components/safe_browsing/features.h"
+#include "components/safe_browsing/triggers/trigger_manager.h"
+#include "components/safe_browsing/triggers/trigger_throttler.h"
+#include "components/safe_browsing/triggers/trigger_util.h"
+#include "components/security_interstitials/content/unsafe_resource.h"
+#include "content/public/browser/browser_task_traits.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/navigation_handle.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/browser/web_contents.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
+#include "services/network/public/cpp/simple_url_loader.h"
+
+namespace safe_browsing {
+
+// Number of milliseconds to allow data collection to run before sending a
+// report (since this trigger runs in the background).
+const int64_t kAdPopupCollectionPeriodMilliseconds = 5000;
+
+// Range of number of milliseconds to wait after a page finished loading before
+// starting a report. Allows ads which load in the background to finish loading.
+const int64_t kMaxAdPopupCollectionStartDelayMilliseconds = 5000;
+const int64_t kMinAdPopupCollectionStartDelayMilliseconds = 500;
+
+// Metric for tracking what the Ad Popup trigger does on each navigation.
+const char kAdPopupTriggerActionMetricName[] =
+ "SafeBrowsing.Triggers.AdPopup.Action";
+
+AdPopupTrigger::AdPopupTrigger(
+ content::WebContents* web_contents,
+ TriggerManager* trigger_manager,
+ PrefService* prefs,
+ scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+ history::HistoryService* history_service)
+ : web_contents_(web_contents),
+ start_report_delay_ms_(
+ base::RandInt(kMinAdPopupCollectionStartDelayMilliseconds,
+ kMaxAdPopupCollectionStartDelayMilliseconds)),
+ finish_report_delay_ms_(kAdPopupCollectionPeriodMilliseconds),
+ trigger_manager_(trigger_manager),
+ prefs_(prefs),
+ url_loader_factory_(url_loader_factory),
+ history_service_(history_service),
+ task_runner_(base::CreateSingleThreadTaskRunnerWithTraits(
+ {content::BrowserThread::UI})),
+ weak_ptr_factory_(this) {}
+
+AdPopupTrigger::~AdPopupTrigger() {}
+
+// static
+void AdPopupTrigger::CreateForWebContents(
+ content::WebContents* web_contents,
+ TriggerManager* trigger_manager,
+ PrefService* prefs,
+ scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+ history::HistoryService* history_service) {
+ DCHECK(web_contents);
+ if (!FromWebContents(web_contents)) {
+ web_contents->SetUserData(UserDataKey(),
+ base::WrapUnique(new AdPopupTrigger(
+ web_contents, trigger_manager, prefs,
+ url_loader_factory, history_service)));
+ }
+}
+
+void AdPopupTrigger::CreateAdPopupReport() {
+ SBErrorOptions error_options =
+ TriggerManager::GetSBErrorDisplayOptions(*prefs_, web_contents_);
+ security_interstitials::UnsafeResource resource;
+ resource.threat_type = SB_THREAT_TYPE_BLOCKED_AD_POPUP;
+ resource.url = web_contents_->GetURL();
+ resource.web_contents_getter = resource.GetWebContentsGetter(
+ web_contents_->GetMainFrame()->GetProcess()->GetID(),
+ web_contents_->GetMainFrame()->GetRoutingID());
+ TriggerManagerReason reason;
+ if (!trigger_manager_->StartCollectingThreatDetailsWithReason(
+ TriggerType::AD_POPUP, web_contents_, resource, url_loader_factory_,
+ history_service_, error_options, &reason)) {
+ if (reason == TriggerManagerReason::DAILY_QUOTA_EXCEEDED) {
+ UMA_HISTOGRAM_ENUMERATION(
+ kAdPopupTriggerActionMetricName,
+ AdPopupTriggerAction::POPUP_DAILY_QUOTA_EXCEEDED);
+ } else {
+ UMA_HISTOGRAM_ENUMERATION(
+ kAdPopupTriggerActionMetricName,
+ AdPopupTriggerAction::POPUP_COULD_NOT_START_REPORT);
+ }
+ return;
+ }
+ // Call into TriggerManager to finish the reports after a short delay. Any
+ // ads that are detected during this delay will be rejected by TriggerManager
+ // because a report is already being collected, so we won't send multiple
+ // reports for the same page.
+ task_runner_->PostDelayedTask(
+ FROM_HERE,
+ base::BindOnce(
+ IgnoreResult(&TriggerManager::FinishCollectingThreatDetails),
+ base::Unretained(trigger_manager_), TriggerType::AD_POPUP,
+ base::Unretained(web_contents_), base::TimeDelta(),
+ /*did_proceed=*/false, /*num_visits=*/0, error_options),
+ base::TimeDelta::FromMilliseconds(finish_report_delay_ms_));
+
+ UMA_HISTOGRAM_ENUMERATION(kAdPopupTriggerActionMetricName,
+ AdPopupTriggerAction::POPUP_REPORTED);
+}
+
+void AdPopupTrigger::PopupWasBlocked(content::RenderFrameHost* render_frame) {
+ UMA_HISTOGRAM_ENUMERATION(kAdPopupTriggerActionMetricName,
+ AdPopupTriggerAction::POPUP_CHECK);
+ if (!DetectGoogleAd(render_frame, web_contents_->GetURL())) {
+ UMA_HISTOGRAM_ENUMERATION(kAdPopupTriggerActionMetricName,
+ AdPopupTriggerAction::POPUP_NO_GOOGLE_AD);
+ return;
+ }
+ // Create a report after a short delay. The delay gives more time for ads to
+ // finish loading in the background. This is best-effort.
+ task_runner_->PostDelayedTask(
+ FROM_HERE,
+ base::BindOnce(&AdPopupTrigger::CreateAdPopupReport,
+ weak_ptr_factory_.GetWeakPtr()),
+ base::TimeDelta::FromMilliseconds(start_report_delay_ms_));
+}
+
+void AdPopupTrigger::SetTaskRunnerForTest(
+ scoped_refptr<base::SingleThreadTaskRunner> task_runner) {
+ task_runner_ = task_runner;
+}
+
+WEB_CONTENTS_USER_DATA_KEY_IMPL(AdPopupTrigger)
+
+} // namespace safe_browsing
diff --git a/components/safe_browsing/triggers/ad_popup_trigger.h b/components/safe_browsing/triggers/ad_popup_trigger.h
new file mode 100644
index 0000000..8523323
--- /dev/null
+++ b/components/safe_browsing/triggers/ad_popup_trigger.h
@@ -0,0 +1,111 @@
+// Copyright 2019 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 COMPONENTS_SAFE_BROWSING_TRIGGERS_AD_POPUP_TRIGGER_H_
+#define COMPONENTS_SAFE_BROWSING_TRIGGERS_AD_POPUP_TRIGGER_H_
+
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "content/public/browser/web_contents_user_data.h"
+
+class PrefService;
+
+namespace history {
+class HistoryService;
+}
+
+namespace network {
+class SharedURLLoaderFactory;
+} // namespace network
+
+namespace safe_browsing {
+class TriggerManager;
+
+// Metric for tracking what the Ad Popup trigger does on each navigation.
+extern const char kAdPopupTriggerActionMetricName[];
+
+enum class AdPopupTriggerAction {
+ // An event occurred that caused the trigger to perform its checks.
+ POPUP_CHECK = 0,
+ // A popup cause by an ad was detected and a report was collected.
+ POPUP_REPORTED = 1,
+ // No ad was detected.
+ POPUP_NO_GOOGLE_AD = 2,
+ // An ad was detected on the page causing a popup and could have been
+ // reported, but the trigger manager rejected the report (eg: because user is
+ // incognito or has not opted into extended reporting).
+ POPUP_COULD_NOT_START_REPORT = 3,
+ // Daily quota for ads that caused blocked popups was met.
+ POPUP_DAILY_QUOTA_EXCEEDED = 4,
+ // New events must be added before kMaxValue, and the value of kMaxValue
+ // updated.
+ kMaxValue = POPUP_DAILY_QUOTA_EXCEEDED
+};
+
+// This class is notified when a popup caused by an ad in the browser is
+// blocked. If the Ad is a Google Ad, this class sends a report to Google.
+// Design doc: go/extending-chrind-q2-2019-1
+class AdPopupTrigger : public content::WebContentsUserData<AdPopupTrigger> {
+ public:
+ ~AdPopupTrigger() override;
+
+ static void CreateForWebContents(
+ content::WebContents* web_contents,
+ TriggerManager* trigger_manager,
+ PrefService* prefs,
+ scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+ history::HistoryService* history_service);
+
+ void PopupWasBlocked(content::RenderFrameHost* render_frame);
+
+ private:
+ friend class AdPopupTriggerTest;
+ friend class content::WebContentsUserData<AdPopupTrigger>;
+
+ AdPopupTrigger(
+ content::WebContents* web_contents,
+ TriggerManager* trigger_manager,
+ PrefService* prefs,
+ scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+ history::HistoryService* history_service);
+
+ // Called to create an ad popup report.
+ void CreateAdPopupReport();
+
+ // Sets a task runner to use for tests.
+ void SetTaskRunnerForTest(
+ scoped_refptr<base::SingleThreadTaskRunner> task_runner);
+
+ // WebContents of the current tab.
+ content::WebContents* web_contents_;
+
+ // The delay (in milliseconds) to wait before starting a report. Can be
+ // ovewritten for tests.
+ int64_t start_report_delay_ms_;
+
+ // The delay (in milliseconds) to wait before finishing a report. Can be
+ // overwritten for tests.
+ int64_t finish_report_delay_ms_;
+
+ // TriggerManager gets called if this trigger detects apopup caused by ad and
+ // wants to collect some data about it. Not owned.
+ TriggerManager* trigger_manager_;
+
+ PrefService* prefs_;
+ scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
+ history::HistoryService* history_service_;
+
+ // Task runner for posting delayed tasks. Normally set to the runner for the
+ // UI thread, but can be overwritten for tests.
+ scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
+
+ base::WeakPtrFactory<AdPopupTrigger> weak_ptr_factory_;
+ WEB_CONTENTS_USER_DATA_KEY_DECL();
+
+ DISALLOW_COPY_AND_ASSIGN(AdPopupTrigger);
+};
+
+} // namespace safe_browsing
+
+#endif // COMPONENTS_SAFE_BROWSING_TRIGGERS_AD_POPUP_TRIGGER_H_
diff --git a/components/safe_browsing/triggers/ad_popup_trigger_unittest.cc b/components/safe_browsing/triggers/ad_popup_trigger_unittest.cc
new file mode 100644
index 0000000..c654921
--- /dev/null
+++ b/components/safe_browsing/triggers/ad_popup_trigger_unittest.cc
@@ -0,0 +1,211 @@
+// Copyright 2019 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 "components/safe_browsing/triggers/ad_popup_trigger.h"
+
+#include "base/metrics/field_trial_params.h"
+#include "base/test/metrics/histogram_tester.h"
+#include "base/test/scoped_feature_list.h"
+#include "base/test/test_simple_task_runner.h"
+#include "components/prefs/testing_pref_service.h"
+#include "components/safe_browsing/features.h"
+#include "components/safe_browsing/triggers/mock_trigger_manager.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/test/navigation_simulator.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "content/public/test/test_renderer_host.h"
+#include "testing/gmock/include/gmock/gmock-generated-function-mockers.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using content::NavigationSimulator;
+using content::RenderFrameHost;
+using content::RenderFrameHostTester;
+
+using testing::_;
+using testing::Return;
+
+namespace safe_browsing {
+
+namespace {
+const char kAdUrl[] = "https://tpc.googlesyndication.com/safeframe/1";
+const char kNonAdUrl[] = "https://foo.com/";
+const char kAdName[] = "google_ads_iframe_1";
+const char kNonAdName[] = "foo";
+} // namespace
+
+class AdPopupTriggerTest : public content::RenderViewHostTestHarness {
+ public:
+ AdPopupTriggerTest() : task_runner_(new base::TestSimpleTaskRunner) {}
+ ~AdPopupTriggerTest() override {}
+
+ void SetUp() override {
+ content::RenderViewHostTestHarness::SetUp();
+
+ // Enable any prefs required for the trigger to run.
+ safe_browsing::RegisterProfilePrefs(prefs_.registry());
+ prefs_.SetBoolean(prefs::kSafeBrowsingExtendedReportingOptInAllowed, true);
+ prefs_.SetBoolean(prefs::kSafeBrowsingScoutReportingEnabled, true);
+ }
+
+ void CreateTrigger() {
+ safe_browsing::AdPopupTrigger::CreateForWebContents(
+ web_contents(), &trigger_manager_, &prefs_, nullptr, nullptr);
+
+ safe_browsing::AdPopupTrigger* ad_popup_trigger =
+ safe_browsing::AdPopupTrigger::FromWebContents(web_contents());
+
+ // Give the trigger a test task runner that we can synchronize on.
+ ad_popup_trigger->SetTaskRunnerForTest(task_runner_);
+ }
+
+ // Returns the final RenderFrameHost after navigation commits.
+ RenderFrameHost* NavigateFrame(const std::string& url,
+ RenderFrameHost* frame) {
+ GURL gurl(url);
+ auto navigation_simulator =
+ NavigationSimulator::CreateRendererInitiated(gurl, frame);
+ navigation_simulator->Commit();
+ return navigation_simulator->GetFinalRenderFrameHost();
+ }
+
+ // Returns the final RenderFrameHost after navigation commits.
+ RenderFrameHost* NavigateMainFrame(const std::string& url) {
+ return NavigateFrame(url, web_contents()->GetMainFrame());
+ }
+
+ // Returns the final RenderFrameHost after navigation commits.
+ RenderFrameHost* CreateAndNavigatePopup(const std::string& url,
+ const std::string& frame_name,
+ RenderFrameHost* parent) {
+ RenderFrameHost* popup_opener_frame =
+ RenderFrameHostTester::For(parent)->AppendChild(frame_name);
+ RenderFrameHost* final_frame_host = NavigateFrame(url, popup_opener_frame);
+ // Call the trigger's PopupWasBlocked event handler directly since it
+ // doesn't happen as part of the navigation. This should check if the frame
+ // opening the popup is an ad.
+ safe_browsing::AdPopupTrigger::FromWebContents(web_contents())
+ ->PopupWasBlocked(final_frame_host);
+ return final_frame_host;
+ }
+
+ void WaitForTaskRunnerIdle() {
+ task_runner_->RunUntilIdle();
+ base::RunLoop().RunUntilIdle();
+ }
+
+ MockTriggerManager* get_trigger_manager() { return &trigger_manager_; }
+ base::HistogramTester* get_histograms() { return &histograms_; }
+
+ private:
+ TestingPrefServiceSimple prefs_;
+ MockTriggerManager trigger_manager_;
+ base::HistogramTester histograms_;
+ scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
+};
+
+TEST_F(AdPopupTriggerTest, PopupWithAds) {
+ // Make sure the trigger fires when there are ads on the page.
+ CreateTrigger();
+ EXPECT_CALL(*get_trigger_manager(),
+ StartCollectingThreatDetailsWithReason(
+ TriggerType::AD_POPUP, web_contents(), _, _, _, _, _))
+ .Times(1)
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(*get_trigger_manager(),
+ FinishCollectingThreatDetails(TriggerType::AD_POPUP,
+ web_contents(), _, _, _, _))
+ .Times(1);
+
+ // This page contains two popups - one originating from an ad subframe and one
+ // from a non ad subframe.
+ RenderFrameHost* main_frame = NavigateMainFrame(kNonAdUrl);
+ CreateAndNavigatePopup(kNonAdUrl, kNonAdName, main_frame);
+ CreateAndNavigatePopup(kNonAdUrl, kAdName, main_frame);
+
+ // Wait for any posted tasks to finish.
+ WaitForTaskRunnerIdle();
+
+ // Two popup navigations, one will cause a report
+ get_histograms()->ExpectBucketCount(kAdPopupTriggerActionMetricName,
+ AdPopupTriggerAction::POPUP_CHECK, 2);
+ get_histograms()->ExpectBucketCount(kAdPopupTriggerActionMetricName,
+ AdPopupTriggerAction::POPUP_REPORTED, 1);
+ get_histograms()->ExpectBucketCount(kAdPopupTriggerActionMetricName,
+ AdPopupTriggerAction::POPUP_NO_GOOGLE_AD,
+ 1);
+}
+
+TEST_F(AdPopupTriggerTest, ReportRejectedByTriggerManager) {
+ // If the trigger manager rejects the report, we don't try to finish/send the
+ // report.
+ CreateTrigger();
+ EXPECT_CALL(*get_trigger_manager(),
+ StartCollectingThreatDetailsWithReason(_, _, _, _, _, _, _))
+ .Times(2);
+ EXPECT_CALL(*get_trigger_manager(),
+ FinishCollectingThreatDetails(_, _, _, _, _, _))
+ .Times(0);
+
+ RenderFrameHost* main_frame = NavigateMainFrame(kAdUrl);
+ CreateAndNavigatePopup(kAdUrl, kAdName, main_frame);
+ CreateAndNavigatePopup(kNonAdUrl, kAdName, main_frame);
+
+ WaitForTaskRunnerIdle();
+
+ get_histograms()->ExpectBucketCount(kAdPopupTriggerActionMetricName,
+ AdPopupTriggerAction::POPUP_CHECK, 2);
+ get_histograms()->ExpectBucketCount(kAdPopupTriggerActionMetricName,
+ AdPopupTriggerAction::POPUP_REPORTED, 0);
+ get_histograms()->ExpectBucketCount(
+ kAdPopupTriggerActionMetricName,
+ AdPopupTriggerAction::POPUP_COULD_NOT_START_REPORT, 2);
+}
+
+TEST_F(AdPopupTriggerTest, PopupWithNoAds) {
+ // Make sure the trigger doesn't fire when there are no ads on the page.
+ CreateTrigger();
+ EXPECT_CALL(*get_trigger_manager(),
+ StartCollectingThreatDetailsWithReason(_, _, _, _, _, _, _))
+ .Times(0);
+ EXPECT_CALL(*get_trigger_manager(),
+ FinishCollectingThreatDetails(_, _, _, _, _, _))
+ .Times(0);
+
+ RenderFrameHost* main_frame = NavigateMainFrame(kNonAdUrl);
+ CreateAndNavigatePopup(kNonAdUrl, kNonAdName, main_frame);
+ CreateAndNavigatePopup(kNonAdUrl, kNonAdName, main_frame);
+
+ get_histograms()->ExpectBucketCount(kAdPopupTriggerActionMetricName,
+ AdPopupTriggerAction::POPUP_CHECK, 2);
+ get_histograms()->ExpectBucketCount(kAdPopupTriggerActionMetricName,
+ AdPopupTriggerAction::POPUP_NO_GOOGLE_AD,
+ 2);
+}
+
+TEST_F(AdPopupTriggerTest, PopupNotFromAdForPageWithAd) {
+ // Make sure that no report is generated when there is an ad on the page but
+ // the popup is caused from a different frame.
+ CreateTrigger();
+ EXPECT_CALL(*get_trigger_manager(),
+ StartCollectingThreatDetailsWithReason(_, _, _, _, _, _, _))
+ .Times(0);
+ EXPECT_CALL(*get_trigger_manager(),
+ FinishCollectingThreatDetails(_, _, _, _, _, _))
+ .Times(0);
+
+ RenderFrameHost* main_frame = NavigateMainFrame(kNonAdUrl);
+ RenderFrameHostTester::For(main_frame)->AppendChild(kAdName);
+ CreateAndNavigatePopup(kNonAdUrl, kNonAdName, main_frame);
+
+ // Two navigations (main frame, one subframe), each with no ad.
+ get_histograms()->ExpectBucketCount(kAdPopupTriggerActionMetricName,
+ AdPopupTriggerAction::POPUP_CHECK, 1);
+ get_histograms()->ExpectBucketCount(kAdPopupTriggerActionMetricName,
+ AdPopupTriggerAction::POPUP_NO_GOOGLE_AD,
+ 1);
+}
+
+} // namespace safe_browsing
diff --git a/components/safe_browsing/triggers/trigger_manager.cc b/components/safe_browsing/triggers/trigger_manager.cc
index ee7457a..9158ec9 100644
--- a/components/safe_browsing/triggers/trigger_manager.cc
+++ b/components/safe_browsing/triggers/trigger_manager.cc
@@ -26,6 +26,7 @@
// For security interstitials, users can change the opt-in while the
// trigger runs, so collection can begin without opt-in.
return false;
+ case TriggerType::AD_POPUP:
case TriggerType::AD_SAMPLE:
// Ad samples happen in the background so the user must already be opted
// in before the trigger is allowed to run.
@@ -111,7 +112,6 @@
bool optin_required_check_ok =
!TriggerNeedsOptInForCollection(trigger_type) ||
error_display_options.is_extended_reporting_enabled;
-
// We start data collection as long as user is not incognito and is able to
// change the Extended Reporting opt-in, and the |trigger_type| has available
// quota. For some triggers we also require extended reporting opt-in in
@@ -128,6 +128,7 @@
return false;
}
}
+
bool TriggerManager::StartCollectingThreatDetails(
const TriggerType trigger_type,
content::WebContents* web_contents,
@@ -160,7 +161,8 @@
if (collectors->threat_details != nullptr)
return false;
- bool should_trim_threat_details = trigger_type == TriggerType::AD_SAMPLE;
+ bool should_trim_threat_details = (trigger_type == TriggerType::AD_POPUP ||
+ trigger_type == TriggerType::AD_SAMPLE);
collectors->threat_details = ThreatDetails::NewThreatDetails(
ui_manager_, web_contents, resource, url_loader_factory, history_service,
referrer_chain_provider_, should_trim_threat_details,
diff --git a/components/safe_browsing/triggers/trigger_throttler.cc b/components/safe_browsing/triggers/trigger_throttler.cc
index 13777d7c..f8f4ed08a 100644
--- a/components/safe_browsing/triggers/trigger_throttler.cc
+++ b/components/safe_browsing/triggers/trigger_throttler.cc
@@ -15,8 +15,10 @@
#include "components/safe_browsing/features.h"
namespace safe_browsing {
+const size_t kAdPopupTriggerDefaultQuota = 1;
const size_t kAdSamplerTriggerDefaultQuota = 10;
const size_t kSuspiciousSiteTriggerDefaultQuota = 5;
+const char kAdPopupTriggerQuotaParam[] = "ad_popup_trigger_quota";
const char kSuspiciousSiteTriggerQuotaParam[] = "suspicious_site_trigger_quota";
const char kTriggerTypeAndQuotaParam[] = "trigger_type_and_quota_csv";
@@ -50,6 +52,14 @@
std::make_pair(TriggerType::SUSPICIOUS_SITE, suspicious_site_quota));
}
+ int ad_popup_quota = base::GetFieldTrialParamByFeatureAsInt(
+ kAdPopupTriggerFeature, kAdPopupTriggerQuotaParam,
+ kAdPopupTriggerDefaultQuota);
+ if (ad_popup_quota > 0) {
+ trigger_type_and_quota_list->push_back(
+ std::make_pair(TriggerType::AD_POPUP, ad_popup_quota));
+ }
+
// If the feature is disabled we just use the default list. Otherwise the list
// from the Finch param will be the one used.
if (!base::FeatureList::IsEnabled(kTriggerThrottlerDailyQuotaFeature)) {
@@ -247,6 +257,13 @@
case TriggerType::GAIA_PASSWORD_REUSE:
case TriggerType::APK_DOWNLOAD:
return kUnlimitedTriggerQuota;
+ case TriggerType::AD_POPUP:
+ // Ad Popup reports are disabled unless they are configured through Finch.
+ if (TryFindQuotaForTrigger(trigger_type, trigger_type_and_quota_list_,
+ "a_from_finch)) {
+ return quota_from_finch;
+ }
+ break;
case TriggerType::AD_SAMPLE:
// Ad Samples have a non-zero default quota, but it can be overwritten
// through Finch.
diff --git a/components/safe_browsing/triggers/trigger_throttler.h b/components/safe_browsing/triggers/trigger_throttler.h
index 10cbcf6..0d8621b 100644
--- a/components/safe_browsing/triggers/trigger_throttler.h
+++ b/components/safe_browsing/triggers/trigger_throttler.h
@@ -40,8 +40,9 @@
GAIA_PASSWORD_REUSE = 3,
SUSPICIOUS_SITE = 4,
APK_DOWNLOAD = 5,
+ AD_POPUP = 6,
kMinTriggerType = SECURITY_INTERSTITIAL,
- kMaxTriggerType = APK_DOWNLOAD,
+ kMaxTriggerType = AD_POPUP,
};
struct TriggerTypeHash {
diff --git a/components/security_interstitials/content/unsafe_resource.cc b/components/security_interstitials/content/unsafe_resource.cc
index f9ed7717..ae046912 100644
--- a/components/security_interstitials/content/unsafe_resource.cc
+++ b/components/security_interstitials/content/unsafe_resource.cc
@@ -45,6 +45,8 @@
// frame load, since they happen after the page is finished loading.
case safe_browsing::SB_THREAT_TYPE_URL_CLIENT_SIDE_PHISHING:
case safe_browsing::SB_THREAT_TYPE_URL_CLIENT_SIDE_MALWARE:
+ // Malicious ad activity reporting happens in the background.
+ case safe_browsing::SB_THREAT_TYPE_BLOCKED_AD_POPUP:
// Ad sampling happens in the background.
case safe_browsing::SB_THREAT_TYPE_AD_SAMPLE:
// Sign-in password reuse warning happens after the page is finished