Revert "Send ThreatDetails report after user done interacting with modal warning"

This reverts commit d1c5cff3.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> Send ThreatDetails report after user done interacting with modal warning
> 
> Bug: 761114
> Change-Id: I8904520f027a6dba423da4002df5191683e95ae0
> Reviewed-on: https://chromium-review.googlesource.com/683245
> Reviewed-by: Luke Z <lpz@chromium.org>
> Commit-Queue: Jialiu Lin <jialiul@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#504842}

TBR=jialiul@chromium.org,lpz@chromium.org

Change-Id: I173227275848903f32f1bd25a90a129d27c45d53
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 761114
Reviewed-on: https://chromium-review.googlesource.com/688645Reviewed-by: Jialiu Lin <jialiul@chromium.org>
Commit-Queue: Jialiu Lin <jialiul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504866}

chrome/browser/safe_browsing/chrome_password_protection_service.cc

@@ -30,7 +30,6 @@
 #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"
@@ -113,7 +112,6 @@ 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) {
@@ -183,7 +181,7 @@ void ChromePasswordProtectionService::FillReferrerChain(
 
 void ChromePasswordProtectionService::ShowModalWarning(
     content::WebContents* web_contents,
-    const std::string& verdict_token) {
+    const std::string& unused_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);
@@ -197,8 +195,6 @@ 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.
@@ -242,8 +238,7 @@ void ChromePasswordProtectionService::OnUserAction(
           NOTREACHED();
           break;
       }
-      MaybeFinishCollectingThreatDetails(
-          web_contents, action == PasswordProtectionService::CHANGE_PASSWORD);
+      // TODO(jialiul): Sends post-warning reporting.
       break;
     case PasswordProtectionService::CHROME_SETTINGS:
       DCHECK_EQ(PasswordProtectionService::CHANGE_PASSWORD, action);
@@ -266,46 +261,6 @@ 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->GetRenderProcessHost()->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();
 }
@@ -616,7 +571,6 @@ 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,7 +10,6 @@
 #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"
@@ -97,16 +96,6 @@ 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_;
   }
@@ -208,7 +197,6 @@ 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,35 +42,26 @@ class ChromePasswordProtectionServiceBrowserTest : public InProcessBrowserTest {
     InProcessBrowserTest::SetUp();
   }
 
-  ChromePasswordProtectionService* GetService(bool is_incognito) {
+  ChromePasswordProtectionService* GetService() {
     return ChromePasswordProtectionService::GetPasswordProtectionService(
-        is_incognito ? browser()->profile()->GetOffTheRecordProfile()
-                     : browser()->profile());
+        browser()->profile());
   }
 
-  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 SimulateGaiaPasswordChange() {
+    browser()->profile()->GetPrefs()->SetString(
+        password_manager::prefs::kSyncPasswordHash, "new_password_hash");
   }
 
