Create NetworkTimeTracker on startup for users in Finch experiment

chrome/browser/browser_process_impl.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/browser_process_impl.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <map>
@@ skipping 1049 matching lines @@
 
 #if !defined(OS_ANDROID)
   storage_monitor::StorageMonitor::Create();
 #endif
 
   child_process_watcher_.reset(new ChromeChildProcessWatcher());
 
   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() {
   DCHECK(!created_icon_manager_ && !icon_manager_);
   created_icon_manager_ = true;
   icon_manager_.reset(new IconManager);
 }
 
 void BrowserProcessImpl::CreateIntranetRedirectDetector() {
   DCHECK(!intranet_redirect_detector_);
@@ skipping 265 matching lines @@
 }
 
 void BrowserProcessImpl::OnAutoupdateTimer() {
   if (CanAutorestartForUpdate()) {
     DLOG(WARNING) << "Detected update.  Restarting browser.";
     RestartBackgroundInstance();
   }
 }
 
 #endif  // (defined(OS_WIN) || defined(OS_LINUX)) && !defined(OS_CHROMEOS)