Make MarkProcessAsAlive only run on UI thread.

Currently this runs on both UI and IO threads, which causes problems for https://codereview.chromium.org/1157243005/ which tries to register trigger handles on the UI thread. Additionally, refactored the timer so that we restart it once we've finished gathering. With a small enough gather cycle + bad luck, it's possible right now for both threads to be accessing the metrics_map_.

BUG=

Review URL: https://codereview.chromium.org/1168503002

Cr-Commit-Position: refs/heads/master@{#332442}

4 files changed, 54 insertions, 7 deletions

diff --git a/chrome/browser/performance_monitor/performance_monitor.cc b/chrome/browser/performance_monitor/performance_monitor.cc
index 5cf2ce4..b386ea6 100644
--- a/chrome/browser/performance_monitor/performance_monitor.cc
+++ b/chrome/browser/performance_monitor/performance_monitor.cc
@@ -118,6 +118,8 @@ void PerformanceMonitor::GatherMetricsMapOnUIThread() {
 void PerformanceMonitor::MarkProcessAsAlive(
     const ProcessMetricsMetadata& process_data,
     int current_update_sequence) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
   const base::ProcessHandle& handle = process_data.handle;
   if (handle == base::kNullProcessHandle) {
     // Process may not be valid yet.
@@ -139,6 +141,9 @@ void PerformanceMonitor::GatherMetricsMapOnIOThread(
     int current_update_sequence) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
+  scoped_ptr<std::vector<ProcessMetricsMetadata>> process_data_list(
+      new std::vector<ProcessMetricsMetadata>());
+
   // Find all child processes (does not include renderers), which has to be
   // done on the IO thread.
   for (content::BrowserChildProcessHostIterator iter; !iter.Done(); ++iter) {
@@ -150,15 +155,38 @@ void PerformanceMonitor::GatherMetricsMapOnIOThread(
       child_process_data.process_subtype = kProcessSubtypePPAPIFlash;
     }
 
-    MarkProcessAsAlive(child_process_data, current_update_sequence);
+    process_data_list->push_back(child_process_data);
   }
 
   // Add the current (browser) process.
   ProcessMetricsMetadata browser_process_data;
   browser_process_data.process_type = content::PROCESS_TYPE_BROWSER;
   browser_process_data.handle = base::GetCurrentProcessHandle();
-  MarkProcessAsAlive(browser_process_data, current_update_sequence);
+  process_data_list->push_back(browser_process_data);
+
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&PerformanceMonitor::MarkProcessesAsAliveOnUIThread,
+                 base::Unretained(this), base::Passed(process_data_list.Pass()),
+                 current_update_sequence));
+}
+
+void PerformanceMonitor::MarkProcessesAsAliveOnUIThread(
+    scoped_ptr<std::vector<ProcessMetricsMetadata>> process_data_list,
+    int current_update_sequence) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  for (size_t i = 0; i < process_data_list->size(); ++i) {
+    MarkProcessAsAlive((*process_data_list)[i], current_update_sequence);
+  }
+
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&PerformanceMonitor::UpdateMetricsOnIOThread,
+                 base::Unretained(this), current_update_sequence));
+}
 
+void PerformanceMonitor::UpdateMetricsOnIOThread(int current_update_sequence) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
   // Update metrics for all watched processes; remove dead entries from the map.
   MetricsMap::iterator iter = metrics_map_.begin();
   while (iter != metrics_map_.end()) {
@@ -171,6 +199,19 @@ void PerformanceMonitor::GatherMetricsMapOnIOThread(
       ++iter;
     }
   }
+
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                          base::Bind(&PerformanceMonitor::RunTriggersUIThread,
+                                     base::Unretained(this)));
+}
+
+void PerformanceMonitor::RunTriggersUIThread() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  for (auto it = metrics_map_.begin(); it != metrics_map_.end(); ++it) {
+    it->second.RunPerformanceTriggers();
+  }
+
+  StartGatherCycle();
 }
 
 }  // namespace performance_monitor
diff --git a/chrome/browser/performance_monitor/performance_monitor.h b/chrome/browser/performance_monitor/performance_monitor.h
index 65d89b6..22bdb58 100644
--- a/chrome/browser/performance_monitor/performance_monitor.h
+++ b/chrome/browser/performance_monitor/performance_monitor.h
@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_PERFORMANCE_MONITOR_PERFORMANCE_MONITOR_H_
 
 #include <map>
+#include <vector>
 
 #include "base/process/process_handle.h"
 #include "base/timer/timer.h"
@@ -48,17 +49,23 @@ class PerformanceMonitor {
   // already present.
   void MarkProcessAsAlive(const ProcessMetricsMetadata& process_data,
                           int current_update_sequence);
+  void MarkProcessesAsAliveOnUIThread(
+      scoped_ptr<std::vector<ProcessMetricsMetadata>> process_data_list,
+      int current_update_sequence);
 
   // Updates the ProcessMetrics map with the current list of processes and
   // gathers metrics from each entry.
   void GatherMetricsMapOnUIThread();
   void GatherMetricsMapOnIOThread(int current_update_sequence);
 
+  void UpdateMetricsOnIOThread(int current_update_sequence);
+  void RunTriggersUIThread();
+
   // A map of currently running ProcessHandles to ProcessMetrics.
   MetricsMap metrics_map_;
 
   // The timer to signal PerformanceMonitor to perform its timed collections.
-  base::RepeatingTimer<PerformanceMonitor> repeating_timer_;
+  base::OneShotTimer<PerformanceMonitor> repeating_timer_;
 
   DISALLOW_COPY_AND_ASSIGN(PerformanceMonitor);
 };
diff --git a/chrome/browser/performance_monitor/process_metrics_history.cc b/chrome/browser/performance_monitor/process_metrics_history.cc
index 10263d8..701a114 100644
--- a/chrome/browser/performance_monitor/process_metrics_history.cc
+++ b/chrome/browser/performance_monitor/process_metrics_history.cc
@@ -47,8 +47,6 @@ void ProcessMetricsHistory::Initialize(
 
 void ProcessMetricsHistory::SampleMetrics() {
   cpu_usage_ = process_metrics_->GetPlatformIndependentCPUUsage();
-
-  RunPerformanceTriggers();
 }
 
 void ProcessMetricsHistory::RunPerformanceTriggers() {
diff --git a/chrome/browser/performance_monitor/process_metrics_history.h b/chrome/browser/performance_monitor/process_metrics_history.h
index 21f8740..186236d 100644
--- a/chrome/browser/performance_monitor/process_metrics_history.h
+++ b/chrome/browser/performance_monitor/process_metrics_history.h
@@ -45,6 +45,9 @@ class ProcessMetricsHistory {
   // Gather metrics for the process and accumulate with past data.
   void SampleMetrics();
 
+  // Triggers any UMA histograms or background traces if cpu_usage is excessive.
+  void RunPerformanceTriggers();
+
   // Used to mark when this object was last updated, so we can cull
   // dead ones.
   void set_last_update_sequence(int new_update_sequence) {
@@ -54,8 +57,6 @@ class ProcessMetricsHistory {
   int last_update_sequence() const { return last_update_sequence_; }
 
  private:
-  void RunPerformanceTriggers();
-
   // May not be fully populated. e.g. no |id| and no |name| for browser and
   // renderer processes.
   ProcessMetricsMetadata process_data_;