Fetch independent metrics on background thread.

Previously, these metrics were loaded on the UI thread which, because
they are in a memory-mapped file, can cause blocking I/O and cause
jank.

Bug: 882892
Change-Id: I23b1808b5233b5bd05a1dbe5e96ace32c73ef46b
Reviewed-on: https://chromium-review.googlesource.com/c/1292271
Commit-Queue: Brian White <bcwhite@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602968}

base/metrics/histogram_flattener.h

@@ -19,12 +19,13 @@ class HistogramSamples;
 // handles the logistics of gathering up available histograms for recording.
 class BASE_EXPORT HistogramFlattener {
  public:
+  virtual ~HistogramFlattener() = default;
+
   virtual void RecordDelta(const HistogramBase& histogram,
                            const HistogramSamples& snapshot) = 0;
 
  protected:
   HistogramFlattener() = default;
-  virtual ~HistogramFlattener() = default;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(HistogramFlattener);

components/metrics/delegating_provider.cc

@@ -55,9 +55,7 @@ void DelegatingProvider::OnAppEnterBackground() {
     provider->OnAppEnterBackground();
 }
 
-bool DelegatingProvider::ProvideIndependentMetrics(
-    SystemProfileProto* system_profile_proto,
-    base::HistogramSnapshotManager* snapshot_manager) {
+bool DelegatingProvider::HasIndependentMetrics() {
   // These are collected seperately for each provider.
   NOTREACHED();
   return false;

components/metrics/delegating_provider.h

@@ -33,9 +33,7 @@ class DelegatingProvider final : public MetricsProvider {
   void OnRecordingEnabled() override;
   void OnRecordingDisabled() override;
   void OnAppEnterBackground() override;
-  bool ProvideIndependentMetrics(
-      SystemProfileProto* system_profile_proto,
-      base::HistogramSnapshotManager* snapshot_manager) override;
+  bool HasIndependentMetrics() override;
   void ProvideSystemProfileMetrics(
       SystemProfileProto* system_profile_proto) override;
   bool HasPreviousSessionData() override;

components/metrics/file_metrics_provider.cc

@@ -22,6 +22,7 @@
 #include "base/task/post_task.h"
 #include "base/task/task_traits.h"
 #include "base/task_runner.h"
+#include "base/task_runner_util.h"
 #include "base/time/time.h"
 #include "components/metrics/metrics_pref_names.h"
 #include "components/metrics/metrics_service.h"
@@ -33,8 +34,8 @@ namespace metrics {
 
 namespace {
 
-const base::Feature kCacheFileMetricData = {"CacheFileMetricData",
-                                            base::FEATURE_ENABLED_BY_DEFAULT};
+const base::Feature kBackgroundIndependentMetrics = {
+    "BackgoundIndependentMetrics", base::FEATURE_ENABLED_BY_DEFAULT};
 
 // These structures provide values used to define how files are opened and
 // accessed. It obviates the need for multiple code-paths within several of
@@ -506,7 +507,11 @@ FileMetricsProvider::AccessResult FileMetricsProvider::CheckAndMapMetricSource(
 
   // Cache the file data while running in a background thread so that there
   // shouldn't be any I/O when the data is accessed from the main thread.
-  if (base::FeatureList::IsEnabled(kCacheFileMetricData))
+  // Files with an internal profile, those from previous runs that include
+  // a full system profile and are fetched via ProvideIndependentMetrics(),
+  // are loaded on a background task and so there's no need to cache the
+  // data in advance.
+  if (source->association != ASSOCIATE_INTERNAL_PROFILE)
     memory_allocator->Cache();
 
   // Create an allocator for the mapped file. Ownership passes to the allocator.
@@ -624,6 +629,38 @@ FileMetricsProvider::AccessResult FileMetricsProvider::HandleFilterSource(
   return ACCESS_RESULT_SUCCESS;
 }
 
+/* static */
+bool FileMetricsProvider::ProvideIndependentMetricsOnTaskRunner(
+    SourceInfo* source,
+    SystemProfileProto* system_profile_proto,
+    base::HistogramSnapshotManager* snapshot_manager) {
+  RecordEmbeddedProfileResult(EMBEDDED_PROFILE_ATTEMPT);
+  base::Time start_time = base::Time::Now();
+  if (PersistentSystemProfile::GetSystemProfile(
+          *source->allocator->memory_allocator(), system_profile_proto)) {
+    system_profile_proto->mutable_stability()->set_from_previous_run(true);
+    RecordHistogramSnapshotsFromSource(snapshot_manager, source);
+    UMA_HISTOGRAM_TIMES("UMA.FileMetricsProvider.EmbeddedProfile.RecordTime",
+                        base::Time::Now() - start_time);
+    RecordEmbeddedProfileResult(EMBEDDED_PROFILE_FOUND);
+    return true;
+  }
+
+  RecordEmbeddedProfileResult(EMBEDDED_PROFILE_DROPPED);
+
+  // TODO(bcwhite): Remove these once crbug/695880 is resolved.
+  int histogram_count = 0;
+  base::PersistentHistogramAllocator::Iterator histogram_iter(
+      source->allocator.get());
+  while (histogram_iter.GetNext()) {
+    ++histogram_count;
+  }
+  UMA_HISTOGRAM_COUNTS_10000(
+      "UMA.FileMetricsProvider.EmbeddedProfile.DroppedHistogramCount",
+      histogram_count);
+  return false;
+}
+
 void FileMetricsProvider::ScheduleSourcesCheck() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
@@ -718,56 +755,62 @@ void FileMetricsProvider::OnDidCreateMetricsLog() {
   sources_for_previous_run_.clear();
 }
 
-bool FileMetricsProvider::ProvideIndependentMetrics(
+bool FileMetricsProvider::HasIndependentMetrics() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  return !sources_with_profile_.empty();
+}
+
+void FileMetricsProvider::ProvideIndependentMetrics(
+    base::OnceCallback<void(bool)> done_callback,
     SystemProfileProto* system_profile_proto,
     base::HistogramSnapshotManager* snapshot_manager) {
   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);
-    base::Time start_time = base::Time::Now();
-    if (PersistentSystemProfile::GetSystemProfile(
-            *source->allocator->memory_allocator(), system_profile_proto)) {
-      RecordHistogramSnapshotsFromSource(snapshot_manager, source);
-      UMA_HISTOGRAM_TIMES("UMA.FileMetricsProvider.EmbeddedProfile.RecordTime",
-                          base::Time::Now() - start_time);
-      success = true;
-      RecordEmbeddedProfileResult(EMBEDDED_PROFILE_FOUND);
-    } else {
-      RecordEmbeddedProfileResult(EMBEDDED_PROFILE_DROPPED);
+  if (sources_with_profile_.empty()) {
+    std::move(done_callback).Run(false);
+    return;
+  }
 
-      // TODO(bcwhite): Remove these once crbug/695880 is resolved.
+  std::unique_ptr<SourceInfo> source =
+      std::move(*sources_with_profile_.begin());
+  sources_with_profile_.pop_front();
+  SourceInfo* source_ptr = source.get();
+  DCHECK(source->allocator);
 
-      int histogram_count = 0;
-      base::PersistentHistogramAllocator::Iterator histogram_iter(
-          source->allocator.get());
-      while (histogram_iter.GetNext()) {
-        ++histogram_count;
-      }
-      UMA_HISTOGRAM_COUNTS_10000(
-          "UMA.FileMetricsProvider.EmbeddedProfile.DroppedHistogramCount",
-          histogram_count);
-    }
+  if (base::FeatureList::IsEnabled(kBackgroundIndependentMetrics)) {
+    // Do the actual work as a background task.
+    base::PostTaskAndReplyWithResult(
+        task_runner_.get(), FROM_HERE,
+        base::BindOnce(
+            &FileMetricsProvider::ProvideIndependentMetricsOnTaskRunner,
+            source_ptr, system_profile_proto, snapshot_manager),
+        base::BindOnce(&FileMetricsProvider::ProvideIndependentMetricsCleanup,
+                       weak_factory_.GetWeakPtr(), std::move(done_callback),
+                       std::move(source)));
+  } else {
+    // Do the actual work now, inline (for performance comparisons).
+    bool success = ProvideIndependentMetricsOnTaskRunner(
+        source_ptr, system_profile_proto, snapshot_manager);
+    ProvideIndependentMetricsCleanup(std::move(done_callback),
+                                     std::move(source), success);
+  }
+}
 
-    // Regardless of whether this source was successfully recorded, it is never
-    // read again.
-    source->read_complete = true;
-    RecordSourceAsRead(source);
-    sources_to_check_.splice(sources_to_check_.end(), sources_with_profile_,
-                             sources_with_profile_.begin());
-    ScheduleSourcesCheck();
+void FileMetricsProvider::ProvideIndependentMetricsCleanup(
+    base::OnceCallback<void(bool)> done_callback,
+    std::unique_ptr<SourceInfo> source,
+    bool success) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-    if (success) {
-      system_profile_proto->mutable_stability()->set_from_previous_run(true);
-      return true;
-    }
-  }
+  // Regardless of whether this source was successfully recorded, it is
+  // never read again.
+  source->read_complete = true;
+  RecordSourceAsRead(source.get());
+  sources_to_check_.push_back(std::move(source));
+  ScheduleSourcesCheck();
 
-  return false;
+  // Execute the chained callback.
+  std::move(done_callback).Run(success);
 }
 
 bool FileMetricsProvider::HasPreviousSessionData() {

components/metrics/file_metrics_provider.h

@@ -267,6 +267,12 @@ class FileMetricsProvider : public MetricsProvider,
   static AccessResult HandleFilterSource(SourceInfo* source,
                                          const base::FilePath& path);
 
+  // The part of ProvideIndependentMetrics that runs as a background task.
+  static bool ProvideIndependentMetricsOnTaskRunner(
+      SourceInfo* source,
+      SystemProfileProto* system_profile_proto,
+      base::HistogramSnapshotManager* snapshot_manager);
+
   // Creates a task to check all monitored sources for updates.
   void ScheduleSourcesCheck();
 
@@ -282,7 +288,9 @@ class FileMetricsProvider : public MetricsProvider,
 
   // metrics::MetricsProvider:
   void OnDidCreateMetricsLog() override;
-  bool ProvideIndependentMetrics(
+  bool HasIndependentMetrics() override;
+  void ProvideIndependentMetrics(
+      base::OnceCallback<void(bool)> done_callback,
       SystemProfileProto* system_profile_proto,
       base::HistogramSnapshotManager* snapshot_manager) override;
   bool HasPreviousSessionData() override;
@@ -292,6 +300,12 @@ class FileMetricsProvider : public MetricsProvider,
   // base::StatisticsRecorder::HistogramProvider:
   void MergeHistogramDeltas() override;
 
+  // The part of ProvideIndependentMetrics that runs after background task.
+  void ProvideIndependentMetricsCleanup(
+      base::OnceCallback<void(bool)> done_callback,
+      std::unique_ptr<SourceInfo> source,
+      bool success);
+
   // A task-runner capable of performing I/O.
   scoped_refptr<base::TaskRunner> task_runner_;
 

components/metrics/file_metrics_provider_unittest.cc

@@ -18,6 +18,7 @@
 #include "base/metrics/sparse_histogram.h"
 #include "base/metrics/statistics_recorder.h"
 #include "base/strings/stringprintf.h"
+#include "base/synchronization/waitable_event.h"
 #include "base/test/test_simple_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
@@ -109,11 +110,25 @@ class FileMetricsProviderTest : public testing::TestWithParam<bool> {
     provider()->MergeHistogramDeltas();
   }
 
+  bool HasIndependentMetrics() { return provider()->HasIndependentMetrics(); }
+
   bool ProvideIndependentMetrics(
       SystemProfileProto* profile_proto,
       base::HistogramSnapshotManager* snapshot_manager) {
-    return provider()->ProvideIndependentMetrics(profile_proto,
-                                                 snapshot_manager);
+    bool success = false;
+    bool success_set = false;
+    provider()->ProvideIndependentMetrics(
+        base::BindOnce(
+            [](bool* success_ptr, bool* set_ptr, bool s) {
+              *success_ptr = s;
+              *set_ptr = true;
+            },
+            &success, &success_set),
+        profile_proto, snapshot_manager);
+
+    RunTasks();
+    CHECK(success_set);
+    return success;
   }
 
   void RecordInitialHistogramSnapshots(
@@ -139,10 +154,10 @@ class FileMetricsProviderTest : public testing::TestWithParam<bool> {
     HistogramFlattenerDeltaRecorder flattener;
     base::HistogramSnapshotManager snapshot_manager(&flattener);
     SystemProfileProto profile_proto;
-    if (!provider()->ProvideIndependentMetrics(&profile_proto,
-                                               &snapshot_manager)) {
-      return 0;
-    }
+    provider()->ProvideIndependentMetrics(base::BindOnce([](bool success) {}),
+                                          &profile_proto, &snapshot_manager);
+
+    RunTasks();
     return flattener.GetRecordedDeltaHistogramNames().size();
   }
 
@@ -545,18 +560,20 @@ TEST_P(FileMetricsProviderTest, AccessDirectoryWithInvalidFiles) {
   // H1 should be skipped and H2 available.
   OnDidCreateMetricsLog();
   RunTasks();
-  EXPECT_EQ(2U, GetIndependentHistogramCount());
   EXPECT_FALSE(base::PathExists(metrics_files.GetPath().AppendASCII("h1.pma")));
   EXPECT_TRUE(base::PathExists(metrics_files.GetPath().AppendASCII("h2.pma")));
   EXPECT_TRUE(base::PathExists(metrics_files.GetPath().AppendASCII("h3.pma")));
   EXPECT_TRUE(base::PathExists(metrics_files.GetPath().AppendASCII("h4.pma")));
 
+  // H2 should be read and the file deleted.
+  EXPECT_EQ(2U, GetIndependentHistogramCount());
+  RunTasks();
+  EXPECT_FALSE(base::PathExists(metrics_files.GetPath().AppendASCII("h2.pma")));
+
   // Nothing else should be found but the last (valid but empty) file will
   // stick around to be processed later (should it get expanded).
-  OnDidCreateMetricsLog();
-  RunTasks();
   EXPECT_EQ(0U, GetIndependentHistogramCount());
-  EXPECT_FALSE(base::PathExists(metrics_files.GetPath().AppendASCII("h2.pma")));
+  RunTasks();
   EXPECT_FALSE(base::PathExists(metrics_files.GetPath().AppendASCII("h3.pma")));
   EXPECT_TRUE(base::PathExists(metrics_files.GetPath().AppendASCII("h4.pma")));
 }
@@ -888,6 +905,7 @@ TEST_P(FileMetricsProviderTest, AccessEmbeddedProfileMetricsWithoutProfile) {
     SystemProfileProto profile;
 
     // A read of metrics with internal profiles should return nothing.
+    EXPECT_FALSE(HasIndependentMetrics());
     EXPECT_FALSE(ProvideIndependentMetrics(&profile, &snapshot_manager));
   }
   EXPECT_TRUE(base::PathExists(metrics_file()));
@@ -929,11 +947,11 @@ TEST_P(FileMetricsProviderTest, AccessEmbeddedProfileMetricsWithProfile) {
 
     // A read of metrics with internal profiles should return one result.
     SystemProfileProto profile;
+    EXPECT_TRUE(HasIndependentMetrics());
     EXPECT_TRUE(ProvideIndependentMetrics(&profile, &snapshot_manager));
+    EXPECT_FALSE(HasIndependentMetrics());
     EXPECT_FALSE(ProvideIndependentMetrics(&profile, &snapshot_manager));
   }
-  EXPECT_TRUE(base::PathExists(metrics_file()));
-  OnDidCreateMetricsLog();
   RunTasks();
   EXPECT_FALSE(base::PathExists(metrics_file()));
 }
@@ -960,6 +978,7 @@ TEST_P(FileMetricsProviderTest, AccessEmbeddedFallbackMetricsWithoutProfile) {
 
     // A read of metrics with internal profiles should return nothing.
     SystemProfileProto profile;
+    EXPECT_FALSE(HasIndependentMetrics());
     EXPECT_FALSE(ProvideIndependentMetrics(&profile, &snapshot_manager));
   }
   EXPECT_TRUE(base::PathExists(metrics_file()));
@@ -1002,11 +1021,11 @@ TEST_P(FileMetricsProviderTest, AccessEmbeddedFallbackMetricsWithProfile) {
 
     // A read of metrics with internal profiles should return one result.
     SystemProfileProto profile;
+    EXPECT_TRUE(HasIndependentMetrics());
     EXPECT_TRUE(ProvideIndependentMetrics(&profile, &snapshot_manager));
+    EXPECT_FALSE(HasIndependentMetrics());
     EXPECT_FALSE(ProvideIndependentMetrics(&profile, &snapshot_manager));
   }
-  EXPECT_TRUE(base::PathExists(metrics_file()));
-  OnDidCreateMetricsLog();
   RunTasks();
   EXPECT_FALSE(base::PathExists(metrics_file()));
 }
@@ -1055,9 +1074,11 @@ TEST_P(FileMetricsProviderTest, AccessEmbeddedProfileMetricsFromDir) {
   base::HistogramSnapshotManager snapshot_manager(&flattener);
   SystemProfileProto profile;
   for (int i = 0; i < file_count; ++i) {
+    EXPECT_TRUE(HasIndependentMetrics()) << i;
     EXPECT_TRUE(ProvideIndependentMetrics(&profile, &snapshot_manager)) << i;
     RunTasks();
   }
+  EXPECT_FALSE(HasIndependentMetrics());
   EXPECT_FALSE(ProvideIndependentMetrics(&profile, &snapshot_manager));
 
   OnDidCreateMetricsLog();

components/metrics/metrics_log.cc

@@ -54,6 +54,7 @@ namespace {
 class IndependentFlattener : public base::HistogramFlattener {
  public:
   explicit IndependentFlattener(MetricsLog* log) : log_(log) {}
+  ~IndependentFlattener() override {}
 
   // base::HistogramFlattener:
   void RecordDelta(const base::HistogramBase& histogram,
@@ -74,6 +75,26 @@ bool IsTestingID(const std::string& id) {
 
 }  // namespace
 
+MetricsLog::IndependentMetricsLoader::IndependentMetricsLoader(
+    std::unique_ptr<MetricsLog> log)
+    : log_(std::move(log)),
+      flattener_(new IndependentFlattener(log_.get())),
+      snapshot_manager_(new base::HistogramSnapshotManager(flattener_.get())) {}
+
+MetricsLog::IndependentMetricsLoader::~IndependentMetricsLoader() = default;
+
+void MetricsLog::IndependentMetricsLoader::Run(
+    base::OnceCallback<void(bool)> done_callback,
+    MetricsProvider* metrics_provider) {
+  metrics_provider->ProvideIndependentMetrics(
+      std::move(done_callback), log_->uma_proto()->mutable_system_profile(),
+      snapshot_manager_.get());
+}
+
+std::unique_ptr<MetricsLog> MetricsLog::IndependentMetricsLoader::ReleaseLog() {
+  return std::move(log_);
+}
+
 MetricsLog::MetricsLog(const std::string& client_id,
                        int session_id,
                        LogType log_type,
@@ -278,15 +299,6 @@ const SystemProfileProto& MetricsLog::RecordEnvironment(
   return *system_profile;
 }
 
-bool MetricsLog::LoadIndependentMetrics(MetricsProvider* metrics_provider) {
-  SystemProfileProto* system_profile = uma_proto()->mutable_system_profile();
-  IndependentFlattener flattener(this);
-  base::HistogramSnapshotManager snapshot_manager(&flattener);
-
-  return metrics_provider->ProvideIndependentMetrics(system_profile,
-                                                     &snapshot_manager);
-}
-
 bool MetricsLog::LoadSavedEnvironmentFromPrefs(PrefService* local_state,
                                                std::string* app_version) {
   DCHECK(!has_environment_);

components/metrics/metrics_log.h

@@ -22,7 +22,9 @@
 class PrefService;
 
 namespace base {
+class HistogramFlattener;
 class HistogramSamples;
+class HistogramSnapshotManager;
 }
 
 namespace metrics {
@@ -45,6 +47,33 @@ class MetricsLog {
     INDEPENDENT_LOG,        // An independent log from a previous session.
   };
 
+  // Loads "independent" metrics from a metrics provider and executes a
+  // callback when complete, which could be immediate or after some
+  // execution on a background thread.
+  class IndependentMetricsLoader {
+   public:
+    explicit IndependentMetricsLoader(std::unique_ptr<MetricsLog> log);
+    ~IndependentMetricsLoader();
+
+    // Call ProvideIndependentMetrics (which may execute on a background thread)
+    // for the |metrics_provider| and execute the |done_callback| when complete
+    // with the result (true if successful). Though this can be called multiple
+    // times to include data from multiple providers, later calls will override
+    // system profile information set by earlier calls.
+    void Run(base::OnceCallback<void(bool)> done_callback,
+             MetricsProvider* metrics_provider);
+
+    // Extract the filled log. No more Run() operations can be done after this.
+    std::unique_ptr<MetricsLog> ReleaseLog();
+
+   private:
+    std::unique_ptr<MetricsLog> log_;
+    std::unique_ptr<base::HistogramFlattener> flattener_;
+    std::unique_ptr<base::HistogramSnapshotManager> snapshot_manager_;
+
+    DISALLOW_COPY_AND_ASSIGN(IndependentMetricsLoader);
+  };
+
   // Creates a new metrics log of the specified type.
   // |client_id| is the identifier for this profile on this installation
   // |session_id| is an integer that's incremented on each application launch
@@ -96,11 +125,6 @@ class MetricsLog {
   const SystemProfileProto& RecordEnvironment(
       DelegatingProvider* delegating_provider);
 
-  // Loads a saved system profile and the associated metrics into the log.
-  // Returns true on success. Keep calling it with fresh logs until it returns
-  // false.
-  bool LoadIndependentMetrics(MetricsProvider* metrics_provider);
-
   // Loads the environment proto that was saved by the last RecordEnvironment()
   // call from prefs. On success, returns true and |app_version| contains the
   // recovered version. Otherwise (if there was no saved environment in prefs

components/metrics/metrics_provider.cc

@@ -33,10 +33,18 @@ void MetricsProvider::OnRecordingDisabled() {
 void MetricsProvider::OnAppEnterBackground() {
 }
 
-bool MetricsProvider::ProvideIndependentMetrics(
+bool MetricsProvider::HasIndependentMetrics() {
+  return false;
+}
+
+void MetricsProvider::ProvideIndependentMetrics(
+    base::OnceCallback<void(bool)> done_callback,
     SystemProfileProto* system_profile_proto,
     base::HistogramSnapshotManager* snapshot_manager) {
-  return false;
+  // Either the method HasIndependentMetrics() has been overridden and this
+  // method has not, or this method being called without regard to Has().
+  // Both are wrong.
+  NOTREACHED();
 }
 
 void MetricsProvider::ProvideSystemProfileMetrics(

components/metrics/metrics_provider.h

@@ -48,11 +48,16 @@ class MetricsProvider {
   // further notification after this callback.
   virtual void OnAppEnterBackground();
 
+  // Returns whether there are "independent" metrics that can be retrieved
+  // with a call to ProvideIndependentMetrics().
+  virtual bool HasIndependentMetrics();
+
   // Provides a complete and independent system profile + metrics for uploading.
-  // Any histograms added to the |snapshot_manager| will also be included. A
-  // return of false indicates there are none. Will be called repeatedly until
-  // there is nothing else.
-  virtual bool ProvideIndependentMetrics(
+  // This may or may not be executed on a background thread and the callback
+  // executed when complete. Ownership of the passed objects remains with the
+  // caller and those objects must live until the callback is executed.
+  virtual void ProvideIndependentMetrics(
+      base::OnceCallback<void(bool)> done_callback,
       SystemProfileProto* system_profile_proto,
       base::HistogramSnapshotManager* snapshot_manager);
 

components/metrics/metrics_service.cc

@@ -869,22 +869,58 @@ void MetricsService::RecordCurrentStabilityHistograms() {
       &histogram_snapshot_manager_);
 }
 
+void MetricsService::PrepareProviderMetricsLogDone(
+    std::unique_ptr<MetricsLog::IndependentMetricsLoader> loader,
+    bool success) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DCHECK(independent_loader_active_);
+  DCHECK(loader);
+
+  if (success) {
+    log_manager_.PauseCurrentLog();
+    log_manager_.BeginLoggingWithLog(loader->ReleaseLog());
+    log_manager_.FinishCurrentLog(log_store());
+    log_manager_.ResumePausedLog();
+  }
+
+  independent_loader_active_ = false;
+}
+
 bool MetricsService::PrepareProviderMetricsLog() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  // Create a new log. This will have some defaut values injected in it but
-  // those will be overwritten when an embedded profile is extracted.
-  std::unique_ptr<MetricsLog> log = CreateLog(MetricsLog::INDEPENDENT_LOG);
+  // If something is still pending, stop now and indicate that there is
+  // still work to do.
+  if (independent_loader_active_)
+    return true;
 
+  // Check each provider in turn for data.
   for (auto& provider : delegating_provider_.GetProviders()) {
-    if (log->LoadIndependentMetrics(provider.get())) {
-      log_manager_.PauseCurrentLog();
-      log_manager_.BeginLoggingWithLog(std::move(log));
-      log_manager_.FinishCurrentLog(log_store());
-      log_manager_.ResumePausedLog();
+    if (provider->HasIndependentMetrics()) {
+      // Create a new log. This will have some default values injected in it
+      // but those will be overwritten when an embedded profile is extracted.
+      std::unique_ptr<MetricsLog> log = CreateLog(MetricsLog::INDEPENDENT_LOG);
+
+      // Give the new log to a loader for management and then run it on the
+      // provider that has something to give. A copy of the pointer is needed
+      // because the unique_ptr may get moved before the value can be used
+      // to call Run().
+      std::unique_ptr<MetricsLog::IndependentMetricsLoader> loader =
+          std::make_unique<MetricsLog::IndependentMetricsLoader>(
+              std::move(log));
+      MetricsLog::IndependentMetricsLoader* loader_ptr = loader.get();
+      loader_ptr->Run(
+          base::BindOnce(&MetricsService::PrepareProviderMetricsLogDone,
+                         self_ptr_factory_.GetWeakPtr(), std::move(loader)),
+          provider.get());
+      independent_loader_active_ = true;
+
+      // Something was found so there may still be more work to do.
       return true;
     }
   }
+
+  // Nothing was found so indicate there is no more work to do.
   return false;
 }
 

components/metrics/metrics_service.h

@@ -308,6 +308,12 @@ class MetricsService : public base::HistogramFlattener {
   // i.e., histograms with the |kUmaStabilityHistogramFlag| flag set.
   void RecordCurrentStabilityHistograms();
 
+  // Handle completion of PrepareProviderMetricsLog which is run as a
+  // background task.
+  void PrepareProviderMetricsLogDone(
+      std::unique_ptr<MetricsLog::IndependentMetricsLoader> loader,
+      bool success);
+
   // Record a single independent profile and associated histogram from
   // metrics providers. If this returns true, one was found and there may
   // be more.
@@ -380,6 +386,9 @@ class MetricsService : public base::HistogramFlattener {
 
   variations::SyntheticTrialRegistry synthetic_trial_registry_;
 
+  // Indicates if loading of independent metrics is currently active.
+  bool independent_loader_active_ = false;
+
   // Redundant marker to check that we completed our shutdown, and set the
   // exited-cleanly bit in the prefs.
   static ShutdownCleanliness clean_shutdown_status_;