Create NetworkTimeTracker on startup for users in Finch experiment

chrome/browser/browser_process_impl.cc

Index: chrome/browser/browser_process_impl.cc
diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc
index 3d002dad6f012a3bcbe1518e242a79e8630b9a7b..5a32e520a3b5b9770b42f817b19ef81080d8e280 100644
--- a/chrome/browser/browser_process_impl.cc
+++ b/chrome/browser/browser_process_impl.cc
@@ -1067,6 +1067,13 @@ void BrowserProcessImpl::PreMainMessageLoopRun() {
   CacheDefaultWebClientState();
 
   platform_part_->PreMainMessageLoopRun();
+
+  if (base::FeatureList::IsEnabled(network_time::kNetworkTimeServiceQuerying)) {
+    network_time_tracker_.reset(new network_time::NetworkTimeTracker(

[Nico, 2016/09/27 15:51:24]

Does this do any amount of real work that could slow down startup?

[estark, 2016/09/27 18:56:24]

It reads a pref and then, in some cases, starts a URLFetcher to the time server.

Since the request is async I think this should not slow down startup. If it does
for some reason, I think we'd get a Chirp alert about it (because we're only
doing this for users in the Finch trial)?

[Nico, 2016/09/27 19:07:23]

Reading prefs is ok. Is the network request needed right at startup? Or could
that be delayed a bit? Startup is a busy place.

[estark, 2016/09/27 19:37:44]

It is, unfortunately, needed right at startup. The scenario is that the user's
system clock is wrong and they start up Chrome, which immediately restores their
tabs, resulting in scary certificate validation errors on all their tabs.
Instead we'd like to have secure time available before those tabs load, so that
we can show a helpful error message ("Please fix your clock") instead of a
generic scary one ("Your connection is not private").

Do you think it would help at all to delay a little bit, like one second? We
could potentially delay showing the certificate validation errors at startup for
a few seconds until the first time query completes.

+        base::WrapUnique(new base::DefaultClock()),
+        base::WrapUnique(new base::DefaultTickClock()), local_state(),
+        system_request_context()));

[mab, 2016/09/26 21:45:09]

Note that the network_time_tracker() getter above also can create a
NetworkTimeTracker.  You could call it here, although it is a bit obscure that
calling that innocently-named getter has the side effect of creating the thing
in question.

[estark, 2016/09/26 21:55:40]

Yeah, I considered that but thought it was a little too weird to call a getter
for its side effects. 

+  }
 }
 
 void BrowserProcessImpl::CreateIconManager() {