Add 4 Variations.SeedDateSkew.* histograms to analyze whether the system clock or the build time co…

chrome/browser/metrics/variations/variations_service.cc

Index: chrome/browser/metrics/variations/variations_service.cc
diff --git a/chrome/browser/metrics/variations/variations_service.cc b/chrome/browser/metrics/variations/variations_service.cc
index 8a56747038c32a04bbc09bcb76f8f3c54bdffba5..44cc5dfc5d6c4b56b2cad90f6643d8699870a018 100644
--- a/chrome/browser/metrics/variations/variations_service.cc
+++ b/chrome/browser/metrics/variations/variations_service.cc
@@ -276,12 +276,13 @@ bool VariationsService::CreateTrialsFromSeed() {
       GetHardwareClass(),
       base::Bind(&OverrideUIString));
 
+  const base::Time now = base::Time::Now();
+
   // Log the "freshness" of the seed that was just used. The freshness is the
   // time between the last successful seed download and now.
   const int64 last_fetch_time_internal =
       local_state_->GetInt64(prefs::kVariationsLastFetchTime);
   if (last_fetch_time_internal) {
-    const base::Time now = base::Time::Now();
     const base::TimeDelta delta =
         now - base::Time::FromInternalValue(last_fetch_time_internal);
     // Log the value in number of minutes.
@@ -289,6 +290,46 @@ bool VariationsService::CreateTrialsFromSeed() {
         1, base::TimeDelta::FromDays(30).InMinutes(), 50);
   }
 
+  // Log the skew between the seed date and the system clock/build time to
+  // analyze whether either could be used to make old variations seeds expire
+  // after some time.
+  const int64 seed_date_internal =
+      local_state_->GetInt64(prefs::kVariationsSeedDate);

[Alexei Svitkine (slow), 2014/10/02 13:00:32]

Note that if we do start using this to make decisions, we'll need to protect
this pref in some way (probably instead of storing it separately, make it a
field in the seed that's received). But we can do that.

[gab, 2014/10/02 14:03:23]

Right that makes sense, we can discuss implementation details if this proves to
be worthwhile avenue.

+  if (seed_date_internal) {
+    const base::Time seed_date =
+        base::Time::FromInternalValue(seed_date_internal);
+    const int system_clock_delta_days = (now - seed_date).InDays();
+    if (system_clock_delta_days < 0) {
+      UMA_HISTOGRAM_CUSTOM_COUNTS("Variations.SeedDateSkew.SystemClockBehindBy",
+                                  -system_clock_delta_days,
+                                  1,
+                                  100,
+                                  30);
+    } else {
+      UMA_HISTOGRAM_CUSTOM_COUNTS("Variations.SeedDateSkew.SystemClockAheadBy",
+                                  system_clock_delta_days,
+                                  0,
+                                  100,
+                                  30);
+    }
+
+    const int build_time_delta_days =
+        (base::GetBuildTime() - seed_date).InDays();
+    if (build_time_delta_days < 0) {
+      UMA_HISTOGRAM_CUSTOM_COUNTS("Variations.SeedDateSkew.BuildTimeBehindBy",
+                                  -build_time_delta_days,
+                                  1,
+                                  100,
+                                  30);
+    } else {
+      UMA_HISTOGRAM_CUSTOM_COUNTS("Variations.SeedDateSkew.BuildTimeAheadBy",

[Alexei Svitkine (slow), 2014/10/02 13:00:32]

I suggest using UMA_HISTOGRAM_COUNTS_100() for all of these, instead of a custom
macro that's not very different.

[gab, 2014/10/02 14:03:23]

UMA_HISTOGRAM_COUNTS_100 is from 1 to 100; I assume that includes 0 despite the
range suggesting it doesn't?

[Alexei Svitkine (slow), 2014/10/02 14:05:00]

Yes, there's always an underflow bucket - in this case 0 to 1.

+                                  build_time_delta_days,
+                                  0,
+                                  100,
+                                  30);
+    }
+  }
+
   return true;
 }