[PartitionAlloc] Don't call SetSystemPageAccess twice

Call to De/RecommitSystemPages is almost always accompanied by
SetSystemPageAccess. On Windows and Fuchsia, literally all
De/RecommitSystemPages do is call SetSystemPageAccess, leading to
a redundant double call. This CLs unifies this behavior, so that only
one function has to be called.

Apart from the double call evasion, there are other behavior changes:
- gin::PageAllocator::SetPermissions, on Posix, will now make pages
  inaccessible when kNoAccess requested
- AddressPoolManager::Alloc, on Windows, will now CHECK instead
  returning false
- PagePool::Take, on Posix, will now CHECK (instead of returning
  nullptr) if failed to set permissions, and on Windows, CHECK is moved
  down the stack
- PartitionBucket::SlowPathAlloc -> PartitionRoot::RecommitSystemPages,
  on Windows, CHECK is moved down the stack
- NormalPageArena::AllocatePage & PageMemory::Allocate, on Windows
  & Posix, CHECK is moved down the stack

Bug: 766882
Change-Id: I961200449abc5271c9e0bed5ad6c4544468169b8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2550119Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Commit-Queue: Bartek Nowierski <bartekn@chromium.org>
Auto-Submit: Bartek Nowierski <bartekn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#831192}

base/allocator/partition_allocator/address_pool_manager.cc

