Make Sber1/2 pref metrics into Nullable Booleans so we can track how often these prefs are unset.

components/safe_browsing_db/safe_browsing_prefs.cc

 // Copyright (c) 2016 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 "base/command_line.h"
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
 #include "components/prefs/pref_service.h"
 #include "components/safe_browsing_db/safe_browsing_prefs.h"
 
@@ skipping 39 matching lines @@
 // The Extended Reporting pref that is currently active, used for UMA metrics.
 // These values are written to logs.  New enum values can be added, but
 // existing enums must never be renumbered or deleted and reused.
 enum ActiveExtendedReportingPref {
   SBER1_PREF = 0,
   SBER2_PREF = 1,
   // New prefs must be added before MAX_SBER_PREF
   MAX_SBER_PREF
 };
 
+// A histogram for tracking a nullable boolean, which can be false, true or
+// null. These values are written to logs. New enum values can be added, but
+// existing enums must never be renumbered or deleted and reused.
+enum NullableBoolean {
+  NULLABLE_BOOLEAN_FALSE = 0,
+  NULLABLE_BOOLEAN_TRUE = 1,
+  NULLABLE_BOOLEAN_NULL = 2,
+  MAX_NULLABLE_BOOLEAN
+};
+
+NullableBoolean GetPrefValueOrNull(const PrefService& prefs,
+                                   const std::string& pref_name) {
+  if (!prefs.HasPrefPath(pref_name)) {
+    return NULLABLE_BOOLEAN_NULL;
+  }
+  return prefs.GetBoolean(pref_name) ? NULLABLE_BOOLEAN_TRUE
+                                     : NULLABLE_BOOLEAN_FALSE;
+}
+
 // Update the correct UMA metric based on which pref was changed and which UI
 // the change was made on.
 void RecordExtendedReportingPrefChanged(
     const PrefService& prefs,
     safe_browsing::ExtendedReportingOptInLocation location) {
   bool pref_value = safe_browsing::IsExtendedReportingEnabled(prefs);

[Jialiu Lin, 2017/03/07 18:07:12]

how about IsExtendedReportingEnabled() function? Maybe we can change its return
type to NullableBoolean as well?

[lpz, 2017/03/07 19:34:54]

I'm not sure the False vs Null distinction is relevant for this function.
Callers are trying to decide "should I do some reporting?", so False and Null
mean the same thing I think.

 
   if (safe_browsing::IsScout(prefs)) {
     switch (location) {
       case safe_browsing::SBER_OPTIN_SITE_CHROME_SETTINGS:
         UMA_HISTOGRAM_BOOLEAN(
             "SafeBrowsing.Pref.Scout.SetPref.SBER2Pref.ChromeSettings",
             pref_value);
         break;
       case safe_browsing::SBER_OPTIN_SITE_ANDROID_SETTINGS:
         UMA_HISTOGRAM_BOOLEAN(
@@ skipping 196 matching lines @@
 void RecordExtendedReportingMetrics(const PrefService& prefs) {
   // This metric tracks the extended browsing opt-in based on whichever setting
   // the user is currently seeing. It tells us whether extended reporting is
   // happening for this user.
   UMA_HISTOGRAM_BOOLEAN("SafeBrowsing.Pref.Extended",
                         IsExtendedReportingEnabled(prefs));
 
   // These metrics track the Scout transition.
   if (prefs.GetBoolean(prefs::kSafeBrowsingScoutGroupSelected)) {
     // Users in the Scout group: currently seeing the Scout opt-in.
-    UMA_HISTOGRAM_BOOLEAN(
+    UMA_HISTOGRAM_ENUMERATION(
         "SafeBrowsing.Pref.Scout.ScoutGroup.SBER1Pref",
-        prefs.GetBoolean(prefs::kSafeBrowsingExtendedReportingEnabled));
-    UMA_HISTOGRAM_BOOLEAN(
+        GetPrefValueOrNull(prefs, prefs::kSafeBrowsingExtendedReportingEnabled),
+        MAX_NULLABLE_BOOLEAN);
+    UMA_HISTOGRAM_ENUMERATION(
         "SafeBrowsing.Pref.Scout.ScoutGroup.SBER2Pref",
-        prefs.GetBoolean(prefs::kSafeBrowsingScoutReportingEnabled));
+        GetPrefValueOrNull(prefs, prefs::kSafeBrowsingScoutReportingEnabled),
+        MAX_NULLABLE_BOOLEAN);
   } else {
     // Users not in the Scout group: currently seeing the SBER opt-in.
-    UMA_HISTOGRAM_BOOLEAN(
+    UMA_HISTOGRAM_ENUMERATION(
         "SafeBrowsing.Pref.Scout.NoScoutGroup.SBER1Pref",
-        prefs.GetBoolean(prefs::kSafeBrowsingExtendedReportingEnabled));
+        GetPrefValueOrNull(prefs, prefs::kSafeBrowsingExtendedReportingEnabled),
+        MAX_NULLABLE_BOOLEAN);
     // The following metric is a corner case. User was previously in the
     // Scout group and was able to opt-in to the Scout pref, but was since
     // removed from the Scout group (eg: by rolling back a Scout experiment).
-    UMA_HISTOGRAM_BOOLEAN(
+    UMA_HISTOGRAM_ENUMERATION(
         "SafeBrowsing.Pref.Scout.NoScoutGroup.SBER2Pref",
-        prefs.GetBoolean(prefs::kSafeBrowsingScoutReportingEnabled));
+        GetPrefValueOrNull(prefs, prefs::kSafeBrowsingScoutReportingEnabled),
+        MAX_NULLABLE_BOOLEAN);
   }
 }
 
 void SetExtendedReportingPrefAndMetric(
     PrefService* prefs,
     bool value,
     ExtendedReportingOptInLocation location) {
   prefs->SetBoolean(GetExtendedReportingPrefName(*prefs), value);
   RecordExtendedReportingPrefChanged(*prefs, location);
 }
@@ skipping 71 matching lines @@
   if (base::FeatureList::IsEnabled(kCanShowScoutOptIn) &&
       !prefs->GetBoolean(prefs::kSafeBrowsingScoutGroupSelected)) {
     prefs->SetBoolean(prefs::kSafeBrowsingScoutGroupSelected, true);
     UMA_HISTOGRAM_ENUMERATION(kScoutTransitionMetricName,
                               CAN_SHOW_SCOUT_OPT_IN_SAW_FIRST_INTERSTITIAL,
                               MAX_REASONS);
   }
 }
 
 }  // namespace safe_browsing