Add finch control of user population in low reputation requests

chrome/browser/safe_browsing/chrome_password_protection_service.cc

Index: chrome/browser/safe_browsing/chrome_password_protection_service.cc
diff --git a/chrome/browser/safe_browsing/chrome_password_protection_service.cc b/chrome/browser/safe_browsing/chrome_password_protection_service.cc
index f979a75967e0cbe8803a9964087a728f4847f0f4..901685e22c0855c9e2b489ef5d3d2bb063d96faf 100644
--- a/chrome/browser/safe_browsing/chrome_password_protection_service.cc
+++ b/chrome/browser/safe_browsing/chrome_password_protection_service.cc
@@ -5,12 +5,17 @@
 #include "chrome/browser/safe_browsing/chrome_password_protection_service.h"
 
 #include "base/feature_list.h"
+#include "base/metrics/field_trial_params.h"
 #include "base/metrics/histogram_macros.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/safe_browsing/safe_browsing_navigation_observer_manager.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
+#include "chrome/browser/sync/profile_sync_service_factory.h"
+#include "components/browser_sync/profile_sync_service.h"
+#include "components/safe_browsing/password_protection/password_protection_request.h"
+#include "components/safe_browsing_db/database_manager.h"
 #include "components/safe_browsing_db/safe_browsing_prefs.h"
 
 using content::BrowserThread;
@@ -22,9 +27,6 @@ namespace {
 const int kPasswordEventAttributionUserGestureLimit = 2;
 }
 
-const base::Feature kPasswordProtectionPingOnly{
-    "PasswordProtectionPingOnly", base::FEATURE_DISABLED_BY_DEFAULT};
-
 ChromePasswordProtectionService::ChromePasswordProtectionService(
     SafeBrowsingService* sb_service,
     Profile* profile)
@@ -41,9 +43,11 @@ ChromePasswordProtectionService::ChromePasswordProtectionService(
 }
 
 ChromePasswordProtectionService::~ChromePasswordProtectionService() {
-  UMA_HISTOGRAM_COUNTS_1000(
-      "PasswordProtection.NumberOfCachedVerdictBeforeShutdown",
-      GetStoredVerdictCount());
+  if (content_settings()) {
+    UMA_HISTOGRAM_COUNTS_1000(
+        "PasswordProtection.NumberOfCachedVerdictBeforeShutdown",
+        GetStoredVerdictCount());
+  }
 }
 
 void ChromePasswordProtectionService::FillReferrerChain(
@@ -72,8 +76,38 @@ bool ChromePasswordProtectionService::IsIncognito() {
   return profile_->IsOffTheRecord();
 }
 
-bool ChromePasswordProtectionService::IsPingingEnabled() {
-  return base::FeatureList::IsEnabled(kPasswordProtectionPingOnly);
+bool ChromePasswordProtectionService::IsPingingEnabled(
+    const base::Feature& feature) {
+  if (!base::FeatureList::IsEnabled(feature)) {
+    return false;
+  }
+
+  bool allowed_incognito =
+      base::GetFieldTrialParamByFeatureAsBool(feature, "incognito", false);
+  if (IsIncognito() && !allowed_incognito)
+    return false;
+
+  bool allowed_extended_reporting = base::GetFieldTrialParamByFeatureAsBool(
+      feature, "extended_reporting", false);
+  if (IsExtendedReporting() && allowed_extended_reporting)
+    return true;  // Ping enabled because this user opted in extended reporting.
+
+  bool allowed_history_sync =
+      base::GetFieldTrialParamByFeatureAsBool(feature, "history_sync", false);
+  if (IsHistorySyncEnabled() && allowed_history_sync)
+    return true;
+
+  return allowed_incognito && allowed_extended_reporting &&

[Nathan Parker, 2017/05/05 20:58:21]

So if we wanted to launch for everyone but not on incognito, I think we'd need
another param "allow_everyone." Is that right? I realize that makes it more
complicated, but we wouldn't want to be stuck unable to launch w/o a code
change.  WDYT?

[Jialiu Lin, 2017/05/05 22:19:32]

Yes, "allow_everyone" is needed for that case.
Code updated accordingly.

+         allowed_history_sync;
+}
+
+bool ChromePasswordProtectionService::IsHistorySyncEnabled() {
+  browser_sync::ProfileSyncService* sync =
+      ProfileSyncServiceFactory::GetInstance()->GetForProfile(profile_);
+  return sync && sync->IsSyncActive() && !sync->IsLocalSyncEnabled() &&
+         sync->GetActiveDataTypes().Has(syncer::HISTORY_DELETE_DIRECTIVES);

[Nathan Parker, 2017/05/05 20:58:21]

What does the HISTORY_DELETE_DIRECTIVES requirement do?  Can you add a comment?

[Jialiu Lin, 2017/05/05 22:19:32]

You got me... I copied this function from web_history_service_factory.cc
https://cs.chromium.org/chromium/src/chrome/browser/history/web_history_servi...

My educated guess: it is a fancy way of saying "HISTORY"  (because it is the
only enum value mentioned "history" in its name in ModelType.)  And the comment
above HISTORY_DELETE_DIRECTIVES isn't very helpful. 
https://cs.chromium.org/chromium/src/components/sync/base/model_type.h?type=c...
 

 }
 
+ChromePasswordProtectionService::ChromePasswordProtectionService()
+    : PasswordProtectionService(nullptr, nullptr, nullptr, nullptr) {}
 }  // namespace safe_browsing