@@ -39,6 +39,7 @@ AddressPoolManager* AddressPoolManager::GetInstance() {
 
 namespace {
 
+// This will crash if the range cannot be decommitted.
 void DecommitPages(void* address, size_t size) {
 #if defined(OS_APPLE)
   // MAP_FIXED replaces an existing mapping with a new one, when the address is
@@ -50,21 +51,17 @@ void DecommitPages(void* address, size_t size) {
                    MAP_FIXED | MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
   PA_CHECK(ptr == address);
 #else
-  SetSystemPagesAccess(address, size, PageInaccessible);
-  DecommitSystemPages(address, size);
+  DecommitSystemPages(address, size, PageUpdatePermissions);
 #endif
 }
 
-bool WARN_UNUSED_RESULT CommitPages(void* address, size_t size) {
+// This will crash if the range cannot be committed.
+void CommitPages(void* address, size_t size) {
 #if defined(OS_APPLE)
   SetSystemPagesAccess(address, size, PageReadWrite);
 #else
-  if (!RecommitSystemPages(address, size, PageReadWrite))
-    return false;
-  SetSystemPagesAccess(address, size, PageReadWrite);
+  RecommitSystemPages(address, size, PageReadWrite, PageUpdatePermissions);
 #endif
-
-  return true;
 }
 
 }  // namespace
@@ -99,9 +96,9 @@ void AddressPoolManager::Remove(pool_handle handle) {
 char* AddressPoolManager::Alloc(pool_handle handle, void*, size_t length) {
   Pool* pool = GetPool(handle);
   char* ptr = reinterpret_cast<char*>(pool->FindChunk(length));
-
-  if (UNLIKELY(!ptr) || !CommitPages(ptr, length))
+  if (UNLIKELY(!ptr))
     return nullptr;
+  CommitPages(ptr, length);
   return ptr;
 }
 

base/allocator/partition_allocator/page_allocator.cc

@@ -206,17 +206,25 @@ void SetSystemPagesAccess(void* address,
   SetSystemPagesAccessInternal(address, length, accessibility);
 }
 
-void DecommitSystemPages(void* address, size_t length) {
+void DecommitSystemPages(
+    void* address,
+    size_t length,
+    PageAccessibilityDisposition accessibility_disposition) {
+  PA_DCHECK(!(reinterpret_cast<uintptr_t>(address) & SystemPageOffsetMask()));
   PA_DCHECK(!(length & SystemPageOffsetMask()));
-  DecommitSystemPagesInternal(address, length);
+  DecommitSystemPagesInternal(address, length, accessibility_disposition);
 }
 
-bool RecommitSystemPages(void* address,
-                         size_t length,
-                         PageAccessibilityConfiguration accessibility) {
+void RecommitSystemPages(
+    void* address,
+    size_t length,
+    PageAccessibilityConfiguration accessibility,
+    PageAccessibilityDisposition accessibility_disposition) {
+  PA_DCHECK(!(reinterpret_cast<uintptr_t>(address) & SystemPageOffsetMask()));
   PA_DCHECK(!(length & SystemPageOffsetMask()));
   PA_DCHECK(accessibility != PageInaccessible);
-  return RecommitSystemPagesInternal(address, length, accessibility);
+  RecommitSystemPagesInternal(address, length, accessibility,
+                              accessibility_disposition);
 }
 
 void DiscardSystemPages(void* address, size_t length) {

base/allocator/partition_allocator/page_allocator.h

@@ -26,6 +26,15 @@ enum PageAccessibilityConfiguration {
   PageReadWriteExecute,
 };
 
+// Use for De/RecommitSystemPages API.
+enum PageAccessibilityDisposition {
+  // Enforces permission update (Decommit will set to PageInaccessible;
+  // Recommit will set to whatever was requested, other than PageInaccessible).
+  PageUpdatePermissions,
+  // Will not update permissions, if the platform supports that (POSIX only).
+  PageKeepPermissionsIfPossible,
+};
+
 // macOS supports tagged memory regions, to help in debugging. On Android,
 // these tags are used to name anonymous mappings.
 enum class PageTag {
@@ -91,41 +100,62 @@ BASE_EXPORT void SetSystemPagesAccess(
     PageAccessibilityConfiguration page_accessibility);
 
 // Decommit one or more system pages starting at |address| and continuing for
-// |length| bytes. |length| must be a multiple of |SystemPageSize()|.
+// |length| bytes. |address| and |length| must be aligned to a system page
+// boundary.
+//
+// |accessibility_disposition| allows to specify whether the pages should be
+// made inaccessible (PageUpdatePermissions), or left as is
+// (PageKeepPermissionsIfPossible, POSIX only). The latter should only be used
+// as an optimization if you really know what you're doing.
+// TODO(bartekn): Ideally, all callers should use PageUpdatePermissions,
+// for better security, but that may lead to a perf regression. Tracked at
+// http://crbug.com/766882.
 //
 // Decommitted means that physical resources (RAM or swap) backing the allocated
 // virtual address range are released back to the system, but the address space
 // is still allocated to the process (possibly using up page table entries or
-// other accounting resources). Any access to a decommitted region of memory
-// is an error and will generate a fault.
+// other accounting resources). Unless PageKeepPermissionsIfPossible disposition
+// is used, any access to a decommitted region of memory is an error and will
+// generate a fault.
 //
 // This operation is not atomic on all platforms.
 //
 // Note: "Committed memory" is a Windows Memory Subsystem concept that ensures
 // processes will not fault when touching a committed memory region. There is
 // no analogue in the POSIX memory API where virtual memory pages are
-// best-effort allocated resources on the first touch. To create a
-// platform-agnostic abstraction, this API simulates the Windows "decommit"
-// state by both discarding the region (allowing the OS to avoid swap
-// operations) and changing the page protections so accesses fault.
-//
-// TODO(ajwong): This currently does not change page protections on POSIX
-// systems due to a perf regression. Tracked at http://crbug.com/766882.
-BASE_EXPORT void DecommitSystemPages(void* address, size_t length);
+// best-effort allocated resources on the first touch. If PageUpdatePermissions
+// disposition is used, this API behaves in a platform-agnostic way by
+// simulating the Windows "decommit" state by both discarding the region
+// (allowing the OS to avoid swap operations) *and* changing the page
+// protections so accesses fault.
+//
+// This API will crash if the operation cannot be performed.
+BASE_EXPORT void DecommitSystemPages(
+    void* address,
+    size_t length,
+    PageAccessibilityDisposition accessibility_disposition);
 
 // Recommit one or more system pages, starting at |address| and continuing for
-// |length| bytes with the given |page_accessibility|. |length| must be a
-// multiple of |SystemPageSize()|.
+// |length| bytes with the given |page_accessibility| (must not be
+// PageInaccsessible). |address| and |length| must be aligned to a system page
+// boundary.
+//
+// |accessibility_disposition| allows to specify whether the page permissions
+// should be set to |page_accessibility| (PageUpdatePermissions), or left as is
+// (PageKeepPermissionsIfPossible, POSIX only). The latter can only be used if
+// the pages were previously accessible and decommitted with
+// PageKeepPermissionsIfPossible. It is ok, however, to recommit with
+// PageUpdatePermissions even if pages were decommitted with
+// PageKeepPermissionsIfPossible (merely losing an optimization).
 //
-// Decommitted system pages must be recommitted with their original permissions
-// before they are used again.
+// This operation is not atomic on all platforms.
 //
-// Returns true if the recommit change succeeded. In most cases you must |CHECK|
-// the result.
-BASE_EXPORT WARN_UNUSED_RESULT bool RecommitSystemPages(
+// This API will crash if the operation cannot be performed.
+BASE_EXPORT void RecommitSystemPages(
     void* address,
     size_t length,
-    PageAccessibilityConfiguration page_accessibility);
+    PageAccessibilityConfiguration page_accessibility,
+    PageAccessibilityDisposition accessibility_disposition);
 
 // Discard one or more system pages starting at |address| and continuing for
 // |length| bytes. |length| must be a multiple of |SystemPageSize()|.

base/allocator/partition_allocator/page_allocator_internals_fuchsia.h

@@ -180,20 +180,30 @@ void DiscardSystemPagesInternal(void* address, size_t length) {
   ZX_CHECK(status == ZX_OK, status);
 }
 
-void DecommitSystemPagesInternal(void* address, size_t length) {
+void DecommitSystemPagesInternal(
+    void* address,
+    size_t length,
+    PageAccessibilityDisposition accessibility_disposition) {
+  // TODO(bartekn): Ignoring accessibility_disposition preserves the behavior
+  // the API had since its conception, but consider similar optimization to
+  // POSIX.
+  SetSystemPagesAccessInternal(address, length, PageInaccessible);
+
   // TODO(https://crbug.com/1022062): Review whether this implementation is
   // still appropriate once DiscardSystemPagesInternal() migrates to a "lazy"
   // discardable API.
   DiscardSystemPagesInternal(address, length);
-
-  SetSystemPagesAccessInternal(address, length, PageInaccessible);
 }
 
-bool RecommitSystemPagesInternal(void* address,
-                                 size_t length,
-                                 PageAccessibilityConfiguration accessibility) {
+void RecommitSystemPagesInternal(
+    void* address,
+    size_t length,
+    PageAccessibilityConfiguration accessibility,
+    PageAccessibilityDisposition accessibility_disposition) {
+  // TODO(bartekn): Ignoring accessibility_disposition preserves the behavior
+  // the API had since its conception, but consider similar optimization to
+  // POSIX.
   SetSystemPagesAccessInternal(address, length, accessibility);
-  return true;
 }
 
 }  // namespace base

base/allocator/partition_allocator/page_allocator_internals_posix.h

@@ -243,21 +243,25 @@ void* TrimMappingInternal(void* base,
   return ret;
 }
 
-void DecommitSystemPagesInternal(void* address, size_t length) {
+void DecommitSystemPagesInternal(
+    void* address,
+    size_t length,
+    PageAccessibilityDisposition accessibility_disposition) {
+  if (accessibility_disposition == PageUpdatePermissions) {
+    SetSystemPagesAccess(address, length, PageInaccessible);
+  }
+
   // In POSIX, there is no decommit concept. Discarding is an effective way of
   // implementing the Windows semantics where the OS is allowed to not swap the
   // pages in the region.
-  //
-  // TODO(ajwong): Also explore setting PageInaccessible to make the protection
-  // semantics consistent between Windows and POSIX. This might have a perf cost
-  // though as both decommit and recommit would incur an extra syscall.
-  // http://crbug.com/766882
   DiscardSystemPages(address, length);
 }
 
-bool RecommitSystemPagesInternal(void* address,
-                                 size_t length,
-                                 PageAccessibilityConfiguration accessibility) {
+void RecommitSystemPagesInternal(
+    void* address,
+    size_t length,
+    PageAccessibilityConfiguration accessibility,
+    PageAccessibilityDisposition accessibility_disposition) {
 #if defined(OS_APPLE)
   // On macOS, to update accounting, we need to make another syscall. For more
   // details, see https://crbug.com/823915.
@@ -267,7 +271,11 @@ bool RecommitSystemPagesInternal(void* address,
   // On POSIX systems, the caller need simply read the memory to recommit it.
   // This has the correct behavior because the API requires the permissions to
   // be the same as before decommitting and all configurations can read.
-  return true;
+  // However, if decommit changed the permissions, recommit has to change them
+  // back.
+  if (accessibility_disposition == PageUpdatePermissions) {
+    SetSystemPagesAccess(address, length, accessibility);
+  }
 }
 
 void DiscardSystemPagesInternal(void* address, size_t length) {

base/allocator/partition_allocator/page_allocator_internals_win.h

@@ -103,14 +103,23 @@ void FreePagesInternal(void* address, size_t length) {
   PA_CHECK(VirtualFree(address, 0, MEM_RELEASE));
 }
 
-void DecommitSystemPagesInternal(void* address, size_t length) {
+void DecommitSystemPagesInternal(
+    void* address,
+    size_t length,
+    PageAccessibilityDisposition accessibility_disposition) {
+  // Ignore accessibility_disposition, because decommitting is equivalent to
+  // making pages inaccessible.
   SetSystemPagesAccess(address, length, PageInaccessible);
 }
 
-bool RecommitSystemPagesInternal(void* address,
-                                 size_t length,
-                                 PageAccessibilityConfiguration accessibility) {
-  return TrySetSystemPagesAccess(address, length, accessibility);
+void RecommitSystemPagesInternal(
+    void* address,
+    size_t length,
+    PageAccessibilityConfiguration accessibility,
+    PageAccessibilityDisposition accessibility_disposition) {
+  // Ignore accessibility_disposition, because decommitting is equivalent to
+  // making pages inaccessible.
+  SetSystemPagesAccess(address, length, accessibility);
 }
 
 void DiscardSystemPagesInternal(void* address, size_t length) {

base/allocator/partition_allocator/page_allocator_unittest.cc

@@ -265,8 +265,9 @@ TEST(PageAllocatorTest, DecommitErasesMemory) {
 
   memset(buffer, 42, size);
 
-  DecommitSystemPages(buffer, size);
-  EXPECT_TRUE(RecommitSystemPages(buffer, size, PageReadWrite));
+  DecommitSystemPages(buffer, size, PageKeepPermissionsIfPossible);
+  RecommitSystemPages(buffer, size, PageReadWrite,
+                      PageKeepPermissionsIfPossible);
 
   uint8_t* recommitted_buffer = reinterpret_cast<uint8_t*>(buffer);
   uint32_t sum = 0;
@@ -290,7 +291,7 @@ TEST(PageAllocatorTest, MappedPagesAccounting) {
 
   EXPECT_EQ(mapped_size_before + size, GetTotalMappedSize());
 
-  DecommitSystemPages(data, size);
+  DecommitSystemPages(data, size, PageKeepPermissionsIfPossible);
   EXPECT_EQ(mapped_size_before + size, GetTotalMappedSize());
 
   FreePages(data, size);

base/allocator/partition_allocator/partition_bucket.cc

@@ -606,7 +606,8 @@ void* PartitionBucket<thread_safe>::SlowPathAlloc(
       decommitted_slot_spans_head = new_slot_span->next_slot_span;
       void* addr = SlotSpanMetadata<thread_safe>::ToPointer(new_slot_span);
       root->RecommitSystemPages(addr,
-                                new_slot_span->bucket->get_bytes_per_span());
+                                new_slot_span->bucket->get_bytes_per_span(),
+                                PageKeepPermissionsIfPossible);
       new_slot_span->Reset();
       *is_already_zeroed = kDecommittedPagesAreAlwaysZeroed;
     }

base/allocator/partition_allocator/partition_page.cc

@@ -5,6 +5,7 @@
 #include "base/allocator/partition_allocator/partition_page.h"
 
 #include "base/allocator/partition_allocator/address_pool_manager.h"
+#include "base/allocator/partition_allocator/page_allocator.h"
 #include "base/allocator/partition_allocator/partition_address_space.h"
 #include "base/allocator/partition_allocator/partition_alloc.h"
 #include "base/allocator/partition_allocator/partition_alloc_check.h"
@@ -165,7 +166,8 @@ void SlotSpanMetadata<thread_safe>::Decommit(PartitionRoot<thread_safe>* root) {
   PA_DCHECK(is_empty());
   PA_DCHECK(!bucket->is_direct_mapped());
   void* addr = SlotSpanMetadata::ToPointer(this);
-  root->DecommitSystemPages(addr, bucket->get_bytes_per_span());
+  root->DecommitSystemPages(addr, bucket->get_bytes_per_span(),
+                            PageKeepPermissionsIfPossible);
 
   // We actually leave the decommitted slot span in the active list. We'll sweep
   // it on to the decommitted list when we next walk the active list.

base/allocator/partition_allocator/partition_root.cc

@@ -5,6 +5,7 @@
 #include "base/allocator/partition_allocator/partition_root.h"
 
 #include "base/allocator/partition_allocator/oom.h"
+#include "base/allocator/partition_allocator/page_allocator.h"
 #include "base/allocator/partition_allocator/partition_alloc_check.h"
 #include "base/allocator/partition_allocator/partition_bucket.h"
 #include "base/allocator/partition_allocator/partition_cookie.h"
@@ -492,18 +493,15 @@ bool PartitionRoot<thread_safe>::ReallocDirectMappedInPlace(
 
     // Shrink by decommitting unneeded pages and making them inaccessible.
     size_t decommit_size = current_slot_size - new_slot_size;
-    DecommitSystemPages(char_ptr + new_slot_size, decommit_size);
-    SetSystemPagesAccess(char_ptr + new_slot_size, decommit_size,
-                         PageInaccessible);
+    DecommitSystemPages(char_ptr + new_slot_size, decommit_size,
+                        PageUpdatePermissions);
   } else if (new_slot_size <=
              DirectMapExtent::FromSlotSpan(slot_span)->map_size) {
     // Grow within the actually allocated memory. Just need to make the
     // pages accessible again.
     size_t recommit_slot_size_growth = new_slot_size - current_slot_size;
-    SetSystemPagesAccess(char_ptr + current_slot_size,
-                         recommit_slot_size_growth, PageReadWrite);
-    RecommitSystemPages(char_ptr + current_slot_size,
-                        recommit_slot_size_growth);
+    RecommitSystemPages(char_ptr + current_slot_size, recommit_slot_size_growth,
+                        PageUpdatePermissions);
 
 #if DCHECK_IS_ON()
     memset(char_ptr + current_slot_size, kUninitializedByte,

base/allocator/partition_allocator/partition_root.h

@@ -212,9 +212,15 @@ struct BASE_EXPORT PartitionRoot {
 
   ALWAYS_INLINE void IncreaseCommittedPages(size_t len);
   ALWAYS_INLINE void DecreaseCommittedPages(size_t len);
-  ALWAYS_INLINE void DecommitSystemPages(void* address, size_t length)
+  ALWAYS_INLINE void DecommitSystemPages(
+      void* address,
+      size_t length,
+      PageAccessibilityDisposition accessibility_disposition)
       EXCLUSIVE_LOCKS_REQUIRED(lock_);
-  ALWAYS_INLINE void RecommitSystemPages(void* address, size_t length)
+  ALWAYS_INLINE void RecommitSystemPages(
+      void* address,
+      size_t length,
+      PageAccessibilityDisposition accessibility_disposition)
       EXCLUSIVE_LOCKS_REQUIRED(lock_);
 
   NOINLINE void OutOfMemory(size_t size);
@@ -770,16 +776,19 @@ ALWAYS_INLINE void PartitionRoot<thread_safe>::DecreaseCommittedPages(
 template <bool thread_safe>
 ALWAYS_INLINE void PartitionRoot<thread_safe>::DecommitSystemPages(
     void* address,
-    size_t length) {
-  ::base::DecommitSystemPages(address, length);
+    size_t length,
+    PageAccessibilityDisposition accessibility_disposition) {
+  ::base::DecommitSystemPages(address, length, accessibility_disposition);
   DecreaseCommittedPages(length);
 }
 
 template <bool thread_safe>
 ALWAYS_INLINE void PartitionRoot<thread_safe>::RecommitSystemPages(
     void* address,
-    size_t length) {
-  PA_CHECK(::base::RecommitSystemPages(address, length, PageReadWrite));
+    size_t length,
+    PageAccessibilityDisposition accessibility_disposition) {
+  ::base::RecommitSystemPages(address, length, PageReadWrite,
+                              accessibility_disposition);
   IncreaseCommittedPages(length);
 }
 

gin/v8_platform.cc

@@ -245,7 +245,8 @@ class PageAllocator : public v8::PageAllocator {
     // On Windows, we can only de-commit the trailing pages. FreePages() will
     // still free all pages in the region including the released tail, so it's
     // safe to just decommit the tail.
-    base::DecommitSystemPages(release_base, release_size);
+    base::DecommitSystemPages(release_base, release_size,
+                              base::PageUpdatePermissions);
 #else
 #error Unsupported platform
 #endif
@@ -257,7 +258,7 @@ class PageAllocator : public v8::PageAllocator {
                       Permission permissions) override {
     // If V8 sets permissions to none, we can discard the memory.
     if (permissions == v8::PageAllocator::Permission::kNoAccess) {
-      base::DecommitSystemPages(address, length);
+      base::DecommitSystemPages(address, length, base::PageUpdatePermissions);
       return true;
     } else {
       return base::TrySetSystemPagesAccess(address, length,

third_party/blink/renderer/platform/heap/impl/gc_info.cc

@@ -108,13 +108,8 @@ void GCInfoTable::Resize() {
   const size_t table_size_delta = new_committed_size - old_committed_size;
 
   // Commit the new size and allow read/write.
-  // TODO(ajwong): SetSystemPagesAccess should be part of RecommitSystemPages to
-  // avoid having two calls here.
-  base::SetSystemPagesAccess(current_table_end, table_size_delta,
-                             base::PageReadWrite);
-  bool ok = base::RecommitSystemPages(current_table_end, table_size_delta,
-                                      base::PageReadWrite);
-  CHECK(ok);
+  base::RecommitSystemPages(current_table_end, table_size_delta,
+                            base::PageReadWrite, base::PageUpdatePermissions);
 
 #if DCHECK_IS_ON()
   // Check that newly-committed memory is zero-initialized.

third_party/blink/renderer/platform/heap/impl/heap_page.cc

@@ -673,13 +673,12 @@ void NormalPageArena::AllocatePage() {
       // Take the first possible page ensuring that this thread actually
       // gets a page and add the rest to the page pool.
       if (!page_memory) {
-        bool result = memory->Commit();
-        // If you hit the CHECK, it will mean that you're hitting the limit
-        // of the number of mmapped regions the OS can support
+        // If you hit the CHECK in the call to Commit(), it means that you're
+        // hitting the limit of the number of mmapped regions the OS can support
         // (e.g., /proc/sys/vm/max_map_count in Linux) or on that Windows you
         // have exceeded the max commit charge across all processes for the
         // system.
-        CHECK(result);
+        memory->Commit();
         page_memory = memory;
       } else {
         GetThreadState()->Heap().GetFreePagePool()->Add(ArenaIndex(), memory);

third_party/blink/renderer/platform/heap/impl/page_memory.cc

@@ -16,15 +16,14 @@ void MemoryRegion::Release() {
   base::FreePages(base_, size_);
 }
 
-bool MemoryRegion::Commit() {
-  CHECK(base::RecommitSystemPages(base_, size_, base::PageReadWrite));
-  return base::TrySetSystemPagesAccess(base_, size_, base::PageReadWrite);
+void MemoryRegion::Commit() {
+  base::RecommitSystemPages(base_, size_, base::PageReadWrite,
+                            base::PageUpdatePermissions);
 }
 
 void MemoryRegion::Decommit() {
   ASAN_UNPOISON_MEMORY_REGION(base_, size_);
-  base::DecommitSystemPages(base_, size_);
-  base::SetSystemPagesAccess(base_, size_, base::PageInaccessible);
+  base::DecommitSystemPages(base_, size_, base::PageUpdatePermissions);
 }
 
 PageMemoryRegion::PageMemoryRegion(Address base,
@@ -130,7 +129,7 @@ PageMemory* PageMemory::Allocate(size_t payload_size, RegionTree* region_tree) {
       PageMemoryRegion::AllocateLargePage(allocation_size, region_tree);
   PageMemory* storage =
       SetupPageMemoryInRegion(page_memory_region, 0, payload_size);
-  CHECK(storage->Commit());
+  storage->Commit();
   return storage;
 }
 

third_party/blink/renderer/platform/heap/impl/page_memory.h

@@ -33,7 +33,7 @@ class MemoryRegion {
   }
 
   void Release();
-  WARN_UNUSED_RESULT bool Commit();
+  void Commit();
   void Decommit();
 
   Address Base() const { return base_; }
@@ -144,9 +144,9 @@ class PageMemory {
     reserved_->PageDeleted(WritableStart());
   }
 
-  WARN_UNUSED_RESULT bool Commit() {
+  void Commit() {
     reserved_->MarkPageUsed(WritableStart());
-    return writable_.Commit();
+    writable_.Commit();
   }
 
   void Decommit() {

third_party/blink/renderer/platform/heap/impl/page_pool.cc

@@ -38,16 +38,13 @@ void PagePool::Add(int index, PageMemory* memory) {
 }
 
 PageMemory* PagePool::Take(int index) {
-  while (PoolEntry* entry = pool_[index]) {
+  if (PoolEntry* entry = pool_[index]) {
     pool_[index] = entry->next;
     PageMemory* memory = entry->data;
     DCHECK(memory);
     delete entry;
-    if (memory->Commit())
-      return memory;
-
-    // We got some memory, but failed to commit it, try again.
-    delete memory;
+    memory->Commit();
+    return memory;
   }
   return nullptr;
 }