Connecting Ad Sampler trigger to TriggerManager.

The trigger remains disabled because it is hard-coded to have 0 quota (to be controlled via Finch in an upcoming CL), so this CL should be a no-op for now. When the trigger notices an ad in some frame on the page it will call TriggerManager to start capturing a trimmed threat report. It will also enqueue a call to finish and send the report 5s later to allow enough time for the asynchronous collection to run. There are some TODOs scattered throughout the code to fill in missing pieces, upcoming CLs will cover these. Bug: 744869 Change-Id: I67c53a57e338c82c299b4d0533700e9aee79b6bd Reviewed-on: https://chromium-review.googlesource.com/612125 Reviewed-by: Alexei Svitkine (very slow) <asvitkine@chromium.org> Reviewed-by: Emily Stark <estark@chromium.org> Reviewed-by: Michael Wasserman <msw@chromium.org> Reviewed-by: Varun Khaneja <vakh@chromium.org> Reviewed-by: Jialiu Lin <jialiul@chromium.org> Commit-Queue: Luke Z <lpz@chromium.org> Cr-Commit-Position: refs/heads/master@{#496289}
qweri0p · Aug 22, 2017 · 40af601 · 40af601
1 parent eda1230
commit 40af601
Show file tree

Hide file tree

Showing 21 changed files with 690 additions and 130 deletions.
diff --git a/chrome/browser/safe_browsing/BUILD.gn b/chrome/browser/safe_browsing/BUILD.gn
@@ -238,13 +238,22 @@ static_library("safe_browsing") {
         "services_delegate_impl.h",
         "signature_evaluator_mac.h",
         "signature_evaluator_mac.mm",
+        "trigger_creator.cc",
+        "trigger_creator.h",
         "v4_test_utils.cc",
         "v4_test_utils.h",
       ]
       deps += [
         "//components/content_settings/core/browser:browser",
+        "//components/prefs:prefs",
         "//components/safe_browsing/common:safe_browsing_prefs",
+        "//components/safe_browsing/triggers:ad_sampler_trigger",
+        "//components/safe_browsing/triggers:trigger_throttler",
+        "//components/safe_browsing/triggers:triggers",
         "//components/safe_browsing_db:safe_browsing_db",
+        "//components/security_interstitials/content:security_interstitial_page",
+        "//content/public/browser:browser",
+        "//net:net",
       ]
       if (is_win) {
         deps += [ "//chrome/browser/safe_browsing/incident_reporting:state_store_data_proto" ]

diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc
@@ -245,23 +245,14 @@ class TestThreatDetailsFactory : public ThreatDetailsFactory {
       WebContents* web_contents,
       const security_interstitials::UnsafeResource& unsafe_resource,
       net::URLRequestContextGetter* request_context_getter,
-      history::HistoryService* history_service) override {
+      history::HistoryService* history_service,
+      bool trim_to_ad_tags) override {
     details_ = new ThreatDetails(delegate, web_contents, unsafe_resource,
                                  request_context_getter, history_service,
-                                 /*trim_to_ad_tags=*/false);
+                                 trim_to_ad_tags);
     return details_;
   }
 
-  ThreatDetails* CreateTrimmedThreatDetails(
-      BaseUIManager* ui_manager,
-      content::WebContents* web_contents,
-      const security_interstitials::UnsafeResource& unsafe_resource,
-      net::URLRequestContextGetter* request_context_getter,
-      history::HistoryService* history_service) override {
-    return CreateThreatDetails(ui_manager, web_contents, unsafe_resource,
-                               request_context_getter, history_service);
-  }
-
   ThreatDetails* get_details() { return details_; }
 
  private:

