Use //base shmem APIs for Perfetto service.

Direct use of handle<shared_buffer> is discouraged; the wrappers around
the base APIs give more indication of whether a given shmem region is
intended by read-only or writable.

Bug: 1306363
Change-Id: Ibe2e864345a2aed1322112e5269c101eb881d166
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3643178
Reviewed-by: Eric Seckler <eseckler@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1002542}

services/tracing/perfetto/perfetto_service.cc

@@ -69,7 +69,8 @@ PerfettoService::PerfettoService(
                                 ? std::move(task_runner_for_testing)
                                 : base::SequencedTaskRunnerHandle::Get()) {
   service_ = perfetto::TracingService::CreateInstance(
-      std::make_unique<MojoSharedMemory::Factory>(), &perfetto_task_runner_);
+      std::make_unique<ChromeBaseSharedMemory::Factory>(),
+      &perfetto_task_runner_);
   // Chromium uses scraping of the shared memory chunks to ensure that data
   // from threads without a MessageLoop doesn't get lost.
   service_->SetSMBScrapingEnabled(true);
@@ -96,17 +97,14 @@ void PerfettoService::BindReceiver(
 void PerfettoService::ConnectToProducerHost(
     mojo::PendingRemote<mojom::ProducerClient> producer_client,
     mojo::PendingReceiver<mojom::ProducerHost> producer_host_receiver,
-    mojo::ScopedSharedBufferHandle shared_memory,
+    base::UnsafeSharedMemoryRegion shared_memory,
     uint64_t shared_memory_buffer_page_size_bytes) {
-  if (!shared_memory.is_valid()) {
-    // Connection requests should always include an SMB.
-    mojo::ReportBadMessage("Producer connection request without SMB");
-    return;
-  }
+  // `shared_memory` is not marked nullable in the Mojom IDL so the region
+  // should always be valid.
+  DCHECK(shared_memory.IsValid());
 
   auto new_producer = std::make_unique<ProducerHost>(&perfetto_task_runner_);
   uint32_t producer_pid = receivers_.current_context();
-  DCHECK(shared_memory.is_valid());
   ProducerHost::InitializationResult result = new_producer->Initialize(
       std::move(producer_client), service_.get(),
       base::StrCat({mojom::kPerfettoProducerNamePrefix,

services/tracing/perfetto/perfetto_service.h

@@ -46,7 +46,7 @@ class PerfettoService : public mojom::PerfettoService {
   void ConnectToProducerHost(
       mojo::PendingRemote<mojom::ProducerClient> producer_client,
       mojo::PendingReceiver<mojom::ProducerHost> producer_host_receiver,
-      mojo::ScopedSharedBufferHandle shared_memory,
+      base::UnsafeSharedMemoryRegion shared_memory,
       uint64_t shared_memory_buffer_page_size_bytes) override;
 
   perfetto::TracingService* GetService() const;

services/tracing/perfetto/producer_host.cc

@@ -33,21 +33,21 @@ ProducerHost::InitializationResult ProducerHost::Initialize(
     mojo::PendingRemote<mojom::ProducerClient> producer_client,
     perfetto::TracingService* service,
     const std::string& name,
-    mojo::ScopedSharedBufferHandle shared_memory,
+    base::UnsafeSharedMemoryRegion shared_memory,
     uint64_t shared_memory_buffer_page_size_bytes) {
   DCHECK(service);
   DCHECK(!producer_endpoint_);
 
   producer_client_.Bind(std::move(producer_client));
 
-  auto shm = std::make_unique<MojoSharedMemory>(std::move(shared_memory));
+  auto shm = std::make_unique<ChromeBaseSharedMemory>(std::move(shared_memory));
   // We may fail to map the buffer provided by the ProducerClient.
   if (!shm->start()) {
     return InitializationResult::kSmbMappingFailed;
   }
 
   size_t shm_size = shm->size();
-  MojoSharedMemory* shm_raw = shm.get();
+  ChromeBaseSharedMemory* shm_raw = shm.get();
 
   // TODO(oysteine): Figure out a uid once we need it.
   producer_endpoint_ = service->ConnectProducer(

services/tracing/perfetto/producer_host.h

@@ -11,6 +11,7 @@
 #include <vector>
 
 #include "base/memory/raw_ptr.h"
+#include "base/memory/unsafe_shared_memory_region.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "services/tracing/public/mojom/perfetto_service.mojom.h"
 #include "third_party/perfetto/include/perfetto/ext/tracing/core/producer.h"
@@ -62,7 +63,7 @@ class ProducerHost : public tracing::mojom::ProducerHost,
       mojo::PendingRemote<mojom::ProducerClient> producer_client,
       perfetto::TracingService* service,
       const std::string& name,
-      mojo::ScopedSharedBufferHandle shared_memory,
+      base::UnsafeSharedMemoryRegion shared_memory,
       uint64_t shared_memory_buffer_page_size_bytes);
 
   // perfetto::Producer implementation.

services/tracing/perfetto/test_utils.cc

@@ -351,9 +351,9 @@ MockProducerHost::MockProducerHost(
   mojo::PendingRemote<mojom::ProducerHost> host_remote;
   auto client_receiver = client.InitWithNewPipeAndPassReceiver();
   Initialize(std::move(client), service->GetService(), producer_name_,
-             static_cast<MojoSharedMemory*>(
+             static_cast<ChromeBaseSharedMemory*>(
                  producer_client->shared_memory_for_testing())
-                 ->Clone(),
+                 ->CloneRegion(),
              PerfettoProducer::kSMBPageSizeBytes);
   receiver_.Bind(host_remote.InitWithNewPipeAndPassReceiver());
   producer_client->BindClientAndHostPipesForTesting(std::move(client_receiver),

services/tracing/public/cpp/perfetto/perfetto_tracing_backend.cc

@@ -247,7 +247,7 @@ class ProducerEndpoint : public perfetto::ProducerEndpoint,
   struct EndpointBindings {
     mojo::PendingReceiver<mojom::ProducerClient> client_receiver;
     mojo::PendingRemote<mojom::ProducerHost> host_remote;
-    std::unique_ptr<MojoSharedMemory> shared_memory;
+    std::unique_ptr<ChromeBaseSharedMemory> shared_memory;
   };
 
   static void OnConnected(
@@ -266,9 +266,9 @@ class ProducerEndpoint : public perfetto::ProducerEndpoint,
     if (!shmem_page_size_bytes)
       shmem_page_size_bytes = kDefaultSMBPageSizeBytes;
     bindings->shared_memory =
-        std::make_unique<MojoSharedMemory>(shmem_size_bytes);
+        std::make_unique<ChromeBaseSharedMemory>(shmem_size_bytes);
 
-    if (!bindings->shared_memory->shared_buffer().is_valid()) {
+    if (!bindings->shared_memory->region().IsValid()) {
       // There's no way to do tracing after an SMB allocation failure, so let's
       // disconnect Perfetto.
       // TODO(skyostil): Record failure in UMA.
@@ -287,7 +287,7 @@ class ProducerEndpoint : public perfetto::ProducerEndpoint,
         ->ConnectToProducerHost(
             std::move(client),
             bindings->host_remote.InitWithNewPipeAndPassReceiver(),
-            bindings->shared_memory->Clone(), shmem_page_size_bytes);
+            bindings->shared_memory->CloneRegion(), shmem_page_size_bytes);
 
     // Bind the interfaces on Perfetto's sequence so we can avoid extra thread
     // hops.
@@ -342,7 +342,7 @@ class ProducerEndpoint : public perfetto::ProducerEndpoint,
   size_t shared_buffer_page_size_kb_ = 0;
 
   // Accessed on arbitrary threads after setup.
-  std::unique_ptr<MojoSharedMemory> shared_memory_;
+  std::unique_ptr<ChromeBaseSharedMemory> shared_memory_;
   std::unique_ptr<perfetto::SharedMemoryArbiter> shared_memory_arbiter_;
 
   base::WeakPtrFactory<ProducerEndpoint> weak_factory_{this};

services/tracing/public/cpp/perfetto/producer_client.cc

@@ -8,6 +8,7 @@
 
 #include "base/bind.h"
 #include "base/containers/adapters.h"
+#include "base/memory/unsafe_shared_memory_region.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/process/process.h"
 #include "base/tracing/tracing_tls.h"
@@ -47,12 +48,12 @@ void ProducerClient::Connect(
     return;
   }
 
-  mojo::ScopedSharedBufferHandle shm;
+  base::UnsafeSharedMemoryRegion shm;
   {
     base::AutoLock lock(lock_);
-    shm = shared_memory_->Clone();
+    shm = shared_memory_->CloneRegion();
   }
-  CHECK(shm.is_valid());
+  CHECK(shm.IsValid());
 
   mojo::PendingRemote<mojom::ProducerClient> client;
   auto client_receiver = client.InitWithNewPipeAndPassReceiver();
@@ -399,12 +400,12 @@ bool ProducerClient::InitSharedMemoryIfNeeded() {
   // The shared memory buffer is always provided by the ProducerClient, but only
   // created upon the first tracing request.
   shared_memory_ =
-      std::make_unique<MojoSharedMemory>(GetPreferredSmbSizeBytes());
+      std::make_unique<ChromeBaseSharedMemory>(GetPreferredSmbSizeBytes());
 
-  // TODO(crbug/1057614): We see shared memory buffer creation fail on windows
+  // TODO(crbug/1057614): We see shared memory region creation fail on windows
   // in the field. Investigate why this can happen. Gather statistics on
   // failure rates.
-  bool valid = shared_memory_->shared_buffer().is_valid();
+  bool valid = shared_memory_->region().IsValid();
   base::UmaHistogramBoolean(kSharedBufferIsValidMetricName, valid);
 
   if (!valid) {

services/tracing/public/cpp/perfetto/producer_client.h

@@ -32,7 +32,7 @@ class SharedMemoryArbiter;
 
 namespace tracing {
 
-class MojoSharedMemory;
+class ChromeBaseSharedMemory;
 
 // This class is the per-process client side of the Perfetto
 // producer, and is responsible for creating specific kinds
@@ -146,7 +146,7 @@ class COMPONENT_EXPORT(TRACING_CPP) ProducerClient
   // TODO(eseckler): Consider accessing |shared_memory_| and
   // |shared_memory_arbiter_| without locks after setup was completed, since we
   // never destroy or unset them.
-  std::unique_ptr<MojoSharedMemory> shared_memory_ GUARDED_BY(lock_);
+  std::unique_ptr<ChromeBaseSharedMemory> shared_memory_ GUARDED_BY(lock_);
   std::unique_ptr<perfetto::SharedMemoryArbiter> shared_memory_arbiter_
       GUARDED_BY(lock_);
 

services/tracing/public/cpp/perfetto/shared_memory.cc

@@ -11,40 +11,40 @@
 namespace tracing {
 
 std::unique_ptr<perfetto::SharedMemory>
-MojoSharedMemory::Factory::CreateSharedMemory(size_t size) {
-  return std::make_unique<MojoSharedMemory>(size);
+ChromeBaseSharedMemory::Factory::CreateSharedMemory(size_t size) {
+  return std::make_unique<ChromeBaseSharedMemory>(size);
 }
 
-MojoSharedMemory::MojoSharedMemory(size_t size) {
-  shared_buffer_ = mojo::SharedBufferHandle::Create(size);
+ChromeBaseSharedMemory::ChromeBaseSharedMemory(size_t size)
+    : region_(base::UnsafeSharedMemoryRegion::Create(size)) {
   // DCHECK rather than CHECK as we handle SMB creation failures as
   // DumpWithoutCrashing in ProducerClient on release builds.
-  DCHECK(shared_buffer_.is_valid());
-  mapping_ = shared_buffer_->Map(size);
-  DCHECK(mapping_);
+  DCHECK(region_.IsValid());
+  mapping_ = region_.Map();
+  DCHECK(mapping_.IsValid());
 }
 
-MojoSharedMemory::MojoSharedMemory(mojo::ScopedSharedBufferHandle shared_memory)
-    : shared_buffer_(std::move(shared_memory)) {
-  mapping_ = shared_buffer_->Map(shared_buffer_->GetSize());
+ChromeBaseSharedMemory::ChromeBaseSharedMemory(
+    base::UnsafeSharedMemoryRegion region)
+    : region_(std::move(region)) {
+  mapping_ = region_.Map();
   // DCHECK rather than CHECK as we handle SMB mapping failures in ProducerHost
   // on release builds.
-  DCHECK(mapping_);
+  DCHECK(mapping_.IsValid());
 }
 
-MojoSharedMemory::~MojoSharedMemory() = default;
+ChromeBaseSharedMemory::~ChromeBaseSharedMemory() = default;
 
-mojo::ScopedSharedBufferHandle MojoSharedMemory::Clone() {
-  return shared_buffer_->Clone(
-      mojo::SharedBufferHandle::AccessMode::READ_WRITE);
+base::UnsafeSharedMemoryRegion ChromeBaseSharedMemory::CloneRegion() {
+  return region_.Duplicate();
 }
 
-void* MojoSharedMemory::start() const {
-  return mapping_.get();
+void* ChromeBaseSharedMemory::start() const {
+  return mapping_.memory();
 }
 
-size_t MojoSharedMemory::size() const {
-  return shared_buffer_->GetSize();
+size_t ChromeBaseSharedMemory::size() const {
+  return region_.GetSize();
 }
 
 }  // namespace tracing

services/tracing/public/cpp/perfetto/shared_memory.h

@@ -8,14 +8,14 @@
 #include <memory>
 
 #include "base/component_export.h"
-#include "mojo/public/cpp/system/platform_handle.h"
+#include "base/memory/shared_memory_mapping.h"
+#include "base/memory/unsafe_shared_memory_region.h"
 #include "third_party/perfetto/include/perfetto/ext/tracing/core/shared_memory.h"
 
 namespace tracing {
 
-// This wraps a Mojo SharedBuffer implementation to be
-// able to provide it to Perfetto.
-class COMPONENT_EXPORT(TRACING_CPP) MojoSharedMemory
+// This wraps //base's shmem implementation for Perfetto to consume.
+class COMPONENT_EXPORT(TRACING_CPP) ChromeBaseSharedMemory
     : public perfetto::SharedMemory {
  public:
   class COMPONENT_EXPORT(TRACING_CPP) Factory
@@ -25,30 +25,27 @@ class COMPONENT_EXPORT(TRACING_CPP) MojoSharedMemory
         size_t size) override;
   };
 
-  explicit MojoSharedMemory(size_t size);
-  explicit MojoSharedMemory(mojo::ScopedSharedBufferHandle shared_memory);
+  explicit ChromeBaseSharedMemory(size_t size);
+  explicit ChromeBaseSharedMemory(base::UnsafeSharedMemoryRegion region);
 
-  MojoSharedMemory(const MojoSharedMemory&) = delete;
-  MojoSharedMemory& operator=(const MojoSharedMemory&) = delete;
+  ChromeBaseSharedMemory(const ChromeBaseSharedMemory&) = delete;
+  ChromeBaseSharedMemory& operator=(const ChromeBaseSharedMemory&) = delete;
 
-  ~MojoSharedMemory() override;
+  ~ChromeBaseSharedMemory() override;
 
-  // Create another wrapping instance of the same SharedMemory buffer,
-  // for sending to other processes.
-  mojo::ScopedSharedBufferHandle Clone();
+  // Clone the region, e.g. for sending to other processes over IPC.
+  base::UnsafeSharedMemoryRegion CloneRegion();
 
-  const mojo::ScopedSharedBufferHandle& shared_buffer() const {
-    return shared_buffer_;
-  }
+  const base::UnsafeSharedMemoryRegion& region() const { return region_; }
 
   // perfetto::SharedMemory implementation. Called internally by Perfetto
   // classes.
   void* start() const override;
   size_t size() const override;
 
  private:
-  mojo::ScopedSharedBufferHandle shared_buffer_;
-  mojo::ScopedSharedBufferMapping mapping_;
+  base::UnsafeSharedMemoryRegion region_;
+  base::WritableSharedMemoryMapping mapping_;
 };
 
 }  // namespace tracing

services/tracing/public/mojom/perfetto_service.mojom

@@ -5,6 +5,7 @@
 module tracing.mojom;
 
 import "mojo/public/mojom/base/file.mojom";
+import "mojo/public/mojom/base/shared_memory.mojom";
 
 // Producer processes register with the format
 // "kPerfettoProducerNamePrefix-PID" when connecting to Chrome's internal
@@ -165,7 +166,7 @@ interface PerfettoService {
   // providing the shared memory buffer that will be used to send trace data.
   ConnectToProducerHost(pending_remote<ProducerClient> producer_client,
                         pending_receiver<ProducerHost> producer_host_receiver,
-                        handle<shared_buffer> shared_memory,
+                        mojo_base.mojom.UnsafeSharedMemoryRegion shared_memory,
                         uint64 shared_memory_buffer_page_size_bytes);
 };