From ecc628e6bee9335684f7400aeaa5e8db956d13e5 Mon Sep 17 00:00:00 2001
From: Olli Pettay <Olli.Pettay@helsinki.fi>
Date: Wed, 12 Mar 2025 10:04:03 +0000
Subject: [PATCH] Bug 1943811, avoid calling SegmentedVector::Length() in hot
 code paths, r=mccr8

Differential Revision: https://phabricator.services.mozilla.com/D240910
---
 dom/bindings/BindingUtils.h        |  7 +------
 mfbt/SegmentedVector.h             |  4 +---
 mfbt/tests/TestSegmentedVector.cpp | 17 +++++++++++++++++
 3 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/dom/bindings/BindingUtils.h b/dom/bindings/BindingUtils.h
index 006ab7168111..fec0be53dd3f 100644
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -3041,13 +3041,8 @@ struct DeferredFinalizerImpl {
   static bool DeferredFinalize(uint32_t aSlice, void* aData) {
     MOZ_ASSERT(aSlice > 0, "nonsensical/useless call with aSlice == 0");
     SmartPtrArray* pointers = static_cast<SmartPtrArray*>(aData);
-    uint32_t oldLen = pointers->Length();
-    if (oldLen < aSlice) {
-      aSlice = oldLen;
-    }
-    uint32_t newLen = oldLen - aSlice;
     pointers->PopLastN(aSlice);
-    if (newLen == 0) {
+    if (pointers->IsEmpty()) {
       delete pointers;
       return true;
     }
diff --git a/mfbt/SegmentedVector.h b/mfbt/SegmentedVector.h
index c22c3e8d1f53..0198d81e9e6d 100644
--- a/mfbt/SegmentedVector.h
+++ b/mfbt/SegmentedVector.h
@@ -224,10 +224,8 @@ class SegmentedVector : private AllocPolicy {
   }
 
   // Equivalent to calling |PopLast| |aNumElements| times, but potentially
-  // more efficient.
+  // more efficient. It is safe to call this even when aNumElements > Length().
   void PopLastN(uint32_t aNumElements) {
-    MOZ_ASSERT(aNumElements <= Length());
-
     Segment* last;
 
     // Pop full segments for as long as we can.  Note that this loop
diff --git a/mfbt/tests/TestSegmentedVector.cpp b/mfbt/tests/TestSegmentedVector.cpp
index dd569ea7b6dc..ef96d6c09a6f 100644
--- a/mfbt/tests/TestSegmentedVector.cpp
+++ b/mfbt/tests/TestSegmentedVector.cpp
@@ -110,6 +110,23 @@ void TestBasics() {
 
   // Verify the contents are what we expect.
   CheckContents(v, 700);
+
+  // Verify PopLastN can take larger than .Length() value as an argument.
+  v.PopLastN(1000);
+  MOZ_RELEASE_ASSERT(v.Length() == 0);
+  MOZ_RELEASE_ASSERT(v.IsEmpty());
+
+  // Fill the vector again.
+  for (i = 0; i < 1000; ++i) {
+    v.InfallibleAppend(std::move(i));
+  }
+  MOZ_RELEASE_ASSERT(!v.IsEmpty());
+  MOZ_RELEASE_ASSERT(v.Length() == 1000);
+
+  // Verify that calling PopLastN with Length() empties the vector.
+  v.PopLastN(v.Length());
+  MOZ_RELEASE_ASSERT(v.Length() == 0);
+  MOZ_RELEASE_ASSERT(v.IsEmpty());
 }
 
 void TestMoveAndSwap() {