diff --git a/chrome/browser/safe_browsing/trigger_creator.cc b/chrome/browser/safe_browsing/trigger_creator.cc
@@ -0,0 +1,52 @@
+// 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/safe_browsing/trigger_creator.h"
+
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/history/history_service_factory.h"
+#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/triggers/ad_sampler_trigger.h"
+#include "components/safe_browsing/triggers/trigger_manager.h"
+#include "components/safe_browsing/triggers/trigger_throttler.h"
+#include "components/security_interstitials/core/base_safe_browsing_error_ui.h"
+#include "content/public/browser/browser_context.h"
+#include "content/public/browser/browser_thread.h"
+#include "net/url_request/url_request_context_getter.h"
+
+namespace safe_browsing {
+
+using SBErrorOptions =
+    security_interstitials::BaseSafeBrowsingErrorUI::SBErrorDisplayOptions;
+
+void TriggerCreator::MaybeCreateTriggersForWebContents(
+    Profile* profile,
+    content::WebContents* web_contents) {
+  if (!g_browser_process->safe_browsing_service() ||
+      !g_browser_process->safe_browsing_service()->trigger_manager()) {
+    return;
+  }
+
+  // We only start triggers for this tab if they are eligible to collect data
+  // (eg: because of opt-ins, available quota, etc). If we skip a trigger but
+  // later opt-in changes or quota becomes available, the trigger won't be
+  // running on old tabs, but that's acceptable. The trigger will be started for
+  // new tabs.
+  TriggerManager* trigger_manager =
+      g_browser_process->safe_browsing_service()->trigger_manager();
+  SBErrorOptions options = TriggerManager::GetSBErrorDisplayOptions(
+      *profile->GetPrefs(), *web_contents);
+  if (trigger_manager->CanStartDataCollection(options,
+                                              TriggerType::AD_SAMPLE)) {
+    safe_browsing::AdSamplerTrigger::CreateForWebContents(
+        web_contents, trigger_manager, profile->GetPrefs(),
+        profile->GetRequestContext(),
+        HistoryServiceFactory::GetForProfile(
+            profile, ServiceAccessType::EXPLICIT_ACCESS));
+  }
+}
+
+}  // namespace safe_browsing
diff --git a/chrome/browser/safe_browsing/trigger_creator.h b/chrome/browser/safe_browsing/trigger_creator.h
@@ -0,0 +1,29 @@
+// 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.
+
+#ifndef CHROME_BROWSER_SAFE_BROWSING_TRIGGER_CREATOR_H_
+#define CHROME_BROWSER_SAFE_BROWSING_TRIGGER_CREATOR_H_
+
+class Profile;
+
+namespace content {
+class WebContents;
+}
+
+namespace safe_browsing {
+
+// Takes care of creation of individual triggers. This functionality lives in a
+// separate class from TriggerManager to avoid circular dependencies.
+// TriggerManager need not know about individual trigger classes, while the
+// trigger classes needs to know about the TriggerManager in order to fire
+// triggers.
+class TriggerCreator {
+ public:
+  static void MaybeCreateTriggersForWebContents(
+      Profile* profile,
+      content::WebContents* web_contents);
+};
+
+}  // namespace safe_browsing
+#endif  // CHROME_BROWSER_SAFE_BROWSING_TRIGGER_CREATOR_H_
diff --git a/chrome/browser/ssl/security_state_tab_helper.cc b/chrome/browser/ssl/security_state_tab_helper.cc
@@ -214,6 +214,7 @@ SecurityStateTabHelper::GetMaliciousContentStatus() const {
       case safe_browsing::SB_THREAT_TYPE_API_ABUSE:
       case safe_browsing::SB_THREAT_TYPE_SUBRESOURCE_FILTER:
       case safe_browsing::SB_THREAT_TYPE_CSD_WHITELIST:
+      case safe_browsing::SB_THREAT_TYPE_AD_SAMPLE:
         // These threat types are not currently associated with
         // interstitials, and thus resources with these threat types are
         // not ever whitelisted or pending whitelisting.

diff --git a/chrome/browser/ui/tab_helpers.cc b/chrome/browser/ui/tab_helpers.cc
@@ -41,6 +41,7 @@
 #include "chrome/browser/previews/previews_infobar_tab_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/resource_coordinator/resource_coordinator_web_contents_observer.h"
+#include "chrome/browser/safe_browsing/trigger_creator.h"
 #include "chrome/browser/sessions/session_tab_helper.h"
 #include "chrome/browser/ssl/security_state_tab_helper.h"
 #include "chrome/browser/subresource_filter/chrome_subresource_filter_client.h"
@@ -166,7 +167,6 @@ void TabHelpers::AttachTabHelpers(WebContents* web_contents) {
 #endif
 
   // --- Common tab helpers ---
-
   autofill::ChromeAutofillClient::CreateForWebContents(web_contents);
   autofill::ContentAutofillDriverFactory::CreateForWebContentsAndDelegate(
       web_contents,
@@ -195,10 +195,11 @@ void TabHelpers::AttachTabHelpers(WebContents* web_contents) {
   ExternalProtocolObserver::CreateForWebContents(web_contents);
   favicon::CreateContentFaviconDriverForWebContents(web_contents);
   FindTabHelper::CreateForWebContents(web_contents);
+
+  Profile* profile =
+      Profile::FromBrowserContext(web_contents->GetBrowserContext());
   history::WebContentsTopSitesObserver::CreateForWebContents(
-      web_contents, TopSitesFactory::GetForProfile(
-                        Profile::FromBrowserContext(
-                            web_contents->GetBrowserContext())).get());
+      web_contents, TopSitesFactory::GetForProfile(profile).get());
   HistoryTabHelper::CreateForWebContents(web_contents);
   InfoBarService::CreateForWebContents(web_contents);
   InstallableManager::CreateForWebContents(web_contents);
@@ -223,7 +224,7 @@ void TabHelpers::AttachTabHelpers(WebContents* web_contents) {
   sync_sessions::SyncSessionsRouterTabHelper::CreateForWebContents(
       web_contents,
       sync_sessions::SyncSessionsWebContentsRouterFactory::GetForProfile(
-          Profile::FromBrowserContext(web_contents->GetBrowserContext())));
+          profile));
   // TODO(vabr): Remove TabSpecificContentSettings from here once their function
   // is taken over by ChromeContentSettingsClient. http://crbug.com/387075
   TabSpecificContentSettings::CreateForWebContents(web_contents);
@@ -259,6 +260,8 @@ void TabHelpers::AttachTabHelpers(WebContents* web_contents) {
   safe_browsing::SafeBrowsingTabObserver::CreateForWebContents(web_contents);
   safe_browsing::SafeBrowsingNavigationObserver::MaybeCreateForWebContents(
       web_contents);
+  safe_browsing::TriggerCreator::MaybeCreateTriggersForWebContents(
+      profile, web_contents);
   TabContentsSyncedTabDelegate::CreateForWebContents(web_contents);
   TabDialogs::CreateForWebContents(web_contents);
   ThumbnailTabHelper::CreateForWebContents(web_contents);
@@ -303,8 +306,7 @@ offline_pages::RecentTabHelper::CreateForWebContents(web_contents);
         web_contents);
   }
 
-  if (predictors::LoadingPredictorFactory::GetForProfile(
-          Profile::FromBrowserContext(web_contents->GetBrowserContext()))) {
+  if (predictors::LoadingPredictorFactory::GetForProfile(profile)) {
     predictors::ResourcePrefetchPredictorTabHelper::CreateForWebContents(
         web_contents);
   }

diff --git a/components/safe_browsing/browser/threat_details.cc b/components/safe_browsing/browser/threat_details.cc
@@ -84,6 +84,8 @@ ClientSafeBrowsingReportRequest::ReportType GetReportTypeFromSBThreatType(
       return ClientSafeBrowsingReportRequest::URL_CLIENT_SIDE_MALWARE;
     case SB_THREAT_TYPE_URL_PASSWORD_PROTECTION_PHISHING:
       return ClientSafeBrowsingReportRequest::URL_PASSWORD_PROTECTION_PHISHING;
+    case SB_THREAT_TYPE_AD_SAMPLE:
+      return ClientSafeBrowsingReportRequest::AD_SAMPLE;
     default:  // Gated by SafeBrowsingBlockingPage::ShouldReportThreatDetails.
       NOTREACHED() << "We should not send report for threat type "
                    << threat_type;
@@ -264,21 +266,11 @@ class ThreatDetailsFactoryImpl : public ThreatDetailsFactory {
       WebContents* web_contents,
       const security_interstitials::UnsafeResource& unsafe_resource,
       net::URLRequestContextGetter* request_context_getter,
-      history::HistoryService* history_service) override {
+      history::HistoryService* history_service,
+      bool trim_to_ad_tags) override {
     return new ThreatDetails(ui_manager, web_contents, unsafe_resource,
                              request_context_getter, history_service,
-                             /*trim_to_ad_tags=*/false);
-  }
-
-  ThreatDetails* CreateTrimmedThreatDetails(
-      BaseUIManager* ui_manager,
-      WebContents* web_contents,
-      const security_interstitials::UnsafeResource& unsafe_resource,
-      net::URLRequestContextGetter* request_context_getter,
-      history::HistoryService* history_service) override {
-    return new ThreatDetails(ui_manager, web_contents, unsafe_resource,
-                             request_context_getter, history_service,
-                             /*trim_to_ad_tags=*/true);
+                             trim_to_ad_tags);
   }
 
  private:
@@ -299,13 +291,15 @@ ThreatDetails* ThreatDetails::NewThreatDetails(
     WebContents* web_contents,
     const UnsafeResource& resource,
     net::URLRequestContextGetter* request_context_getter,
-    history::HistoryService* history_service) {
+    history::HistoryService* history_service,
+    bool trim_to_ad_tags) {
   // Set up the factory if this has not been done already (tests do that
   // before this method is called).
   if (!factory_)
     factory_ = g_threat_details_factory_impl.Pointer();
   return factory_->CreateThreatDetails(ui_manager, web_contents, resource,
-                                       request_context_getter, history_service);
+                                       request_context_getter, history_service,
+                                       trim_to_ad_tags);
 }
 
 // Create a ThreatDetails for the given tab. Runs in the UI thread.

diff --git a/components/safe_browsing/browser/threat_details.h b/components/safe_browsing/browser/threat_details.h
@@ -77,7 +77,8 @@ class ThreatDetails : public base::RefCountedThreadSafe<
       content::WebContents* web_contents,
       const UnsafeResource& resource,
       net::URLRequestContextGetter* request_context_getter,
-      history::HistoryService* history_service);
+      history::HistoryService* history_service,
+      bool trim_to_ad_tags);
 
   // Makes the passed |factory| the factory used to instantiate
   // SafeBrowsingBlockingPage objects. Useful for tests.
