[heap profiler] Replace SharedMemoryBuffer with mojo::BigString

BUG=923459

Change-Id: I008e2aa9fea0d5d7d674cb72e141d6d949b463db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1575734
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Alexei Filippov <alph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#654237}
diff --git a/components/services/heap_profiling/connection_manager.cc b/components/services/heap_profiling/connection_manager.cc
index 57c1b99..9f2cd9d 100644
--- a/components/services/heap_profiling/connection_manager.cc
+++ b/components/services/heap_profiling/connection_manager.cc
@@ -7,7 +7,6 @@
 #include "base/bind.h"
 #include "base/json/string_escape.h"
 #include "base/metrics/histogram_macros.h"
-#include "base/task/post_task.h"
 #include "components/services/heap_profiling/json_exporter.h"
 #include "components/services/heap_profiling/public/cpp/client.h"
 
@@ -33,7 +32,7 @@
   VmRegions vm_regions;
 
   // Collects the results.
-  std::vector<memory_instrumentation::mojom::SharedBufferWithSizePtr> results;
+  std::vector<memory_instrumentation::mojom::HeapProfileResultPtr> results;
 
  private:
   friend class base::RefCountedThreadSafe<DumpProcessesForTracingTracking>;
@@ -156,7 +155,7 @@
   // Early out if there are no connections.
   if (connections_.empty()) {
     std::move(callback).Run(
-        std::vector<memory_instrumentation::mojom::SharedBufferWithSizePtr>());
+        std::vector<memory_instrumentation::mojom::HeapProfileResultPtr>());
     return;
   }
 
@@ -176,26 +175,20 @@
   }
 }
 
