memory-infra: add ProcessType and expose data in RequestGlobalDump()

services/resource_coordinator/memory/coordinator/coordinator_impl.cc

Index: services/resource_coordinator/memory/coordinator/coordinator_impl.cc
diff --git a/services/resource_coordinator/memory/coordinator/coordinator_impl.cc b/services/resource_coordinator/memory/coordinator/coordinator_impl.cc
index 55d08e9e086ecda227628abb8c22ef4d27004a82..235e62dd32a7bd2c446c75111b55a11e068678f3 100644
--- a/services/resource_coordinator/memory/coordinator/coordinator_impl.cc
+++ b/services/resource_coordinator/memory/coordinator/coordinator_impl.cc
@@ -34,8 +34,11 @@ CoordinatorImpl::CoordinatorImpl(bool initialize_memory_dump_manager)
     : failed_memory_dump_count_(0),
       initialize_memory_dump_manager_(initialize_memory_dump_manager) {
   if (initialize_memory_dump_manager) {
+    // TODO(primiano): the current state where the coordinator also creates a
+    // client (ProcessLocalDumpManagerImpl) is contra-intuitive. BrowserMainLoop
+    // should be doing this. Fix in the next CL.

[fmeawad, 2017/05/10 18:00:16]

nit: Remove Fix in the next CL

[Primiano Tucci (use gerrit), 2017/05/11 12:50:02]

Done.

     ProcessLocalDumpManagerImpl::CreateInstance(
-        ProcessLocalDumpManagerImpl::Config(this));
+        ProcessLocalDumpManagerImpl::Config(this, mojom::ProcessType::BROWSER));
     base::trace_event::MemoryDumpManager::GetInstance()->set_tracing_process_id(
         mojom::kServiceTracingProcessId);
   }
@@ -81,7 +84,8 @@ void CoordinatorImpl::RequestGlobalMemoryDump(
                 << base::trace_event::MemoryDumpLevelOfDetailToString(
                        args.level_of_detail)
                 << ") is already in the queue";
-        callback.Run(args.dump_guid, false /* success */);
+        callback.Run(args.dump_guid, false /* success */,
+                     mojom::GlobalMemoryDumpPtr());

[dcheng, 2017/05/11 05:55:26]

Or nullptr (here and elsewhere)

[Primiano Tucci (use gerrit), 2017/05/11 12:50:02]

Done.

         return;
       }
     }
@@ -121,8 +125,7 @@ void CoordinatorImpl::UnregisterProcessLocalDumpManager(
     DCHECK(!queued_memory_dump_requests_.empty());
     OnProcessMemoryDumpResponse(
         process_manager, queued_memory_dump_requests_.front().args.dump_guid,
-        false /* success */,
-        base::Optional<base::trace_event::MemoryDumpCallbackResult>());
+        false /* success */, mojom::ProcessMemoryDumpPtr());
   }
 }
 
@@ -150,15 +153,20 @@ void CoordinatorImpl::OnProcessMemoryDumpResponse(
     mojom::ProcessLocalDumpManager* process_manager,
     uint64_t dump_guid,
     bool success,
-    const base::Optional<base::trace_event::MemoryDumpCallbackResult>& result) {
+    mojom::ProcessMemoryDumpPtr process_memory_dump) {
   auto it = pending_process_managers_.find(process_manager);
 
-  DCHECK(!queued_memory_dump_requests_.empty());
-  if (queued_memory_dump_requests_.front().args.dump_guid != dump_guid ||
+  if (queued_memory_dump_requests_.empty() ||
+      queued_memory_dump_requests_.front().args.dump_guid != dump_guid ||
       it == pending_process_managers_.end()) {
     VLOG(1) << "Received unexpected memory dump response: " << dump_guid;
     return;
   }
+  if (process_memory_dump) {
+    queued_memory_dump_requests_.front().process_memory_dumps.push_back(

[fmeawad, 2017/05/10 18:00:16]

We will probably need to pass the process_manager or the identity as well so
that we can resolve the extra_process_dumps in the Finalize method, but I can
add that to my CL.

[Primiano Tucci (use gerrit), 2017/05/11 12:50:02]

Yeah let's see how the discussion on pid settles

+        std::move(process_memory_dump));
+  }
+
   pending_process_managers_.erase(it);
 
   if (!success) {
@@ -166,6 +174,7 @@ void CoordinatorImpl::OnProcessMemoryDumpResponse(
     VLOG(1) << base::trace_event::MemoryDumpManager::kLogPrefix
             << " failed because of NACK from provider";
   }
+
   FinalizeGlobalMemoryDumpIfAllManagersReplied();
 }
 
@@ -173,13 +182,20 @@ void CoordinatorImpl::FinalizeGlobalMemoryDumpIfAllManagersReplied() {
   if (pending_process_managers_.size() > 0)
     return;
 
-  DCHECK(!queued_memory_dump_requests_.empty());
-  {
-    const auto& callback = queued_memory_dump_requests_.front().callback;
-    const bool global_success = failed_memory_dump_count_ == 0;
-    callback.Run(queued_memory_dump_requests_.front().args.dump_guid,
-                 global_success);
+  if (queued_memory_dump_requests_.empty()) {
+    NOTREACHED();
+    return;
   }
+
+  // TODO(hjd,fmeawad): At this point the |process_memory_dumps| accumulated in
+  // queued_memory_dump_requests_.front() should be normalized (merge
+  // |extra_process_dump|, compute CMM) into a GlobalMemoryDumpPtr and passed
+  // below.
+
+  const auto& callback = queued_memory_dump_requests_.front().callback;
+  const bool global_success = failed_memory_dump_count_ == 0;
+  callback.Run(queued_memory_dump_requests_.front().args.dump_guid,
+               global_success, mojom::GlobalMemoryDumpPtr());
   queued_memory_dump_requests_.pop_front();
 
   // Schedule the next queued dump (if applicable).