Give access to a network time kept in the variation service.

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 82b751050c7f361dc0c06ddddebff08bb3699da9..8a8c8e2f77ccadb6d5de0f282fcc6458d2993884 100644
--- a/chrome/browser/metrics/variations/variations_service.cc
+++ b/chrome/browser/metrics/variations/variations_service.cc
@@ -41,6 +41,15 @@ const int kMaxRetrySeedFetch = 5;
 // Time between seed fetches, in hours.
 const int kSeedFetchPeriodHours = 5;
 
+// TODO(mad): To be moved to NetworkTimeService when available.
+// base::TimeTicks::Now() is documented to have a resolution of
+// ~1-15ms.
+const int64 kTicksResolutionMs = 15;
+
+// For the sources that are supported (HTTP date headers, TLS
+// handshake), the resolution of the server time is 1 second.
+const int64 kServerTimeResolutionMs = 1000;
+
 // Maps Study_Channel enum values to corresponding chrome::VersionInfo::Channel
 // enum values.
 chrome::VersionInfo::Channel ConvertStudyChannelToVersionChannel(
@@ -187,6 +196,19 @@ void VariationsService::StartRepeatedVariationsSeedFetch() {
                this, &VariationsService::FetchVariationsSeed);
 }
 
+bool VariationsService::GetNetworkTime(base::Time* network_time,
+                                       base::TimeDelta* uncertainty) const {
+  if (network_time_.is_null())
+    return false;
+  DCHECK(!network_time_ticks_.is_null());
+  DCHECK(network_time);
+  *network_time = network_time_ + (base::TimeTicks::Now() -
+                                   network_time_ticks_);
+  if (uncertainty)
+    *uncertainty = network_time_uncertainty_;
+  return true;
+}
+
 #if defined(OS_WIN)
 void VariationsService::StartGoogleUpdateRegistrySync() {
   registry_syncer_.RequestRegistrySync();
@@ -256,17 +278,58 @@ void VariationsService::OnURLFetchComplete(const net::URLFetcher* source) {
   }
 
   // Log the response code.
+  int response_code = request->GetResponseCode();

[Alexei Svitkine (slow), 2013/02/01 19:38:55]

Nit: const

[MAD, 2013/02/01 22:01:41]

Done.

   UMA_HISTOGRAM_CUSTOM_ENUMERATION("Variations.SeedFetchResponseCode",
-      net::HttpUtil::MapStatusCodeForHistogram(request->GetResponseCode()),
+      net::HttpUtil::MapStatusCodeForHistogram(response_code),
       net::HttpUtil::GetStatusCodesForHistogram());
 
   const base::TimeDelta latency =
       base::TimeTicks::Now() - last_request_started_time_;
 
-  if (request->GetResponseCode() != 200) {
+  base::Time response_date;
+  if (response_code == 200 || response_code == 304) {

[Alexei Svitkine (slow), 2013/02/01 19:38:55]

Can you change the code to use the constants from net/http/http_status_code.h?

[MAD, 2013/02/01 22:01:41]

Done.

[MAD, 2013/02/01 22:01:41]

Done.

+    bool success = request->GetResponseHeaders()->GetDateValue(&response_date);
+    DCHECK(success || response_date.is_null());
+
+    if (!response_date.is_null()) {
+#ifndef NDEBUG
+      if (!network_time_.is_null()) {
+        // Check that we are tracking time correctly.
+        base::Time current_network_time;
+        base::TimeDelta uncertainty;
+        success = GetNetworkTime(&current_network_time, &uncertainty);
+        DCHECK(success);
+        // Account for response_date; s own innaccuracy.
+        uncertainty +=
+            base::TimeDelta::FromMilliseconds(kServerTimeResolutionMs) +
+            latency + 2 * base::TimeDelta::FromMilliseconds(kTicksResolutionMs);
+        DCHECK(current_network_time >= response_date - uncertainty);
+        DCHECK(current_network_time <= response_date + uncertainty);
+        DVLOG(1) << current_network_time.ToInternalValue() << " VS "
+                 << response_date.ToInternalValue() << ", is within: "
+                 << uncertainty.ToInternalValue();
+      }
+#endif  // ifndef NDEBUG
+      // Update network time on every request to limit dependency on ticks lag.
+      // TODO(mad): Find a heuristic to avoid augmenting the
+      // network_time_uncertainty_ too much by a particularly long latency.
+      // Maybe only update when we either improve accuracy or drifted too far
+      // from response_date.
+      network_time_ = response_date;
+      // Estimate that the time was set midway through the latency time.
+      network_time_ticks_ = base::TimeTicks::Now() - latency / 2;
+      // We can't assume a better time than the resolution of the server time
+      // and we involve 4 ticks value, each with their own uncertainty, 1 & 2
+      // are the ones used to compute the latency, 3 is the Now() above and 4
+      // will be the Now() used in GetNetworkTime().
+      network_time_uncertainty_ =
+          base::TimeDelta::FromMilliseconds(kServerTimeResolutionMs) +
+          latency + 4 * base::TimeDelta::FromMilliseconds(kTicksResolutionMs);
+    }  // if (!response_date.is_null())
+  } else if (response_code != 200) {
     DVLOG(1) << "Variations server request returned non-200 response code: "
-             << request->GetResponseCode();
-    if (request->GetResponseCode() == 304)
+             << response_code;
+    if (response_code == 304)
       UMA_HISTOGRAM_MEDIUM_TIMES("Variations.FetchNotModifiedLatency", latency);
     else
       UMA_HISTOGRAM_MEDIUM_TIMES("Variations.FetchOtherLatency", latency);
@@ -278,10 +341,6 @@ void VariationsService::OnURLFetchComplete(const net::URLFetcher* source) {
   bool success = request->GetResponseAsString(&seed_data);
   DCHECK(success);
 
-  base::Time response_date;
-  success = request->GetResponseHeaders()->GetDateValue(&response_date);
-  DCHECK(success || response_date.is_null());
-
   StoreSeedData(seed_data, response_date, g_browser_process->local_state());
 }