-void ConnectionManager::HeapProfileRetrieved(
-    scoped_refptr<DumpProcessesForTracingTracking> tracking,
-    base::ProcessId pid,
-    mojom::ProcessType process_type,
-    bool strip_path_from_mapped_files,
+bool ConnectionManager::ConvertProfileToExportParams(
+    mojom::HeapProfilePtr profile,
     uint32_t sampling_rate,
-    mojom::HeapProfilePtr profile) {
-  AllocationMap allocations;
+    ExportParams* params) {
+  AllocationMap allocs;
   ContextMap context_map;
   AddressToStringMap string_map;
 
-  bool success = true;
   for (const mojom::HeapProfileSamplePtr& sample : profile->samples) {
     int context_id = 0;
     if (sample->context_id) {
       auto it = profile->strings.find(sample->context_id);
-      if (it == profile->strings.end()) {
-        success = false;
-        break;
-      }
+      if (it == profile->strings.end())
+        return false;
       const std::string& context = it->second;
       // Escape the strings early, to simplify exporting a heap dump.
       std::string escaped_context;
@@ -235,7 +228,7 @@
 
     std::vector<Address> stack(sample->stack.begin(), sample->stack.end());
     AllocationMetrics& metrics =
-        allocations
+        allocs
             .emplace(std::piecewise_construct,
                      std::forward_as_tuple(sample->allocator, std::move(stack),
                                            context_id),
@@ -253,87 +246,46 @@
     string_map.emplace(str.first, std::move(quoted_string));
   }
 
-  DCHECK(success);
-  DoDumpOneProcessForTracing(std::move(tracking), pid, process_type,
-                             strip_path_from_mapped_files, success,
-                             std::move(allocations), std::move(context_map),
-                             std::move(string_map));
+  params->allocs = std::move(allocs);
+  params->context_map = std::move(context_map);
+  params->mapped_strings = std::move(string_map);
+  return true;
 }
 
-void ConnectionManager::DoDumpOneProcessForTracing(
+void ConnectionManager::HeapProfileRetrieved(
     scoped_refptr<DumpProcessesForTracingTracking> tracking,
     base::ProcessId pid,
     mojom::ProcessType process_type,
     bool strip_path_from_mapped_files,
-    bool success,
-    AllocationMap counts,
-    ContextMap context,
-    AddressToStringMap mapped_strings) {
+    uint32_t sampling_rate,
+    mojom::HeapProfilePtr profile) {
   // All code paths through here must issue the callback when waiting_responses
   // is 0 or the browser will wait forever for the dump.
   DCHECK(tracking->waiting_responses > 0);
 
-  if (!success) {
-    tracking->waiting_responses--;
-    if (tracking->waiting_responses == 0)
-      std::move(tracking->callback).Run(std::move(tracking->results));
-    return;
+  ExportParams params;
+  bool success =
+      ConvertProfileToExportParams(std::move(profile), sampling_rate, &params);
+  if (success) {
+    params.process_type = process_type;
+    params.strip_path_from_mapped_files = strip_path_from_mapped_files;
+    params.next_id = next_id_;
+
+    auto it = tracking->vm_regions.find(pid);
+    if (it != tracking->vm_regions.end())
+      params.maps = std::move(it->second);
+
+    memory_instrumentation::mojom::HeapProfileResultPtr result =
+        memory_instrumentation::mojom::HeapProfileResult::New();
+    result->pid = pid;
+    result->json = ExportMemoryMapsAndV2StackTraceToJSON(&params);
+    tracking->results.push_back(std::move(result));
+    next_id_ = params.next_id;
   }
 
-  ExportParams params;
-  params.allocs = std::move(counts);
-  params.context_map = std::move(context);
-  params.mapped_strings = std::move(mapped_strings);
-  params.process_type = process_type;
-  params.strip_path_from_mapped_files = strip_path_from_mapped_files;
-  params.next_id = next_id_;
-
-  auto it = tracking->vm_regions.find(pid);
-  if (it != tracking->vm_regions.end())
-    params.maps = std::move(it->second);
-
-  std::string reply = ExportMemoryMapsAndV2StackTraceToJSON(&params);
-  size_t reply_size = reply.size();
-  next_id_ = params.next_id;
-
-  base::PostTaskWithTraitsAndReplyWithResult(
-      FROM_HERE,
-      {base::TaskPriority::BEST_EFFORT, base::MayBlock(),
-       base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN},
-      base::BindOnce(
-          [](size_t size) {
-            // This call sends a synchronous IPC to the browser process.
-            return mojo::SharedBufferHandle::Create(size);
-          },
-          reply_size),
-      base::BindOnce(
-          [](std::string reply, base::ProcessId pid,
-             scoped_refptr<DumpProcessesForTracingTracking> tracking,
-             mojo::ScopedSharedBufferHandle buffer) {
-            if (!buffer.is_valid()) {
-              DLOG(ERROR) << "Could not create Mojo shared buffer";
-            } else {
-              mojo::ScopedSharedBufferMapping mapping =
-                  buffer->Map(reply.size());
-              if (!mapping) {
-                DLOG(ERROR) << "Could not map Mojo shared buffer";
-              } else {
-                memcpy(mapping.get(), reply.c_str(), reply.size());
-
-                memory_instrumentation::mojom::SharedBufferWithSizePtr result =
-                    memory_instrumentation::mojom::SharedBufferWithSize::New();
-                result->buffer = std::move(buffer);
-                result->size = reply.size();
-                result->pid = pid;
-                tracking->results.push_back(std::move(result));
-              }
-            }
-
-            // When all responses complete, issue done callback.
-            if (--tracking->waiting_responses == 0)
-              std::move(tracking->callback).Run(std::move(tracking->results));
-          },
-          std::move(reply), pid, std::move(tracking)));
+  // When all responses complete, issue done callback.
+  if (--tracking->waiting_responses == 0)
+    std::move(tracking->callback).Run(std::move(tracking->results));
 }
 
 }  // namespace heap_profiling
diff --git a/components/services/heap_profiling/connection_manager.h b/components/services/heap_profiling/connection_manager.h
index 01d65b4..cb576e9 100644
--- a/components/services/heap_profiling/connection_manager.h
+++ b/components/services/heap_profiling/connection_manager.h
@@ -24,6 +24,8 @@
 
 namespace heap_profiling {
 
+struct ExportParams;
+
 using VmRegions =
     base::flat_map<base::ProcessId,
                    std::vector<memory_instrumentation::mojom::VmRegionPtr>>;
@@ -73,15 +75,9 @@
       uint32_t sampling_rate,
       mojom::HeapProfilePtr profile);
 
-  void DoDumpOneProcessForTracing(
-      scoped_refptr<DumpProcessesForTracingTracking> tracking,
-      base::ProcessId pid,
-      mojom::ProcessType process_type,
-      bool strip_path_from_mapped_files,
-      bool success,
-      AllocationMap counts,
-      ContextMap context,
-      AddressToStringMap mapped_strings);
+  bool ConvertProfileToExportParams(mojom::HeapProfilePtr profile,
+                                    uint32_t sampling_rate,
+                                    ExportParams* out_params);
 
   // Notification that a connection is complete. Unlike OnNewConnection which
   // is signaled by the pipe server, this is signaled by the allocation tracker
diff --git a/services/resource_coordinator/memory_instrumentation/coordinator_impl.cc b/services/resource_coordinator/memory_instrumentation/coordinator_impl.cc
index cf3d934..a158a45 100644
--- a/services/resource_coordinator/memory_instrumentation/coordinator_impl.cc
+++ b/services/resource_coordinator/memory_instrumentation/coordinator_impl.cc
@@ -423,9 +423,8 @@
             .IsArgumentFilterEnabled();
     heap_profiler_->DumpProcessesForTracing(
         strip_path_from_mapped_files,
-        base::BindRepeating(&CoordinatorImpl::OnDumpProcessesForTracing,
-                            weak_ptr_factory_.GetWeakPtr(),
-                            request->dump_guid));
+        base::BindOnce(&CoordinatorImpl::OnDumpProcessesForTracing,
+                       weak_ptr_factory_.GetWeakPtr(), request->dump_guid));
 
     base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
         FROM_HERE,
@@ -538,7 +537,7 @@
 
 void CoordinatorImpl::OnDumpProcessesForTracing(
     uint64_t dump_guid,
-    std::vector<mojom::SharedBufferWithSizePtr> buffers) {
+    std::vector<mojom::HeapProfileResultPtr> heap_profile_results) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   QueuedRequest* request = GetCurrentRequest();
   if (!request || request->dump_guid != dump_guid) {
@@ -547,23 +546,9 @@
 
   request->heap_dump_in_progress = false;
 
-  for (auto& buffer_ptr : buffers) {
-    mojo::ScopedSharedBufferHandle& buffer = buffer_ptr->buffer;
-    uint32_t size = buffer_ptr->size;
-
-    if (!buffer->is_valid())
-      continue;
-
-    mojo::ScopedSharedBufferMapping mapping = buffer->Map(size);
-    if (!mapping) {
-      DLOG(ERROR) << "Failed to map buffer";
-      continue;
-    }
-
-    const char* char_buffer = static_cast<const char*>(mapping.get());
-    std::string json(char_buffer, char_buffer + size);
+  for (auto& result : heap_profile_results) {
     base::trace_event::TraceArguments args(
-        "dumps", std::make_unique<StringWrapper>(std::move(json)));
+        "dumps", std::make_unique<StringWrapper>(std::move(result->json)));
 
     // Using the same id merges all of the heap dumps into a single detailed
     // dump node in the UI.
@@ -572,7 +557,7 @@
         base::trace_event::TraceLog::GetCategoryGroupEnabled(
             base::trace_event::MemoryDumpManager::kTraceCategory),
         "periodic_interval", trace_event_internal::kGlobalScope, dump_guid,
-        buffer_ptr->pid, &args, TRACE_EVENT_FLAG_HAS_ID);
+        result->pid, &args, TRACE_EVENT_FLAG_HAS_ID);
   }
 
   FinalizeGlobalMemoryDumpIfAllManagersReplied();
diff --git a/services/resource_coordinator/memory_instrumentation/coordinator_impl.h b/services/resource_coordinator/memory_instrumentation/coordinator_impl.h
index d56cb08..c3432de 100644
--- a/services/resource_coordinator/memory_instrumentation/coordinator_impl.h
+++ b/services/resource_coordinator/memory_instrumentation/coordinator_impl.h
@@ -139,7 +139,7 @@
   // Callback of DumpProcessesForTracing.
   void OnDumpProcessesForTracing(
       uint64_t dump_guid,
-      std::vector<mojom::SharedBufferWithSizePtr> buffers);
+      std::vector<mojom::HeapProfileResultPtr> heap_profile_results);
 
   void RemovePendingResponse(mojom::ClientProcess*,
                              QueuedRequest::PendingResponse::Type);
diff --git a/services/resource_coordinator/public/mojom/memory_instrumentation/memory_instrumentation.mojom b/services/resource_coordinator/public/mojom/memory_instrumentation/memory_instrumentation.mojom
index d6af2047..aab6920 100644
--- a/services/resource_coordinator/public/mojom/memory_instrumentation/memory_instrumentation.mojom
+++ b/services/resource_coordinator/public/mojom/memory_instrumentation/memory_instrumentation.mojom
@@ -4,6 +4,7 @@
 
 module memory_instrumentation.mojom;
 
+import "mojo/public/mojom/base/big_string.mojom";
 import "mojo/public/mojom/base/process_id.mojom";
 import "mojo/public/mojom/base/time.mojom";
 
@@ -240,10 +241,9 @@
       (bool success, map<mojo_base.mojom.ProcessId, RawOSMemDump> dumps);
 };
 
