Revert "[Paint Preview] Split PaintPreviewCompositor memory metrics from the...

Revert "[Paint Preview] Split PaintPreviewCompositor memory metrics from the general Utility process metrics"

This reverts commit 8711fb9a.

Reason for revert: broke Cast Audio Linux bot, https://ci.chromium.org/p/chromium/builders/ci/Cast%20Audio%20Linux/87604

Original change's description:
> [Paint Preview] Split PaintPreviewCompositor memory metrics from the general Utility process metrics
>
> Change-Id: I6067aad3959c054181389c1e1b82e464eacd9a4b
> Bug: 1135217
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2441263
> Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
> Reviewed-by: François Doray <fdoray@chromium.org>
> Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
> Reviewed-by: Calder Kitagawa <ckitagawa@chromium.org>
> Commit-Queue: Yashar Dabiran <yashard@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#815285}

TBR=yfriedman@chromium.org,fdoray@chromium.org,rkaplow@chromium.org,ckitagawa@chromium.org,yashard@chromium.org

Change-Id: Iba7597dd7f400b21f3e464c7849d4852a69c1b9d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1135217
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2461900Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815311}

chrome/browser/DEPS

@@ -252,7 +252,6 @@ include_rules = [
   "+components/services/heap_profiling",
   "+components/services/language_detection/public/cpp",
   "+components/services/language_detection/public/mojom",
-  "+components/services/paint_preview_compositor/public/mojom",
   "+components/services/patch/content",
   "+components/services/patch/public",
   "+components/services/print_compositor/public",

chrome/browser/metrics/process_memory_metrics_emitter.cc

@@ -27,7 +27,6 @@
 #include "components/performance_manager/public/graph/page_node.h"
 #include "components/performance_manager/public/graph/process_node.h"
 #include "components/performance_manager/public/performance_manager.h"
-#include "components/services/paint_preview_compositor/public/mojom/paint_preview_compositor.mojom.h"
 #include "content/public/browser/audio_service_info.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/common/content_switches.h"
@@ -621,8 +620,7 @@ void EmitGpuMemoryMetrics(const GlobalMemoryDump::ProcessDump& pmd,
   builder.Record(ukm_recorder);
 }
 
-void EmitUtilityMemoryMetrics(HistogramProcessType ptype,
-                              const GlobalMemoryDump::ProcessDump& pmd,
+void EmitUtilityMemoryMetrics(const GlobalMemoryDump::ProcessDump& pmd,
                               ukm::SourceId ukm_source_id,
                               ukm::UkmRecorder* ukm_recorder,
                               const base::Optional<base::TimeDelta>& uptime,
@@ -630,7 +628,38 @@ void EmitUtilityMemoryMetrics(HistogramProcessType ptype,
   Memory_Experimental builder(ukm_source_id);
   builder.SetProcessType(static_cast<int64_t>(
       memory_instrumentation::mojom::ProcessType::UTILITY));
-  EmitProcessUmaAndUkm(pmd, ptype, uptime, record_uma, &builder);
+  EmitProcessUmaAndUkm(pmd, HistogramProcessType::kUtility, uptime, record_uma,
+                       &builder);
+
+  builder.Record(ukm_recorder);
+}
+
+void EmitAudioServiceMemoryMetrics(
+    const GlobalMemoryDump::ProcessDump& pmd,
+    ukm::SourceId ukm_source_id,
+    ukm::UkmRecorder* ukm_recorder,
+    const base::Optional<base::TimeDelta>& uptime,
+    bool record_uma) {
+  Memory_Experimental builder(ukm_source_id);
+  builder.SetProcessType(static_cast<int64_t>(
+      memory_instrumentation::mojom::ProcessType::UTILITY));
+  EmitProcessUmaAndUkm(pmd, HistogramProcessType::kAudioService, uptime,
+                       record_uma, &builder);
+
+  builder.Record(ukm_recorder);
+}
+
+void EmitNetworkServiceMemoryMetrics(
+    const GlobalMemoryDump::ProcessDump& pmd,
+    ukm::SourceId ukm_source_id,
+    ukm::UkmRecorder* ukm_recorder,
+    const base::Optional<base::TimeDelta>& uptime,
+    bool record_uma) {
+  Memory_Experimental builder(ukm_source_id);
+  builder.SetProcessType(static_cast<int64_t>(
+      memory_instrumentation::mojom::ProcessType::UTILITY));
+  EmitProcessUmaAndUkm(pmd, HistogramProcessType::kNetworkService, uptime,
+                       record_uma, &builder);
 
   builder.Record(ukm_recorder);
 }
@@ -861,22 +890,20 @@ void ProcessMemoryMetricsEmitter::CollateResults() {
         break;
       }
       case memory_instrumentation::mojom::ProcessType::UTILITY: {
-        HistogramProcessType ptype;
         if (pmd.pid() == content::GetProcessIdForAudioService()) {
-          ptype = HistogramProcessType::kAudioService;
+          EmitAudioServiceMemoryMetrics(
+              pmd, ukm::UkmRecorder::GetNewSourceID(), GetUkmRecorder(),
+              GetProcessUptime(now, pmd.pid()), emit_metrics_for_all_processes);
         } else if (pmd.service_name() ==
                    network::mojom::NetworkService::Name_) {
-          ptype = HistogramProcessType::kNetworkService;
-        } else if (pmd.service_name() ==
-                   paint_preview::mojom::PaintPreviewCompositorCollection::
-                       Name_) {
-          ptype = HistogramProcessType::kPaintPreviewCompositor;
+          EmitNetworkServiceMemoryMetrics(
+              pmd, ukm::UkmRecorder::GetNewSourceID(), GetUkmRecorder(),
+              GetProcessUptime(now, pmd.pid()), emit_metrics_for_all_processes);
         } else {
-          ptype = HistogramProcessType::kUtility;
+          EmitUtilityMemoryMetrics(
+              pmd, ukm::UkmRecorder::GetNewSourceID(), GetUkmRecorder(),
+              GetProcessUptime(now, pmd.pid()), emit_metrics_for_all_processes);
         }
-        EmitUtilityMemoryMetrics(
-            ptype, pmd, ukm::UkmRecorder::GetNewSourceID(), GetUkmRecorder(),
-            GetProcessUptime(now, pmd.pid()), emit_metrics_for_all_processes);
         break;
       }
       case memory_instrumentation::mojom::ProcessType::PLUGIN:

chrome/browser/metrics/process_memory_metrics_emitter_unittest.cc

@@ -14,17 +14,14 @@
 #include "base/test/metrics/histogram_tester.h"
 #include "build/build_config.h"
 #include "chrome/browser/metrics/renderer_uptime_tracker.h"
-#include "components/services/paint_preview_compositor/public/mojom/paint_preview_compositor.mojom.h"
 #include "components/ukm/test_ukm_recorder.h"
 #include "content/public/test/browser_task_environment.h"
 #include "services/metrics/public/cpp/ukm_builders.h"
 #include "services/metrics/public/cpp/ukm_recorder.h"
-#include "services/resource_coordinator/public/cpp/memory_instrumentation/browser_metrics.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using GlobalMemoryDump = memory_instrumentation::GlobalMemoryDump;
 using GlobalMemoryDumpPtr = memory_instrumentation::mojom::GlobalMemoryDumpPtr;
-using HistogramProcessType = memory_instrumentation::HistogramProcessType;
 using ProcessMemoryDumpPtr =
     memory_instrumentation::mojom::ProcessMemoryDumpPtr;
 using OSMemDumpPtr = memory_instrumentation::mojom::OSMemDumpPtr;
@@ -476,87 +473,45 @@ MetricMap GetExpectedAudioServiceMetrics() {
   });
 }
 
-void PopulatePaintPreviewCompositorMetrics(GlobalMemoryDumpPtr& global_dump,
-                                           MetricMap& metrics_mb) {
-  auto process_memory_dump =
-      memory_instrumentation::mojom::ProcessMemoryDump::New();
-  process_memory_dump->service_name =
-      paint_preview::mojom::PaintPreviewCompositorCollection::Name_;
-  ProcessMemoryDumpPtr pmd(std::move(process_memory_dump));
-  pmd->process_type = ProcessType::UTILITY;
-  OSMemDumpPtr os_dump =
-      GetFakeOSMemDump(GetResidentValue(metrics_mb) * 1024,
-                       metrics_mb["PrivateMemoryFootprint"] * 1024,
-                       metrics_mb["SharedMemoryFootprint"] * 1024
-#if defined(OS_LINUX) || defined(OS_CHROMEOS) || defined(OS_ANDROID)
-                       // accessing PrivateSwapFootprint on other OSes will
-                       // modify metrics_mb to create the value, which leads to
-                       // expectation failures.
-                       ,
-                       metrics_mb["PrivateSwapFootprint"] * 1024
-#endif
-      );
-  pmd->os_dump = std::move(os_dump);
-  global_dump->process_dumps.push_back(std::move(pmd));
-}
-
-MetricMap GetExpectedPaintPreviewCompositorMetrics() {
-  return MetricMap({
-    {"ProcessType", static_cast<int64_t>(ProcessType::UTILITY)},
-#if !defined(OS_MAC)
-        {"Resident", 10},
-#endif
-        {"PrivateMemoryFootprint", 30}, {"SharedMemoryFootprint", 35},
-#if defined(OS_LINUX) || defined(OS_CHROMEOS) || defined(OS_ANDROID)
-        {"PrivateSwapFootprint", 50},
-#endif
-  });
-}
-
 void PopulateMetrics(GlobalMemoryDumpPtr& global_dump,
-                     HistogramProcessType ptype,
+                     ProcessType ptype,
                      MetricMap& metrics_mb) {
   switch (ptype) {
-    case HistogramProcessType::kAudioService:
-      PopulateAudioServiceMetrics(global_dump, metrics_mb);
-      return;
-    case HistogramProcessType::kBrowser:
+    case ProcessType::BROWSER:
       PopulateBrowserMetrics(global_dump, metrics_mb);
       return;
-    case HistogramProcessType::kGpu:
-      PopulateGpuMetrics(global_dump, metrics_mb);
+    case ProcessType::RENDERER:
+      PopulateRendererMetrics(global_dump, metrics_mb, 101);
       return;
-    case HistogramProcessType::kPaintPreviewCompositor:
-      PopulatePaintPreviewCompositorMetrics(global_dump, metrics_mb);
+    case ProcessType::GPU:
+      PopulateGpuMetrics(global_dump, metrics_mb);
       return;
-    case HistogramProcessType::kRenderer:
-      PopulateRendererMetrics(global_dump, metrics_mb, 101);
+    case ProcessType::UTILITY:
+      PopulateAudioServiceMetrics(global_dump, metrics_mb);
       return;
-    case HistogramProcessType::kExtension:
-    case HistogramProcessType::kNetworkService:
-    case HistogramProcessType::kUtility:
+    case ProcessType::PLUGIN:
+    case ProcessType::OTHER:
+    case ProcessType::ARC:
       break;
   }
 
   // We shouldn't reach here.
-  CHECK(false);
+  FAIL() << "Unknown process type case " << ptype << ".";
 }
 
-MetricMap GetExpectedProcessMetrics(HistogramProcessType ptype) {
+MetricMap GetExpectedProcessMetrics(ProcessType ptype) {
   switch (ptype) {
-    case HistogramProcessType::kAudioService:
-      return GetExpectedAudioServiceMetrics();
-    case HistogramProcessType::kBrowser:
+    case ProcessType::BROWSER:
       return GetExpectedBrowserMetrics();
-    case HistogramProcessType::kGpu:
-      return GetExpectedGpuMetrics();
-    case HistogramProcessType::kPaintPreviewCompositor:
-      return GetExpectedPaintPreviewCompositorMetrics();
-    case HistogramProcessType::kRenderer:
+    case ProcessType::RENDERER:
       return GetExpectedRendererMetrics();
-    case HistogramProcessType::kExtension:
-    case HistogramProcessType::kNetworkService:
-    case HistogramProcessType::kUtility:
+    case ProcessType::GPU:
+      return GetExpectedGpuMetrics();
+    case ProcessType::UTILITY:
+      return GetExpectedAudioServiceMetrics();
+    case ProcessType::PLUGIN:
+    case ProcessType::OTHER:
+    case ProcessType::ARC:
       break;
   }
 
@@ -630,7 +585,7 @@ ProcessInfoVector GetProcessInfo(ukm::TestUkmRecorder& ukm_recorder) {
 }  // namespace
 
 class ProcessMemoryMetricsEmitterTest
-    : public testing::TestWithParam<HistogramProcessType> {
+    : public testing::TestWithParam<ProcessType> {
  public:
   ProcessMemoryMetricsEmitterTest() {}
   ~ProcessMemoryMetricsEmitterTest() override {}
@@ -685,14 +640,12 @@ TEST_P(ProcessMemoryMetricsEmitterTest, CollectsSingleProcessUKMs) {
   CheckMemoryUkmEntryMetrics(expected_entries);
 }
 
-INSTANTIATE_TEST_SUITE_P(
-    SinglePtype,
-    ProcessMemoryMetricsEmitterTest,
-    testing::Values(HistogramProcessType::kBrowser,
-                    HistogramProcessType::kRenderer,
-                    HistogramProcessType::kGpu,
-                    HistogramProcessType::kPaintPreviewCompositor,
-                    HistogramProcessType::kAudioService));
+INSTANTIATE_TEST_SUITE_P(SinglePtype,
+                         ProcessMemoryMetricsEmitterTest,
+                         testing::Values(ProcessType::BROWSER,
+                                         ProcessType::RENDERER,
+                                         ProcessType::GPU,
+                                         ProcessType::UTILITY));
 
 TEST_F(ProcessMemoryMetricsEmitterTest, CollectsExtensionProcessUKMs) {
   MetricMap expected_metrics = GetExpectedRendererMetrics();
@@ -715,17 +668,10 @@ TEST_F(ProcessMemoryMetricsEmitterTest, CollectsExtensionProcessUKMs) {
 }
 
 TEST_F(ProcessMemoryMetricsEmitterTest, CollectsManyProcessUKMsSingleDump) {
-  std::vector<HistogramProcessType> entries_ptypes = {
-      HistogramProcessType::kBrowser,
-      HistogramProcessType::kRenderer,
-      HistogramProcessType::kGpu,
-      HistogramProcessType::kAudioService,
-      HistogramProcessType::kPaintPreviewCompositor,
-      HistogramProcessType::kPaintPreviewCompositor,
-      HistogramProcessType::kAudioService,
-      HistogramProcessType::kGpu,
-      HistogramProcessType::kRenderer,
-      HistogramProcessType::kBrowser,
+  std::vector<ProcessType> entries_ptypes = {
+      ProcessType::BROWSER,  ProcessType::RENDERER, ProcessType::GPU,
+      ProcessType::UTILITY,  ProcessType::UTILITY,  ProcessType::GPU,
+      ProcessType::RENDERER, ProcessType::BROWSER,
   };
 
   GlobalMemoryDumpPtr global_dump(
@@ -747,15 +693,11 @@ TEST_F(ProcessMemoryMetricsEmitterTest, CollectsManyProcessUKMsSingleDump) {
 }
 
 TEST_F(ProcessMemoryMetricsEmitterTest, CollectsManyProcessUKMsManyDumps) {
-  std::vector<std::vector<HistogramProcessType>> entries_ptypes = {
-      {HistogramProcessType::kBrowser, HistogramProcessType::kRenderer,
-       HistogramProcessType::kGpu,
-       HistogramProcessType::kPaintPreviewCompositor,
-       HistogramProcessType::kAudioService},
-      {HistogramProcessType::kBrowser, HistogramProcessType::kRenderer,
-       HistogramProcessType::kGpu,
-       HistogramProcessType::kPaintPreviewCompositor,
-       HistogramProcessType::kAudioService},
+  std::vector<std::vector<ProcessType>> entries_ptypes = {
+      {ProcessType::BROWSER, ProcessType::RENDERER, ProcessType::GPU,
+       ProcessType::UTILITY},
+      {ProcessType::UTILITY, ProcessType::GPU, ProcessType::RENDERER,
+       ProcessType::BROWSER},
   };
 
   std::vector<MetricMap> entries_metrics;

services/resource_coordinator/public/cpp/memory_instrumentation/browser_metrics.cc

@@ -18,7 +18,6 @@ const char kBrowserHistogramName[] = "Browser";
 const char kExtensionHistogramName[] = "Extension";
 const char kGpuHistogramName[] = "Gpu";
 const char kNetworkServiceHistogramName[] = "NetworkService";
-const char kPaintPreviewCompositorHistogramName[] = "PaintPreviewCompositor";
 const char kRendererHistogramName[] = "Renderer";
 const char kUtilityHistogramName[] = "Utility";
 
@@ -38,8 +37,6 @@ const char* HistogramProcessTypeToString(HistogramProcessType type) {
       return kGpuHistogramName;
     case HistogramProcessType::kNetworkService:
       return kNetworkServiceHistogramName;
-    case HistogramProcessType::kPaintPreviewCompositor:
-      return kPaintPreviewCompositorHistogramName;
     case HistogramProcessType::kRenderer:
       return kRendererHistogramName;
     case HistogramProcessType::kUtility:

services/resource_coordinator/public/cpp/memory_instrumentation/browser_metrics.h

@@ -29,7 +29,6 @@ enum class HistogramProcessType {
   kExtension,
   kGpu,
   kNetworkService,
-  kPaintPreviewCompositor,
   kRenderer,
   kUtility,
 };

tools/metrics/histograms/histograms_xml/memory/histograms.xml

@@ -1631,65 +1631,6 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   </summary>
 </histogram>
 
-<histogram name="Memory.PaintPreviewCompositor.PrivateMemoryFootprint"
-    units="MB" expires_after="2021-04-10">
-  <owner>ckitagawa@chromium.org</owner>
-  <owner>fredmello@chromium.org</owner>
-  <owner>mahmoudi@chromium.org</owner>
-  <summary>
-    A rough estimate of the private memory footprint of the paint preview
-    compositor process.
-
-    Recorded at Poisson sampled time intervals with a mean of 5 minutes on
-    Android and 30 minutes on other platforms.
-  </summary>
-</histogram>
-
-<histogram name="Memory.PaintPreviewCompositor.PrivateSwapFootprint" units="MB"
-    expires_after="2021-04-10">
-  <owner>ckitagawa@chromium.org</owner>
-  <owner>fredmello@chromium.org</owner>
-  <owner>mahmoudi@chromium.org</owner>
-  <summary>
-    An amount of private memory of the paint preview compositor process placed
-    in swap (VmSwap).
-
-    Recorded at Poisson sampled time intervals with a mean of 5 minutes on
-    Android and 30 minutes on other platforms.
-  </summary>
-</histogram>
-
-<histogram name="Memory.PaintPreviewCompositor.ResidentSet" units="MiB"
-    expires_after="2021-04-10">
-  <owner>ckitagawa@chromium.org</owner>
-  <owner>fredmello@chromium.org</owner>
-  <owner>mahmoudi@chromium.org</owner>
-  <summary>
-    The size of the resident memory in a paint preview compositor process. This
-    is influenced by factors we control (e.g. memory that is not accessed can be
-    swapped) and factors we don't control (e.g. an unrelated process using a lot
-    of memory can force memory in our process to be swapped). Recorded once on
-    Windows/Linux/ChromeOS/Android.
-
-    Recorded at Poisson sampled time intervals with a mean of 5 minutes on
-    Android and 30 minutes on other platforms.
-  </summary>
-</histogram>
-
-<histogram name="Memory.PaintPreviewCompositor.SharedMemoryFootprint"
-    units="MB" expires_after="2021-04-10">
-  <owner>ckitagawa@chromium.org</owner>
-  <owner>fredmello@chromium.org</owner>
-  <owner>mahmoudi@chromium.org</owner>
-  <summary>
-    A rough estimate of the shared memory footprint of the paint preview
-    compositor process.
-
-    Recorded at Poisson sampled time intervals with a mean of 5 minutes on
-    Android and 30 minutes on other platforms.
-  </summary>
-</histogram>
-
 <histogram name="Memory.ParkableString.CompressedSizeKb" units="KB"
     expires_after="M87">
   <owner>lizeb@chromium.org</owner>