Delay asynchronous work from the metrics clients until after startup.

chrome/browser/metrics/chrome_metrics_services_manager_client.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 "chrome/browser/metrics/chrome_metrics_services_manager_client.h"
 
 #include "base/command_line.h"
 #include "base/feature_list.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
@@ skipping 21 matching lines @@
 
 // Metrics reporting feature. This feature, along with user consent, controls if
 // recording and reporting are enabled. If the feature is enabled, but no
 // consent is given, then there will be no recording or reporting.
 const base::Feature kMetricsReportingFeature{"MetricsReporting",
                                              base::FEATURE_ENABLED_BY_DEFAULT};
 
 // Posts |GoogleUpdateSettings::StoreMetricsClientInfo| on blocking pool thread
 // because it needs access to IO and cannot work from UI thread.
 void PostStoreMetricsClientInfo(const metrics::ClientInfo& client_info) {
-  content::BrowserThread::GetBlockingPool()->PostTask(
-      FROM_HERE,
+  content::BrowserThread::PostAfterStartupTask(
+      FROM_HERE, content::BrowserThread::GetBlockingPool(),

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

Hmm, in the following CL I'm clearing the client info if consent is toggled off:

https://codereview.chromium.org/2222903004/

I wonder if there's any chance of a race with this change.

For example, if this schedules a task, then the "toggle checkbox off" code runs
and then this task runs, then we will get data written.

Now, I find it hard to reason about whether the above is possible, since I'm not
sure what "After startup" means. Is it arbitrarily delayed by some random value?

If so, I can imagine this going wrong. Ideally, every call to
StoreMetricsClientInfo should use the same sequenced runner. Which might be a
bit hard to structure.

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

If there is, the race condition already exists today (an unsequenced PostTask to
a TaskRunner provides absolutely no guarantees of ordering. The extra delay
might highlight the race but it won't cause it.

From offline discussion : true but today people would have to change settings
manually super early whereas with this CL they have up to a 10 seconds window
after first paint during which they could disable UMA and have this delayed task
re-save state after the fact later. This can perhaps be handled by a
CancellableFlag?

       base::Bind(&GoogleUpdateSettings::StoreMetricsClientInfo, client_info));
 }
 
 // Only clients that were given an opt-out metrics-reporting consent flow are
 // eligible for sampling.
 bool IsClientEligibleForSampling() {
   return metrics::GetMetricsReportingDefaultState(
              g_browser_process->local_state()) ==
          metrics::EnableMetricsDefault::OPT_OUT;
 }
@@ skipping 121 matching lines @@
 ChromeMetricsServicesManagerClient::GetMetricsStateManager() {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (!metrics_state_manager_) {
     metrics_state_manager_ = metrics::MetricsStateManager::Create(
         local_state_, enabled_state_provider_.get(),
         base::Bind(&PostStoreMetricsClientInfo),
         base::Bind(&GoogleUpdateSettings::LoadMetricsClientInfo));
   }
   return metrics_state_manager_.get();
 }