Bug 1951925 - Fix base content memory usage regressions r=nika

This disarms a footgun in the FreezableMapping::Freeze API, which was
causing read-only mappings in the global stylesheet cache to fail. These
failures caused content processes to re-load the style sheets,
increasing memory usage.

Differential Revision: https://phabricator.services.mozilla.com/D240928

dom/ipc/MemMapSnapshot.cpp

@@ -32,7 +32,7 @@ Result<Ok, nsresult> MemMapSnapshot::Init(size_t aSize) {
 Result<ReadOnlySharedMemoryHandle, nsresult> MemMapSnapshot::Finalize() {
   MOZ_ASSERT(mMem);
 
-  auto [_, readOnlyHandle] = std::move(mMem).Freeze();
+  auto readOnlyHandle = std::move(mMem).Freeze();
   if (NS_WARN_IF(!readOnlyHandle)) {
     return Err(NS_ERROR_FAILURE);
   }

gfx/thebes/SharedFontList.cpp

@@ -804,7 +804,7 @@ bool FontList::AppendShmBlock(uint32_t aSizeNeeded) {
     MOZ_CRASH("failed to create shared memory");
     return false;
   }
-  auto [newShm, readOnly] = std::move(handle).Map().Freeze();
+  auto [readOnly, newShm] = std::move(handle).Map().FreezeWithMutableMapping();
   if (!newShm || !newShm.Address()) {
     MOZ_CRASH("failed to map shared memory");
     return false;

intl/hyphenation/glue/nsHyphenator.cpp

@@ -109,8 +109,7 @@ static ipc::ReadOnlySharedMemoryHandle CopyToShmem(const CompiledData* aData) {
   }
 
   memcpy(buffer, mapped_hyph_compiled_data_ptr(aData), size);
-  auto [_, readOnlyHandle] = std::move(map).Freeze();
+  return std::move(map).Freeze();
-  return std::move(readOnlyHandle);
 }
 
 static ipc::ReadOnlySharedMemoryHandle LoadFromURI(nsIURI* aURI,
@@ -161,13 +160,7 @@ static ipc::ReadOnlySharedMemoryHandle LoadFromURI(nsIURI* aURI,
       return nullptr;
     }
 
-    auto [_, readOnlyHandle] = std::move(map).Freeze();
+    return std::move(map).Freeze();
-
-    if (!readOnlyHandle) {
-      return nullptr;
-    }
-
-    return std::move(readOnlyHandle);
   }
 
   // Read from the URI into a temporary buffer, compile it, then copy the

ipc/glue/SharedMemoryMapping.cpp

@@ -164,10 +164,15 @@ FreezableMapping::Mapping(FreezableHandle&& aHandle, uint64_t aOffset,
   MapSubregion(mHandle, aOffset, aSize, aFixedAddress, false);
 }
 
-std::tuple<MutableMapping, ReadOnlyHandle> FreezableMapping::Freeze() && {
+ReadOnlyHandle FreezableMapping::Freeze() && {
+  return std::move(*this).Unmap().Freeze();
+}
+
+std::tuple<ReadOnlyHandle, MutableMapping>
+FreezableMapping::FreezeWithMutableMapping() && {
   auto handle = std::move(mHandle);
-  return std::make_tuple(ConvertMappingTo<Type::Mutable>(std::move(*this)),
+  return std::make_tuple(std::move(handle).Freeze(),
-                         std::move(handle).Freeze());
+                         ConvertMappingTo<Type::Mutable>(std::move(*this)));
 }
 
 FreezableHandle FreezableMapping::Unmap() && {

ipc/glue/SharedMemoryMapping.h

@@ -220,13 +220,18 @@ struct Mapping<Type::Freezable> : MappingData<false> {
   Mapping(FreezableHandle&& aHandle, uint64_t aOffset, size_t aSize,
           void* aFixedAddress = nullptr);
 
+  /**
+   * Freeze the shared memory region.
+   */
+  ReadOnlyHandle Freeze() &&;
+
   /**
    * Freeze the shared memory region.
    *
    * The returned Mapping will still be valid and writable until it is deleted,
    * however no new writable mappings can be created.
    */
-  std::tuple<MutableMapping, ReadOnlyHandle> Freeze() &&;
+  std::tuple<ReadOnlyHandle, MutableMapping> FreezeWithMutableMapping() &&;
 
   /**
    * Unmap the shared memory, returning the freezable handle.

ipc/gtest/TestSharedMemory.cpp

@@ -226,12 +226,22 @@ TEST(IPCSharedMemoryMapping, FreezableFreeze)
   auto handle = shared_memory::CreateFreezable(1);
 
   auto mapping = std::move(handle).Map();
-  auto [m, roHandle] = std::move(mapping).Freeze();
+  auto roHandle = std::move(mapping).Freeze();
   ASSERT_SHMEM(mapping, 0);
-  ASSERT_SHMEM(m, 1);
   ASSERT_SHMEM(roHandle, 1);
 }
 
+TEST(IPCSharedMemoryMapping, FreezableFreezeWithMutableMapping)
+{
+  auto handle = shared_memory::CreateFreezable(1);
+
+  auto mapping = std::move(handle).Map();
+  auto [roHandle, m] = std::move(mapping).FreezeWithMutableMapping();
+  ASSERT_SHMEM(mapping, 0);
+  ASSERT_SHMEM(roHandle, 1);
+  ASSERT_SHMEM(m, 1);
+}
+
 TEST(IPCSharedMemoryMapping, FreezableUnmap)
 {
   auto handle = shared_memory::CreateFreezable(1);
@@ -258,7 +268,7 @@ TEST(IPCSharedMemory, FreezeAndMapRW)
   *mem = 'A';
 
   // Freeze
-  auto [rwMapping, roHandle] = std::move(mapping).Freeze();
+  auto [roHandle, rwMapping] = std::move(mapping).FreezeWithMutableMapping();
   ASSERT_TRUE(rwMapping);
   ASSERT_TRUE(roHandle);
 
@@ -287,7 +297,7 @@ TEST(IPCSharedMemory, FreezeAndReprotect)
   *mem = 'A';
 
   // Freeze
-  auto [rwMapping, roHandle] = std::move(mapping).Freeze();
+  auto [roHandle, rwMapping] = std::move(mapping).FreezeWithMutableMapping();
   ASSERT_TRUE(rwMapping);
   ASSERT_TRUE(roHandle);
 
@@ -387,7 +397,8 @@ TEST(IPCSharedMemory, ROCopyAndWrite)
   auto handle = ipc::shared_memory::CreateFreezable(1);
   ASSERT_TRUE(handle);
 
-  auto [rwMapping, roHandle] = std::move(handle).Map().Freeze();
+  auto [roHandle, rwMapping] =
+      std::move(handle).Map().FreezeWithMutableMapping();
   ASSERT_TRUE(rwMapping);
   ASSERT_TRUE(roHandle);
 
@@ -412,7 +423,8 @@ TEST(IPCSharedMemory, ROCopyAndRewrite)
   auto handle = ipc::shared_memory::CreateFreezable(1);
   ASSERT_TRUE(handle);
 
-  auto [rwMapping, roHandle] = std::move(handle).Map().Freeze();
+  auto [roHandle, rwMapping] =
+      std::move(handle).Map().FreezeWithMutableMapping();
   ASSERT_TRUE(rwMapping);
   ASSERT_TRUE(roHandle);
 

js/xpconnect/src/XPCSelfHostedShmem.cpp

@@ -56,7 +56,7 @@ void xpc::SelfHostedShmem::InitFromParent(ContentType aXdr) {
   void* address = mapping.Address();
   memcpy(address, aXdr.Elements(), aXdr.LengthBytes());
 
-  std::tie(std::ignore, mHandle) = std::move(mapping).Freeze();
+  mHandle = std::move(mapping).Freeze();
   mMem = mHandle.Map();
 }
 

layout/style/GlobalStyleSheetCache.cpp

@@ -393,7 +393,7 @@ void GlobalStyleSheetCache::InitSharedSheetsInParent() {
 
   // Finished writing into the shared memory.  Freeze it, so that a process
   // can't confuse other processes by changing the UA style sheet contents.
-  auto [_, readOnlyHandle] = std::move(mapping).Freeze();
+  auto readOnlyHandle = std::move(mapping).Freeze();
   if (NS_WARN_IF(!readOnlyHandle)) {
     return;
   }