Migrate ProcessesEventRouter to the new task manager

chrome/browser/task_management/sampling/task_group.cc

Index: chrome/browser/task_management/sampling/task_group.cc
diff --git a/chrome/browser/task_management/sampling/task_group.cc b/chrome/browser/task_management/sampling/task_group.cc
index 9911136d9cf37e55f6f44e50358605e1b95c6571..c94439077effbc644d0150b65a4405b0d2633e5a 100644
--- a/chrome/browser/task_management/sampling/task_group.cc
+++ b/chrome/browser/task_management/sampling/task_group.cc
@@ -16,6 +16,16 @@ namespace task_management {
 
 namespace {
 
+// A mask for the refresh types that are done in the background thread.
+const int kBackgroundRefreshTypesMask =
+    REFRESH_TYPE_CPU |
+    REFRESH_TYPE_MEMORY |
+    REFRESH_TYPE_IDLE_WAKEUPS |
+#if defined(OS_LINUX)
+    REFRESH_TYPE_FD_COUNT |
+#endif  // defined(OS_LINUX)
+    REFRESH_TYPE_PRIORITY;
+
 inline bool IsResourceRefreshEnabled(RefreshType refresh_type,
                                      int refresh_flags) {
   return (refresh_flags & refresh_type) != 0;
@@ -56,11 +66,14 @@ void GetWindowsHandles(base::ProcessHandle handle,
 TaskGroup::TaskGroup(
     base::ProcessHandle proc_handle,
     base::ProcessId proc_id,
+    const base::Closure& on_background_calculations_done,
     const scoped_refptr<base::SequencedTaskRunner>& blocking_pool_runner)
     : process_handle_(proc_handle),
       process_id_(proc_id),
+      on_background_calculations_done_(on_background_calculations_done),
       worker_thread_sampler_(nullptr),
-      tasks_(),
+      expected_on_bg_done_flags_(kBackgroundRefreshTypesMask),
+      current_on_bg_done_flags_(0),
       cpu_usage_(0.0),
       memory_usage_(),
       gpu_memory_(-1),
@@ -123,6 +136,8 @@ void TaskGroup::Refresh(
     int64_t refresh_flags) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
+  expected_on_bg_done_flags_ = refresh_flags & kBackgroundRefreshTypesMask;

[ncarter (slow), 2016/02/17 21:21:19]

Is it possible for the next Refresh event to fire, before we hear back from the
blocking pool for the previous refresh? Does anything weird happen if it does?

[afakhry, 2016/02/19 17:35:09]

I think it could, since we are using a sequenced task runner for the background
thread. So, I think if we have a lot a lot of tasks, with a lot of background
refresh types enabled, it could happen that (at a one-second refresh interval)
the periodic Refresh() happens before all background operations finishes.

There shouldn't be anything weird if that happens. We only fire the event
TaskManagerObserver::OnTasksRefreshedWithBackgroundCalculations() when all
TaskGroups have heard back from their background samplers for all the enabled
refresh types.

We clear |current_on_bg_done_flags_| only when all observers have been notified
with OnTasksRefreshedWithBackgroundCalculations() at which point we are ready
for another cycle for this event.

So This event means we have ready data for the background calculations like the
CPU or memory ***from the most recent completed background operation***. That
could be from an operation triggered by the current Refresh() or the previous
one. The thing is once we have a complete snapshot of all background data, we
broadcast this event, reset |current_on_bg_done_flags_|, repeat.

[ncarter (slow), 2016/02/19 18:19:03]

OK.

I do worry a bit about the case where sampling takes longer than 1s on average
(due e.g. to extreme load), and we wind up with a queue of Refresh() tasks that
grows faster than it drains.

+
   // First refresh the enabled non-expensive resources usages on the UI thread.
   // 1- Refresh all the tasks as well as the total network usage (if enabled).
   const bool network_usage_refresh_enabled =
@@ -132,9 +147,8 @@ void TaskGroup::Refresh(
     Task* task = task_pair.second;
     task->Refresh(update_interval, refresh_flags);
 
-    if (network_usage_refresh_enabled && task->ReportsNetworkUsage()) {
+    if (network_usage_refresh_enabled && task->ReportsNetworkUsage())
       per_process_network_usage_ += task->network_usage();
-    }
   }
 
   // 2- Refresh GPU memory (if enabled).
@@ -214,18 +228,21 @@ void TaskGroup::OnCpuRefreshDone(double cpu_usage) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   cpu_usage_ = cpu_usage;
+  OnBackgroundRefreshTypeFinished(REFRESH_TYPE_CPU);
 }
 
 void TaskGroup::OnMemoryUsageRefreshDone(MemoryUsageStats memory_usage) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   memory_usage_ = memory_usage;
+  OnBackgroundRefreshTypeFinished(REFRESH_TYPE_MEMORY);
 }
 
 void TaskGroup::OnIdleWakeupsRefreshDone(int idle_wakeups_per_second) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   idle_wakeups_per_second_ = idle_wakeups_per_second;
+  OnBackgroundRefreshTypeFinished(REFRESH_TYPE_IDLE_WAKEUPS);
 }
 
 #if defined(OS_LINUX)
@@ -233,6 +250,7 @@ void TaskGroup::OnOpenFdCountRefreshDone(int open_fd_count) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   open_fd_count_ = open_fd_count;
+  OnBackgroundRefreshTypeFinished(REFRESH_TYPE_FD_COUNT);
 }
 #endif  // defined(OS_LINUX)
 
@@ -240,6 +258,15 @@ void TaskGroup::OnProcessPriorityDone(bool is_backgrounded) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   is_backgrounded_ = is_backgrounded;
+  OnBackgroundRefreshTypeFinished(REFRESH_TYPE_PRIORITY);
+}
+
+void TaskGroup::OnBackgroundRefreshTypeFinished(int64_t finished_refresh_type) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  current_on_bg_done_flags_ |= finished_refresh_type;

[ncarter (slow), 2016/02/17 21:21:19]

current_on_bg_done_flags_ is never cleared back to 0. Is that intentional?

Since the TaskGroup can persist across extension function invocations, it seems
like subsequent queries might notify too early, when the first (rather than
last) background type reports back "refresh done", and I imagine that the other
types would be stale at this point?

[afakhry, 2016/02/19 17:35:09]

Oh sorry it was clear enough. |current_on_bg_done_flags_| is actually cleared in
the ClearCurrentBackgroundCalculationsFlags() in task_group.h. I moved the
definition to the .cc file.

+  if (current_on_bg_done_flags_ == expected_on_bg_done_flags_)

[ncarter (slow), 2016/02/17 21:21:19]

What if refresh_flags (at Refresh() time) didn't specify *all* of the
kBackgroundRefreshTypesMask types? In that case it seems like we'll never run
the callback (because e.g. OnIdleWakeupsRefreshDone won't happen if
REFRESH_TYPE_IDLE_WAKEUPS wasn't requested?)

[afakhry, 2016/02/19 17:35:09]

Thanks! I added a line to TaskGroup:Refresh() to take care of this case when a
refresh type gets disabled.

+    on_background_calculations_done_.Run();
 }
 
 }  // namespace task_management