Show all system process in the task_manager.

chrome/browser/task_management/providers/arc/arc_process_task_provider.cc

Index: chrome/browser/task_management/providers/arc/arc_process_task_provider.cc
diff --git a/chrome/browser/task_management/providers/arc/arc_process_task_provider.cc b/chrome/browser/task_management/providers/arc/arc_process_task_provider.cc
index dff7a7b38730e30f4304c6b68ef2c41b523005e3..4d94689feab8140f077389564cb18064ae096e5e 100644
--- a/chrome/browser/task_management/providers/arc/arc_process_task_provider.cc
+++ b/chrome/browser/task_management/providers/arc/arc_process_task_provider.cc
@@ -23,7 +23,8 @@ namespace task_management {
 
 namespace {
 
-const int kUpdateProcessListDelaySeconds = 1;
+const int kUpdateAppProcessListDelaySeconds = 1;
+const int kUpdateSystemProcessListDelaySeconds = 3;
 
 }  // namespace
 
@@ -44,10 +45,8 @@ Task* ArcProcessTaskProvider::GetTaskOfUrlRequest(int origin_pid,
   return nullptr;
 }
 
-void ArcProcessTaskProvider::OnUpdateProcessList(
+void ArcProcessTaskProvider::UpdateAppProcessList(
     const std::vector<ArcProcess>& processes) {
-  TRACE_EVENT0("browser", "ArcProcessTaskProvider::OnUpdateProcessList");
-
   if (!is_updating_)
     return;
 
@@ -85,32 +84,106 @@ void ArcProcessTaskProvider::OnUpdateProcessList(
     NotifyObserverTaskRemoved(nspid_to_task_[entry].get());
     nspid_to_task_.erase(entry);
   }
-  ScheduleNextRequest();
 }
 
-void ArcProcessTaskProvider::RequestProcessList() {
+void ArcProcessTaskProvider::UpdateSystemProcessList(

[cylee1, 2016/06/02 20:58:45]

The logic is almost identical to the original function. Is it possible to
consolidate the two function?
Also can you unify them to use either nspid or pid as key?

[Hsu-Cheng, 2016/06/29 10:33:57]

Done.

+    const std::vector<ArcProcess>& processes) {
+  if (!is_updating_)
+    return;
+
+  set<ProcessId> pid_to_remove;
+  for (const auto& it : pid_to_sys_task_)
+    pid_to_remove.insert(it.first);
+
+  for (const auto& it : processes) {
+    if (pid_to_remove.erase(it.pid) == 0) {
+      // New arc process.
+      std::unique_ptr<ArcProcessTask>& task = pid_to_sys_task_[it.pid];
+      // After calling NotifyObserverTaskAdded(), the raw pointer of |task| is
+      // remebered somewhere else. One should not (implicitly) delete the
+      // referenced object before calling NotifyObserverTaskRemoved() first
+      // (crbug.com/587707).
+      DCHECK(!task.get()) <<
+          "Task with the same pid should not be added twice.";
+      task.reset(new ArcProcessTask(
+          it.pid, it.nspid, it.process_name, it.process_state));
+      NotifyObserverTaskAdded(task.get());
+    }
+  }
+
+  for (const auto& it : pid_to_remove) {
+    // Stale arc process.
+    NotifyObserverTaskRemoved(pid_to_sys_task_[it].get());
+    pid_to_sys_task_.erase(it);
+  }
+}
+
+void ArcProcessTaskProvider::OnUpdateAppProcessList(

[cylee1, 2016/06/02 20:58:45]

I don't quite understand the benefit of separating UpdateAppProcessList and
OnUpdateAppProcessList.
Can you explain?

[Hsu-Cheng, 2016/06/29 10:33:57]

Consolidated the logic here. This is because the code would get more complex
when ScheduleNext***Request() become a closure argument and wrap one more level
into Request***ProcessList(). (Imagine how to merge OnUpdate***ProcessList into
Request***ProcessList) Appreciate any hints if there is simpler design here. I
stuck here for a while.

+    const std::vector<ArcProcess>& processes) {
+  TRACE_EVENT0("browser", "ArcProcessTaskProvider::OnUpdateAppProcessList");
+
+  UpdateAppProcessList(processes);
+  ScheduleNextAppRequest();
+}
+
+void ArcProcessTaskProvider::OnUpdateSystemProcessList(
+    const std::vector<ArcProcess>& processes) {
+  TRACE_EVENT0("browser", "ArcProcessTaskProvider::OnUpdateSystemProcessList");
+
+  UpdateSystemProcessList(processes);
+  ScheduleNextSystemRequest();
+}
+
+void ArcProcessTaskProvider::RequestAppProcessList() {
+  arc::ArcProcessService* arc_process_service =
+      arc::ArcProcessService::Get();
+  if (!arc_process_service ||
+      !arc_process_service->RequestAppProcessList(
+          base::Bind(&ArcProcessTaskProvider::OnUpdateAppProcessList,
+                     weak_ptr_factory_.GetWeakPtr()))) {
+    VLOG(2) << "ARC process instance is not ready.";
+    ScheduleNextAppRequest();
+  }
+}
+
+void ArcProcessTaskProvider::RequestSystemProcessList() {
   arc::ArcProcessService* arc_process_service =
       arc::ArcProcessService::Get();
   if (!arc_process_service ||
-      !arc_process_service->RequestProcessList(
-          base::Bind(&ArcProcessTaskProvider::OnUpdateProcessList,
+      !arc_process_service->RequestSystemProcessList(
+          base::Bind(&ArcProcessTaskProvider::OnUpdateSystemProcessList,
                      weak_ptr_factory_.GetWeakPtr()))) {
     VLOG(2) << "ARC process instance is not ready.";
-    ScheduleNextRequest();
+    ScheduleNextSystemRequest();
   }
 }
 
 void ArcProcessTaskProvider::StartUpdating() {
   is_updating_ = true;
-  RequestProcessList();
+  RequestAppProcessList();
+  RequestSystemProcessList();
 }
 
 void ArcProcessTaskProvider::StopUpdating() {
   is_updating_ = false;
   nspid_to_task_.clear();
+  pid_to_sys_task_.clear();
+}
+
+void ArcProcessTaskProvider::ScheduleNextAppRequest() {
+  if (!is_updating_)
+    return;
+  // TODO(nya): Remove this timer once ARC starts to send us UpdateProcessList
+  // message when the process list changed. As of today, ARC does not send
+  // the process list unless we request it by RequestProcessList message.
+  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE,
+      base::Bind(&ArcProcessTaskProvider::RequestAppProcessList,
+                 weak_ptr_factory_.GetWeakPtr()),
+      base::TimeDelta::FromSeconds(kUpdateAppProcessListDelaySeconds));
 }
 
-void ArcProcessTaskProvider::ScheduleNextRequest() {
+void ArcProcessTaskProvider::ScheduleNextSystemRequest() {
   if (!is_updating_)
     return;
   // TODO(nya): Remove this timer once ARC starts to send us UpdateProcessList
@@ -118,9 +191,9 @@ void ArcProcessTaskProvider::ScheduleNextRequest() {
   // the process list unless we request it by RequestProcessList message.
   base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
       FROM_HERE,
-      base::Bind(&ArcProcessTaskProvider::RequestProcessList,
+      base::Bind(&ArcProcessTaskProvider::RequestSystemProcessList,
                  weak_ptr_factory_.GetWeakPtr()),
-      base::TimeDelta::FromSeconds(kUpdateProcessListDelaySeconds));
+      base::TimeDelta::FromSeconds(kUpdateSystemProcessListDelaySeconds));
 }
 
 }  // namespace task_management