@@ -261,13 +262,8 @@ class ThreatDetailsFactory {
       content::WebContents* web_contents,
       const security_interstitials::UnsafeResource& unsafe_resource,
       net::URLRequestContextGetter* request_context_getter,
-      history::HistoryService* history_service) = 0;
-  virtual ThreatDetails* CreateTrimmedThreatDetails(
-      BaseUIManager* ui_manager,
-      content::WebContents* web_contents,
-      const security_interstitials::UnsafeResource& unsafe_resource,
-      net::URLRequestContextGetter* request_context_getter,
-      history::HistoryService* history_service) = 0;
+      history::HistoryService* history_service,
+      bool trim_to_ad_tags) = 0;
 };
 
 }  // namespace safe_browsing

diff --git a/components/safe_browsing/proto/csd.proto b/components/safe_browsing/proto/csd.proto
@@ -934,6 +934,7 @@ message ClientSafeBrowsingReportRequest {
     DANGEROUS_DOWNLOAD_BY_API = 10;
     URL_PASSWORD_PROTECTION_PHISHING = 12;
     DANGEROUS_DOWNLOAD_OPENED = 13;
+    AD_SAMPLE = 14;
   }
 
   message HTTPHeader {

diff --git a/components/safe_browsing/triggers/BUILD.gn b/components/safe_browsing/triggers/BUILD.gn
@@ -20,6 +20,7 @@ source_set("triggers") {
     "//components/prefs:prefs",
     "//components/safe_browsing:safe_browsing",
     "//components/safe_browsing/browser:browser",
+    "//net:net",
   ]
 }
 
@@ -39,6 +40,8 @@ source_set("ad_sampler_trigger") {
     "ad_sampler_trigger.h",
   ]
   deps = [
+    ":trigger_throttler",
+    ":triggers",
     "//base:base",
     "//components/safe_browsing:features",
     "//content/public/browser",
@@ -48,16 +51,21 @@ source_set("ad_sampler_trigger") {
 source_set("unit_tests") {
   testonly = true
   sources = [
+    "ad_sampler_trigger_unittest.cc",
     "trigger_manager_unittest.cc",
     "trigger_throttler_unittest.cc",
   ]
   deps = [
+    ":ad_sampler_trigger",
     ":trigger_throttler",
     ":triggers",
     "//base",
+    "//base/test:test_support",
     "//components/prefs:test_support",
+    "//components/safe_browsing:features",
     "//components/safe_browsing/browser:browser",
     "//content/test:test_support",
+    "//testing/gmock",
     "//testing/gtest",
   ]
 }