Move SetupMetrics() to PreMainMessageLoopRun

chrome/browser/chrome_browser_main.cc

Index: chrome/browser/chrome_browser_main.cc
diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc
index d418834126caf5e42a8ca5db409f325f9226017c..618333ae5a3dc198792898b43a23db0e60a5ad89 100644
--- a/chrome/browser/chrome_browser_main.cc
+++ b/chrome/browser/chrome_browser_main.cc
@@ -1211,8 +1211,6 @@ int ChromeBrowserMainParts::PreCreateThreadsImpl() {
 
   // Task Scheduler initialization needs to be here for the following reasons:
   //   * After |SetupFieldTrials()|: Initialization uses variations.
-  //   * Before |SetupMetrics()|: |SetupMetrics()| uses the blocking pool. The
-  //         Task Scheduler must do any necessary redirection before then.
   //   * Near the end of |PreCreateThreads()|: The TaskScheduler needs to be
   //         created before any other threads are (by contract) but it creates
   //         threads itself so instantiating it earlier is also incorrect.
@@ -1220,8 +1218,6 @@ int ChromeBrowserMainParts::PreCreateThreadsImpl() {
   // shutdown call may also need to be moved.
   task_scheduler_util::InitializeDefaultBrowserTaskScheduler();
 
-  SetupMetrics();
-
   // ChromeOS needs ResourceBundle::InitSharedInstance to be called before this.
   browser_process_->PreCreateThreads();
 
@@ -1415,6 +1411,10 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() {
   SCOPED_UMA_HISTOGRAM_LONG_TIMER("Startup.PreMainMessageLoopRunImplLongTime");
   const base::TimeTicks start_time_step1 = base::TimeTicks::Now();
 
+  // This must occur at PreMainMessageLoopRun because |SetupMetrics()| uses the
+  // blocking pool, which is disabled until the CreateThreads phase of startup.
+  SetupMetrics();

[Alexei Svitkine (slow), 2016/11/24 21:32:48]

Do we have stats on how much later this happens than the previous location?

Reason I ask is that from this moment the next upload is scheduled after the
given timer interval. So if this part of startup happens later, it will also
affect timing of metrics reports. If the time difference is negligible then it's
probably fine. Otherwise, we may want to decrease the initial timer to avoid
introducing a change in metrics reporting time as a result of this.

[gab, 2016/11/25 13:16:08]

We do not have specific stats but as you can see @
https://docs.google.com/document/d/1MN7I9f_7lUthZkmDaTBHsCtOUB8ApiJ8y-AIbs6ub...
:

the biggest chunk of startup is PreMainMessageLoopRun and things following it.

The only thing that runs between the end of PreCreateThreadsImpl and the
beginning of PreMainMessageLoopRunImpl is content::CreateThreads and
content::BrowserThreadsCreated :
https://cs.chromium.org/chromium/src/content/browser/browser_main_loop.h?q=Pr...

I've never seen those as problematic in startup traces.

The timing difference is therefore negligible IMO.

[robliao, 2016/11/28 14:41:48]

Agreed. A few things do occur at CreateThreads and PreCreateThreads, but those
are mostly quick operations like instantiations.

+
 #if defined(OS_WIN)
   // Windows parental controls calls can be slow, so we do an early init here
   // that calculates this value off of the UI thread.