Delay asynchronous work from the metrics clients until after startup.

chrome/browser/metrics/chrome_metrics_service_client.cc

 // Copyright 2014 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/metrics/chrome_metrics_service_client.h"
 
 #include <stddef.h>
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/persistent_histogram_allocator.h"
 #include "base/path_service.h"
 #include "base/rand_util.h"
 #include "base/strings/string16.h"
 #include "base/threading/platform_thread.h"
 #include "build/build_config.h"
+#include "chrome/browser/after_startup_task_utils.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/google/google_brand.h"
 #include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
 #include "chrome/browser/metrics/chrome_stability_metrics_provider.h"
 #include "chrome/browser/metrics/https_engagement_metrics_provider.h"
 #include "chrome/browser/metrics/metrics_reporting_state.h"
 #include "chrome/browser/metrics/sampling_metrics_provider.h"
 #include "chrome/browser/metrics/subprocess_metrics_provider.h"
 #include "chrome/browser/metrics/time_ticks_experiment_win.h"
@@ skipping 96 matching lines @@
   metrics::FileMetricsProvider::RegisterPrefs(
       registry, installer::kSetupHistogramAllocatorName);
 #endif
 }
 
 std::unique_ptr<metrics::FileMetricsProvider>
 CreateInstallerFileMetricsProvider(bool metrics_reporting_enabled) {
   // Fetch a worker-pool for performing I/O tasks that are not allowed on
   // the main UI thread.
   scoped_refptr<base::TaskRunner> task_runner =
-      content::BrowserThread::GetBlockingPool()
-          ->GetTaskRunnerWithShutdownBehavior(
-              base::SequencedWorkerPool::CONTINUE_ON_SHUTDOWN);
+      new AfterStartupTaskUtils::Runner(
+          content::BrowserThread::GetBlockingPool()
+              ->GetTaskRunnerWithShutdownBehavior(
+                  base::SequencedWorkerPool::CONTINUE_ON_SHUTDOWN));
 
   // Create an object to monitor files of metrics and include them in reports.
   std::unique_ptr<metrics::FileMetricsProvider> file_metrics_provider(
       new metrics::FileMetricsProvider(task_runner,
                                        g_browser_process->local_state()));
 
   // Create the full pathname of the file holding browser metrics.
   base::FilePath metrics_file;
   if (base::PathService::Get(chrome::DIR_USER_DATA, &metrics_file)) {
     metrics_file =
@@ skipping 258 matching lines @@
           new SubprocessMetricsProvider()));
 
   // Register metrics providers.
 #if defined(ENABLE_EXTENSIONS)
   metrics_service_->RegisterMetricsProvider(
       std::unique_ptr<metrics::MetricsProvider>(
           new ExtensionsMetricsProvider(metrics_state_manager_)));
 #endif
   metrics_service_->RegisterMetricsProvider(
       std::unique_ptr<metrics::MetricsProvider>(
-          new metrics::NetworkMetricsProvider(
-              content::BrowserThread::GetBlockingPool())));
+          new metrics::NetworkMetricsProvider(new AfterStartupTaskUtils::Runner(
+              content::BrowserThread::GetBlockingPool()))));

[Alexei Svitkine (slow), 2016/08/12 18:28:18]

Similar to AntiVirusMetricsProvider, this could delay getting the data in time.

 
   // Currently, we configure OmniboxMetricsProvider to not log events to UMA
   // if there is a single incognito session visible. In the future, it may
   // be worth revisiting this to still log events from non-incognito sessions.
   metrics_service_->RegisterMetricsProvider(
       std::unique_ptr<metrics::MetricsProvider>(new OmniboxMetricsProvider(
           base::Bind(&chrome::IsIncognitoSessionActive))));
   metrics_service_->RegisterMetricsProvider(
       std::unique_ptr<metrics::MetricsProvider>(
           new ChromeStabilityMetricsProvider(
@@ skipping 39 matching lines @@
 #if defined(OS_WIN)
   google_update_metrics_provider_ = new GoogleUpdateMetricsProviderWin;
   metrics_service_->RegisterMetricsProvider(
       std::unique_ptr<metrics::MetricsProvider>(
           google_update_metrics_provider_));
 
   metrics_service_->RegisterMetricsProvider(
       std::unique_ptr<metrics::MetricsProvider>(
           new browser_watcher::WatcherMetricsProviderWin(
               chrome::GetBrowserExitCodesRegistryPath(),
-              content::BrowserThread::GetBlockingPool())));
+              new AfterStartupTaskUtils::Runner(
+                  content::BrowserThread::GetBlockingPool()))));

[Alexei Svitkine (slow), 2016/08/12 18:28:18]

This seems fine. Used to clean up registry data when metrics is off.

 
-  antivirus_metrics_provider_ = new AntiVirusMetricsProvider(
-      content::BrowserThread::GetBlockingPool()
-          ->GetTaskRunnerWithShutdownBehavior(
-              base::SequencedWorkerPool::CONTINUE_ON_SHUTDOWN));
+  antivirus_metrics_provider_ =
+      new AntiVirusMetricsProvider(new AfterStartupTaskUtils::Runner(

[Alexei Svitkine (slow), 2016/08/12 18:28:18]

This will delay running GetAntiVirusProductsOnFileThread(). (Which is not well
named, as an aside.)

If it runs too late, ProvideSystemProfileMetrics() will not have AV info in
time.

Seems like this could affect behavior. I would want us to verify things or add a
histogram to measure the impact of this, rather than just blindly landing this.

[gab, 2016/08/15 19:39:56]

For startup's sake on slow machines (forgetting the initial re-ordering premise
of this CL for a second), should we not delay the first UMA ping until this
information has been gathered instead of rushing to acquire it while the machine
is under heavy load on startup? This doesn't appear critical to painting the
first frame and hence we should be happy to delay it.

Most users will complete startup in the first 30 seconds and then would schedule
these tasks (with a random 0-10s delay, see after_startup_task_utils.cc). So for
most users we should have this information in time (1 minute IIUC), for others,
gathering it is probably hurting anyways and delaying the first UMA ping (or
flagging it as missing the info) sounds better to me.

From offline discussion : delaying regular ping is okay but not stability ping
(I'm not sure about details here).

+          content::BrowserThread::GetBlockingPool()
+              ->GetTaskRunnerWithShutdownBehavior(
+                  base::SequencedWorkerPool::CONTINUE_ON_SHUTDOWN)));
 
   metrics_service_->RegisterMetricsProvider(
       std::unique_ptr<metrics::MetricsProvider>(antivirus_metrics_provider_));
 #endif  // defined(OS_WIN)
 
 #if defined(ENABLE_PLUGINS)
   plugin_metrics_provider_ =
       new PluginMetricsProvider(g_browser_process->local_state());
   metrics_service_->RegisterMetricsProvider(
       std::unique_ptr<metrics::MetricsProvider>(plugin_metrics_provider_));
@@ skipping 293 matching lines @@
   }
 }
 
 void ChromeMetricsServiceClient::OnURLOpenedFromOmnibox(OmniboxLog* log) {
   metrics_service_->OnApplicationNotIdle();
 }
 
 bool ChromeMetricsServiceClient::IsUMACellularUploadLogicEnabled() {
   return metrics::IsCellularLogicEnabled();
 }