Move the thread hop for UMA user actions from content:: to base::.

components/metrics/metrics_service.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.
 
 //------------------------------------------------------------------------------
 // Description of the life cycle of a instance of MetricsService.
 //
 //  OVERVIEW
 //
 // A MetricsService instance is typically created at application startup.  It is
@@ skipping 286 matching lines @@
       idle_since_last_transmission_(false),
       session_id_(-1),
       data_use_tracker_(DataUseTracker::Create(local_state_)),
       self_ptr_factory_(this),
       state_saver_factory_(this) {
   DCHECK(IsSingleThreaded());
   DCHECK(state_manager_);
   DCHECK(client_);
   DCHECK(local_state_);
 
+  base::SetRecordActionTaskRunner(client_->GetMainThreadTaskRunner());

[Alexei Svitkine (slow), 2016/04/13 14:43:12]

I wasn't following the full discussion here. I like the task runner approach,
however it seems a bit strange to do this in MetricsService, since it's kind of
orthogonal to the functionality of MetricsService.

I would expect this to be done somewhere like StatisticsRecorder::Initialize()
and then you don't need the plumbing of the main thread task runner through the
client interface. (StatisticsRecorder::Initialize can then be made to take a
TaskRunner param).

Apologies if this was discussed previously as I didn't read through the previous
discussions here.

[beaudoin, 2016/04/13 16:19:36]

This wasn't specifically discussed.

Here's what was discussed: at some point we need to be able to set a different
task runner from the renderer (if and only if Chrome is running in multi-process
mode). This will allow base::RecordAction to work from the browser or renderer
process, even when Chrome runs in single process mode.

I took a quick look and StatisticsRecorder::Initialize is called from a great
many places, although mostly tests. In terms of added plumbing I don't know what
it entails. It looks like it's called from the renderer even though RecordAction
is not currently supported there. 

In a sense, since this task runner is only used by RecordAction and the likes, I
feel it does work well in user_metrics' constructor. Ultimately, though, I'll
leave the final call to you and Ilya. 

[Ilya Sherman, 2016/04/13 19:23:15]

I'm not too particular about where this initialization step happens.  The
MetricsService seemed like a natural enough place, since it's also the class
that registers a callback.  However, it wouldn't be out of place to set the task
runner from ChromeMetricsServiceClient::Initialize(), or from the
StatisticsRecorder::Initialize() method, or just about any metrics
initialization code.  StatisticsRecorder is a bit stranger a choice than others
because, currently, that class deals exclusively with histograms... but it
wouldn't be unreasonable to expand its role slightly.

I think, all else equal, I'd be happiest with whatever codepath is simplest to
support, i.e. requires the least plumbing of extra virtual methods, etc.

+
   // Set the install date if this is our first run.
   int64_t install_date = local_state_->GetInt64(prefs::kInstallDate);
   if (install_date == 0)
     local_state_->SetInt64(prefs::kInstallDate, base::Time::Now().ToTimeT());
 }
 
 MetricsService::~MetricsService() {
   DisableRecording();
 }
 
@@ skipping 873 matching lines @@
                  base::Time::Now().ToTimeT());
 }
 
 void MetricsService::SkipAndDiscardUpload() {
   log_manager_.DiscardStagedLog();
   scheduler_->UploadCancelled();
   log_upload_in_progress_ = false;
 }
 
 }  // namespace metrics