Reland: Send ThreatDetails after user done interacting with modal warning

Previous change reverted because it was commited at the same time as
https://chromium-review.googlesource.com/673124 that caused conflict.

TBR=lpz@chromium.org

Bug: 761114
Change-Id: If9031596189dff08fec6ec3f0ef2db82544767ca
Reviewed-on: https://chromium-review.googlesource.com/690596Reviewed-by: Jialiu Lin <jialiul@chromium.org>
Commit-Queue: Jialiu Lin <jialiul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505186}

chrome/browser/safe_browsing/chrome_password_protection_service.cc

@@ -30,6 +30,7 @@
 #include "components/safe_browsing/db/database_manager.h"
 #include "components/safe_browsing/features.h"
 #include "components/safe_browsing/password_protection/password_protection_request.h"
+#include "components/safe_browsing/triggers/trigger_throttler.h"
 #include "components/signin/core/browser/account_info.h"
 #include "components/signin/core/browser/account_tracker_service.h"
 #include "components/signin/core/browser/signin_manager.h"
@@ -112,6 +113,7 @@ ChromePasswordProtectionService::ChromePasswordProtectionService(
               ServiceAccessType::EXPLICIT_ACCESS),
           HostContentSettingsMapFactory::GetForProfile(profile)),
       ui_manager_(sb_service->ui_manager()),