-  void SimulateAction(ChromePasswordProtectionService* service,
-                      ChromePasswordProtectionService::WarningUIType ui_type,
+  void SimulateAction(ChromePasswordProtectionService::WarningUIType ui_type,
                       ChromePasswordProtectionService::WarningAction action) {
-    for (auto& observer : service->observer_list_) {
+    for (auto& observer : GetService()->observer_list_) {
       if (ui_type == observer.GetObserverType()) {
         observer.InvokeActionForTesting(action);
       }
     }
   }
 
-  void SimulateGaiaPasswordChanged(ChromePasswordProtectionService* service) {
-    service->OnGaiaPasswordChanged();
-  }
+  void SimulateGaiaPasswordChanged() { GetService()->OnGaiaPasswordChanged(); }
 
   void GetSecurityInfo(content::WebContents* web_contents,
                        security_state::SecurityInfo* out_security_info) {
@@ -79,8 +70,8 @@ class ChromePasswordProtectionServiceBrowserTest : public InProcessBrowserTest {
     helper->GetSecurityInfo(out_security_info);
   }
 
-  void SetDefaultProfileEmail(ChromePasswordProtectionService* service) {
-    service->account_info_->email = "foo@bar.com";
+  void SetDefaultProfileEmail() {
+    GetService()->account_info_->email = "foo@bar.com";
   }
 
  protected:
@@ -90,8 +81,8 @@ class ChromePasswordProtectionServiceBrowserTest : public InProcessBrowserTest {
 
 IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
                        SuccessfullyChangePassword) {
-  ChromePasswordProtectionService* service = GetService(/*is_incognito=*/false);
-  SetDefaultProfileEmail(service);
+  ChromePasswordProtectionService* service = GetService();
+  SetDefaultProfileEmail();
   Profile* profile = browser()->profile();
   content::WebContents* web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
@@ -121,7 +112,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(service, ChromePasswordProtectionService::MODAL_DIALOG,
+  SimulateAction(ChromePasswordProtectionService::MODAL_DIALOG,
                  ChromePasswordProtectionService::CHANGE_PASSWORD);
   content::WebContents* new_web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
@@ -135,7 +126,7 @@ IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
             new_web_contents->GetVisibleURL());
 
   // Simulates clicking "Change password" button on the chrome://settings card.
-  SimulateAction(service, ChromePasswordProtectionService::CHROME_SETTINGS,
+  SimulateAction(ChromePasswordProtectionService::CHROME_SETTINGS,
                  ChromePasswordProtectionService::CHANGE_PASSWORD);
   base::RunLoop().RunUntilIdle();
   // Verify myaccount.google.com or Google signin page should be opened in a
@@ -148,7 +139,7 @@ IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
                   .DomainIs("google.com"));
 
   // Simulates user finished changing password.
-  SimulateGaiaPasswordChanged(service);
+  SimulateGaiaPasswordChanged();
   base::RunLoop().RunUntilIdle();
   EXPECT_FALSE(
       ChromePasswordProtectionService::ShouldShowChangePasswordSettingUI(
@@ -161,7 +152,7 @@ IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
 
 IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
                        MarkSiteAsLegitimate) {
-  ChromePasswordProtectionService* service = GetService(/*is_incognito=*/false);
+  ChromePasswordProtectionService* service = GetService();
   Profile* profile = browser()->profile();
   content::WebContents* web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
@@ -191,7 +182,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(service, ChromePasswordProtectionService::MODAL_DIALOG,
+  SimulateAction(ChromePasswordProtectionService::MODAL_DIALOG,
                  ChromePasswordProtectionService::IGNORE_WARNING);
   base::RunLoop().RunUntilIdle();
   // No new tab opens. SecurityInfo doesn't change.
@@ -220,7 +211,7 @@ IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
 
 IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
                        OpenChromeSettingsViaPageInfo) {
-  ChromePasswordProtectionService* service = GetService(/*is_incognito=*/false);
+  ChromePasswordProtectionService* service = GetService();
   Profile* profile = browser()->profile();
   content::WebContents* web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
@@ -231,7 +222,7 @@ IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
   service->ShowModalWarning(web_contents, "unused_token");
   base::RunLoop().RunUntilIdle();
   // Simulates clicking "Ignore" to close dialog.
-  SimulateAction(service, ChromePasswordProtectionService::MODAL_DIALOG,
+  SimulateAction(ChromePasswordProtectionService::MODAL_DIALOG,
                  ChromePasswordProtectionService::IGNORE_WARNING);
   base::RunLoop().RunUntilIdle();
   ASSERT_TRUE(
@@ -262,7 +253,7 @@ IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
 IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
                        VeriryUnhandledPasswordReuse) {
   histograms_.ExpectTotalCount(kGaiaPasswordChangeHistogramName, 0);
-  ChromePasswordProtectionService* service = GetService(/*is_incognito=*/false);
+  ChromePasswordProtectionService* service = GetService();
   ASSERT_TRUE(service);
   Profile* profile = browser()->profile();
   ui_test_utils::NavigateToURL(browser(), embedded_test_server()->GetURL("/"));
@@ -282,7 +273,7 @@ IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
           profile));
 
   // Opens a new browser window.
-  Browser* browser2 = CreateBrowser(profile);
+  Browser* browser2 = CreateBrowser(browser()->profile());
   // Shows modal dialog on this new web_contents.
   content::WebContents* new_web_contents =
       browser2->tab_strip_model()->GetActiveWebContents();
@@ -295,7 +286,7 @@ IN_PROC_BROWSER_TEST_F(ChromePasswordProtectionServiceBrowserTest,
           profile));
 
   // Simulates a Gaia password change.
-  SimulateGaiaPasswordChange(/*is_incognito=*/false);
+  SimulateGaiaPasswordChange();
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(0u, service->unhandled_password_reuses().size());
   EXPECT_FALSE(

components/safe_browsing/browser/threat_details.cc

@@ -85,8 +85,6 @@ 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,9 +25,6 @@ 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;
@@ -43,11 +40,6 @@ 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,7 +79,6 @@ 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,7 +21,6 @@ extern const char kTriggerTypeAndQuotaParam[];
 enum class TriggerType {
   SECURITY_INTERSTITIAL = 1,
   AD_SAMPLE = 2,
-  GAIA_PASSWORD_REUSE = 3,
 };
 
 struct TriggerTypeHash {