Add UMA metrics to phishguard pings

chrome/browser/safe_browsing/chrome_password_protection_service.cc

 // 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/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;
 
 namespace safe_browsing {
 
 namespace {
+
 // The number of user gestures we trace back for login event attribution.
 const int kPasswordEventAttributionUserGestureLimit = 2;
-}
+
+}  // namespace
 
 ChromePasswordProtectionService::ChromePasswordProtectionService(
     SafeBrowsingService* sb_service,
     Profile* profile)
     : PasswordProtectionService(
           sb_service->database_manager(),
           sb_service->url_request_context(),
           HistoryServiceFactory::GetForProfile(
               profile,
               ServiceAccessType::EXPLICIT_ACCESS),
@@ skipping 32 matching lines @@
   return IsExtendedReportingEnabled(*profile_->GetPrefs());
 }
 
 bool ChromePasswordProtectionService::IsIncognito() {
   DCHECK(profile_);
   return profile_->IsOffTheRecord();
 }
 
 bool ChromePasswordProtectionService::IsPingingEnabled(
     const base::Feature& feature) {
+  bool is_password_entry_ping =
+      feature.name == kProtectedPasswordEntryPinging.name;

[Nathan Parker, 2017/05/11 23:36:31]

Add a DCHECK() that feature is one of the two expected feature types, to ensure
we modify this code if we add another.

[Jialiu Lin, 2017/05/12 02:12:32]

Good point.

   if (!base::FeatureList::IsEnabled(feature)) {
+    if (is_password_entry_ping) {
+      UMA_HISTOGRAM_ENUMERATION(kPasswordEntryRequestOutcomeHistogramName,
+                                DISABLED_DUE_TO_FEATURE_DISABLED, MAX_OUTCOME);
+    } else {
+      UMA_HISTOGRAM_ENUMERATION(kPasswordOnFocusRequestOutcomeHistogramName,
+                                DISABLED_DUE_TO_FEATURE_DISABLED, MAX_OUTCOME);
+    }
     return false;
   }
 
   bool allowed_incognito =
       base::GetFieldTrialParamByFeatureAsBool(feature, "incognito", false);
-  if (IsIncognito() && !allowed_incognito)
+  if (IsIncognito() && !allowed_incognito) {
+    if (is_password_entry_ping) {
+      UMA_HISTOGRAM_ENUMERATION(kPasswordEntryRequestOutcomeHistogramName,

[Nathan Parker, 2017/05/11 23:36:31]

An idea (you decide if this is any better): You could simplify this code to
return the DISABLED_DUE_TO_* value and have the caller (a wrapper function) put
that in the appropriate histogram.  If the value is SUCCESS (or whatever enum is
appropriate), then the caller would return "true".

[Jialiu Lin, 2017/05/12 02:12:32]

SG. code refactored.

+                                DISABLED_DUE_TO_INCOGNITO, MAX_OUTCOME);
+    } else {
+      UMA_HISTOGRAM_ENUMERATION(kPasswordOnFocusRequestOutcomeHistogramName,
+                                DISABLED_DUE_TO_INCOGNITO, MAX_OUTCOME);
+    }
     return false;
+  }
 
   bool allowed_all_population =
       base::GetFieldTrialParamByFeatureAsBool(feature, "all_population", false);
   if (!allowed_all_population) {
     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;
+
+    if (is_password_entry_ping) {
+      UMA_HISTOGRAM_ENUMERATION(kPasswordEntryRequestOutcomeHistogramName,
+                                DISABLED_DUE_TO_USER_POPULATION, MAX_OUTCOME);
+    } else {
+      UMA_HISTOGRAM_ENUMERATION(kPasswordOnFocusRequestOutcomeHistogramName,
+                                DISABLED_DUE_TO_USER_POPULATION, MAX_OUTCOME);
+    }
   }
 
   return allowed_all_population;
 }
 
 bool ChromePasswordProtectionService::IsHistorySyncEnabled() {
   browser_sync::ProfileSyncService* sync =
       ProfileSyncServiceFactory::GetInstance()->GetForProfile(profile_);
   return sync && sync->IsSyncActive() && !sync->IsLocalSyncEnabled() &&
          sync->GetActiveDataTypes().Has(syncer::HISTORY_DELETE_DIRECTIVES);
 }
 
 ChromePasswordProtectionService::ChromePasswordProtectionService()
     : PasswordProtectionService(nullptr, nullptr, nullptr, nullptr) {}
 }  // namespace safe_browsing