Migrate ProcessesEventRouter to the new task manager

chrome/browser/extensions/api/processes/processes_api.cc

 // Copyright (c) 2012 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 "chrome/browser/extensions/api/processes/processes_api.h"
 
-#include <stddef.h>
-#include <stdint.h>
-#include <utility>
-
-#include "base/callback.h"
-#include "base/json/json_writer.h"
 #include "base/lazy_instance.h"
-#include "base/location.h"
-#include "base/metrics/histogram.h"
-#include "base/single_thread_task_runner.h"
+#include "base/metrics/histogram_macros.h"
+#include "base/process/process.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
-#include "base/thread_task_runner_handle.h"
-#include "base/values.h"
-#include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/extensions/api/processes/processes_api_constants.h"
 #include "chrome/browser/extensions/api/tabs/tabs_constants.h"
-#include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/task_manager/resource_provider.h"
-#include "chrome/browser/task_manager/task_manager.h"
-#include "content/public/browser/browser_context.h"
-#include "content/public/browser/notification_details.h"
-#include "content/public/browser/notification_service.h"
-#include "content/public/browser/notification_source.h"
-#include "content/public/browser/notification_types.h"
+#include "chrome/browser/task_management/task_manager_interface.h"
+#include "chrome/common/extensions/api/processes.h"
+#include "content/public/browser/browser_child_process_host.h"
+#include "content/public/browser/child_process_data.h"
 #include "content/public/browser/render_process_host.h"
-#include "content/public/browser/render_view_host.h"
-#include "content/public/browser/render_widget_host.h"
-#include "content/public/browser/render_widget_host_iterator.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/result_codes.h"
-#include "extensions/browser/event_router.h"
-#include "extensions/browser/extension_function_registry.h"
-#include "extensions/browser/extension_function_util.h"
 #include "extensions/common/error_utils.h"
