Log the freshness of the Variations seed in a histogram.

chrome/browser/metrics/variations/variations_service.cc

 // Copyright (c) 2012 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/metrics/variations/variations_service.h"
 
 #include <set>
 
 #include "base/base64.h"
 #include "base/build_time.h"
@@ skipping 159 matching lines @@
     return false;
 
   chrome::VersionInfo::Channel channel = GetChannelForVariations();
   for (int i = 0; i < seed.study_size(); ++i) {
     if (ShouldAddStudy(seed.study(i), current_version_info, reference_date,
                        channel)) {
       CreateTrialFromStudy(seed.study(i), reference_date);
     }
   }
 
+  // Log the "freshness" of the seed that was just used. The freshness is the
+  // time between the last successful seed download and now.
+  base::Time now = base::Time::Now();

[Alexei Svitkine (slow), 2013/02/22 16:26:25]

Move this into the if statement. Make it const.

[SteveT, 2013/02/22 18:12:03]

Done.

+  int64 last_fetch_time_internal =

[Alexei Svitkine (slow), 2013/02/22 16:26:25]

Nit: const.

[SteveT, 2013/02/22 18:12:03]

Done.

+      local_state_->GetInt64(prefs::kVariationsLastFetchTime);
+  if (last_fetch_time_internal) {
+    DCHECK_GE(now.ToInternalValue(), last_fetch_time_internal);

[Alexei Svitkine (slow), 2013/02/22 16:26:25]

This may happen if the user changed his system time between, which is something
that could happen (and can't really be avoided). Don't DCHECK on it.

I think we can just log these anyway - so that we have the info about them in
the logs (assuming the histogram supports negative values).

[SteveT, 2013/02/22 18:12:03]

Removed the DCHECK.

I think the histogram might just log it in the smallest bucket. Ilya, can you
comment on what happens if we pass a negative sample into a histogram?

[Ilya Sherman, 2013/02/22 23:26:16]

Yes, negative values end up in the lowest bucket, which is the underflow bucket.

+    base::TimeDelta delta =

[Alexei Svitkine (slow), 2013/02/22 16:26:25]

Nit: const.

[SteveT, 2013/02/22 18:12:03]

Done.

+        now - base::Time::FromInternalValue(last_fetch_time_internal);
+    // Log the value in number of minutes.
+    UMA_HISTOGRAM_CUSTOM_COUNTS("Variations.SeedFreshness", delta.InMinutes(),
+        1, base::TimeDelta::FromDays(7).InMinutes(), 50);

[Alexei Svitkine (slow), 2013/02/22 16:26:25]

I would say let's use 30 days, just to get a more complete story (for folks who
don't relaunch their browser as much).

How did you come up with the 50 for the last parameter?

[SteveT, 2013/02/22 18:12:03]

Done.
UMA_HISTOGRAM_CUSTOM_COUNTS_10000 has 50 buckets. 7 days = 10080 minutes and our
unit is in minutes. We didn't use UMA_HISTOGRAM_CUSTOM_COUNTS_10000 because
expanding out the InMinutes() here makes the code easier to read.

+  }
+
   return true;
 }
 
 void VariationsService::StartRepeatedVariationsSeedFetch() {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
   // Check that |CreateTrialsFromSeed| was called, which is necessary to
   // retrieve the serial number that will be sent to the server.
   DCHECK(create_trials_from_seed_called_);
 
@@ skipping 18 matching lines @@
 
 void VariationsService::SetCreateTrialsFromSeedCalledForTesting(bool called) {
   create_trials_from_seed_called_ = called;
 }
 
 // static
 void VariationsService::RegisterPrefs(PrefRegistrySimple* registry) {
   registry->RegisterStringPref(prefs::kVariationsSeed, std::string());
   registry->RegisterInt64Pref(prefs::kVariationsSeedDate,
                               base::Time().ToInternalValue());
+  registry->RegisterInt64Pref(prefs::kVariationsLastFetchTime, 0);
 }
 
 // static
 VariationsService* VariationsService::Create(PrefService* local_state) {
 // This is temporarily disabled for Android. See http://crbug.com/168224
 #if !defined(GOOGLE_CHROME_BUILD) || defined(OS_ANDROID)
   // Unless the URL was provided, unsupported builds should return NULL to
   // indicate that the service should not be used.
   if (!CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kVariationsServerURL))
@@ skipping 53 matching lines @@
       response_code == net::HTTP_NOT_MODIFIED) {
     bool success = request->GetResponseHeaders()->GetDateValue(&response_date);
     DCHECK(success || response_date.is_null());
 
     if (!response_date.is_null()) {
       network_time_tracker_.UpdateNetworkTime(
           response_date,
           base::TimeDelta::FromMilliseconds(kServerTimeResolutionMs),
           latency);
     }
+
+    // Record the time of the most recent successful fetch.
+    local_state_->SetInt64(prefs::kVariationsLastFetchTime,
+                           base::Time::Now().ToInternalValue());
   }
 
   if (response_code != net::HTTP_OK) {
     DVLOG(1) << "Variations server request returned non-HTTP_OK response code: "
              << response_code;
     if (response_code == net::HTTP_NOT_MODIFIED)
       UMA_HISTOGRAM_MEDIUM_TIMES("Variations.FetchNotModifiedLatency", latency);
     else
       UMA_HISTOGRAM_MEDIUM_TIMES("Variations.FetchOtherLatency", latency);
     return;
@@ skipping 294 matching lines @@
                                       variation_id);
     }
   }
 
   trial->SetForced();
   if (IsStudyExpired(study, reference_date))
     trial->Disable();
 }
 
 }  // namespace chrome_variations