+      trigger_manager_(sb_service->trigger_manager()),
       profile_(profile),
       navigation_observer_manager_(sb_service->navigation_observer_manager()),
       pref_change_registrar_(new PrefChangeRegistrar) {
@@ -181,7 +183,7 @@ void ChromePasswordProtectionService::FillReferrerChain(
 
 void ChromePasswordProtectionService::ShowModalWarning(
     content::WebContents* web_contents,
-    const std::string& unused_verdict_token) {
+    const std::string& verdict_token) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   // TODO(jialiul): Use verdict_token field in post warning report.
   UpdateSecurityState(SB_THREAT_TYPE_PASSWORD_REUSE, web_contents);
@@ -195,6 +197,8 @@ void ChromePasswordProtectionService::ShowModalWarning(
     unhandled_password_reuses_[Origin(web_contents->GetLastCommittedURL())] =
         GetLastCommittedNavigationID(web_contents);
   }
+  // Starts preparing post-warning report.
+  MaybeStartThreatDetailsCollection(web_contents, verdict_token);
 }
 
 // TODO(jialiul): Handle user actions in separate functions.
@@ -238,7 +242,8 @@ void ChromePasswordProtectionService::OnUserAction(
           NOTREACHED();
           break;
       }
-      // TODO(jialiul): Sends post-warning reporting.
+      MaybeFinishCollectingThreatDetails(
+          web_contents, action == PasswordProtectionService::CHANGE_PASSWORD);
       break;
     case PasswordProtectionService::CHROME_SETTINGS:
       DCHECK_EQ(PasswordProtectionService::CHANGE_PASSWORD, action);
@@ -261,6 +266,46 @@ void ChromePasswordProtectionService::RemoveObserver(Observer* observer) {
   observer_list_.RemoveObserver(observer);
 }
 
+void ChromePasswordProtectionService::MaybeStartThreatDetailsCollection(
+    content::WebContents* web_contents,
+    const std::string& token) {
+  // |trigger_manager_| can be null in test.
+  if (!trigger_manager_)
+    return;
+
+  security_interstitials::UnsafeResource resource;
+  resource.threat_type = SB_THREAT_TYPE_PASSWORD_REUSE;
+  resource.url = web_contents->GetLastCommittedURL();
+  resource.web_contents_getter = resource.GetWebContentsGetter(
+      web_contents->GetMainFrame()->GetProcess()->GetID(),
+      web_contents->GetMainFrame()->GetRoutingID());
+  resource.token = token;
+  // Ignores the return of |StartCollectingThreatDetails()| here and let
+  // TriggerManager decide whether it should start data collection.
+  trigger_manager_->StartCollectingThreatDetails(
+      safe_browsing::TriggerType::GAIA_PASSWORD_REUSE, web_contents, resource,
+      profile_->GetRequestContext(), /*history_service=*/nullptr,
+      TriggerManager::GetSBErrorDisplayOptions(*profile_->GetPrefs(),
+                                               *web_contents));
+}
+
+void ChromePasswordProtectionService::MaybeFinishCollectingThreatDetails(
+    content::WebContents* web_contents,
+    bool did_proceed) {
+  // |trigger_manager_| can be null in test.
+  if (!trigger_manager_)
+    return;
+
+  // Since we don't keep track the threat details in progress, it is safe to
+  // ignore the result of |FinishCollectingThreatDetails()|. TriggerManager will
+  // take care of whether report should be sent.
+  trigger_manager_->FinishCollectingThreatDetails(
+      safe_browsing::TriggerType::GAIA_PASSWORD_REUSE, web_contents,
+      base::TimeDelta::FromMilliseconds(0), did_proceed, /*num_visit=*/0,
+      TriggerManager::GetSBErrorDisplayOptions(*profile_->GetPrefs(),
+                                               *web_contents));
+}
+
 PrefService* ChromePasswordProtectionService::GetPrefs() {
   return profile_->GetPrefs();
 }
@@ -571,6 +616,7 @@ ChromePasswordProtectionService::ChromePasswordProtectionService(
                                 nullptr,
                                 content_setting_map.get()),
       ui_manager_(ui_manager),
+      trigger_manager_(nullptr),
       profile_(profile) {
   InitializeAccountInfo();
 }

chrome/browser/safe_browsing/chrome_password_protection_service.h

@@ -10,6 +10,7 @@
 #include "base/observer_list.h"
 #include "build/build_config.h"
 #include "components/safe_browsing/password_protection/password_protection_service.h"
+#include "components/safe_browsing/triggers/trigger_manager.h"
 #include "components/sync/protocol/user_event_specifics.pb.h"
 #include "ui/base/ui_features.h"
 #include "url/origin.h"
@@ -96,6 +97,16 @@ class ChromePasswordProtectionService : public PasswordProtectionService {
   // Called during the destruction of the observer subclass.
   virtual void RemoveObserver(Observer* observer);
 
+  // Starts collecting threat details if user has extended reporting enabled and
+  // is not in incognito mode.
+  void MaybeStartThreatDetailsCollection(content::WebContents* web_contents,
+                                         const std::string& token);
+
+  // Sends threat details if user has extended reporting enabled and is not in
+  // incognito mode.
+  void MaybeFinishCollectingThreatDetails(content::WebContents* web_contents,
+                                          bool did_proceed);
+
   const std::map<Origin, int64_t>& unhandled_password_reuses() const {
     return unhandled_password_reuses_;
   }
@@ -197,6 +208,7 @@ class ChromePasswordProtectionService : public PasswordProtectionService {
       scoped_refptr<SafeBrowsingUIManager> ui_manager);
 
   scoped_refptr<SafeBrowsingUIManager> ui_manager_;
+  TriggerManager* trigger_manager_;
   // Profile associated with this instance.
   Profile* profile_;
   // AccountInfo associated with this |profile_|.

chrome/browser/safe_browsing/chrome_password_protection_service_browsertest.cc

@@ -42,26 +42,35 @@ class ChromePasswordProtectionServiceBrowserTest : public InProcessBrowserTest {
     InProcessBrowserTest::SetUp();
   }
 
-  ChromePasswordProtectionService* GetService() {
+  ChromePasswordProtectionService* GetService(bool is_incognito) {
     return ChromePasswordProtectionService::GetPasswordProtectionService(
-        browser()->profile());
+        is_incognito ? browser()->profile()->GetOffTheRecordProfile()
+                     : browser()->profile());
   }
 
-  void SimulateGaiaPasswordChange() {
-    browser()->profile()->GetPrefs()->SetString(
-        password_manager::prefs::kSyncPasswordHash, "new_password_hash");
+  void SimulateGaiaPasswordChange(bool is_incognito) {
+    if (is_incognito) {
+      browser()->profile()->GetOffTheRecordProfile()->GetPrefs()->SetString(
+          password_manager::prefs::kSyncPasswordHash, "new_password_hash");
+    } else {
+      browser()->profile()->GetPrefs()->SetString(
+          password_manager::prefs::kSyncPasswordHash, "new_password_hash");
+    }
   }
 
-  void SimulateAction(ChromePasswordProtectionService::WarningUIType ui_type,
+  void SimulateAction(ChromePasswordProtectionService* service,
+                      ChromePasswordProtectionService::WarningUIType ui_type,
                       ChromePasswordProtectionService::WarningAction action) {
-    for (auto& observer : GetService()->observer_list_) {
+    for (auto& observer : service->observer_list_) {
       if (ui_type == observer.GetObserverType()) {
         observer.InvokeActionForTesting(action);
       }
     }
   }
 
-  void SimulateGaiaPasswordChanged() { GetService()->OnGaiaPasswordChanged(); }
+  void SimulateGaiaPasswordChanged(ChromePasswordProtectionService* service) {
+    service->OnGaiaPasswordChanged();
+  }
 
   void GetSecurityInfo(content::WebContents* web_contents,
                        security_state::SecurityInfo* out_security_info) {
@@ -70,8 +79,8 @@ class ChromePasswordProtectionServiceBrowserTest : public InProcessBrowserTest {
     helper->GetSecurityInfo(out_security_info);
   }
 
-  void SetDefaultProfileEmail() {
-    GetService()->account_info_->email = "foo@bar.com";
+  void SetDefaultProfileEmail(ChromePasswordProtectionService* service) {
+    service->account_info_->email = "foo@bar.com";
   }
 
  protected:
@@ -81,8 +90,8 @@ class ChromePasswordProtectionServiceBrowserTest : public InProcessBrowserTest {
 
 IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
                        SuccessfullyChangePassword) {
-  ChromePasswordProtectionService* service = GetService();
-  SetDefaultProfileEmail();
+  ChromePasswordProtectionService* service = GetService(/*is_incognito=*/false);
+  SetDefaultProfileEmail(service);
   Profile* profile = browser()->profile();
   content::WebContents* web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
@@ -112,7 +121,7 @@ IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
 
   // Simulates clicking "Change Password" button on the modal dialog.
   // There should be only 1 observer in the list.
-  SimulateAction(ChromePasswordProtectionService::MODAL_DIALOG,
+  SimulateAction(service, ChromePasswordProtectionService::MODAL_DIALOG,
                  ChromePasswordProtectionService::CHANGE_PASSWORD);
   content::WebContents* new_web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
@@ -126,7 +135,7 @@ IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
             new_web_contents->GetVisibleURL());
 
   // Simulates clicking "Change password" button on the chrome://settings card.
-  SimulateAction(ChromePasswordProtectionService::CHROME_SETTINGS,
+  SimulateAction(service, ChromePasswordProtectionService::CHROME_SETTINGS,
                  ChromePasswordProtectionService::CHANGE_PASSWORD);
   base::RunLoop().RunUntilIdle();
   // Verify myaccount.google.com or Google signin page should be opened in a
@@ -139,7 +148,7 @@ IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
                   .DomainIs("google.com"));
 
   // Simulates user finished changing password.
-  SimulateGaiaPasswordChanged();
+  SimulateGaiaPasswordChanged(service);
   base::RunLoop().RunUntilIdle();
   EXPECT_FALSE(
       ChromePasswordProtectionService::ShouldShowChangePasswordSettingUI(
@@ -152,7 +161,7 @@ IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
 
 IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
                        MarkSiteAsLegitimate) {
-  ChromePasswordProtectionService* service = GetService();
+  ChromePasswordProtectionService* service = GetService(/*is_incognito=*/false);
   Profile* profile = browser()->profile();
   content::WebContents* web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
@@ -182,7 +191,7 @@ IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
 
   // Simulates clicking "Ignore" button on the modal dialog.
   // There should be only 1 observer in the list.
-  SimulateAction(ChromePasswordProtectionService::MODAL_DIALOG,
+  SimulateAction(service, ChromePasswordProtectionService::MODAL_DIALOG,
                  ChromePasswordProtectionService::IGNORE_WARNING);
   base::RunLoop().RunUntilIdle();
   // No new tab opens. SecurityInfo doesn't change.
@@ -211,7 +220,7 @@ IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
 
 IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
                        OpenChromeSettingsViaPageInfo) {
-  ChromePasswordProtectionService* service = GetService();
+  ChromePasswordProtectionService* service = GetService(/*is_incognito=*/false);
   Profile* profile = browser()->profile();
   content::WebContents* web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
@@ -222,7 +231,7 @@ IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
   service->ShowModalWarning(web_contents, "unused_token");
   base::RunLoop().RunUntilIdle();
   // Simulates clicking "Ignore" to close dialog.
-  SimulateAction(ChromePasswordProtectionService::MODAL_DIALOG,
+  SimulateAction(service, ChromePasswordProtectionService::MODAL_DIALOG,
                  ChromePasswordProtectionService::IGNORE_WARNING);
   base::RunLoop().RunUntilIdle();
   ASSERT_TRUE(
@@ -253,7 +262,7 @@ IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
 IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
                        VeriryUnhandledPasswordReuse) {
   histograms_.ExpectTotalCount(kGaiaPasswordChangeHistogramName, 0);
-  ChromePasswordProtectionService* service = GetService();
+  ChromePasswordProtectionService* service = GetService(/*is_incognito=*/false);
   ASSERT_TRUE(service);
   Profile* profile = browser()->profile();
   ui_test_utils::NavigateToURL(browser(), embedded_test_server()->GetURL("/"));
@@ -273,7 +282,7 @@ IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
           profile));
 
   // Opens a new browser window.
-  Browser* browser2 = CreateBrowser(browser()->profile());
+  Browser* browser2 = CreateBrowser(profile);
   // Shows modal dialog on this new web_contents.
   content::WebContents* new_web_contents =
       browser2->tab_strip_model()->GetActiveWebContents();
@@ -286,7 +295,7 @@ IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
           profile));
 
   // Simulates a Gaia password change.
-  SimulateGaiaPasswordChange();
+  SimulateGaiaPasswordChange(/*is_incognito=*/false);
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(0u, service->unhandled_password_reuses().size());
   EXPECT_FALSE(

components/safe_browsing/browser/threat_details.cc

@@ -85,6 +85,8 @@ ClientSafeBrowsingReportRequest::ReportType GetReportTypeFromSBThreatType(
       return ClientSafeBrowsingReportRequest::URL_CLIENT_SIDE_MALWARE;
     case SB_THREAT_TYPE_AD_SAMPLE:
       return ClientSafeBrowsingReportRequest::AD_SAMPLE;
+    case SB_THREAT_TYPE_PASSWORD_REUSE:
+      return ClientSafeBrowsingReportRequest::URL_PASSWORD_PROTECTION_PHISHING;
     default:  // Gated by SafeBrowsingBlockingPage::ShouldReportThreatDetails.
       NOTREACHED() << "We should not send report for threat type "
                    << threat_type;

components/safe_browsing/triggers/trigger_manager.cc

@@ -25,6 +25,9 @@ bool TriggerNeedsScout(const TriggerType trigger_type) {
     case TriggerType::AD_SAMPLE:
       // Ad samples need Scout-level opt-in.
       return true;
+    case TriggerType::GAIA_PASSWORD_REUSE:
+      // Gaia password reuses only need legacy SBER opt-in.
+      return false;
   }
   // By default, require Scout so we are more restrictive on data collection.
   return true;
@@ -40,6 +43,11 @@ bool TriggerNeedsOptInForCollection(const TriggerType trigger_type) {
       // Ad samples happen in the background so the user must already be opted
       // in before the trigger is allowed to run.
       return true;
+    case TriggerType::GAIA_PASSWORD_REUSE:
+      // For Gaia password reuses, it is unlikely for users to change opt-in
+      // while the trigger runs, so we require opt-in for collection to avoid
+      // overheads.
+      return true;
   }
   // By default, require opt-in for all triggers.
   return true;

components/safe_browsing/triggers/trigger_throttler.cc

@@ -79,6 +79,7 @@ size_t GetDailyQuotaForTrigger(
     const std::vector<TriggerTypeAndQuotaItem>& trigger_quota_list) {
   switch (trigger_type) {
     case TriggerType::SECURITY_INTERSTITIAL:
+    case TriggerType::GAIA_PASSWORD_REUSE:
       return kUnlimitedTriggerQuota;
     case TriggerType::AD_SAMPLE:
       // These triggers have quota configured via Finch, lookup the value in

components/safe_browsing/triggers/trigger_throttler.h

@@ -21,6 +21,7 @@ extern const char kTriggerTypeAndQuotaParam[];
 enum class TriggerType {
   SECURITY_INTERSTITIAL = 1,
   AD_SAMPLE = 2,
+  GAIA_PASSWORD_REUSE = 3,
 };
 
 struct TriggerTypeHash {