Implement IPC::ParamTraits for PlatformSharedMemoryRegion

This CL adds the support for passing PlatfromSharedMemoryRegions
between processes via IPC messages.

Bug: 795291
Change-Id: I13b22226866b0a4e5d97e66f4ce93e5ad28fdaac
Reviewed-on: https://chromium-review.googlesource.com/937514
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Matthew Cary <mattcary@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552192}

base/memory/platform_shared_memory_region.h

@@ -88,6 +88,7 @@ class BASE_EXPORT PlatformSharedMemoryRegion {
     kReadOnly,  // ReadOnlySharedMemoryRegion
     kWritable,  // WritableSharedMemoryRegion
     kUnsafe,    // UnsafeSharedMemoryRegion
+    kMaxValue = kUnsafe
   };
 
 // Platform-specific shared memory type used by this class.

base/memory/platform_shared_memory_region_unittest.cc

@@ -21,30 +21,14 @@ namespace subtle {
 
 const size_t kRegionSize = 1024;
 
-class PlatformSharedMemoryRegionTest : public ::testing::Test {
- public:
-  SharedMemoryMapping MapAt(PlatformSharedMemoryRegion* region,
-                            off_t offset,
-                            size_t bytes) {
-    void* memory = nullptr;
-    size_t mapped_size = 0;
-    if (!region->MapAt(offset, bytes, &memory, &mapped_size))
-      return {};
-
-    return SharedMemoryMapping(memory, bytes, mapped_size, region->GetGUID());
-  }
-
-  void* GetMemory(SharedMemoryMapping* mapping) {
-    return mapping->raw_memory_ptr();
-  }
-};
+class PlatformSharedMemoryRegionTest : public ::testing::Test {};
 
 // Tests that a default constructed region is invalid and produces invalid
 // mappings.
 TEST_F(PlatformSharedMemoryRegionTest, DefaultConstructedRegionIsInvalid) {
   PlatformSharedMemoryRegion region;
   EXPECT_FALSE(region.IsValid());
-  SharedMemoryMapping mapping = MapAt(&region, 0, kRegionSize);
+  WritableSharedMemoryMapping mapping = MapForTesting(&region);
   EXPECT_FALSE(mapping.IsValid());
   PlatformSharedMemoryRegion duplicate = region.Duplicate();
   EXPECT_FALSE(duplicate.IsValid());
@@ -151,7 +135,8 @@ TEST_F(PlatformSharedMemoryRegionTest, MapAtOutOfTheRegionLimitsTest) {
   PlatformSharedMemoryRegion region =
       PlatformSharedMemoryRegion::CreateWritable(kRegionSize);
   ASSERT_TRUE(region.IsValid());
-  SharedMemoryMapping mapping = MapAt(&region, 0, region.GetSize() + 1);
+  WritableSharedMemoryMapping mapping =
+      MapAtForTesting(&region, 0, region.GetSize() + 1);
   EXPECT_FALSE(mapping.IsValid());
 }
 
@@ -166,7 +151,7 @@ TEST_F(PlatformSharedMemoryRegionTest, MapAtWithOverflowTest) {
   // |size| + |offset| should be below the region size due to overflow but
   // mapping a region with these parameters should be invalid.
   EXPECT_LT(size + offset, region.GetSize());
-  SharedMemoryMapping mapping = MapAt(&region, offset, size);
+  WritableSharedMemoryMapping mapping = MapAtForTesting(&region, offset, size);
   EXPECT_FALSE(mapping.IsValid());
 }
 
@@ -192,12 +177,12 @@ TEST_F(PlatformSharedMemoryRegionTest, MapCurrentAndMaxProtectionSetCorrectly) {
       PlatformSharedMemoryRegion::CreateWritable(kRegionSize);
   ASSERT_TRUE(region.IsValid());
   ASSERT_TRUE(region.ConvertToReadOnly());
-  SharedMemoryMapping ro_mapping = MapAt(&region, 0, kRegionSize);
+  WritableSharedMemoryMapping ro_mapping = MapForTesting(&region);
   ASSERT_TRUE(ro_mapping.IsValid());
 
   vm_region_basic_info_64 basic_info;
   mach_vm_size_t dummy_size = 0;
-  void* temp_addr = GetMemory(&ro_mapping);
+  void* temp_addr = ro_mapping.memory();
   MachVMRegionResult result = GetBasicInfo(
       mach_task_self(), &dummy_size,
       reinterpret_cast<mach_vm_address_t*>(&temp_addr), &basic_info);

base/memory/read_only_shared_memory_region.cc

@@ -88,8 +88,10 @@ bool ReadOnlySharedMemoryRegion::IsValid() const {
 ReadOnlySharedMemoryRegion::ReadOnlySharedMemoryRegion(
     subtle::PlatformSharedMemoryRegion handle)
     : handle_(std::move(handle)) {
-  CHECK_EQ(handle_.GetMode(),
-           subtle::PlatformSharedMemoryRegion::Mode::kReadOnly);
+  if (handle_.IsValid()) {
+    CHECK_EQ(handle_.GetMode(),
+             subtle::PlatformSharedMemoryRegion::Mode::kReadOnly);
+  }
 }
 
 }  // namespace base

base/memory/shared_memory_mapping.h

@@ -14,7 +14,6 @@ namespace base {
 
 namespace subtle {
 class PlatformSharedMemoryRegion;
-class PlatformSharedMemoryRegionTest;
 }  // namespace subtle
 
 // Base class for scoped handles to a shared memory mapping created from a
@@ -70,7 +69,6 @@ class BASE_EXPORT SharedMemoryMapping {
   void* raw_memory_ptr() const { return memory_; }
 
  private:
-  friend class subtle::PlatformSharedMemoryRegionTest;
   friend class SharedMemoryTracker;
 
   void Unmap();
@@ -126,7 +124,10 @@ class BASE_EXPORT WritableSharedMemoryMapping : public SharedMemoryMapping {
   void* memory() const { return raw_memory_ptr(); }
 
  private:
-  friend class subtle::PlatformSharedMemoryRegion;
+  friend WritableSharedMemoryMapping MapAtForTesting(
+      subtle::PlatformSharedMemoryRegion* region,
+      off_t offset,
+      size_t size);
   friend class ReadOnlySharedMemoryRegion;
   friend class WritableSharedMemoryRegion;
   friend class UnsafeSharedMemoryRegion;

base/memory/shared_memory_region_unittest.cc

@@ -32,36 +32,19 @@ template <typename SharedMemoryRegionType>
 class SharedMemoryRegionTest : public ::testing::Test {
  public:
   void SetUp() override {
-    std::tie(region_, rw_mapping_) = CreateWithMapping(kRegionSize);
+    std::tie(region_, rw_mapping_) =
+        CreateMappedRegion<SharedMemoryRegionType>(kRegionSize);
     ASSERT_TRUE(region_.IsValid());
     ASSERT_TRUE(rw_mapping_.IsValid());
     memset(rw_mapping_.memory(), 'G', kRegionSize);
     EXPECT_TRUE(IsMemoryFilledWithByte(rw_mapping_.memory(), kRegionSize, 'G'));
   }
 
-  static std::pair<SharedMemoryRegionType, WritableSharedMemoryMapping>
-  CreateWithMapping(size_t size) {
-    SharedMemoryRegionType region = SharedMemoryRegionType::Create(size);
-    WritableSharedMemoryMapping mapping = region.Map();
-    return {std::move(region), std::move(mapping)};
-  }
-
  protected:
   SharedMemoryRegionType region_;
   WritableSharedMemoryMapping rw_mapping_;
 };
 
-// Template specialization of SharedMemoryRegionTest<>::CreateWithMapping() for
-// the ReadOnlySharedMemoryRegion. We need this because
-// ReadOnlySharedMemoryRegion::Create() has a different return type.
-template <>
-std::pair<ReadOnlySharedMemoryRegion, WritableSharedMemoryMapping>
-SharedMemoryRegionTest<ReadOnlySharedMemoryRegion>::CreateWithMapping(
-    size_t size) {
-  MappedReadOnlyRegion mapped_region = ReadOnlySharedMemoryRegion::Create(size);
-  return {std::move(mapped_region.region), std::move(mapped_region.mapping)};
-}
-
 typedef ::testing::Types<WritableSharedMemoryRegion,
                          UnsafeSharedMemoryRegion,
                          ReadOnlySharedMemoryRegion>
@@ -176,7 +159,7 @@ TYPED_TEST(SharedMemoryRegionTest, MapAt) {
 
   TypeParam region;
   WritableSharedMemoryMapping rw_mapping;
-  std::tie(region, rw_mapping) = TestFixture::CreateWithMapping(kDataSize);
+  std::tie(region, rw_mapping) = CreateMappedRegion<TypeParam>(kDataSize);
   ASSERT_TRUE(region.IsValid());
   ASSERT_TRUE(rw_mapping.IsValid());
   uint32_t* ptr = static_cast<uint32_t*>(rw_mapping.memory());
@@ -202,7 +185,7 @@ TYPED_TEST(SharedMemoryRegionTest, MapAtNotAlignedOffsetFails) {
 
   TypeParam region;
   WritableSharedMemoryMapping rw_mapping;
-  std::tie(region, rw_mapping) = TestFixture::CreateWithMapping(kDataSize);
+  std::tie(region, rw_mapping) = CreateMappedRegion<TypeParam>(kDataSize);
   ASSERT_TRUE(region.IsValid());
   ASSERT_TRUE(rw_mapping.IsValid());
   off_t offset = kDataSize / 2;

base/memory/unsafe_shared_memory_region.cc

@@ -67,8 +67,10 @@ bool UnsafeSharedMemoryRegion::IsValid() const {
 UnsafeSharedMemoryRegion::UnsafeSharedMemoryRegion(
     subtle::PlatformSharedMemoryRegion handle)
     : handle_(std::move(handle)) {
-  CHECK_EQ(handle_.GetMode(),
-           subtle::PlatformSharedMemoryRegion::Mode::kUnsafe);
+  if (handle_.IsValid()) {
+    CHECK_EQ(handle_.GetMode(),
+             subtle::PlatformSharedMemoryRegion::Mode::kUnsafe);
+  }
 }
 
 }  // namespace base

base/memory/writable_shared_memory_region.cc

@@ -75,8 +75,10 @@ bool WritableSharedMemoryRegion::IsValid() const {
 WritableSharedMemoryRegion::WritableSharedMemoryRegion(
     subtle::PlatformSharedMemoryRegion handle)
     : handle_(std::move(handle)) {
-  CHECK_EQ(handle_.GetMode(),
-           subtle::PlatformSharedMemoryRegion::Mode::kWritable);
+  if (handle_.IsValid()) {
+    CHECK_EQ(handle_.GetMode(),
+             subtle::PlatformSharedMemoryRegion::Mode::kWritable);
+  }
 }
 
 }  // namespace base

base/test/test_shared_memory_util.cc

@@ -159,4 +159,29 @@ bool CheckReadOnlyPlatformSharedMemoryRegionForTesting(
 
 #endif  // !OS_NACL
 
+WritableSharedMemoryMapping MapForTesting(
+    subtle::PlatformSharedMemoryRegion* region) {
+  return MapAtForTesting(region, 0, region->GetSize());
+}
+
+WritableSharedMemoryMapping MapAtForTesting(
+    subtle::PlatformSharedMemoryRegion* region,
+    off_t offset,
+    size_t size) {
+  void* memory = nullptr;
+  size_t mapped_size = 0;
+  if (!region->MapAt(offset, size, &memory, &mapped_size))
+    return {};
+
+  return WritableSharedMemoryMapping(memory, size, mapped_size,
+                                     region->GetGUID());
+}
+
+template <>
+std::pair<ReadOnlySharedMemoryRegion, WritableSharedMemoryMapping>
+CreateMappedRegion(size_t size) {
+  MappedReadOnlyRegion mapped_region = ReadOnlySharedMemoryRegion::Create(size);
+  return {std::move(mapped_region.region), std::move(mapped_region.mapping)};
+}
+
 }  // namespace base

base/test/test_shared_memory_util.h

@@ -6,7 +6,10 @@
 #define BASE_TEST_TEST_SHARED_MEMORY_UTIL_H_
 
 #include "base/memory/platform_shared_memory_region.h"
+#include "base/memory/read_only_shared_memory_region.h"
 #include "base/memory/shared_memory_handle.h"
+#include "base/memory/shared_memory_mapping.h"
+#include "testing/gtest/include/gtest/gtest.h"
 
 namespace base {
 
@@ -19,6 +22,35 @@ bool CheckReadOnlySharedMemoryHandleForTesting(SharedMemoryHandle handle);
 bool CheckReadOnlyPlatformSharedMemoryRegionForTesting(
     subtle::PlatformSharedMemoryRegion region);
 
+// Creates a scoped mapping from a PlatformSharedMemoryRegion. It's useful for
+// PlatformSharedMemoryRegion testing to not leak mapped memory.
+// WritableSharedMemoryMapping is used for wrapping because it has max
+// capabilities but the actual permission depends on the |region|'s mode.
+// This must not be used in production where PlatformSharedMemoryRegion should
+// be wrapped with {Writable,Unsafe,ReadOnly}SharedMemoryRegion.
+WritableSharedMemoryMapping MapAtForTesting(
+    subtle::PlatformSharedMemoryRegion* region,
+    off_t offset,
+    size_t size);
+
+WritableSharedMemoryMapping MapForTesting(
+    subtle::PlatformSharedMemoryRegion* region);
+
+template <typename SharedMemoryRegionType>
+std::pair<SharedMemoryRegionType, WritableSharedMemoryMapping>
+CreateMappedRegion(size_t size) {
+  SharedMemoryRegionType region = SharedMemoryRegionType::Create(size);
+  WritableSharedMemoryMapping mapping = region.Map();
+  return {std::move(region), std::move(mapping)};
+}
+
+// Template specialization of CreateMappedRegion<>() for
+// the ReadOnlySharedMemoryRegion. We need this because
+// ReadOnlySharedMemoryRegion::Create() has a different return type.
+template <>
+std::pair<ReadOnlySharedMemoryRegion, WritableSharedMemoryMapping>
+CreateMappedRegion(size_t size);
+
 }  // namespace base
 
 #endif  // BASE_TEST_TEST_SHARED_MEMORY_UTIL_H_