Fix CrashMemoryMetricsReporterImpl to report memory usage upon renderer OOM crash

Change-Id: I5a96979b123311ed5aa4dda35c39d1eaa7bc98da
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1975377
Commit-Queue: Takashi Sakamoto <tasak@google.com>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#731863}

third_party/blink/public/common/oom_intervention/oom_intervention_types.h

@@ -16,17 +16,17 @@ namespace blink {
 // the arm64 and renderer in the arm32.
 
 struct OomInterventionMetrics {
-  uint64_t current_private_footprint_kb;
-  uint64_t current_swap_kb;
-  uint64_t current_vm_size_kb;
+  uint64_t current_private_footprint_kb = 0;
+  uint64_t current_swap_kb = 0;
+  uint64_t current_vm_size_kb = 0;
 
   // Stores the total of V8, BlinkGC and PartitionAlloc memory usage.
-  uint64_t current_blink_usage_kb;
+  uint64_t current_blink_usage_kb = 0;
 
   // Indicates whether the crash was because of virtual address space OOM.
   // This holds only 0 or 1 as a value but because of the reason stated above,
   // uses uint64_t instead of boolean.
-  uint64_t allocation_failed;
+  uint64_t allocation_failed = 0;
 };
 
 }  // namespace blink

third_party/blink/renderer/controller/crash_memory_metrics_reporter_impl.cc

@@ -37,50 +37,54 @@ CrashMemoryMetricsReporterImpl::CrashMemoryMetricsReporterImpl() {
       CrashMemoryMetricsReporterImpl::OnOOMCallback);
 }
 
-CrashMemoryMetricsReporterImpl::~CrashMemoryMetricsReporterImpl() = default;
+CrashMemoryMetricsReporterImpl::~CrashMemoryMetricsReporterImpl() {
+  MemoryUsageMonitor::Instance().RemoveObserver(this);
+}
 
 void CrashMemoryMetricsReporterImpl::SetSharedMemory(
     base::UnsafeSharedMemoryRegion shared_metrics_buffer) {
   // This method should be called only once per process.
   DCHECK(!shared_metrics_mapping_.IsValid());
   shared_metrics_mapping_ = shared_metrics_buffer.Map();
+  MemoryUsageMonitor::Instance().AddObserver(this);
 }
 
 void CrashMemoryMetricsReporterImpl::OnMemoryPing(MemoryUsage usage) {
-  WriteIntoSharedMemory(
-      CrashMemoryMetricsReporterImpl::MemoryUsageToMetrics(usage));
+  DCHECK(IsMainThread());
+  last_reported_metrics_ =
+      CrashMemoryMetricsReporterImpl::MemoryUsageToMetrics(usage);
+  WriteIntoSharedMemory();
 }
 
-void CrashMemoryMetricsReporterImpl::WriteIntoSharedMemory(
-    const OomInterventionMetrics& metrics) {
+void CrashMemoryMetricsReporterImpl::WriteIntoSharedMemory() {
   if (!shared_metrics_mapping_.IsValid())
     return;
   auto* metrics_shared =
       shared_metrics_mapping_.GetMemoryAs<OomInterventionMetrics>();
-  memcpy(metrics_shared, &metrics, sizeof(OomInterventionMetrics));
+  *metrics_shared = last_reported_metrics_;
 }
 
 void CrashMemoryMetricsReporterImpl::OnOOMCallback() {
   // TODO(yuzus: Support allocation failures on other threads as well.
   if (!IsMainThread())
     return;
+  CrashMemoryMetricsReporterImpl& instance =
+      CrashMemoryMetricsReporterImpl::Instance();
   // If shared_metrics_mapping_ is not set, it means OnNoMemory happened before
   // initializing render process host sets the shared memory.
-  if (!CrashMemoryMetricsReporterImpl::Instance()
-           .shared_metrics_mapping_.IsValid())
+  if (!instance.shared_metrics_mapping_.IsValid())
     return;
   // Else, we can send the allocation_failed bool.
-  OomInterventionMetrics metrics;
   // TODO(yuzus): Report this UMA on all the platforms. Currently this is only
   // reported on Android.
-  metrics.allocation_failed = 1;  // true
-  CrashMemoryMetricsReporterImpl::Instance().WriteIntoSharedMemory(metrics);
+  instance.last_reported_metrics_.allocation_failed = 1;  // true
+  instance.WriteIntoSharedMemory();
 }
 
 // static
 OomInterventionMetrics CrashMemoryMetricsReporterImpl::MemoryUsageToMetrics(
     MemoryUsage usage) {
-  OomInterventionMetrics metrics = {};
+  OomInterventionMetrics metrics;
 
   DCHECK(!std::isnan(usage.private_footprint_bytes));
   DCHECK(!std::isnan(usage.swap_bytes));

third_party/blink/renderer/controller/crash_memory_metrics_reporter_impl.h

@@ -31,9 +31,6 @@ class CONTROLLER_EXPORT CrashMemoryMetricsReporterImpl
   void SetSharedMemory(
       base::UnsafeSharedMemoryRegion shared_metrics_buffer) override;
 
-  // MemoryUsageMonitor::Observer:
-  void OnMemoryPing(MemoryUsage) override;
-
   // This method tracks when an allocation failure occurs. It should be hooked
   // into all platform allocation failure handlers in a process such as
   // base::TerminateBecauseOutOfMemory() and OOM_CRASH() in Partition Alloc.
@@ -47,8 +44,12 @@ class CONTROLLER_EXPORT CrashMemoryMetricsReporterImpl
  private:
   FRIEND_TEST_ALL_PREFIXES(OomInterventionImplTest, CalculateProcessFootprint);
 
-  void WriteIntoSharedMemory(const OomInterventionMetrics& metrics);
+  // MemoryUsageMonitor::Observer:
+  void OnMemoryPing(MemoryUsage) override;
+
+  void WriteIntoSharedMemory();
 
+  OomInterventionMetrics last_reported_metrics_;
   base::WritableSharedMemoryMapping shared_metrics_mapping_;
   mojo::Receiver<mojom::blink::CrashMemoryMetricsReporter> receiver_{this};
 };