Use network time for bad clock interstitial.

components/ssl_errors/error_classification.cc

Index: components/ssl_errors/error_classification.cc
diff --git a/components/ssl_errors/error_classification.cc b/components/ssl_errors/error_classification.cc
index 7ab390bc62adc8d5e8918e574f77f7654a99c4bf..dcba52c2545e96449401ed98e05496d500dd267c 100644
--- a/components/ssl_errors/error_classification.cc
+++ b/components/ssl_errors/error_classification.cc
@@ -16,6 +16,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
+#include "components/network_time/network_time_tracker.h"
 #include "components/ssl_errors/error_info.h"
 #include "components/url_formatter/url_formatter.h"
 #include "net/base/network_change_notifier.h"
@@ -128,12 +129,10 @@ void RecordUMAStatistics(bool overridable,
                             ssl_errors::ErrorInfo::END_OF_ENUM);
   switch (type) {
     case ssl_errors::ErrorInfo::CERT_DATE_INVALID: {
-      if (IsUserClockInThePast(base::Time::NowFromSystemTime())) {
-        RecordSSLInterstitialCause(overridable, CLOCK_PAST);
-      } else if (IsUserClockInTheFuture(base::Time::NowFromSystemTime())) {
-        RecordSSLInterstitialCause(overridable, CLOCK_FUTURE);
-      } else if (cert.HasExpired() &&
-                 (current_time - cert.valid_expiry()).InDays() < 28) {
+      // Note: not reached when displaying the bad clock interstitial.
+      // See |RecordUMAStatisticsForClockInterstitial| below.
+      if (cert.HasExpired() &&
+          (current_time - cert.valid_expiry()).InDays() < 28) {
         RecordSSLInterstitialCause(overridable, EXPIRED_RECENTLY);
       }
       break;
@@ -181,30 +180,60 @@ void RecordUMAStatistics(bool overridable,
                             net::NetworkChangeNotifier::CONNECTION_LAST);
 }
 
-bool IsUserClockInThePast(const base::Time& time_now) {
-  base::Time build_time;
-  if (!g_testing_build_time.Get().is_null()) {
-    build_time = g_testing_build_time.Get();
+void RecordUMAStatisticsForClockInterstitial(bool overridable,

[estark, 2016/03/16 00:51:36]

Note for future: it seems inevitable that we'll add a new metric in
RecordUMAStatistics() and forget to add it here as well. Maybe (in a future CL)
we should separate out all the metrics that aren't conditioned on the error
(just ssl_error_type and connection_type at the moment) into a helper function
or something.

[mab, 2016/03/17 00:38:09]

Eh, why don't I just get it done.  How does this look?

+                                             ssl_errors::ClockState clock_state,
+                                             int cert_error) {
+  ssl_errors::ErrorInfo::ErrorType type =
+      ssl_errors::ErrorInfo::NetErrorToErrorType(cert_error);
+  DCHECK(type == ssl_errors::ErrorInfo::CERT_DATE_INVALID);
+  UMA_HISTOGRAM_ENUMERATION("interstitial.ssl_error_type", type,
+                            ssl_errors::ErrorInfo::END_OF_ENUM);
+  if (clock_state == ssl_errors::CLOCK_STATE_FUTURE) {
+    RecordSSLInterstitialCause(overridable, CLOCK_FUTURE);
+  } else if (clock_state == ssl_errors::CLOCK_STATE_PAST) {
+    RecordSSLInterstitialCause(overridable, CLOCK_PAST);
   } else {
-    build_time = base::GetBuildTime();
+    NOTREACHED();
   }
-
-  if (time_now < build_time - base::TimeDelta::FromDays(2))
-    return true;
-  return false;
+  UMA_HISTOGRAM_ENUMERATION("interstitial.ssl.connection_type",
+                            net::NetworkChangeNotifier::GetConnectionType(),
+                            net::NetworkChangeNotifier::CONNECTION_LAST);
 }
 
-bool IsUserClockInTheFuture(const base::Time& time_now) {
-  base::Time build_time;
-  if (!g_testing_build_time.Get().is_null()) {
-    build_time = g_testing_build_time.Get();
-  } else {
-    build_time = base::GetBuildTime();
+ClockState GetClockState(
+    const base::Time& now_system,
+    const network_time::NetworkTimeTracker* network_time_tracker) {
+  base::Time now_network;
+  base::TimeDelta uncertainty;
+  const base::TimeDelta kNetworkTimeFudge = base::TimeDelta::FromMinutes(5);
+  ClockState network_state = CLOCK_STATE_UNKNOWN;
+  if (network_time_tracker->GetNetworkTime(&now_network, &uncertainty)) {
+    if (now_system < now_network - uncertainty - kNetworkTimeFudge) {
+      network_state = CLOCK_STATE_PAST;
+    } else if (now_system > now_network + uncertainty + kNetworkTimeFudge) {
+      network_state = CLOCK_STATE_FUTURE;
+    } else {
+      network_state = CLOCK_STATE_OK;
+    }
   }
 
-  if (time_now > build_time + base::TimeDelta::FromDays(365))
-    return true;
-  return false;
+  ClockState build_time_state = CLOCK_STATE_UNKNOWN;
+  base::Time build_time = g_testing_build_time.Get().is_null()
+                              ? base::GetBuildTime()
+                              : g_testing_build_time.Get();
+  if (now_system < build_time - base::TimeDelta::FromDays(2)) {
+    build_time_state = CLOCK_STATE_PAST;
+  } else if (now_system > build_time + base::TimeDelta::FromDays(365)) {
+    build_time_state = CLOCK_STATE_FUTURE;
+  }
+
+  UMA_HISTOGRAM_ENUMERATION("interstitial.ssl.clockstate.network",
+                            network_state, CLOCK_STATE_MAX);
+  UMA_HISTOGRAM_ENUMERATION("interstitial.ssl.clockstate.build_time",
+                            build_time_state, CLOCK_STATE_MAX);
+
+  return network_state == CLOCK_STATE_UNKNOWN ? build_time_state
+                                              : network_state;
 }
 
 void SetBuildTimeForTesting(const base::Time& testing_time) {