When network time is unavailable, record the reason in UMA

components/ssl_errors/error_classification.cc

 // Copyright 2015 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 "components/ssl_errors/error_classification.h"
 
 #include <limits.h>
 #include <stddef.h>
 
 #include <vector>
@@ skipping 20 matching lines @@
 #include "base/win/windows_version.h"
 #endif
 
 using base::Time;
 using base::TimeTicks;
 using base::TimeDelta;
 
 namespace ssl_errors {
 namespace {
 
+// Describes the result of getting network time and if it was
+// unavailable, why it was unavailable. This enum is being histogrammed
+// so do not reorder or remove values.
+enum NetworkClockState {
+  // The clock state relative to network time is unknown because the
+  // NetworkTimeTracker has no information from the network.
+  NETWORK_CLOCK_STATE_UNKNOWN_NO_SYNC = 0,
+  // The clock state relative to network time is unknown because the
+  // user's clock has fallen out of sync with the latest information
+  // from the network (due to e.g. suspend/resume).
+  NETWORK_CLOCK_STATE_UNKNOWN_SYNC_LOST,
+  // The clock is "close enough" to the network time.
+  NETWORK_CLOCK_STATE_OK,
+  // The clock is in the past relative to network time.
+  NETWORK_CLOCK_STATE_PAST,
+  // The clock is in the future relative to network time.
+  NETWORK_CLOCK_STATE_FUTURE,
+  NETWORK_CLOCK_STATE_MAX
+};
+
 // Events for UMA. Do not reorder or change!
 enum SSLInterstitialCause {
   CLOCK_PAST,
   CLOCK_FUTURE,
   WWW_SUBDOMAIN_MATCH,
   SUBDOMAIN_MATCH,
   SUBDOMAIN_INVERSE_MATCH,
   SUBDOMAIN_OUTSIDE_WILDCARD,
   HOST_NAME_NOT_KNOWN_TLD,
   LIKELY_MULTI_TENANT_HOSTING,
@@ skipping 149 matching lines @@
     NOTREACHED();
   }
 }
 
 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;
-    }
+  NetworkClockState network_state = NETWORK_CLOCK_STATE_MAX;
+  network_time::NetworkTimeTracker::NetworkTimeResult network_time_result =
+      network_time_tracker->GetNetworkTime(&now_network, &uncertainty);
+  switch (network_time_result) {
+    case network_time::NetworkTimeTracker::NETWORK_TIME_AVAILABLE:
+      if (now_system < now_network - uncertainty - kNetworkTimeFudge) {
+        network_state = NETWORK_CLOCK_STATE_PAST;
+      } else if (now_system > now_network + uncertainty + kNetworkTimeFudge) {
+        network_state = NETWORK_CLOCK_STATE_FUTURE;

[mab, 2016/08/16 19:04:52]

I'm not sure how to fix this, but it sounds a bit funny.  It's not the network
clock that's in the future, but the system clock!

[estark, 2016/08/18 12:15:11]

Changed it to NETWORK_CLOCK_STATE_CLOCK_IN_FUTURE (/PAST) which is kinda clunky
but hopefully slightly less confusing.

[mab, 2016/08/19 01:51:07]

_DINOSAURS and _FLYING_CARS?

+      } else {
+        network_state = NETWORK_CLOCK_STATE_OK;
+      }
+      break;
+    case network_time::NetworkTimeTracker::NETWORK_TIME_SYNC_LOST:
+      network_state = NETWORK_CLOCK_STATE_UNKNOWN_SYNC_LOST;
+      break;
+    case network_time::NetworkTimeTracker::NETWORK_TIME_NO_SYNC:
+      network_state = NETWORK_CLOCK_STATE_UNKNOWN_NO_SYNC;
+      break;
   }
 
   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.network2",
+                            network_time_result, NETWORK_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;
+  switch (network_state) {
+    case NETWORK_CLOCK_STATE_UNKNOWN_NO_SYNC:
+    case NETWORK_CLOCK_STATE_UNKNOWN_SYNC_LOST:
+      return build_time_state;
+    case NETWORK_CLOCK_STATE_OK:
+      return CLOCK_STATE_OK;
+    case NETWORK_CLOCK_STATE_PAST:
+      return CLOCK_STATE_PAST;
+    case NETWORK_CLOCK_STATE_FUTURE:
+      return CLOCK_STATE_FUTURE;
+    case NETWORK_CLOCK_STATE_MAX:
+      NOTREACHED();
+      return CLOCK_STATE_UNKNOWN;
+  }
+
+  NOTREACHED();
+  return CLOCK_STATE_UNKNOWN;
 }
 
 void SetBuildTimeForTesting(const base::Time& testing_time) {
   g_testing_build_time.Get() = testing_time;
 }
 
 bool IsHostNameKnownTLD(const std::string& host_name) {
   size_t tld_length = net::registry_controlled_domains::GetRegistryLength(
       host_name, net::registry_controlled_domains::EXCLUDE_UNKNOWN_REGISTRIES,
       net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES);
@@ skipping 194 matching lines @@
   return std::find(dns_names_domain.begin(), dns_names_domain.end() - 1,
                    host_name_domain) != dns_names_domain.end() - 1;
 }
 
 bool IsHostnameNonUniqueOrDotless(const std::string& hostname) {
   return net::IsHostnameNonUnique(hostname) ||
          hostname.find('.') == std::string::npos;
 }
 
 }  // namespace ssl_errors