+#include "third_party/WebKit/public/web/WebCache.h"
 
 namespace extensions {
 
-namespace keys = processes_api_constants;
-namespace errors = processes_api_constants;
+namespace errors {
+const char kNotAllowedToTerminate[] = "Not allowed to terminate process: *.";
+const char kProcessNotFound[] = "Process not found: *.";
+}  // namespace errors
 
 namespace {
 
-#if defined(ENABLE_TASK_MANAGER)
-
-base::DictionaryValue* CreateCacheData(
+base::LazyInstance<BrowserContextKeyedAPIFactory<ProcessesAPI> >
+    g_processes_api_factory = LAZY_INSTANCE_INITIALIZER;
+
+int64_t GetRefreshTypesFlagOnlyEssentialData() {
+  // This is the only non-optional data in the Process as defined by the API in
+  // processes.idl.
+  return task_management::REFRESH_TYPE_NACL;
+}
+
+// This does not include memory. The memory refresh flag will only be added once
+// a listener to OnUpdatedWithMemory event is added.
+int64_t GetRefreshTypesFlagIncludeOptionalData() {
+  return GetRefreshTypesFlagOnlyEssentialData() |
+      task_management::REFRESH_TYPE_CPU |
+      task_management::REFRESH_TYPE_NETWORK_USAGE |
+      task_management::REFRESH_TYPE_SQLITE_MEMORY |
+      task_management::REFRESH_TYPE_V8_MEMORY |
+      task_management::REFRESH_TYPE_WEBCACHE_STATS;
+}
+
+scoped_ptr<api::processes::Cache> CreateCacheData(
     const blink::WebCache::ResourceTypeStat& stat) {
-
-  base::DictionaryValue* cache = new base::DictionaryValue();
-  cache->SetDouble(keys::kCacheSize, static_cast<double>(stat.size));
-  cache->SetDouble(keys::kCacheLiveSize, static_cast<double>(stat.liveSize));
+  scoped_ptr<api::processes::Cache> cache(new api::processes::Cache);
+  cache->size = static_cast<double>(stat.size);
+  cache->live_size = static_cast<double>(stat.liveSize);
   return cache;
 }
 
-void SetProcessType(base::DictionaryValue* result,
-                    TaskManagerModel* model,
-                    int index) {
-  // Determine process type.
-  std::string type = keys::kProcessTypeOther;
-  task_manager::Resource::Type resource_type = model->GetResourceType(index);
-  switch (resource_type) {
-    case task_manager::Resource::BROWSER:
-      type = keys::kProcessTypeBrowser;
-      break;
-    case task_manager::Resource::RENDERER:
-      type = keys::kProcessTypeRenderer;
-      break;
-    case task_manager::Resource::EXTENSION:
-      type = keys::kProcessTypeExtension;
-      break;
-    case task_manager::Resource::NOTIFICATION:
-      type = keys::kProcessTypeNotification;
-      break;
-    case task_manager::Resource::PLUGIN:
-      type = keys::kProcessTypePlugin;
-      break;
-    case task_manager::Resource::WORKER:
-      type = keys::kProcessTypeWorker;
-      break;
-    case task_manager::Resource::NACL:
-      type = keys::kProcessTypeNacl;
-      break;
-    case task_manager::Resource::UTILITY:
-      type = keys::kProcessTypeUtility;
-      break;
-    case task_manager::Resource::GPU:
-      type = keys::kProcessTypeGPU;
-      break;
-    case task_manager::Resource::ZYGOTE:
-    case task_manager::Resource::SANDBOX_HELPER:
-    case task_manager::Resource::UNKNOWN:
-      type = keys::kProcessTypeOther;
-      break;
-    default:
-      NOTREACHED() << "Unknown resource type.";
-  }
-  result->SetString(keys::kTypeKey, type);
-}
-
-base::ListValue* GetTabsForProcess(int process_id) {
-  base::ListValue* tabs_list = new base::ListValue();
-
-  // The tabs list only makes sense for render processes, so if we don't find
-  // one, just return the empty list.
-  content::RenderProcessHost* rph =
-      content::RenderProcessHost::FromID(process_id);
-  if (rph == NULL)
-    return tabs_list;
-
-  int tab_id = -1;
-  // We need to loop through all the RVHs to ensure we collect the set of all
-  // tabs using this renderer process.
-  scoped_ptr<content::RenderWidgetHostIterator> widgets(
-      content::RenderWidgetHost::GetRenderWidgetHosts());
-  while (content::RenderWidgetHost* widget = widgets->GetNextHost()) {
-    if (widget->GetProcess()->GetID() != process_id)
-      continue;
-
-    content::RenderViewHost* host = content::RenderViewHost::From(widget);
-    content::WebContents* contents =
-        content::WebContents::FromRenderViewHost(host);
-    if (contents) {
-      tab_id = ExtensionTabUtil::GetTabId(contents);
-      if (tab_id != -1)
-        tabs_list->Append(new base::FundamentalValue(tab_id));
-    }
-  }
-
-  return tabs_list;
-}
-
-// This function creates a Process object to be returned to the extensions
-// using these APIs. For memory details, which are not added by this function,
-// the callers need to use AddMemoryDetails.
-base::DictionaryValue* CreateProcessFromModel(int process_id,
-                                              TaskManagerModel* model,
-                                              int index,
-                                              bool include_optional) {
-  base::DictionaryValue* result = new base::DictionaryValue();
-  size_t mem;
-
-  result->SetInteger(keys::kIdKey, process_id);
-  result->SetInteger(keys::kOsProcessIdKey, model->GetProcessId(index));
-  SetProcessType(result, model, index);
-  result->SetString(keys::kTitleKey, model->GetResourceTitle(index));
-  result->SetString(keys::kProfileKey,
-      model->GetResourceProfileName(index));
-  result->SetInteger(keys::kNaClDebugPortKey,
-                     model->GetNaClDebugStubPort(index));
-
-  result->Set(keys::kTabsListKey, GetTabsForProcess(process_id));
+api::processes::ProcessType GetProcessType(
+    task_management::Task::Type task_type) {
+  switch (task_type) {
+    case task_management::Task::BROWSER:
+      return api::processes::PROCESS_TYPE_BROWSER;
+
+    case task_management::Task::RENDERER:
+      return api::processes::PROCESS_TYPE_RENDERER;
+
+    case task_management::Task::EXTENSION:
+    case task_management::Task::GUEST:
+      return api::processes::PROCESS_TYPE_EXTENSION;
+
+    case task_management::Task::PLUGIN:
+      return api::processes::PROCESS_TYPE_PLUGIN;
+
+    case task_management::Task::WORKER:
+      return api::processes::PROCESS_TYPE_WORKER;
+
+    case task_management::Task::NACL:
+      return api::processes::PROCESS_TYPE_NACL;
+
+    case task_management::Task::UTILITY:
+      return api::processes::PROCESS_TYPE_UTILITY;
+
+    case task_management::Task::GPU:
+      return api::processes::PROCESS_TYPE_GPU;
+
+    case task_management::Task::UNKNOWN:
+    case task_management::Task::ARC:
+    case task_management::Task::SANDBOX_HELPER:
+    case task_management::Task::ZYGOTE:
+      return api::processes::PROCESS_TYPE_OTHER;
+  }
+
+  NOTREACHED() << "Unknown task type.";
+  return api::processes::PROCESS_TYPE_NONE;
+}
+
+// Fills |out_process| with the data of the process in which the task with |id|
+// is running. If |include_optional| is true, this function will fill the
+// optional fields in |api::processes::Process| except for |private_memory|,
+// which should be filled later if needed.
+void FillProcessData(
+    task_management::TaskId id,
+    task_management::TaskManagerInterface* task_manager,
+    bool include_optional,
+    api::processes::Process* out_process) {
+  DCHECK(out_process);
+
+  out_process->id = task_manager->GetChildProcessUniqueId(id);
+  out_process->os_process_id = task_manager->GetProcessId(id);
+  out_process->type = GetProcessType(task_manager->GetType(id));
+  out_process->profile = base::UTF16ToUTF8(task_manager->GetProfileName(id));
+  out_process->nacl_debug_port = task_manager->GetNaClDebugStubPort(id);
+
+  // Collect the tab IDs of all the tasks sharing this renderer if any.
+  const task_management::TaskIdList tasks_on_process =
+      task_manager->GetIdsOfTasksSharingSameProcess(id);
+  for (const auto& task_id : tasks_on_process) {
+    linked_ptr<api::processes::TaskInfo> task_info(
+        new api::processes::TaskInfo);
+    task_info->title = base::UTF16ToUTF8(task_manager->GetTitle(task_id));
+    const int tab_id = task_manager->GetTabId(task_id);
+    if (tab_id != -1)
+      task_info->tab_id.reset(new int(tab_id));
+
+    out_process->tasks.push_back(task_info);
+  }
 
   // If we don't need to include the optional properties, just return now.
   if (!include_optional)
-    return result;
-
-  result->SetDouble(keys::kCpuKey, model->GetCPUUsage(index));
-
-  if (model->GetV8Memory(index, &mem))
-    result->SetDouble(keys::kJsMemoryAllocatedKey,
-        static_cast<double>(mem));
-
-  if (model->GetV8MemoryUsed(index, &mem))
-    result->SetDouble(keys::kJsMemoryUsedKey,
-        static_cast<double>(mem));
-
-  if (model->GetSqliteMemoryUsedBytes(index, &mem))
-    result->SetDouble(keys::kSqliteMemoryKey,
-        static_cast<double>(mem));
+    return;
+
+  out_process->cpu.reset(new double(task_manager->GetCpuUsage(id)));
+
+  out_process->network.reset(new double(static_cast<double>(
+      task_manager->GetProcessTotalNetworkUsage(id))));
+
+  int64_t v8_allocated;
+  int64_t v8_used;
+  if (task_manager->GetV8Memory(id, &v8_allocated, & v8_used)) {
+    out_process->js_memory_allocated.reset(new double(static_cast<double>(
+        v8_allocated)));
+    out_process->js_memory_used.reset(new double(static_cast<double>(v8_used)));
+  }
+
+  const int64_t sqlite_bytes = task_manager->GetSqliteMemoryUsed(id);
+  if (sqlite_bytes != -1) {
+    out_process->sqlite_memory.reset(new double(static_cast<double>(
+        sqlite_bytes)));
+  }
 
   blink::WebCache::ResourceTypeStats cache_stats;
-  if (model->GetWebCoreCacheStats(index, &cache_stats)) {
-    result->Set(keys::kImageCacheKey,
-                CreateCacheData(cache_stats.images));
-    result->Set(keys::kScriptCacheKey,
-                CreateCacheData(cache_stats.scripts));
-    result->Set(keys::kCssCacheKey,
-                CreateCacheData(cache_stats.cssStyleSheets));
-  }
-
-  // Network is reported by the TaskManager per resource (tab), not per
-  // process, therefore we need to iterate through the group of resources
-  // and aggregate the data.
-  int64_t net = 0;
-  int length = model->GetGroupRangeForResource(index).second;
-  for (int i = 0; i < length; ++i)
-    net += model->GetNetworkUsage(index + i);
-  result->SetDouble(keys::kNetworkKey, static_cast<double>(net));
-
-  return result;
-}
-
-// Since memory details are expensive to gather, we don't do it by default.
-// This function is a helper to add memory details data to an existing
-// Process object representation.
-void AddMemoryDetails(base::DictionaryValue* result,
-                      TaskManagerModel* model,
-                      int index) {
-  size_t mem;
-  int64_t pr_mem =
-      model->GetPrivateMemory(index, &mem) ? static_cast<int64_t>(mem) : -1;
-  result->SetDouble(keys::kPrivateMemoryKey, static_cast<double>(pr_mem));
-}
-
-#endif  // defined(ENABLE_TASK_MANAGER)
+  if (task_manager->GetWebCacheStats(id, &cache_stats)) {
+    out_process->image_cache = CreateCacheData(cache_stats.images);
+    out_process->script_cache = CreateCacheData(cache_stats.scripts);
+    out_process->css_cache = CreateCacheData(cache_stats.cssStyleSheets);
+  }
+}
 
 }  // namespace
 
+////////////////////////////////////////////////////////////////////////////////
+// ProcessesEventRouter:
+////////////////////////////////////////////////////////////////////////////////
+
 ProcessesEventRouter::ProcessesEventRouter(content::BrowserContext* context)
-    : browser_context_(context), listeners_(0), task_manager_listening_(false) {
-#if defined(ENABLE_TASK_MANAGER)
-  model_ = TaskManager::GetInstance()->model();
-  model_->AddObserver(this);
-
-  registrar_.Add(this, content::NOTIFICATION_RENDER_WIDGET_HOST_HANG,
-      content::NotificationService::AllSources());
-  registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
-      content::NotificationService::AllSources());
-#endif  // defined(ENABLE_TASK_MANAGER)
+    : task_management::TaskManagerObserver(
+          base::TimeDelta::FromSeconds(1),
+          GetRefreshTypesFlagIncludeOptionalData()),
+      browser_context_(context),
+      listeners_(0) {
 }
 
 ProcessesEventRouter::~ProcessesEventRouter() {
-#if defined(ENABLE_TASK_MANAGER)
-  registrar_.Remove(this, content::NOTIFICATION_RENDER_WIDGET_HOST_HANG,
-      content::NotificationService::AllSources());
-  registrar_.Remove(this, content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
-      content::NotificationService::AllSources());
-
-  if (task_manager_listening_)
-    model_->StopListening();
-
-  model_->RemoveObserver(this);
-#endif  // defined(ENABLE_TASK_MANAGER)
+  if (observed_task_manager()) {
+    task_management::TaskManagerInterface::GetTaskManager()->RemoveObserver(
+        this);
+  }

[ncarter (slow), 2016/02/11 00:33:54]

Given the need to conditionally unsubscribe in the dtor, maybe ScopedObserver
would be helpful.

[afakhry, 2016/02/17 02:27:13]

I think ScopedObserver might be an overkill. It seems to be used when there is a
need to observe many Sources. In our case we only observe a single task manager.
The above condition is there for our sanity just in case something skipped
ListenerRemoved() below.

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

Acknowledged.

 }
 
 void ProcessesEventRouter::ListenerAdded() {
-#if defined(ENABLE_TASK_MANAGER)
-  // The task manager has its own ref count to balance other callers of
-  // StartUpdating/StopUpdating.
-  model_->StartUpdating();
-#endif  // defined(ENABLE_TASK_MANAGER)
-  ++listeners_;
+  // Do we need to update memory usage?
+  if (HasEventListeners(api::processes::OnUpdatedWithMemory::kEventName))
+    AddRefreshType(task_management::REFRESH_TYPE_MEMORY);
+
+  if (listeners_++ == 0) {
+   // The first listener to be added.
+   task_management::TaskManagerInterface::GetTaskManager()->AddObserver(this);

[ncarter (slow), 2016/02/11 00:33:55]

indentation is off here.

[afakhry, 2016/02/17 02:27:14]

Done.

+  }
 }
 
 void ProcessesEventRouter::ListenerRemoved() {
-  DCHECK_GT(listeners_, 0);
-  --listeners_;
-#if defined(ENABLE_TASK_MANAGER)
-  // The task manager has its own ref count to balance other callers of
-  // StartUpdating/StopUpdating.
-  model_->StopUpdating();
-#endif  // defined(ENABLE_TASK_MANAGER)
-}
-
-void ProcessesEventRouter::StartTaskManagerListening() {
-#if defined(ENABLE_TASK_MANAGER)
-  if (!task_manager_listening_) {
-    model_->StartListening();
-    task_manager_listening_ = true;
-  }
-#endif  // defined(ENABLE_TASK_MANAGER)
-}
-
-void ProcessesEventRouter::Observe(
-    int type,
-    const content::NotificationSource& source,
-    const content::NotificationDetails& details) {
-
-  switch (type) {
-    case content::NOTIFICATION_RENDER_WIDGET_HOST_HANG:
-      ProcessHangEvent(
-          content::Source<content::RenderWidgetHost>(source).ptr());
-      break;
-    case content::NOTIFICATION_RENDERER_PROCESS_CLOSED:
-      ProcessClosedEvent(
-          content::Source<content::RenderProcessHost>(source).ptr(),
-          content::Details<content::RenderProcessHost::RendererClosedDetails>(
-              details).ptr());
-      break;
-    default:
-      NOTREACHED() << "Unexpected observe of type " << type;
-  }
-  return;
-}
-
-void ProcessesEventRouter::OnItemsAdded(int start, int length) {
-#if defined(ENABLE_TASK_MANAGER)
-  DCHECK_EQ(length, 1);
-  int index = start;
-
-  std::string event(keys::kOnCreated);
-  if (!HasEventListeners(event))
-    return;
-
-  // If the item being added is not the first one in the group, find the base
-  // index and use it for retrieving the process data.
-  if (!model_->IsResourceFirstInGroup(start)) {
-    index = model_->GetGroupIndexForResource(start);
-  }
-
-  scoped_ptr<base::ListValue> args(new base::ListValue());
-  base::DictionaryValue* process = CreateProcessFromModel(
-      model_->GetUniqueChildProcessId(index), model_, index, false);
-  DCHECK(process != NULL);
-
-  if (process == NULL)
-    return;
-
-  args->Append(process);
-
-  DispatchEvent(events::PROCESSES_ON_CREATED, keys::kOnCreated,
-                std::move(args));
-#endif  // defined(ENABLE_TASK_MANAGER)
-}
-
-void ProcessesEventRouter::OnItemsChanged(int start, int length) {
-#if defined(ENABLE_TASK_MANAGER)
-  // If we don't have any listeners, return immediately.
-  if (listeners_ == 0)
-    return;
-
-  if (!model_)
-    return;
-
-  // We need to know which type of onUpdated events to fire and whether to
-  // collect memory or not.
-  std::string updated_event(keys::kOnUpdated);
-  std::string updated_event_memory(keys::kOnUpdatedWithMemory);
-  bool updated = HasEventListeners(updated_event);
-  bool updated_memory = HasEventListeners(updated_event_memory);
-
-  DCHECK(updated || updated_memory);
-
-  IDMap<base::DictionaryValue> processes_map;
-  for (int i = start; i < start + length; i++) {
-    if (model_->IsResourceFirstInGroup(i)) {
-      int id = model_->GetUniqueChildProcessId(i);
-      base::DictionaryValue* process = CreateProcessFromModel(id, model_, i,
-                                                              true);
-      processes_map.AddWithID(process, i);
+  // No more listeners for memory usage?
+  if (!HasEventListeners(api::processes::OnUpdatedWithMemory::kEventName))
+    RemoveRefreshType(task_management::REFRESH_TYPE_MEMORY);
+
+  if (--listeners_ == 0) {
+    // Last listener to be removed.
+    task_management::TaskManagerInterface::GetTaskManager()->RemoveObserver(
+        this);
+  }
+}
+
+void ProcessesEventRouter::OnTaskAdded(task_management::TaskId id) {
+  if (!HasEventListeners(api::processes::OnCreated::kEventName))
+    return;
+
+  // Is this the first task to be created on the process and hence its creation
+  // corresponds to the creation of the process?
+  if (observed_task_manager()->GetNumberOfTasksOnSameProcess(id) != 1)
+    return;
+
+  // Ignore tasks that don't have a child host ID that is greater than 0 (i.e.
+  // browser process task, ARC tasks).
+  if (observed_task_manager()->GetChildProcessUniqueId(id) <= 0)
+    return;
+
+  api::processes::Process process;
+  FillProcessData(id, observed_task_manager(), false, &process);
+  DispatchEvent(events::PROCESSES_ON_CREATED,
+                api::processes::OnCreated::kEventName,
+                api::processes::OnCreated::Create(process));
+}
+
+void ProcessesEventRouter::OnTaskToBeRemoved(task_management::TaskId id) {
+  if (!HasEventListeners(api::processes::OnExited::kEventName))
+    return;
+
+  // Is this the last task to be removed from the process, and hence its removal
+  // corresponds to the termination of the process?
+  if (observed_task_manager()->GetNumberOfTasksOnSameProcess(id) != 1)
+    return;
+
+  // Ignore tasks that don't have a child host ID that is greater than 0 (i.e.
+  // browser process task, ARC tasks).
+  if (observed_task_manager()->GetChildProcessUniqueId(id) <= 0)
+    return;
+
+  int exit_code = 0;
+  base::TerminationStatus status = base::TERMINATION_STATUS_STILL_RUNNING;
+  observed_task_manager()->GetTerminationStatus(id, &status, &exit_code);
+
+  DispatchEvent(events::PROCESSES_ON_EXITED,
+                api::processes::OnExited::kEventName,
+                api::processes::OnExited::Create(
+                    observed_task_manager()->GetChildProcessUniqueId(id),
+                    status,
+                    exit_code));
+
+}
+
+void ProcessesEventRouter::OnTasksRefreshed(
+    const task_management::TaskIdList& task_ids) {
+  const bool has_on_updated_listeners =
+      HasEventListeners(api::processes::OnUpdated::kEventName);
+  const bool has_on_updated_with_memory_listeners =
+      HasEventListeners(api::processes::OnUpdatedWithMemory::kEventName);
+
+  if (!has_on_updated_listeners && !has_on_updated_with_memory_listeners)
+    return;
+
+  // Get the data of tasks sharing the same process only once.
+  std::set<base::ProcessId> seen_processes;
+  base::DictionaryValue processes_dictionary;
+  for (const auto& task_id : task_ids) {
+    // We are not interested in tasks, but rather the processes on which they
+    // run.
+    const base::ProcessId proc_id =
+        observed_task_manager()->GetProcessId(task_id);

[ncarter (slow), 2016/02/11 00:33:55]

Why not de-dup on child_process_host_id?

[afakhry, 2016/02/17 02:27:13]

child_process_host_id isn't unique enough unfortunately. All ARC processes will
report a value of content::ChildProcessHost::kInvalidUniqueID (-1) -- 0 prior to
this patch -- so it's safer to dedup with something that is surely unique.

+    if (seen_processes.count(proc_id))
+      continue;
+
+    seen_processes.insert(proc_id);

[ncarter (slow), 2016/02/11 00:33:55]

In OnTaskAdded, we filter out tasks if ChildProcessUniqueId <= 0. Should we do
the same here?

[afakhry, 2016/02/17 02:27:13]

We actually want to report the browser process, but ignore ARC processes. 
I got this wrong idea that we don't want to report the browser process from the
test here:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/test/data/e...

This is one of the reasons why I'm wondering why can't we just use the OS PIDs
for this API instead of the child_process_host_ids.

+    const int child_process_host_id =
+        observed_task_manager()->GetChildProcessUniqueId(task_id);
+    api::processes::Process process;
+    FillProcessData(task_id, observed_task_manager(), true, &process);
+
+    if (has_on_updated_with_memory_listeners) {
+      // Append the private memory usage to the process data.
+      const int64_t private_memory =
+          observed_task_manager()->GetPrivateMemoryUsage(task_id);
+      process.private_memory.reset(new double(static_cast<double>(
+          private_memory)));
     }
-  }
-
-  int id;
-  std::string idkey(keys::kIdKey);
-  base::DictionaryValue* processes = new base::DictionaryValue();
-
-  if (updated) {
-    IDMap<base::DictionaryValue>::iterator it(&processes_map);
-    for (; !it.IsAtEnd(); it.Advance()) {
-      if (!it.GetCurrentValue()->GetInteger(idkey, &id))
-        continue;
-
-      // Store each process indexed by the string version of its id.
-      processes->Set(base::IntToString(id), it.GetCurrentValue());
-    }
-
-    scoped_ptr<base::ListValue> args(new base::ListValue());
-    args->Append(processes);
-    DispatchEvent(events::PROCESSES_ON_UPDATED, keys::kOnUpdated,
-                  std::move(args));
-  }
-
-  if (updated_memory) {
-    IDMap<base::DictionaryValue>::iterator it(&processes_map);
-    for (; !it.IsAtEnd(); it.Advance()) {
-      if (!it.GetCurrentValue()->GetInteger(idkey, &id))
-        continue;
-
-      AddMemoryDetails(it.GetCurrentValue(), model_, it.GetCurrentKey());
-
-      // Store each process indexed by the string version of its id if we didn't
-      // already insert it as part of the onUpdated processing above.
-      if (!updated)
-        processes->Set(base::IntToString(id), it.GetCurrentValue());
-    }
-
-    scoped_ptr<base::ListValue> args(new base::ListValue());
-    args->Append(processes);
+
+    // Store each process indexed by the string version of its ChildProcessHost
+    // ID.
+    processes_dictionary.Set(base::IntToString(child_process_host_id),
+                             process.ToValue());
+  }
+
+  // Done with data collection. Now dispatch the appropriate events according to
+  // the present listeners.
+  DCHECK(has_on_updated_listeners || has_on_updated_with_memory_listeners);
+  if (has_on_updated_listeners) {
+    api::processes::OnUpdated::Processes processes;
+    processes.additional_properties.MergeDictionary(&processes_dictionary);
+    // NOTE: If there are listeners to the updates with memory as well,
+    // listeners to onUpdated (without memory) will also get the memory info
+    // of processes as an added bonus.

[ncarter (slow), 2016/02/11 22:16:49]

This looks like a behavior change versus the old code. If so, why make it? It
doesn't seem like an improvement to sometimes randomly populate extra fields on
an event -- it just seems like a source of possible nondeterminism.

[afakhry, 2016/02/17 02:27:14]

You're right. It is a behavior change. I did this intentionally. The reason was
to fill in only a **single** dictionary with the needed data (with memory if
there are listeners to onUpdatedWithMemory(), and without memory if we only have
listeners to onUpdated()) with one single iteration. See the old implementation
to see why I was motivated to do this.

Since listeners know what they listen to, so listeners to onUpdated() only
wouldn't be affected if |process['privateMemory']| is defined or not. But
listeners to onUpdatedWithMemory() are guaranteed to have it defined all the
time.

Let me know if I need to change this, it seems to me a nice improvement without
any effect on clients.

+    DispatchEvent(events::PROCESSES_ON_UPDATED,
+                  api::processes::OnUpdated::kEventName,
+                  api::processes::OnUpdated::Create(processes));
+  }
+
+  if (has_on_updated_with_memory_listeners) {
+    api::processes::OnUpdatedWithMemory::Processes processes;
+    processes.additional_properties.MergeDictionary(&processes_dictionary);
     DispatchEvent(events::PROCESSES_ON_UPDATED_WITH_MEMORY,
-                  keys::kOnUpdatedWithMemory, std::move(args));
-  }
-#endif  // defined(ENABLE_TASK_MANAGER)
-}
-
-void ProcessesEventRouter::OnItemsToBeRemoved(int start, int length) {
-#if defined(ENABLE_TASK_MANAGER)
-  DCHECK_EQ(length, 1);
-
-  // Process exit for renderer processes has the data about exit code and
-  // termination status, therefore we will rely on notifications and not on
-  // the Task Manager data. We do use the rest of this method for non-renderer
-  // processes.
-  if (model_->GetResourceType(start) == task_manager::Resource::RENDERER)
-    return;
-
-  // The callback function parameters.
-  scoped_ptr<base::ListValue> args(new base::ListValue());
-
-  // First arg: The id of the process that was closed.
-  args->Append(new base::FundamentalValue(
-      model_->GetUniqueChildProcessId(start)));
-
-  // Second arg: The exit type for the process.
-  args->Append(new base::FundamentalValue(0));
-
-  // Third arg: The exit code for the process.
-  args->Append(new base::FundamentalValue(0));
-
-  DispatchEvent(events::PROCESSES_ON_EXITED, keys::kOnExited, std::move(args));
-#endif  // defined(ENABLE_TASK_MANAGER)
-}
-
-void ProcessesEventRouter::ProcessHangEvent(content::RenderWidgetHost* widget) {
-#if defined(ENABLE_TASK_MANAGER)
-  std::string event(keys::kOnUnresponsive);
-  if (!HasEventListeners(event))
-    return;
-
-  base::DictionaryValue* process = NULL;
-  int count = model_->ResourceCount();
-  int id = widget->GetProcess()->GetID();
-
-  for (int i = 0; i < count; ++i) {
-    if (model_->IsResourceFirstInGroup(i)) {
-      if (id == model_->GetUniqueChildProcessId(i)) {
-        process = CreateProcessFromModel(id, model_, i, false);
-        break;
-      }
-    }
-  }
-
-  if (process == NULL)
-    return;
-
-  scoped_ptr<base::ListValue> args(new base::ListValue());
-  args->Append(process);
-
-  DispatchEvent(events::PROCESSES_ON_UNRESPONSIVE, keys::kOnUnresponsive,
-                std::move(args));
-#endif  // defined(ENABLE_TASK_MANAGER)
-}
-
-void ProcessesEventRouter::ProcessClosedEvent(
-    content::RenderProcessHost* rph,
-    content::RenderProcessHost::RendererClosedDetails* details) {
-#if defined(ENABLE_TASK_MANAGER)
-  // The callback function parameters.
-  scoped_ptr<base::ListValue> args(new base::ListValue());
-
-  // First arg: The id of the process that was closed.
-  args->Append(new base::FundamentalValue(rph->GetID()));
-
-  // Second arg: The exit type for the process.
-  args->Append(new base::FundamentalValue(details->status));
-
-  // Third arg: The exit code for the process.
-  args->Append(new base::FundamentalValue(details->exit_code));
-
-  DispatchEvent(events::PROCESSES_ON_EXITED, keys::kOnExited, std::move(args));
-#endif  // defined(ENABLE_TASK_MANAGER)
+                  api::processes::OnUpdatedWithMemory::kEventName,
+                  api::processes::OnUpdatedWithMemory::Create(processes));
+  }
+}
+
+void ProcessesEventRouter::OnTaskUnresponsive(task_management::TaskId id) {
+  if (!HasEventListeners(api::processes::OnUnresponsive::kEventName))
+    return;
+
+  api::processes::Process process;
+  FillProcessData(id, observed_task_manager(), false, &process);
+  DispatchEvent(events::PROCESSES_ON_UNRESPONSIVE,
+                api::processes::OnUnresponsive::kEventName,
+                api::processes::OnUnresponsive::Create(process));
 }
 
 void ProcessesEventRouter::DispatchEvent(
     events::HistogramValue histogram_value,
     const std::string& event_name,
-    scoped_ptr<base::ListValue> event_args) {
+    scoped_ptr<base::ListValue> event_args) const {
   EventRouter* event_router = EventRouter::Get(browser_context_);
   if (event_router) {
     scoped_ptr<Event> event(
         new Event(histogram_value, event_name, std::move(event_args)));
     event_router->BroadcastEvent(std::move(event));
   }
 }
 
-bool ProcessesEventRouter::HasEventListeners(const std::string& event_name) {
+bool ProcessesEventRouter::HasEventListeners(
+    const std::string& event_name) const {
   EventRouter* event_router = EventRouter::Get(browser_context_);
   return event_router && event_router->HasEventListener(event_name);
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// ProcessesAPI:
+////////////////////////////////////////////////////////////////////////////////
+
 ProcessesAPI::ProcessesAPI(content::BrowserContext* context)
     : browser_context_(context) {
   EventRouter* event_router = EventRouter::Get(browser_context_);
-  event_router->RegisterObserver(this, processes_api_constants::kOnUpdated);
+  // Monitor when the following events are being listened to in order to know
+  // when to start the task manager.
+  event_router->RegisterObserver(this, api::processes::OnUpdated::kEventName);
+  event_router->RegisterObserver(
+      this, api::processes::OnUpdatedWithMemory::kEventName);
+  event_router->RegisterObserver(this, api::processes::OnCreated::kEventName);
   event_router->RegisterObserver(this,
-                                 processes_api_constants::kOnUpdatedWithMemory);
-  ExtensionFunctionRegistry* registry =
-      ExtensionFunctionRegistry::GetInstance();
-  registry->RegisterFunction<GetProcessIdForTabFunction>();
-  registry->RegisterFunction<TerminateFunction>();
-  registry->RegisterFunction<GetProcessInfoFunction>();
+                                 api::processes::OnUnresponsive::kEventName);
+  event_router->RegisterObserver(this, api::processes::OnExited::kEventName);
 }
 
 ProcessesAPI::~ProcessesAPI() {
+  // This object has already been unregistered as an observer in Shutdown().
 }
 
-void ProcessesAPI::Shutdown() {
-  EventRouter::Get(browser_context_)->UnregisterObserver(this);
-}
-
-static base::LazyInstance<BrowserContextKeyedAPIFactory<ProcessesAPI> >
-    g_factory = LAZY_INSTANCE_INITIALIZER;
-
 // static
 BrowserContextKeyedAPIFactory<ProcessesAPI>*
 ProcessesAPI::GetFactoryInstance() {
-  return g_factory.Pointer();
+  return g_processes_api_factory.Pointer();
 }
 
 // static
 ProcessesAPI* ProcessesAPI::Get(content::BrowserContext* context) {
   return BrowserContextKeyedAPIFactory<ProcessesAPI>::Get(context);
 }
 
-ProcessesEventRouter* ProcessesAPI::processes_event_router() {
-  if (!processes_event_router_)
-    processes_event_router_.reset(new ProcessesEventRouter(browser_context_));
-  return processes_event_router_.get();
+void ProcessesAPI::Shutdown() {
+  EventRouter::Get(browser_context_)->UnregisterObserver(this);
 }
 
 void ProcessesAPI::OnListenerAdded(const EventListenerInfo& details) {
-  // We lazily tell the TaskManager to start updating when listeners to the
-  // processes.onUpdated or processes.onUpdatedWithMemory events arrive.
+  // The ProcessesEventRouter will observe the TaskManager as long as there are
+  // listeners for the processes.onUpdated or processes.onUpdatedWithMemory
+  // events.
   processes_event_router()->ListenerAdded();
 }
 
 void ProcessesAPI::OnListenerRemoved(const EventListenerInfo& details) {
   // If a processes.onUpdated or processes.onUpdatedWithMemory event listener
   // is removed (or a process with one exits), then we let the extension API
   // know that it has one fewer listener.
   processes_event_router()->ListenerRemoved();
 }
 
-GetProcessIdForTabFunction::GetProcessIdForTabFunction() : tab_id_(-1) {
-}
-
-bool GetProcessIdForTabFunction::RunAsync() {
-#if defined(ENABLE_TASK_MANAGER)
-  EXTENSION_FUNCTION_VALIDATE(args_->GetInteger(0, &tab_id_));
-
-  // Add a reference, which is balanced in GetProcessIdForTab to keep the object
-  // around and allow for the callback to be invoked.
+ProcessesEventRouter* ProcessesAPI::processes_event_router() {
+  if (!processes_event_router_.get())
+    processes_event_router_.reset(new ProcessesEventRouter(browser_context_));
+  return processes_event_router_.get();
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// ProcessesGetProcessIdForTabFunction:
+////////////////////////////////////////////////////////////////////////////////
+
+ExtensionFunction::ResponseAction ProcessesGetProcessIdForTabFunction::Run() {
+  // For this function, the task manager doesn't even need to be running.
+  scoped_ptr<api::processes::GetProcessIdForTab::Params> params(
+      api::processes::GetProcessIdForTab::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get());
+
+  const int tab_id = params->tab_id;
+  content::WebContents* contents = nullptr;
+  int tab_index = -1;
+  if (!ExtensionTabUtil::GetTabById(
+          tab_id,
+          Profile::FromBrowserContext(browser_context()),
+          include_incognito(),
+          nullptr,
+          nullptr,
+          &contents,
+          &tab_index)) {
+    return RespondNow(Error(tabs_constants::kTabNotFoundError,
+                            base::IntToString(tab_id)));
+  }
+
+  const int process_id = contents->GetRenderProcessHost()->GetID();
+  return RespondNow(ArgumentList(
+      api::processes::GetProcessIdForTab::Results::Create(process_id)));
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// ProcessesTerminateFunction:
+////////////////////////////////////////////////////////////////////////////////
+
+ExtensionFunction::ResponseAction ProcessesTerminateFunction::Run() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  // For this function, the task manager doesn't even need to be running.
+  scoped_ptr<api::processes::Terminate::Params> params(
+      api::processes::Terminate::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get());
+
+  child_process_host_id_ = params->process_id;
+  if (child_process_host_id_ == 0) {

[ncarter (slow), 2016/02/11 22:16:49]

You use == 0 here, but elsewhere we use <= 0.

[afakhry, 2016/02/17 02:27:13]

Thanks for highlighting the discrepancy. 

I now changed ArcProcessTask::GetChildProcessUniqueID() to return 
content::ChildProcessHost::kInvalidUniqueID (which is -1) instead of 0, to
reserve the value 0 for the browser process. 

Now I want to keep the < 0 case separate and report it as an invalid argument.

+    // Cannot kill the browser process.
+    return RespondNow(Error(errors::kNotAllowedToTerminate,
+                            base::IntToString(child_process_host_id_)));
+  }
+
+  content::BrowserThread::PostTaskAndReplyWithResult(
+      content::BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(&ProcessesTerminateFunction::GetProcessHandleIO,
+                 this,
+                 child_process_host_id_),
+      base::Bind(&ProcessesTerminateFunction::OnProcessHandleUI, this));
+
+  // Promise to respond later.
+  return RespondLater();
+}
+
+base::ProcessHandle ProcessesTerminateFunction::GetProcessHandleIO(
+    int child_process_host_id) const {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+  auto* host = content::BrowserChildProcessHost::FromID(child_process_host_id);
+  if (host)
+    return host->GetData().handle;
+
+  return base::kNullProcessHandle;
+}
+
+void ProcessesTerminateFunction::OnProcessHandleUI(base::ProcessHandle handle) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  if (handle == base::kNullProcessHandle) {
+    // If we didn't get anything from BrowserChildProcessHost, then it could be
+    // a renderer.
+    auto* render_host =

[ncarter (slow), 2016/02/11 22:16:49]

process, process_host, rph, or render_process_host would be more conventional
names for this variable.

[afakhry, 2016/02/17 02:27:14]

Done.

+        content::RenderProcessHost::FromID(child_process_host_id_);
+    if (render_host)
+      handle = render_host->GetHandle();
+  }
+
+  if (handle == base::kNullProcessHandle) {
+    Respond(Error(errors::kProcessNotFound,
+                  base::IntToString(child_process_host_id_)));
+    return;
+  }
+
+  if (handle == base::GetCurrentProcessHandle()) {
+    // Cannot kill the browser process.
+    Respond(Error(errors::kNotAllowedToTerminate,
+                  base::IntToString(child_process_host_id_)));
+    return;
+  }
+
+  base::Process process = base::Process::Open(base::GetProcId(handle));
+  const bool did_terminate =
+      process.Terminate(content::RESULT_CODE_KILLED, true);
+  if (did_terminate)
+    UMA_HISTOGRAM_COUNTS("ChildProcess.KilledByExtensionAPI", 1);
+
+  Respond(ArgumentList(
+      api::processes::Terminate::Results::Create(did_terminate)));
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// ProcessesGetProcessInfoFunction:
+////////////////////////////////////////////////////////////////////////////////
+
+ProcessesGetProcessInfoFunction::ProcessesGetProcessInfoFunction()
+    : task_management::TaskManagerObserver(
+          base::TimeDelta::FromSeconds(1),
+          GetRefreshTypesFlagOnlyEssentialData()) {
+}
+
+ExtensionFunction::ResponseAction ProcessesGetProcessInfoFunction::Run() {
+  scoped_ptr<api::processes::GetProcessInfo::Params> params(
+      api::processes::GetProcessInfo::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get());
+  if (params->process_ids.as_integer)
+    process_host_ids_.push_back(*params->process_ids.as_integer);
+  else
+    process_host_ids_.swap(*params->process_ids.as_integers);
+  include_memory_ = params->include_memory;
+

[ncarter (slow), 2016/02/11 22:16:49]

Shouldn't we add REFRESH_TYPE_MEMORY based on the value of include_memory_ ?

[afakhry, 2016/02/17 02:27:14]

Yep, my bad! Done.

+  // Keep this object alive until the OnTasksRefreshed() is received.
   AddRef();
 
-  // If the task manager is already listening, just post a task to execute
-  // which will invoke the callback once we have returned from this function.
-  // Otherwise, wait for the notification that the task manager is done with
-  // the data gathering.
-  if (ProcessesAPI::Get(GetProfile())
-          ->processes_event_router()
-          ->is_task_manager_listening()) {
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE,
-        base::Bind(&GetProcessIdForTabFunction::GetProcessIdForTab, this));
-  } else {
-    TaskManager::GetInstance()->model()->RegisterOnDataReadyCallback(
-        base::Bind(&GetProcessIdForTabFunction::GetProcessIdForTab, this));
-
-    ProcessesAPI::Get(GetProfile())
-        ->processes_event_router()
-        ->StartTaskManagerListening();
-  }
-
-  return true;
-#else
-  error_ = errors::kExtensionNotSupported;
-  return false;
-#endif  // defined(ENABLE_TASK_MANAGER)
-}
-
-void GetProcessIdForTabFunction::GetProcessIdForTab() {
-  content::WebContents* contents = NULL;
-  int tab_index = -1;
-  if (!ExtensionTabUtil::GetTabById(tab_id_,
-                                    GetProfile(),
-                                    include_incognito(),
-                                    NULL,
-                                    NULL,
-                                    &contents,
-                                    &tab_index)) {
-    error_ = ErrorUtils::FormatErrorMessage(tabs_constants::kTabNotFoundError,
-                                            base::IntToString(tab_id_));
-    SetResult(new base::FundamentalValue(-1));
-    SendResponse(false);
-  } else {
-    int process_id = contents->GetRenderProcessHost()->GetID();
-    SetResult(new base::FundamentalValue(process_id));
-    SendResponse(true);
-  }
-
-  // Balance the AddRef in the RunAsync.
-  Release();
-}
-
-TerminateFunction::TerminateFunction() : process_id_(-1) {
-}
-
-bool TerminateFunction::RunAsync() {
-#if defined(ENABLE_TASK_MANAGER)
-  EXTENSION_FUNCTION_VALIDATE(args_->GetInteger(0, &process_id_));
-
-  // Add a reference, which is balanced in TerminateProcess to keep the object
-  // around and allow for the callback to be invoked.
-  AddRef();
-
-  // If the task manager is already listening, just post a task to execute
-  // which will invoke the callback once we have returned from this function.
-  // Otherwise, wait for the notification that the task manager is done with
-  // the data gathering.
-  if (ProcessesAPI::Get(GetProfile())
-          ->processes_event_router()
-          ->is_task_manager_listening()) {
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::Bind(&TerminateFunction::TerminateProcess, this));
-  } else {
-    TaskManager::GetInstance()->model()->RegisterOnDataReadyCallback(
-        base::Bind(&TerminateFunction::TerminateProcess, this));
-
-    ProcessesAPI::Get(GetProfile())
-        ->processes_event_router()
-        ->StartTaskManagerListening();
-  }
-
-  return true;
-#else
-  error_ = errors::kExtensionNotSupported;
-  return false;
-#endif  // defined(ENABLE_TASK_MANAGER)
-}
-
-
-void TerminateFunction::TerminateProcess() {
-#if defined(ENABLE_TASK_MANAGER)
-  TaskManagerModel* model = TaskManager::GetInstance()->model();
-
-  bool found = false;
-  for (int i = 0, count = model->ResourceCount(); i < count; ++i) {
-    if (!model->IsResourceFirstInGroup(i) ||
-        process_id_ != model->GetUniqueChildProcessId(i)) {
-      continue;
-    }
-    base::ProcessHandle process_handle = model->GetProcess(i);
-    if (process_handle == base::GetCurrentProcessHandle()) {
-      // Cannot kill the browser process.
-      // TODO(kalman): Are there other sensitive processes?
-      error_ = ErrorUtils::FormatErrorMessage(errors::kNotAllowedToTerminate,
-                                              base::IntToString(process_id_));
-    } else {
-      base::Process process =
-          base::Process::DeprecatedGetProcessFromHandle(process_handle);
-      bool did_terminate = process.Terminate(content::RESULT_CODE_KILLED, true);
-      if (did_terminate)
-        UMA_HISTOGRAM_COUNTS("ChildProcess.KilledByExtensionAPI", 1);
-      SetResult(new base::FundamentalValue(did_terminate));
-    }
-    found = true;
-    break;
-  }
-  if (!found) {
-    error_ = ErrorUtils::FormatErrorMessage(errors::kProcessNotFound,
-                                            base::IntToString(process_id_));
-  }
-
-  SendResponse(error_.empty());
-
-  // Balance the AddRef in the RunAsync.
-  Release();
-#else
-  error_ = errors::kExtensionNotSupported;
-  SendResponse(false);
-#endif  // defined(ENABLE_TASK_MANAGER)
-}
-
-GetProcessInfoFunction::GetProcessInfoFunction()
-#if defined(ENABLE_TASK_MANAGER)
-  : memory_(false)
-#endif
-    {
-}
-
-GetProcessInfoFunction::~GetProcessInfoFunction() {
-}
-
-bool GetProcessInfoFunction::RunAsync() {
-#if defined(ENABLE_TASK_MANAGER)
-  base::Value* processes = NULL;
-
-  EXTENSION_FUNCTION_VALIDATE(args_->Get(0, &processes));
-  EXTENSION_FUNCTION_VALIDATE(args_->GetBoolean(1, &memory_));
-  EXTENSION_FUNCTION_VALIDATE(ReadOneOrMoreIntegers(processes, &process_ids_));

[ncarter (slow), 2016/02/11 22:16:49]

Looks like this was the only call to this function, and ReadOneOrMoreIntegers is
the only function in its util file, so I think you can delete
extensions/browser/extension_function_util.h/.cc!

[afakhry, 2016/02/17 02:27:13]

Done. Thanks for catching this.

-
-  // Add a reference, which is balanced in GatherProcessInfo to keep the object
-  // around and allow for the callback to be invoked.
-  AddRef();
-
-  // If the task manager is already listening, just post a task to execute
-  // which will invoke the callback once we have returned from this function.
-  // Otherwise, wait for the notification that the task manager is done with
-  // the data gathering.
-  if (ProcessesAPI::Get(GetProfile())
-          ->processes_event_router()
-          ->is_task_manager_listening()) {
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE,
-        base::Bind(&GetProcessInfoFunction::GatherProcessInfo, this));
-  } else {
-    TaskManager::GetInstance()->model()->RegisterOnDataReadyCallback(
-        base::Bind(&GetProcessInfoFunction::GatherProcessInfo, this));
-
-    ProcessesAPI::Get(GetProfile())
-        ->processes_event_router()
-        ->StartTaskManagerListening();
-  }
-  return true;
-
-#else
-  error_ = errors::kExtensionNotSupported;
-  return false;
-#endif  // defined(ENABLE_TASK_MANAGER)
-}
-
-void GetProcessInfoFunction::GatherProcessInfo() {
-#if defined(ENABLE_TASK_MANAGER)
-  TaskManagerModel* model = TaskManager::GetInstance()->model();
-  base::DictionaryValue* processes = new base::DictionaryValue();
-
+  // The task manager needs to be enabled for this function.
+  // Start observing the task manager and wait for the next refresh event.
+  task_management::TaskManagerInterface::GetTaskManager()->AddObserver(this);
+
+  return RespondLater();
+}
+
+void ProcessesGetProcessInfoFunction::OnTasksRefreshed(

[ncarter (slow), 2016/02/11 22:16:49]

What motivates making this function being a TM observer?

[afakhry, 2016/02/17 02:27:13]

As we discussed offline, to make this independent from the presence of listeners
to onUpdated(), onCreated(), ... etc. This way, it's only observing the task
manager only for the lifetime of this function.

+    const task_management::TaskIdList& task_ids) {
   // If there are no process IDs specified, it means we need to return all of
   // the ones we know of.
-  if (process_ids_.size() == 0) {
-    int resources = model->ResourceCount();
-    for (int i = 0; i < resources; ++i) {
-      if (model->IsResourceFirstInGroup(i)) {
-        int id = model->GetUniqueChildProcessId(i);
-        base::DictionaryValue* d = CreateProcessFromModel(id, model, i, false);
-        if (memory_)
-          AddMemoryDetails(d, model, i);
-        processes->Set(base::IntToString(id), d);
-      }
+  const bool specific_processes_requested = !process_host_ids_.empty();
+  std::set<base::ProcessId> seen_processes;
+  // Create the results object as defined in the generated API from process.idl
+  // and fill it with the processes info.
+  api::processes::GetProcessInfo::Results::Processes processes;
+  for (const auto& task_id : task_ids) {
+    const base::ProcessId proc_id =
+        observed_task_manager()->GetProcessId(task_id);
+    if (seen_processes.count(proc_id))
+      continue;
+
+    const int child_process_host_id =
+        observed_task_manager()->GetChildProcessUniqueId(task_id);
+
+    if (specific_processes_requested) {
+      // Note: we can't use |!process_host_ids_.empty()| directly in the above
+      // condition as we will erase from |process_host_ids_| below.
+      auto itr = std::find(process_host_ids_.begin(),

[ncarter (slow), 2016/02/11 22:16:49]

std::find needs <algorithm>

[afakhry, 2016/02/17 02:27:14]

Right. Done. By the way, why trybots don't fail to compile on a missing thing
like this?

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

It's almost certainly transitively included by some other header file. C++ is
silly.

+                           process_host_ids_.end(),
+                           child_process_host_id);
+      if (itr == process_host_ids_.end())
+        continue;
+
+      // If found, we remove it from |process_host_ids|, so that at the end if
+      // anything remains in |process_host_ids|, those were invalid arguments
+      // that will be reported on the console.
+      process_host_ids_.erase(itr);
     }
-  } else {
-    int resources = model->ResourceCount();
-    for (int i = 0; i < resources; ++i) {
-      if (model->IsResourceFirstInGroup(i)) {
-        int id = model->GetUniqueChildProcessId(i);
-        std::vector<int>::iterator proc_id = std::find(process_ids_.begin(),
-                                                       process_ids_.end(), id);
-        if (proc_id != process_ids_.end()) {
-          base::DictionaryValue* d =
-              CreateProcessFromModel(id, model, i, false);
-          if (memory_)
-            AddMemoryDetails(d, model, i);
-          processes->Set(base::IntToString(id), d);
-
-          process_ids_.erase(proc_id);
-          if (process_ids_.size() == 0)
-            break;
-        }
-      }
+
+    seen_processes.insert(proc_id);
+
+    // We do not include the optional data in this function results.
+    api::processes::Process process;
+    FillProcessData(task_id, observed_task_manager(), false, &process);
+
+    if (include_memory_) {
+      // Append the private memory usage to the process data.
+      const int64_t private_memory =
+          observed_task_manager()->GetPrivateMemoryUsage(task_id);
+      process.private_memory.reset(new double(static_cast<double>(
+          private_memory)));
     }
-    // If not all processes were found, log them to the extension's console to
-    // help the developer, but don't fail the API call.
-    for (int pid : process_ids_) {
-      WriteToConsole(content::CONSOLE_MESSAGE_LEVEL_ERROR,
-                     ErrorUtils::FormatErrorMessage(errors::kProcessNotFound,
-                                                    base::IntToString(pid)));
-    }
-  }
-
-  SetResult(processes);
-  SendResponse(true);
-
-  // Balance the AddRef in the RunAsync.
+
+    // Store each process indexed by the string version of its
+    // ChildProcessHost ID.
+    processes.additional_properties.Set(
+        base::IntToString(child_process_host_id),
+        process.ToValue());
+  }
+
+  // Report the invalid host ids sent in the arguments.
+  for (const auto& host_id : process_host_ids_) {
+    WriteToConsole(content::CONSOLE_MESSAGE_LEVEL_ERROR,
+                   ErrorUtils::FormatErrorMessage(errors::kProcessNotFound,
+                                                  base::IntToString(host_id)));
+  }
+
+  // Send the response.
+  Respond(ArgumentList(
+      api::processes::GetProcessInfo::Results::Create(processes)));
+
+  // Stop observing the task manager, and balance the AddRef() in Run().
+  task_management::TaskManagerInterface::GetTaskManager()->RemoveObserver(this);
   Release();
-#endif  // defined(ENABLE_TASK_MANAGER)
-}
+}
+
+ProcessesGetProcessInfoFunction::~ProcessesGetProcessInfoFunction() {}
 
 }  // namespace extensions