[Cleanup] Migrate the FileMetricsProvider to use the Task Scheduler.

components/metrics/file_metrics_provider.cc

 // Copyright 2016 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 "components/metrics/file_metrics_provider.h"
 
 #include "base/command_line.h"
 #include "base/files/file.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/file_util.h"
 #include "base/files/memory_mapped_file.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_base.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/persistent_histogram_allocator.h"
 #include "base/metrics/persistent_memory_allocator.h"
 #include "base/strings/string_piece.h"
 #include "base/task_runner.h"
+#include "base/task_scheduler/post_task.h"
+#include "base/task_scheduler/task_traits.h"
 #include "base/time/time.h"
 #include "components/metrics/metrics_pref_names.h"
 #include "components/metrics/metrics_service.h"
 #include "components/metrics/persistent_system_profile.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 
 namespace metrics {
 
 namespace {
@@ skipping 56 matching lines @@
 
 void DeleteFileWhenPossible(const base::FilePath& path) {
   // Open (with delete) and then immediately close the file by going out of
   // scope. This is the only cross-platform safe way to delete a file that may
   // be open elsewhere, a distinct possibility given the asynchronous nature
   // of the delete task.
   base::File file(path, base::File::FLAG_OPEN | base::File::FLAG_READ |
                             base::File::FLAG_DELETE_ON_CLOSE);
 }
 
+// A task runner to use for testing.
+base::TaskRunner* g_task_runner_for_testing = nullptr;
+
+// Returns a task runner appropriate for running background tasks that perform
+// file I/O.
+scoped_refptr<base::TaskRunner> CreateBackgroundTaskRunner() {
+  if (g_task_runner_for_testing)
+    return scoped_refptr<base::TaskRunner>(g_task_runner_for_testing);
+
+  return base::CreateTaskRunnerWithTraits(
+      {base::MayBlock(), base::TaskPriority::BACKGROUND,
+       base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN});

[bcwhite, 2017/07/04 14:50:31]

I don't think there's anything in here that needs "continue".  They can all be
"skip".

[Ilya Sherman, 2017/07/05 18:15:56]

Done.

+}
+
 }  // namespace
 
 // This structure stores all the information about the sources being monitored
 // and their current reporting state.
 struct FileMetricsProvider::SourceInfo {
   SourceInfo(SourceType source_type, SourceAssociation source_association)
       : type(source_type), association(source_association) {}
   ~SourceInfo() {}
 
   // How to access this source (file/dir, atomic/active).
@@ skipping 20 matching lines @@
   bool read_complete = false;
 
   // Once a file has been recognized as needing to be read, it is mapped
   // into memory and assigned to an |allocator| object.
   std::unique_ptr<base::PersistentHistogramAllocator> allocator;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(SourceInfo);
 };
 
-FileMetricsProvider::FileMetricsProvider(
-    const scoped_refptr<base::TaskRunner>& task_runner,
-    PrefService* local_state)
-    : task_runner_(task_runner),
+FileMetricsProvider::FileMetricsProvider(PrefService* local_state)
+    : task_runner_(CreateBackgroundTaskRunner()),
       pref_service_(local_state),
       weak_factory_(this) {
   base::StatisticsRecorder::RegisterHistogramProvider(
       weak_factory_.GetWeakPtr());
 }
 
 FileMetricsProvider::~FileMetricsProvider() {}
 
 void FileMetricsProvider::RegisterSource(const base::FilePath& path,
                                          SourceType type,
                                          SourceAssociation source_association,
                                          const base::StringPiece prefs_key) {
-  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   // Ensure that kSourceOptions has been filled for this type.
   DCHECK_GT(arraysize(kSourceOptions), static_cast<size_t>(type));
 
   std::unique_ptr<SourceInfo> source(new SourceInfo(type, source_association));
   source->prefs_key = prefs_key.as_string();
 
   switch (source->type) {
     case SOURCE_HISTOGRAMS_ACTIVE_FILE:
       DCHECK(prefs_key.empty());
@@ skipping 28 matching lines @@
 }
 
 // static
 void FileMetricsProvider::RegisterPrefs(PrefRegistrySimple* prefs,
                                         const base::StringPiece prefs_key) {
   prefs->RegisterInt64Pref(metrics::prefs::kMetricsLastSeenPrefix +
                            prefs_key.as_string(), 0);
 }
 
 // static
+void FileMetricsProvider::SetTaskRunnerForTesting(
+    const scoped_refptr<base::TaskRunner>& task_runner) {
+  g_task_runner_for_testing = task_runner.get();

[bcwhite, 2017/07/04 14:50:31]

Delete the existing one if set.

[Ilya Sherman, 2017/07/05 18:15:56]

There's no ownership, so it doesn't make sense to delete anything.  I did add a
DCHECK, though, that this is never set twice in the same test.

[bcwhite, 2017/07/05 18:31:51]

Acknowledged.

+}
+
+// static
 bool FileMetricsProvider::LocateNextFileInDirectory(SourceInfo* source) {
   DCHECK_EQ(SOURCE_HISTOGRAMS_ATOMIC_DIR, source->type);
   DCHECK(!source->directory.empty());
 
   // Open the directory and find all the files, remembering the oldest that
   // has not been read. They can be removed and/or ignored if they're older
   // than the last-check time.
   base::Time oldest_file_time = base::Time::Now();
   base::FilePath oldest_file_path;
   base::FilePath file_path;
@@ skipping 220 matching lines @@
     snapshot_manager->PrepareFinalDelta(histogram.get());
     ++histogram_count;
   }
 
   source->read_complete = true;
   DVLOG(1) << "Reported " << histogram_count << " histograms from "
            << source->path.value();
 }
 
 void FileMetricsProvider::ScheduleSourcesCheck() {
-  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (sources_to_check_.empty())
     return;
 
   // Create an independent list of sources for checking. This will be Owned()
   // by the reply call given to the task-runner, to be deleted when that call
   // has returned. It is also passed Unretained() to the task itself, safe
   // because that must complete before the reply runs.
   SourceInfoList* check_list = new SourceInfoList();
   std::swap(sources_to_check_, *check_list);
   task_runner_->PostTaskAndReply(
       FROM_HERE,
       base::Bind(&FileMetricsProvider::CheckAndMergeMetricSourcesOnTaskRunner,
                  base::Unretained(check_list)),
       base::Bind(&FileMetricsProvider::RecordSourcesChecked,
                  weak_factory_.GetWeakPtr(), base::Owned(check_list)));
 }
 
 void FileMetricsProvider::RecordSourcesChecked(SourceInfoList* checked) {
-  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   // Sources that still have an allocator at this point are read/write "active"
   // files that may need their contents merged on-demand. If there is no
   // allocator (not a read/write file) but a read was done on the task-runner,
   // try again immediately to see if more is available (in a directory of
   // files). Otherwise, remember the source for checking again at a later time.
   bool did_read = false;
   for (auto iter = checked->begin(); iter != checked->end();) {
     auto temp = iter++;
     SourceInfo* source = temp->get();
@@ skipping 20 matching lines @@
   // is possible.
   if (did_read)
     ScheduleSourcesCheck();
 }
 
 void FileMetricsProvider::DeleteFileAsync(const base::FilePath& path) {
   task_runner_->PostTask(FROM_HERE, base::Bind(DeleteFileWhenPossible, path));
 }
 
 void FileMetricsProvider::RecordSourceAsRead(SourceInfo* source) {
-  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   // Persistently record the "last seen" timestamp of the source file to
   // ensure that the file is never read again unless it is modified again.
   if (pref_service_ && !source->prefs_key.empty()) {
     pref_service_->SetInt64(
         metrics::prefs::kMetricsLastSeenPrefix + source->prefs_key,
         source->last_seen.ToInternalValue());
   }
 }
 
 void FileMetricsProvider::OnDidCreateMetricsLog() {
-  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   // Schedule a check to see if there are new metrics to load. If so, they
   // will be reported during the next collection run after this one. The
   // check is run off of the worker-pool so as to not cause delays on the
   // main UI thread (which is currently where metric collection is done).
   ScheduleSourcesCheck();
 
   // Clear any data for initial metrics since they're always reported
   // before the first call to this method. It couldn't be released after
   // being reported in RecordInitialHistogramSnapshots because the data
   // will continue to be used by the caller after that method returns. Once
   // here, though, all actions to be done on the data have been completed.
   for (const std::unique_ptr<SourceInfo>& source : sources_for_previous_run_)
     DeleteFileAsync(source->path);
   sources_for_previous_run_.clear();
 }
 
 bool FileMetricsProvider::ProvideIndependentMetrics(
     SystemProfileProto* system_profile_proto,
     base::HistogramSnapshotManager* snapshot_manager) {
-  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   while (!sources_with_profile_.empty()) {
     SourceInfo* source = sources_with_profile_.begin()->get();
     DCHECK(source->allocator);
 
     bool success = false;
     RecordEmbeddedProfileResult(EMBEDDED_PROFILE_ATTEMPT);
     if (PersistentSystemProfile::GetSystemProfile(
             *source->allocator->memory_allocator(), system_profile_proto)) {
       RecordHistogramSnapshotsFromSource(snapshot_manager, source);
@@ skipping 10 matching lines @@
     sources_to_check_.splice(sources_to_check_.end(), sources_with_profile_,
                              sources_with_profile_.begin());
     if (success)
       return true;
   }
 
   return false;
 }
 
 bool FileMetricsProvider::HasInitialStabilityMetrics() {
-  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   // Measure the total time spent checking all sources as well as the time
   // per individual file. This method is called during startup and thus blocks
   // the initial showing of the browser window so it's important to know the
   // total delay.
   SCOPED_UMA_HISTOGRAM_TIMER("UMA.FileMetricsProvider.InitialCheckTime.Total");
 
   // Check all sources for previous run to see if they need to be read.
   for (auto iter = sources_for_previous_run_.begin();
        iter != sources_for_previous_run_.end();) {
@@ skipping 35 matching lines @@
                                      sources_for_previous_run_, temp);
       }
     }
   }
 
   return !sources_for_previous_run_.empty();
 }
 
 void FileMetricsProvider::RecordInitialHistogramSnapshots(
     base::HistogramSnapshotManager* snapshot_manager) {
-  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   // Measure the total time spent processing all sources as well as the time
   // per individual file. This method is called during startup and thus blocks
   // the initial showing of the browser window so it's important to know the
   // total delay.
   SCOPED_UMA_HISTOGRAM_TIMER(
       "UMA.FileMetricsProvider.InitialSnapshotTime.Total");
 
   for (const std::unique_ptr<SourceInfo>& source : sources_for_previous_run_) {
     SCOPED_UMA_HISTOGRAM_TIMER(
         "UMA.FileMetricsProvider.InitialSnapshotTime.File");
 
     // The source needs to have an allocator attached to it in order to read
     // histograms out of it.
     DCHECK(!source->read_complete);
     DCHECK(source->allocator);
 
     // Dump all histograms contained within the source to the snapshot-manager.
     RecordHistogramSnapshotsFromSource(snapshot_manager, source.get());
 
     // Update the last-seen time so it isn't read again unless it changes.
     RecordSourceAsRead(source.get());
   }
 }
 
 void FileMetricsProvider::MergeHistogramDeltas() {
-  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   // Measure the total time spent processing all sources as well as the time
   // per individual file. This method is called on the UI thread so it's
   // important to know how much total "jank" may be introduced.
   SCOPED_UMA_HISTOGRAM_TIMER("UMA.FileMetricsProvider.SnapshotTime.Total");
 
   for (std::unique_ptr<SourceInfo>& source : sources_mapped_) {
     SCOPED_UMA_HISTOGRAM_TIMER("UMA.FileMetricsProvider.SnapshotTime.File");
     MergeHistogramDeltasFromSource(source.get());
   }
 }
 
 }  // namespace metrics