Revert "Refactor dependency on memory_instrumentation.mojom out of process.mojom"

This reverts commit 9e12e07d.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 688212 as the
culprit for failures in the build cycles as shown on:
https://analysis.chromium.org/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzllMTJlMDdkMzQwZjJkZDNiY2IyYjdhMWU3MTJhYTAzMGVmODNlNWEM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.linux/linux-ozone-rel/89256

Sample Failed Step: compile

Original change's description:
> Refactor dependency on memory_instrumentation.mojom out of process.mojom
> 
> Deprecates use of memory_instrumentation.mojom.GlobalMemoryDump struct
> in process.mojom and ArcProcessService. Uses new struct ArcMemoryDump
> instead.
> 
> manager for Android memory footprint
> 
> Bug: b:139190852
> Test: Build cheet_x86 and flash betty-arcnext, check Chrome OS task
> Change-Id: Ib21dfcf8ab7c23dfd60573341dfdddbcb744f166
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1749456
> Commit-Queue: Willie Koomson <wvk@google.com>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Dominick Ng <dominickn@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#688212}


Change-Id: I642591284693919f312d8e1af3d3713a5a5e0874
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: b:139190852
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1761229
Cr-Commit-Position: refs/heads/master@{#688220}

chrome/browser/chromeos/arc/process/arc_process_service.cc

@@ -34,6 +34,8 @@
 #include "components/arc/arc_util.h"
 #include "components/arc/session/arc_bridge_service.h"
 #include "content/public/browser/browser_thread.h"
+#include "services/resource_coordinator/public/cpp/memory_instrumentation/global_memory_dump.h"
+#include "services/resource_coordinator/public/mojom/memory_instrumentation/memory_instrumentation.mojom.h"
 
 namespace arc {
 
@@ -222,18 +224,20 @@ std::vector<ArcProcess> UpdateAndReturnProcessList(
   return FilterProcessList(pid_map, std::move(processes));
 }
 
-std::vector<mojom::ArcMemoryDumpPtr> UpdateAndReturnMemoryInfo(
+std::unique_ptr<memory_instrumentation::GlobalMemoryDump>
+UpdateAndReturnMemoryInfo(
     scoped_refptr<ArcProcessService::NSPidToPidMap> nspid_map,
-    std::vector<mojom::ArcMemoryDumpPtr> process_dumps) {
+    memory_instrumentation::mojom::GlobalMemoryDumpPtr dump) {
   if (!arc::IsArcVmEnabled()) {
     ArcProcessService::NSPidToPidMap& pid_map = *nspid_map;
     // Cleanup dead processes in pid_map
+    // TODO(wvk) should we be cleaning dead processes here too ?
     base::flat_set<ProcessId> nspid_to_remove;
     for (const auto& entry : pid_map)
       nspid_to_remove.insert(entry.first);
 
     bool unmapped_nspid = false;
-    for (const auto& proc : process_dumps) {
+    for (const auto& proc : dump->process_dumps) {
       // erase() returns 0 if couldn't find the key (new process)
       if (nspid_to_remove.erase(proc->pid) == 0) {
         pid_map[proc->pid] = base::kNullProcessId;
@@ -247,14 +251,14 @@ std::vector<mojom::ArcMemoryDumpPtr> UpdateAndReturnMemoryInfo(
       UpdateNspidToPidMap(nspid_map);
 
     // Return memory info only for processes that have a mapping nspid->pid
-    for (auto& proc : process_dumps) {
+    for (auto& proc : dump->process_dumps) {
       auto it = pid_map.find(proc->pid);
       proc->pid = it == pid_map.end() ? kNullProcessId : it->second;
     }
-    base::EraseIf(process_dumps,
+    base::EraseIf(dump->process_dumps,
                   [](const auto& proc) { return proc->pid == kNullProcessId; });
   }
-  return process_dumps;
+  return memory_instrumentation::GlobalMemoryDump::MoveFrom(std::move(dump));
 }
 
 void Reset(scoped_refptr<ArcProcessService::NSPidToPidMap> pid_map) {
@@ -397,12 +401,12 @@ void ArcProcessService::OnReceiveProcessList(
 
 void ArcProcessService::OnReceiveMemoryInfo(
     RequestMemoryInfoCallback callback,
-    std::vector<mojom::ArcMemoryDumpPtr> process_dumps) {
+    memory_instrumentation::mojom::GlobalMemoryDumpPtr dump) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   base::PostTaskAndReplyWithResult(
       task_runner_.get(), FROM_HERE,
       base::BindOnce(&UpdateAndReturnMemoryInfo, nspid_to_pid_,
-                     std::move(process_dumps)),
+                     std::move(dump)),
       std::move(callback));
 }
 

chrome/browser/chromeos/arc/process/arc_process_service.h

@@ -72,8 +72,8 @@ class ArcProcessService : public KeyedService,
   using OptionalArcProcessList = base::Optional<std::vector<ArcProcess>>;
   using RequestProcessListCallback =
       base::OnceCallback<void(OptionalArcProcessList)>;
-  using RequestMemoryInfoCallback =
-      base::OnceCallback<void(std::vector<mojom::ArcMemoryDumpPtr>)>;
+  using RequestMemoryInfoCallback = base::OnceCallback<void(
+      std::unique_ptr<memory_instrumentation::GlobalMemoryDump>)>;
 
   ArcProcessService(content::BrowserContext* context,
                     ArcBridgeService* bridge_service);
@@ -124,8 +124,9 @@ class ArcProcessService : public KeyedService,
   void OnReceiveProcessList(
       RequestProcessListCallback callback,
       std::vector<mojom::RunningAppProcessInfoPtr> processes);
-  void OnReceiveMemoryInfo(RequestMemoryInfoCallback callback,
-                           std::vector<mojom::ArcMemoryDumpPtr> process_dumps);
+  void OnReceiveMemoryInfo(
+      RequestMemoryInfoCallback callback,
+      memory_instrumentation::mojom::GlobalMemoryDumpPtr dump);
   void OnGetSystemProcessList(RequestMemoryInfoCallback callback,
                               std::vector<ArcProcess> processes);
   // ConnectionObserver<mojom::ProcessInstance> overrides.

chrome/browser/task_manager/sampling/arc_shared_sampler.cc

@@ -71,7 +71,7 @@ void ArcSharedSampler::Refresh() {
 
 void ArcSharedSampler::OnReceiveMemoryDump(
     int type,
-    std::vector<arc::mojom::ArcMemoryDumpPtr> process_dump) {
+    std::unique_ptr<memory_instrumentation::GlobalMemoryDump> dump) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   pending_memory_dump_types_ &= ~type;
 
@@ -80,11 +80,14 @@ void ArcSharedSampler::OnReceiveMemoryDump(
   else
     last_system_refresh = base::Time::Now();
 
-  for (const auto& proc : process_dump) {
-    auto it = callbacks_.find(proc->pid);
+  if (!dump)
+    return;
+  for (const auto& pmd : dump->process_dumps()) {
+    auto it = callbacks_.find(pmd.pid());
     if (it == callbacks_.end())
       continue;
-    const MemoryFootprintBytes result = proc->private_footprint_kb * 1024;
+    const MemoryFootprintBytes result =
+        pmd.os_dump().private_footprint_kb * 1024;
     it->second.Run(base::make_optional<MemoryFootprintBytes>(result));
   }
 }

chrome/browser/task_manager/sampling/arc_shared_sampler.h

@@ -6,7 +6,6 @@
 #define CHROME_BROWSER_TASK_MANAGER_SAMPLING_ARC_SHARED_SAMPLER_H_
 
 #include <stdint.h>
-
 #include <map>
 
 #include "base/callback.h"
@@ -14,7 +13,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
 #include "base/process/process_handle.h"
-#include "components/arc/mojom/process.mojom.h"
+#include "services/resource_coordinator/public/cpp/memory_instrumentation/memory_instrumentation.h"
 
 namespace task_manager {
 
@@ -46,7 +45,7 @@ class ArcSharedSampler {
   // Called when ArcProcessService returns memory dump.
   void OnReceiveMemoryDump(
       int dump_type,
-      std::vector<arc::mojom::ArcMemoryDumpPtr> process_dumps);
+      std::unique_ptr<memory_instrumentation::GlobalMemoryDump> dump);
 
   // Holds callbacks registered by TaskGroup objects.
   CallbacksMap callbacks_;

components/arc/mojom/process.mojom

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
-// Next MinVersion: 9
+// Next MinVersion: 8
 
 module arc.mojom;
 
@@ -109,19 +109,7 @@ struct RunningAppProcessInfo {
   int64 last_activity_time;
 };
 
-// Describes the memory usage of an ARC process.
-struct ArcMemoryDump {
-  // PID (within ARC's PID namespace) of the process.
-  uint32 pid;
-
-  // Resident Set Size (RSS) in kilobytes
-  uint32 resident_set_kb = 0;
-
-  // Private footprint (RSS + swap usage) in kilobytes
-  uint32 private_footprint_kb = 0;
-};
-
-// Next Method ID: 10
+// Next Method ID: 8
 interface ProcessInstance {
   // Requests ARC instance to kill a process.
   [MinVersion=1]
@@ -132,22 +120,11 @@ interface ProcessInstance {
 
   // Requests memory usage dumps for all ARC application processes.
   [MinVersion=7]
-  RequestApplicationProcessMemoryInfoDeprecated@6()
-    => (memory_instrumentation.mojom.GlobalMemoryDump dump);
+  RequestApplicationProcessMemoryInfo@6()
+      => (memory_instrumentation.mojom.GlobalMemoryDump dump);
 
   // Requests memory usage dumps for all ARC system processes.
   [MinVersion=7]
-  RequestSystemProcessMemoryInfoDeprecated@7(array<uint32> nspids)
-    => (memory_instrumentation.mojom.GlobalMemoryDump dump);
-
-  // Requests memory usage dumps for all ARC application processes.
-  [MinVersion=8]
-  RequestApplicationProcessMemoryInfo@8()
-    => (array<ArcMemoryDump> process_dumps);
-
-  // Requests memory usage dumps for all ARC system processes.
-  [MinVersion=8]
-  RequestSystemProcessMemoryInfo@9(array<uint32> nspids)
-    => (array<ArcMemoryDump> process_dumps);
-
+  RequestSystemProcessMemoryInfo@7(array<uint32> nspids)
+      => (memory_instrumentation.mojom.GlobalMemoryDump dump);
 };