commit 201c1ed8687f67d647430dbf2c396f5c8b32ac86
parent e1408b070296d99c1be8b2992d350cf1291488bb
Author: Matthew Gaudet <mgaudet@mozilla.com>
Date: Wed, 1 Oct 2025 21:38:48 +0000
Bug 1983154 - Support JS::Value in TraceableFifo r=jonco
Differential Revision: https://phabricator.services.mozilla.com/D264130
Diffstat:
5 files changed, 344 insertions(+), 9 deletions(-)
diff --git a/js/public/GCVector.h b/js/public/GCVector.h
@@ -155,6 +155,12 @@ class GCVector {
void popBack() { return vector.popBack(); }
T popCopy() { return vector.popCopy(); }
+ void swap(GCVector& other) {
+ // Note, this only will work for MinInlineCapacity of zero.
+ // See Bug 1987683.
+ vector.swap(other.vector);
+ }
+
size_t sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const {
return vector.sizeOfExcludingThis(mallocSizeOf);
}
diff --git a/js/src/ds/TraceableFifo.h b/js/src/ds/TraceableFifo.h
@@ -8,6 +8,7 @@
#define js_TraceableFifo_h
#include "ds/Fifo.h"
+#include "js/GCVector.h"
#include "js/RootingAPI.h"
#include "js/TracingAPI.h"
@@ -30,8 +31,9 @@ namespace js {
// must either be used with Rooted, or barriered and traced manually.
template <typename T, size_t MinInlineCapacity = 0,
typename AllocPolicy = TempAllocPolicy>
-class TraceableFifo : public js::Fifo<T, MinInlineCapacity, AllocPolicy> {
- using Base = js::Fifo<T, MinInlineCapacity, AllocPolicy>;
+class TraceableFifo
+ : public js::Fifo<T, MinInlineCapacity, AllocPolicy, JS::GCVector> {
+ using Base = js::Fifo<T, MinInlineCapacity, AllocPolicy, JS::GCVector>;
public:
explicit TraceableFifo(AllocPolicy alloc = AllocPolicy())
@@ -44,12 +46,8 @@ class TraceableFifo : public js::Fifo<T, MinInlineCapacity, AllocPolicy> {
TraceableFifo& operator=(const TraceableFifo&) = delete;
void trace(JSTracer* trc) {
- for (size_t i = 0; i < this->front_.length(); ++i) {
- JS::GCPolicy<T>::trace(trc, &this->front_[i], "fifo element");
- }
- for (size_t i = 0; i < this->rear_.length(); ++i) {
- JS::GCPolicy<T>::trace(trc, &this->rear_[i], "fifo element");
- }
+ this->front_.trace(trc);
+ this->rear_.trace(trc);
}
};
diff --git a/js/src/jsapi-tests/moz.build b/js/src/jsapi-tests/moz.build
@@ -144,6 +144,7 @@ UNIFIED_SOURCES += [
"testThreadingMutex.cpp",
"testThreadingThread.cpp",
"testToSignedOrUnsignedInteger.cpp",
+ "testTraceableFifoValue.cpp",
"testTypedArrays.cpp",
"testUbiNode.cpp",
"testUncaughtSymbol.cpp",
diff --git a/js/src/jsapi-tests/testTraceableFifoValue.cpp b/js/src/jsapi-tests/testTraceableFifoValue.cpp
@@ -0,0 +1,329 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*-
+ * vim: set ts=8 sts=2 et sw=2 tw=80:
+ */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "ds/TraceableFifo.h"
+#include "js/PropertyAndElement.h" // JS_DefineProperty, JS_GetProperty
+#include "js/RootingAPI.h"
+
+#include "jsapi-tests/tests.h"
+
+using namespace js;
+
+BEGIN_TEST(testTraceableFifoValueBasic) {
+ JS::Rooted<TraceableFifo<JS::Value>> fifo(cx, TraceableFifo<JS::Value>(cx));
+
+ // Test empty state
+ CHECK(fifo.empty());
+ CHECK(fifo.length() == 0);
+
+ // Test pushBack with various JS::Value types
+ CHECK(fifo.pushBack(JS::UndefinedValue()));
+ CHECK(fifo.pushBack(JS::NullValue()));
+ CHECK(fifo.pushBack(JS::Int32Value(42)));
+ CHECK(fifo.pushBack(JS::BooleanValue(true)));
+ CHECK(fifo.pushBack(JS::DoubleValue(3.14)));
+
+ CHECK(!fifo.empty());
+ CHECK(fifo.length() == 5);
+
+ // Test FIFO behavior - first in, first out
+ CHECK(fifo.front().isUndefined());
+ fifo.popFront();
+
+ CHECK(fifo.front().isNull());
+ fifo.popFront();
+
+ CHECK(fifo.front().isInt32() && fifo.front().toInt32() == 42);
+ fifo.popFront();
+
+ CHECK(fifo.front().isBoolean() && fifo.front().toBoolean() == true);
+ fifo.popFront();
+
+ CHECK(fifo.front().isDouble() && fifo.front().toDouble() == 3.14);
+ fifo.popFront();
+
+ CHECK(fifo.empty());
+ CHECK(fifo.length() == 0);
+
+ return true;
+}
+END_TEST(testTraceableFifoValueBasic)
+
+BEGIN_TEST(testTraceableFifoValueGCSurvival) {
+ JS::Rooted<TraceableFifo<JS::Value>> fifo(cx, TraceableFifo<JS::Value>(cx));
+
+ // Create objects and add them to fifo
+ const size_t numObjects = 15;
+ for (size_t i = 0; i < numObjects; ++i) {
+ JS::RootedObject obj(cx, JS_NewPlainObject(cx));
+ CHECK(obj);
+
+ // Add a property to make objects identifiable
+ JS::RootedValue indexVal(cx, JS::NumberValue(static_cast<double>(i)));
+ CHECK(JS_DefineProperty(cx, obj, "testIndex", indexVal, 0));
+
+ JS::RootedValue objVal(cx, JS::ObjectValue(*obj));
+ CHECK(fifo.pushBack(objVal));
+ }
+
+ CHECK(fifo.length() == numObjects);
+
+ // Trigger multiple GC cycles to ensure objects are properly traced
+ JS_GC(cx);
+ JS_GC(cx);
+ JS_GC(cx);
+
+ // Verify all objects survived and have correct properties
+ for (size_t i = 0; i < numObjects; ++i) {
+ CHECK(!fifo.empty());
+ CHECK(fifo.front().isObject());
+
+ JS::RootedObject obj(cx, &fifo.front().toObject());
+ CHECK(obj);
+
+ JS::RootedValue indexVal(cx);
+ CHECK(JS_GetProperty(cx, obj, "testIndex", &indexVal));
+ CHECK(indexVal.isNumber() && indexVal.toNumber() == static_cast<double>(i));
+
+ fifo.popFront();
+ }
+
+ CHECK(fifo.empty());
+
+ return true;
+}
+END_TEST(testTraceableFifoValueGCSurvival)
+
+BEGIN_TEST(testTraceableFifoValueStrings) {
+ JS::Rooted<TraceableFifo<JS::Value>> fifo(cx, TraceableFifo<JS::Value>(cx));
+
+ // Test with various string types
+ const char* testStrings[] = {"hello",
+ "world",
+ "TraceableFifo",
+ "JavaScript",
+ "garbage collection",
+ "SpiderMonkey",
+ "test string with spaces"};
+
+ // Add strings to fifo
+ for (const char* str : testStrings) {
+ JS::RootedString jsStr(cx, JS_NewStringCopyZ(cx, str));
+ CHECK(jsStr);
+ JS::RootedValue strVal(cx, JS::StringValue(jsStr));
+ CHECK(fifo.pushBack(strVal));
+ }
+
+ CHECK(fifo.length() == std::size(testStrings));
+
+ // Trigger GC to ensure strings survive
+ JS_GC(cx);
+
+ // Verify strings in FIFO order
+ for (const char* expected : testStrings) {
+ CHECK(!fifo.empty());
+ CHECK(fifo.front().isString());
+
+ JS::RootedString str(cx, fifo.front().toString());
+ CHECK(str);
+
+ bool match = false;
+ CHECK(JS_StringEqualsAscii(cx, str, expected, strlen(expected), &match));
+ CHECK(match);
+
+ fifo.popFront();
+ }
+
+ CHECK(fifo.empty());
+
+ return true;
+}
+END_TEST(testTraceableFifoValueStrings)
+
+BEGIN_TEST(testTraceableFifoValueMixed) {
+ JS::Rooted<TraceableFifo<JS::Value>> fifo(cx, TraceableFifo<JS::Value>(cx));
+
+ // Mix different value types
+ CHECK(fifo.pushBack(JS::Int32Value(100)));
+
+ JS::RootedString str(cx, JS_NewStringCopyZ(cx, "mixed"));
+ CHECK(str);
+ CHECK(fifo.pushBack(JS::StringValue(str)));
+
+ JS::RootedObject obj(cx, JS_NewPlainObject(cx));
+ CHECK(obj);
+ CHECK(fifo.pushBack(JS::ObjectValue(*obj)));
+
+ CHECK(fifo.pushBack(JS::BooleanValue(false)));
+ CHECK(fifo.pushBack(JS::UndefinedValue()));
+
+ CHECK(fifo.length() == 5);
+
+ // Force GC between operations
+ JS_GC(cx);
+
+ // Verify mixed types maintain order and survive GC
+ CHECK(fifo.front().isInt32() && fifo.front().toInt32() == 100);
+ fifo.popFront();
+
+ CHECK(fifo.front().isString());
+ JS::RootedString retrievedStr(cx, fifo.front().toString());
+ bool match = false;
+ CHECK(JS_StringEqualsAscii(cx, retrievedStr, "mixed", 5, &match));
+ CHECK(match);
+ fifo.popFront();
+
+ CHECK(fifo.front().isObject());
+ CHECK(&fifo.front().toObject() == obj);
+ fifo.popFront();
+
+ CHECK(fifo.front().isBoolean() && !fifo.front().toBoolean());
+ fifo.popFront();
+
+ CHECK(fifo.front().isUndefined());
+ fifo.popFront();
+
+ CHECK(fifo.empty());
+
+ return true;
+}
+END_TEST(testTraceableFifoValueMixed)
+
+BEGIN_TEST(testTraceableFifoValueEmplaceBack) {
+ JS::Rooted<TraceableFifo<JS::Value>> fifo(cx, TraceableFifo<JS::Value>(cx));
+
+ // Test emplaceBack with different value construction
+ CHECK(fifo.emplaceBack(JS::UndefinedValue()));
+ CHECK(fifo.emplaceBack(JS::Int32Value(999)));
+ CHECK(fifo.emplaceBack(JS::DoubleValue(2.718)));
+
+ CHECK(fifo.length() == 3);
+
+ // Verify emplaced values
+ CHECK(fifo.front().isUndefined());
+ fifo.popFront();
+
+ CHECK(fifo.front().isInt32() && fifo.front().toInt32() == 999);
+ fifo.popFront();
+
+ CHECK(fifo.front().isDouble() && fifo.front().toDouble() == 2.718);
+ fifo.popFront();
+
+ CHECK(fifo.empty());
+
+ return true;
+}
+END_TEST(testTraceableFifoValueEmplaceBack)
+
+BEGIN_TEST(testTraceableFifoValueClear) {
+ JS::Rooted<TraceableFifo<JS::Value>> fifo(cx, TraceableFifo<JS::Value>(cx));
+
+ // Fill with many values
+ for (int i = 0; i < 30; ++i) {
+ if (i % 3 == 0) {
+ CHECK(fifo.pushBack(JS::Int32Value(i)));
+ } else if (i % 3 == 1) {
+ JS::RootedObject obj(cx, JS_NewPlainObject(cx));
+ CHECK(obj);
+ CHECK(fifo.pushBack(JS::ObjectValue(*obj)));
+ } else {
+ JS::RootedString str(cx, JS_NewStringCopyZ(cx, "test"));
+ CHECK(str);
+ CHECK(fifo.pushBack(JS::StringValue(str)));
+ }
+ }
+
+ CHECK(fifo.length() == 30);
+ CHECK(!fifo.empty());
+
+ // Clear the fifo
+ fifo.clear();
+
+ CHECK(fifo.length() == 0);
+ CHECK(fifo.empty());
+
+ // Verify we can still use it after clear
+ CHECK(fifo.pushBack(JS::Int32Value(42)));
+ CHECK(fifo.length() == 1);
+ CHECK(fifo.front().isInt32() && fifo.front().toInt32() == 42);
+
+ return true;
+}
+END_TEST(testTraceableFifoValueClear)
+
+BEGIN_TEST(testTraceableFifoValueLargeScale) {
+ JS::Rooted<TraceableFifo<JS::Value>> fifo(cx, TraceableFifo<JS::Value>(cx));
+
+ // Large scale test with GC pressure
+ const size_t largeCount = 100;
+
+ for (size_t i = 0; i < largeCount; ++i) {
+ // Create different types of values
+ switch (i % 4) {
+ case 0: {
+ CHECK(fifo.pushBack(JS::Int32Value(static_cast<int32_t>(i))));
+ break;
+ }
+ case 1: {
+ JS::RootedObject obj(cx, JS_NewPlainObject(cx));
+ CHECK(obj);
+ CHECK(fifo.pushBack(JS::ObjectValue(*obj)));
+ break;
+ }
+ case 2: {
+ JS::RootedString str(cx, JS_NewStringCopyZ(cx, "large"));
+ CHECK(str);
+ CHECK(fifo.pushBack(JS::StringValue(str)));
+ break;
+ }
+ case 3: {
+ CHECK(fifo.pushBack(JS::DoubleValue(static_cast<double>(i) + 0.5)));
+ break;
+ }
+ }
+
+ // Periodic GC to test tracing under pressure
+ if (i % 25 == 0) {
+ JS_GC(cx);
+ }
+ }
+
+ CHECK(fifo.length() == largeCount);
+
+ // Final GC sweep
+ JS_GC(cx);
+ JS_GC(cx);
+
+ // Verify all values are intact
+ for (size_t i = 0; i < largeCount; ++i) {
+ CHECK(!fifo.empty());
+
+ switch (i % 4) {
+ case 0:
+ CHECK(fifo.front().isInt32());
+ CHECK(fifo.front().toInt32() == static_cast<int32_t>(i));
+ break;
+ case 1:
+ CHECK(fifo.front().isObject());
+ break;
+ case 2:
+ CHECK(fifo.front().isString());
+ break;
+ case 3:
+ CHECK(fifo.front().isDouble());
+ CHECK(fifo.front().toDouble() == static_cast<double>(i) + 0.5);
+ break;
+ }
+
+ fifo.popFront();
+ }
+
+ CHECK(fifo.empty());
+
+ return true;
+}
+END_TEST(testTraceableFifoValueLargeScale)
diff --git a/mfbt/Vector.h b/mfbt/Vector.h
@@ -1625,7 +1625,8 @@ inline size_t Vector<T, N, AP>::sizeOfIncludingThis(
template <typename T, size_t N, class AP>
inline void Vector<T, N, AP>::swap(Vector& aOther) {
- static_assert(N == 0, "still need to implement this for N != 0");
+ static_assert(N == 0,
+ "still need to implement this for N != 0 (Bug 1987683)");
// This only works when inline storage is always empty.
if (!usingInlineStorage() && aOther.usingInlineStorage()) {
