Index: chrome/browser/chromeos/arc/arc_process_service.cc |
diff --git a/chrome/browser/chromeos/arc/arc_process_service.cc b/chrome/browser/chromeos/arc/arc_process_service.cc |
index b84ddafaba4b03172f5bf00609169c12baa8cc19..a74711e343522470c3eec1a964b4424b64c86814 100644 |
--- a/chrome/browser/chromeos/arc/arc_process_service.cc |
+++ b/chrome/browser/chromeos/arc/arc_process_service.cc |
@@ -9,10 +9,12 @@ |
#include "chrome/browser/chromeos/arc/arc_process_service.h" |
+#include <algorithm> |
#include <queue> |
#include <set> |
#include <string> |
+#include "base/callback.h" |
#include "base/process/process.h" |
#include "base/process/process_iterator.h" |
#include "base/task_runner_util.h" |
@@ -23,11 +25,56 @@ namespace arc { |
namespace { |
-const char kSequenceToken[] = "arc_process_service"; |
- |
// Weak pointer. This class is owned by ArcServiceManager. |
ArcProcessService* g_arc_process_service = nullptr; |
+// Matches the process name "/init" in the process tree and get the |
+// corresponding process ID. |
+base::ProcessId GetArcInitProcessId( |
+ const base::ProcessIterator::ProcessEntries& entry_list) { |
+ for (const base::ProcessEntry& entry : entry_list) { |
+ // TODO(nya): Add more constraints to avoid mismatches. |
+ std::string process_name = |
Luis Héctor Chávez
2016/07/15 16:19:41
I know you didn't write this code, but can you imp
Hsu-Cheng
2016/07/27 08:07:59
Done.
|
+ !entry.cmd_line_args().empty() ? entry.cmd_line_args()[0] : ""; |
+ if (process_name == "/init") { |
+ return entry.pid(); |
+ } |
+ } |
+ return base::kNullProcessId; |
+} |
+ |
+std::vector<arc::ArcProcess> GetArcSystemProcessList() { |
+ std::vector<arc::ArcProcess> ret_processes; |
+ const base::ProcessIterator::ProcessEntries& entry_list = |
+ base::ProcessIterator(nullptr).Snapshot(); |
+ base::ProcessId arc_init_pid = GetArcInitProcessId(entry_list); |
cylee1
2016/07/15 23:17:03
nit: can be const
Hsu-Cheng
2016/07/27 08:07:59
Done.
|
+ |
+ if (arc_init_pid == base::kNullProcessId) { |
+ return ret_processes; |
+ } |
+ |
+ // Enumerate the child processes of ARC init for gathering ARC System |
+ // Processes. |
+ for (const base::ProcessEntry& entry : entry_list) { |
+ // TODO(hctsai): For now, we only gather direct child process of init, need |
+ // to get the processes below. For example, installd might |
+ // fork dex2oat and it can be executed for minutes. |
+ if (entry.parent_pid() == arc_init_pid) { |
+ const base::ProcessId child_pid = entry.pid(); |
+ const base::ProcessId child_nspid = |
+ base::Process(child_pid).GetPidInNamespace(); |
+ const std::string process_name = |
cylee1
2016/07/15 23:17:03
nit: can move inside the if
Hsu-Cheng
2016/07/27 08:07:59
Done.
|
+ !entry.cmd_line_args().empty() ? entry.cmd_line_args()[0] : ""; |
+ if (child_nspid != base::kNullProcessId) { |
+ ret_processes.emplace_back(child_nspid, child_pid, process_name, |
+ mojom::ProcessState::PERSISTENT); |
+ } |
+ } |
+ } |
+ |
+ return ret_processes; |
+} |
+ |
} // namespace |
using base::kNullProcessId; |
@@ -40,21 +87,18 @@ using std::vector; |
ArcProcessService::ArcProcessService(ArcBridgeService* bridge_service) |
: ArcService(bridge_service), |
- worker_pool_(new SequencedWorkerPool(1, "arc_process_manager")), |
weak_ptr_factory_(this) { |
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); |
arc_bridge_service()->process()->AddObserver(this); |
DCHECK(!g_arc_process_service); |
g_arc_process_service = this; |
// Not intended to be used from the creating thread. |
Luis Héctor Chávez
2016/07/15 16:19:42
Remove?
Hsu-Cheng
2016/07/27 08:07:59
Done.
|
- thread_checker_.DetachFromThread(); |
} |
ArcProcessService::~ArcProcessService() { |
DCHECK(g_arc_process_service == this); |
g_arc_process_service = nullptr; |
arc_bridge_service()->process()->RemoveObserver(this); |
- worker_pool_->Shutdown(); |
} |
// static |
@@ -65,19 +109,25 @@ ArcProcessService* ArcProcessService::Get() { |
void ArcProcessService::OnInstanceReady() { |
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); |
- worker_pool_->PostNamedSequencedWorkerTask( |
- kSequenceToken, |
+ heavy_task_runner_->PostTask( |
cylee1
2016/07/15 23:17:03
Are you sure SingleThreadTaskRunner work as intend
Hsu-Cheng
2016/07/27 08:07:59
Done.
|
FROM_HERE, |
- base::Bind(&ArcProcessService::Reset, |
- weak_ptr_factory_.GetWeakPtr())); |
+ base::Bind(&ArcProcessService::Reset, weak_ptr_factory_.GetWeakPtr())); |
Luis Héctor Chávez
2016/07/15 16:19:41
This is still problematic, since WeakPtrs are not
cylee1
2016/07/15 23:17:03
To be more clear, you should not use (dereference)
Hsu-Cheng
2016/07/27 08:07:59
I did move on this way. The NSPidToPidMap class is
|
} |
void ArcProcessService::Reset() { |
- DCHECK(thread_checker_.CalledOnValidThread()); |
- nspid_to_pid_.clear(); |
+ nspid_to_pid_->clear(); |
cylee1
2016/07/15 23:17:03
Shouldn't we only modify the object in the dedicat
Hsu-Cheng
2016/07/27 08:07:59
Done.
|
+} |
+ |
+void ArcProcessService::RequestSystemProcessList( |
+ RequestProcessListCallback callback) { |
+ DCHECK_CURRENTLY_ON(content::BrowserThread::UI); |
+ |
+ base::PostTaskAndReplyWithResult(heavy_task_runner_.get(), FROM_HERE, |
+ base::Bind(&GetArcSystemProcessList), |
+ callback); |
} |
-bool ArcProcessService::RequestProcessList( |
+bool ArcProcessService::RequestAppProcessList( |
RequestProcessListCallback callback) { |
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); |
@@ -93,102 +143,72 @@ bool ArcProcessService::RequestProcessList( |
return true; |
} |
+// static |
void ArcProcessService::OnReceiveProcessList( |
const RequestProcessListCallback& callback, |
- mojo::Array<arc::mojom::RunningAppProcessInfoPtr> mojo_processes) { |
+ mojo::Array<arc::mojom::RunningAppProcessInfoPtr> instance_processes) { |
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); |
- auto raw_processes = new vector<mojom::RunningAppProcessInfoPtr>(); |
- mojo_processes.Swap(raw_processes); |
- |
- auto ret_processes = new vector<ArcProcess>(); |
- // Post to its dedicated worker thread to avoid race condition. |
- // Since no two tasks with the same token should be run at the same. |
- // Note: GetSequencedTaskRunner's shutdown behavior defaults to |
- // SKIP_ON_SHUTDOWN (ongoing task blocks shutdown). |
- // So in theory using Unretained(this) should be fine since the life cycle |
- // of |this| is the same as the main browser. |
- // To be safe I still use weak pointers, but weak_ptrs can only bind to |
- // methods without return values. That's why I can't use |
- // PostTaskAndReplyWithResult but handle the return object by myself. |
- auto runner = worker_pool_->GetSequencedTaskRunner( |
- worker_pool_->GetNamedSequenceToken(kSequenceToken)); |
- runner->PostTaskAndReply( |
- FROM_HERE, |
- base::Bind(&ArcProcessService::UpdateAndReturnProcessList, |
- weak_ptr_factory_.GetWeakPtr(), |
- base::Owned(raw_processes), |
- base::Unretained(ret_processes)), |
- base::Bind(&ArcProcessService::CallbackRelay, |
- weak_ptr_factory_.GetWeakPtr(), |
- callback, |
- base::Owned(ret_processes))); |
-} |
- |
-void ArcProcessService::CallbackRelay( |
- const RequestProcessListCallback& callback, |
- const vector<ArcProcess>* ret_processes) { |
- DCHECK_CURRENTLY_ON(content::BrowserThread::UI); |
- callback.Run(*ret_processes); |
+ base::PostTaskAndReplyWithResult( |
+ heavy_task_runner_.get(), FROM_HERE, |
+ base::Bind(&ArcProcessService::UpdateAndReturnProcessList, nspid_to_pid_, |
+ base::Passed(&instance_processes)), |
+ callback); |
} |
-void ArcProcessService::UpdateAndReturnProcessList( |
- const vector<arc::mojom::RunningAppProcessInfoPtr>* raw_processes, |
- vector<ArcProcess>* ret_processes) { |
- DCHECK(thread_checker_.CalledOnValidThread()); |
- |
- // Cleanup dead pids in the cache |nspid_to_pid_|. |
+// static |
+vector<ArcProcess> ArcProcessService::UpdateAndReturnProcessList( |
+ scoped_refptr<NSPidToPidMap> pid_map, |
+ mojo::Array<arc::mojom::RunningAppProcessInfoPtr> processes) { |
+ // Cleanup dead pids in the cache |pid_map|. |
set<ProcessId> nspid_to_remove; |
- for (const auto& entry : nspid_to_pid_) { |
+ for (const auto& entry : *pid_map) { |
cylee1
2016/07/15 23:17:03
I think you can create a reference to the derefere
Hsu-Cheng
2016/07/27 08:07:59
Done.
|
nspid_to_remove.insert(entry.first); |
} |
bool unmapped_nspid = false; |
- for (const auto& entry : *raw_processes) { |
+ for (const auto& entry : processes) { |
// erase() returns 0 if coudln't find the key. It means a new process. |
if (nspid_to_remove.erase(entry->pid) == 0) { |
- nspid_to_pid_[entry->pid] = kNullProcessId; |
+ (*pid_map)[entry->pid] = base::kNullProcessId; |
unmapped_nspid = true; |
} |
} |
for (const auto& entry : nspid_to_remove) { |
- nspid_to_pid_.erase(entry); |
+ (*pid_map).erase(entry); |
} |
// The operation is costly so avoid calling it when possible. |
if (unmapped_nspid) { |
- UpdateNspidToPidMap(); |
+ UpdateNspidToPidMap(*pid_map); |
} |
- PopulateProcessList(raw_processes, ret_processes); |
+ return FilterProcessList(*pid_map, std::move(processes)); |
} |
-void ArcProcessService::PopulateProcessList( |
- const vector<arc::mojom::RunningAppProcessInfoPtr>* raw_processes, |
- vector<ArcProcess>* ret_processes) { |
- DCHECK(thread_checker_.CalledOnValidThread()); |
- |
- for (const auto& entry : *raw_processes) { |
- const auto it = nspid_to_pid_.find(entry->pid); |
- // In case the process already dies so couldn't find corresponding pid. |
- if (it != nspid_to_pid_.end() && it->second != kNullProcessId) { |
- ArcProcess arc_process(entry->pid, it->second, entry->process_name, |
- entry->process_state); |
+// static |
+vector<ArcProcess> ArcProcessService::FilterProcessList( |
+ NSPidToPidMap& pid_map, |
cylee1
2016/07/15 23:17:03
const ?
Hsu-Cheng
2016/07/27 08:07:59
Done.
|
+ mojo::Array<arc::mojom::RunningAppProcessInfoPtr> processes) { |
+ vector<ArcProcess> ret_processes; |
+ for (const auto& entry : processes) { |
+ const auto it = pid_map.find(entry->pid); |
+ if (it != pid_map.end() && it->second != base::kNullProcessId) { |
+ ArcProcess arc_process(entry->pid, pid_map[entry->pid], |
+ entry->process_name, entry->process_state); |
// |entry->packages| is provided only when process.mojom's verion is >=4. |
if (entry->packages) { |
for (const auto& package : entry->packages) { |
arc_process.packages().push_back(package.get()); |
} |
} |
- ret_processes->push_back(std::move(arc_process)); |
+ ret_processes.push_back(std::move(arc_process)); |
} |
} |
+ return ret_processes; |
} |
-// Computes a map from PID in ARC namespace to PID in system namespace. |
-// The returned map contains ARC processes only. |
-void ArcProcessService::UpdateNspidToPidMap() { |
- DCHECK(thread_checker_.CalledOnValidThread()); |
- |
+// static |
+void ArcProcessService::UpdateNspidToPidMap(NSPidToPidMap& pid_map) { |
TRACE_EVENT0("browser", "ArcProcessService::UpdateNspidToPidMap"); |
// NB: Despite of its name, ProcessIterator::Snapshot() may return |
@@ -197,27 +217,13 @@ void ArcProcessService::UpdateNspidToPidMap() { |
const base::ProcessIterator::ProcessEntries& entry_list = |
base::ProcessIterator(nullptr).Snapshot(); |
- // System may contain many different namespaces so several different |
- // processes may have the same nspid. We need to get the proper subset of |
- // processes to create correct nspid -> pid map. |
- |
// Construct the process tree. |
// NB: This can contain a loop in case of race conditions. |
map<ProcessId, vector<ProcessId> > process_tree; |
for (const base::ProcessEntry& entry : entry_list) |
process_tree[entry.parent_pid()].push_back(entry.pid()); |
- // Find the ARC init process. |
- ProcessId arc_init_pid = kNullProcessId; |
- for (const base::ProcessEntry& entry : entry_list) { |
- // TODO(nya): Add more constraints to avoid mismatches. |
- std::string process_name = |
- !entry.cmd_line_args().empty() ? entry.cmd_line_args()[0] : ""; |
- if (process_name == "/init") { |
- arc_init_pid = entry.pid(); |
- break; |
- } |
- } |
+ ProcessId arc_init_pid = GetArcInitProcessId(entry_list); |
// Enumerate all processes under ARC init and create nspid -> pid map. |
if (arc_init_pid != kNullProcessId) { |
@@ -237,10 +243,9 @@ void ArcProcessService::UpdateNspidToPidMap() { |
// All ARC processes should be in namespace so nspid is usually non-null, |
// but this can happen if the process has already gone. |
// Only add processes we're interested in (those appear as keys in |
- // |nspid_to_pid_|). |
- if (nspid != kNullProcessId && |
- nspid_to_pid_.find(nspid) != nspid_to_pid_.end()) |
- nspid_to_pid_[nspid] = pid; |
+ // |pid_map|). |
+ if (nspid != kNullProcessId && pid_map.find(nspid) != pid_map.end()) |
+ pid_map[nspid] = pid; |
for (ProcessId child_pid : process_tree[pid]) |
queue.push(child_pid); |