-struct SharedBufferWithSize {
-  handle<shared_buffer> buffer;
-  uint32 size;
+struct HeapProfileResult {
   mojo_base.mojom.ProcessId pid;
+  mojo_base.mojom.BigString json;
 };
 
 // HeapProfilers expose a single interface to memory_instrumentation, allowing
@@ -254,17 +254,17 @@
 // with the Coordinator (see RegisterHeapProfiler) and is invoked when a memory
 // dump is requested (via Coordinator::RequestGlobalMemoryDump).
 interface HeapProfiler {
-  // Dumps the memory log of all profiled processes into shared buffers. The
-  // contents of each shared buffer is a JSON string compatible with
+  // Dumps the memory log of all profiled processes into BigString's.
+  // The contents of each BigString is a JSON string compatible with
   // TRACE_EVENT* macros. Processes that fail to dump will be omitted from
-  // |buffers|. When |strip_path_from_mapped_files| is true, only the base name
+  // |result|. When |strip_path_from_mapped_files| is true, only the base name
   // of mapped files is emitted. This prevents usernames from sneaking into the
   // trace.
   // |strip_path_from_mapped_files| should only be true for traces that will be
   // uploaded to the crash servers - this strips potential PII, but prevents
   // symbolization of local builds.
   DumpProcessesForTracing(bool strip_path_from_mapped_files) =>
-      (array<SharedBufferWithSize> buffers);
+      (array<HeapProfileResult> results);
 };
 
 // Implemented by resource_coordinator to provide additional information needed
diff --git a/third_party/blink/renderer/platform/BUILD.gn b/third_party/blink/renderer/platform/BUILD.gn
index 9e48c00..8420081 100644
--- a/third_party/blink/renderer/platform/BUILD.gn
+++ b/third_party/blink/renderer/platform/BUILD.gn
@@ -1232,7 +1232,6 @@
     "mhtml/serialized_resource.h",
     "mhtml/shared_buffer_chunk_reader.cc",
     "mhtml/shared_buffer_chunk_reader.h",
-    "mojo/big_string_mojom_traits.cc",
     "mojo/big_string_mojom_traits.h",
     "mojo/bluetooth_struct_traits.cc",
     "mojo/bluetooth_struct_traits.h",
diff --git a/third_party/blink/renderer/platform/mojo/big_string_mojom_traits.cc b/third_party/blink/renderer/platform/mojo/big_string_mojom_traits.cc
deleted file mode 100644
index a0c0200..0000000
--- a/third_party/blink/renderer/platform/mojo/big_string_mojom_traits.cc
+++ /dev/null
@@ -1,43 +0,0 @@
-// Copyright 2016 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "third_party/blink/renderer/platform/mojo/big_string_mojom_traits.h"
-
-#include <cstring>
-
-#include "base/containers/span.h"
-#include "mojo/public/cpp/base/big_buffer.h"
-#include "mojo/public/cpp/base/big_buffer_mojom_traits.h"
-#include "third_party/blink/renderer/platform/wtf/text/string_utf8_adaptor.h"
-
-namespace mojo {
-
-// static
-mojo_base::BigBuffer StructTraits<mojo_base::mojom::BigStringDataView,
-                                  WTF::String>::data(const WTF::String& input) {
-  WTF::StringUTF8Adaptor adaptor(input);
-  return mojo_base::BigBuffer(base::as_bytes(base::make_span(adaptor)));
-}
-
-// static
-bool StructTraits<mojo_base::mojom::BigStringDataView, WTF::String>::Read(
-    mojo_base::mojom::BigStringDataView data,
-    WTF::String* out) {
-  mojo_base::BigBuffer buffer;
-  if (!data.ReadData(&buffer))
-    return false;
-  size_t size = buffer.size();
-  if (size % sizeof(char))
-    return false;
-  // An empty |mojo_base::BigBuffer| may have a null |data()| if empty.
-  if (!size) {
-    *out = g_empty_string;
-  } else {
-    *out = WTF::String::FromUTF8(reinterpret_cast<const char*>(buffer.data()),
-                                 size / sizeof(char));
-  }
-  return true;
-}
-
-}  // namespace mojo
diff --git a/third_party/blink/renderer/platform/mojo/big_string_mojom_traits.h b/third_party/blink/renderer/platform/mojo/big_string_mojom_traits.h
index 4ea78d8..d157f8e 100644
--- a/third_party/blink/renderer/platform/mojo/big_string_mojom_traits.h
+++ b/third_party/blink/renderer/platform/mojo/big_string_mojom_traits.h
@@ -8,10 +8,7 @@
 #include "mojo/public/cpp/bindings/struct_traits.h"
 #include "mojo/public/mojom/base/big_string.mojom-blink.h"
 #include "third_party/blink/renderer/platform/platform_export.h"
-
-namespace mojo_base {
-class BigBuffer;
-}
+#include "third_party/blink/renderer/platform/wtf/text/string_utf8_adaptor.h"
 
 namespace mojo {
 
@@ -21,8 +18,27 @@
   static bool IsNull(const WTF::String& input) { return input.IsNull(); }
   static void SetToNull(WTF::String* output) { *output = WTF::String(); }
 
-  static mojo_base::BigBuffer data(const WTF::String& input);
-  static bool Read(mojo_base::mojom::BigStringDataView, WTF::String* out);
+  static mojo_base::BigBuffer data(const WTF::String& input) {
+    WTF::StringUTF8Adaptor adaptor(input);
+    return mojo_base::BigBuffer(base::as_bytes(base::make_span(adaptor)));
+  }
+
+  static bool Read(mojo_base::mojom::BigStringDataView data, WTF::String* out) {
+    mojo_base::BigBuffer buffer;
+    if (!data.ReadData(&buffer))
+      return false;
+    size_t size = buffer.size();
+    if (size % sizeof(char))
+      return false;
+    // An empty |mojo_base::BigBuffer| may have a null |data()| if empty.
+    if (!size) {
+      *out = g_empty_string;
+    } else {
+      *out = WTF::String::FromUTF8(reinterpret_cast<const char*>(buffer.data()),
+                                   size / sizeof(char));
+    }
+    return true;
+  }
 };
 
 }  // namespace mojo