Issue #3206 Back out stub pref for WeakRef/FinalizationRegistry API>

While this should work with some tracing fixes, there are subtleties that
continue to cause issues with the stub variant of the API.

Resolves #3206

dom/workers/RuntimeService.cpp

@@ -301,7 +301,6 @@ LoadContextOptions(const char* aPrefName, void* /* aClosure */)
                 .setAsyncStack(GetWorkerPref<bool>(NS_LITERAL_CSTRING("asyncstack")))
                 .setWerror(GetWorkerPref<bool>(NS_LITERAL_CSTRING("werror")))
                 .setStreams(GetWorkerPref<bool>(NS_LITERAL_CSTRING("streams")))
-                .setWeakRefs(GetWorkerPref<bool>(NS_LITERAL_CSTRING("weakrefs")))
                 .setExtraWarnings(GetWorkerPref<bool>(NS_LITERAL_CSTRING("strict")))
                 .setArrayProtoValues(GetWorkerPref<bool>(
                       NS_LITERAL_CSTRING("array_prototype_values")));

dom/workers/WorkerPrefs.h

@@ -35,7 +35,6 @@ WORKER_SIMPLE_PREF("dom.serviceWorkers.openWindow.enabled", OpenWindowEnabled, O
 WORKER_SIMPLE_PREF("dom.storageManager.enabled", StorageManagerEnabled, STORAGEMANAGER_ENABLED)
 WORKER_SIMPLE_PREF("dom.push.enabled", PushEnabled, PUSH_ENABLED)
 WORKER_SIMPLE_PREF("dom.streams.enabled", StreamsEnabled, STREAMS_ENABLED)
-WORKER_SIMPLE_PREF("javascript.options.weakrefs", WeakRefsEnabled, WEAKREFS_ENABLED)
 WORKER_SIMPLE_PREF("dom.requestcontext.enabled", RequestContextEnabled, REQUESTCONTEXT_ENABLED)
 WORKER_SIMPLE_PREF("gfx.offscreencanvas.enabled", OffscreenCanvasEnabled, OFFSCREENCANVAS_ENABLED)
 WORKER_SIMPLE_PREF("dom.webkitBlink.dirPicker.enabled", WebkitBlinkDirectoryPickerEnabled, DOM_WEBKITBLINK_DIRPICKER_WEBKITBLINK)

js/src/builtin/FinalizationRegistryObject.cpp

@@ -66,28 +66,10 @@ struct WeakCell {
     WeakCellKind kind;
     WeakRef<JSObject*> object;
     WeakRef<JS::Symbol*> symbol;
-    bool enabled;
 
-    WeakCell()
-        : kind(WeakCellKind::Empty)
-        , object(nullptr)
-        , symbol(nullptr)
-        , enabled(true)
-    {}
-
-    explicit WeakCell(JSObject* obj, bool enabled)
-        : kind(WeakCellKind::Object)
-        , object(obj)
-        , symbol(nullptr)
-        , enabled(enabled)
-    {}
-
-    explicit WeakCell(JS::Symbol* sym, bool enabled)
-        : kind(WeakCellKind::Symbol)
-        , object(nullptr)
-        , symbol(sym)
-        , enabled(enabled)
-    {}
+    WeakCell() : kind(WeakCellKind::Empty), object(nullptr), symbol(nullptr) {}
+    explicit WeakCell(JSObject* obj) : kind(WeakCellKind::Object), object(obj), symbol(nullptr) {}
+    explicit WeakCell(JS::Symbol* sym) : kind(WeakCellKind::Symbol), object(nullptr), symbol(sym) {}
 
     void clear() {
         kind = WeakCellKind::Empty;
@@ -119,9 +101,7 @@ struct WeakCell {
             if (!target)
                 return;
 
-            if (!enabled || IsInsideNursery(target)) {
-                // Weak edges must be tenured.
-                // Trace strongly if inside nursery or WeakRef disabled.
+            if (IsInsideNursery(target)) {
                 TraceManuallyBarrieredEdge(trc, object.unsafeGet(), name);
             } else {
                 if (trc->isMarkingTracer() && !target->asTenured().zone()->isCollecting())
@@ -131,14 +111,9 @@ struct WeakCell {
         } else if (kind == WeakCellKind::Symbol) {
             JS::Symbol* target = symbol.unbarrieredGet();
             if (target) {
-                if (!enabled) {
-                    // Trace strongly if WeakRef disabled.
-                    TraceManuallyBarrieredEdge(trc, symbol.unsafeGet(), name);
-                } else {
-                    if (trc->isMarkingTracer() && !target->asTenured().zone()->isCollecting())
-                        return;
-                    TraceWeakEdge(trc, &symbol, name);
-                }
+                if (trc->isMarkingTracer() && !target->asTenured().zone()->isCollecting())
+                    return;
+                TraceWeakEdge(trc, &symbol, name);
             }
         }
     }
@@ -149,16 +124,14 @@ struct WeakCell {
             if (!target)
                 return;
 
-            if (!enabled) {
-              // Do nothing
-            } else if (IsInsideNursery(target)) {
+            if (IsInsideNursery(target)) {
                 TraceManuallyBarrieredEdge(trc, object.unsafeGet(), name);
             } else if (target->asTenured().zone()->isCollecting()) {
                 TraceWeakEdge(trc, &object, name);
             }
         } else if (kind == WeakCellKind::Symbol) {
             JS::Symbol* target = symbol.unbarrieredGet();
-            if (enabled && target && target->asTenured().zone()->isCollecting())
+            if (target && target->asTenured().zone()->isCollecting())
                 TraceWeakEdge(trc, &symbol, name);
         }
     }
@@ -170,24 +143,22 @@ struct FinalizationRecord {
     WeakCell unregisterToken;
     bool active;
     bool queued;
-    bool enabled;
 
     FinalizationRecord(HandleValue targetValue, HandleValue heldValue,
-                       HandleValue unregisterTokenValue, bool enabled)
+                       HandleValue unregisterTokenValue)
       : heldValue(heldValue),
         active(true),
-        queued(false),
-        enabled(enabled)
+        queued(false)
     {
         if (targetValue.isObject())
-            target = WeakCell(NormalizeWeakObject(&targetValue.toObject()), enabled);
+            target = WeakCell(NormalizeWeakObject(&targetValue.toObject()));
         else
-            target = WeakCell(targetValue.toSymbol(), enabled);
+            target = WeakCell(targetValue.toSymbol());
 
         if (unregisterTokenValue.isObject())
-            unregisterToken = WeakCell(NormalizeWeakObject(&unregisterTokenValue.toObject()), enabled);
+            unregisterToken = WeakCell(NormalizeWeakObject(&unregisterTokenValue.toObject()));
         else if (unregisterTokenValue.isSymbol())
-            unregisterToken = WeakCell(unregisterTokenValue.toSymbol(), enabled);
+            unregisterToken = WeakCell(unregisterTokenValue.toSymbol());
     }
 
     void trace(JSTracer* trc) {
@@ -246,15 +217,13 @@ struct FinalizationRegistryObject::Data {
     FinalizationRecordVector records;
     FinalizationRecordVector cleanupQueue;
     bool queuedForCleanup;
-    bool enabled;
 
-    explicit Data(JSObject* cleanupCallback, bool enabled)
+    explicit Data(JSObject* cleanupCallback)
       : cleanupCallback(cleanupCallback),
         cleanupJob(nullptr),
         records(SystemAllocPolicy()),
         cleanupQueue(SystemAllocPolicy()),
-        queuedForCleanup(false),
-        enabled(enabled)
+        queuedForCleanup(false)
     {}
 
     ~Data() {
@@ -263,13 +232,11 @@ struct FinalizationRegistryObject::Data {
     }
 
     void trace(JSTracer* trc) {
-        if (enabled) {
-          JS::TraceEdge(trc, &cleanupCallback, "FinalizationRegistry cleanup callback");
-          if (cleanupJob.unbarrieredGet())
-              JS::TraceEdge(trc, &cleanupJob, "FinalizationRegistry cleanup job");
-          for (size_t i = 0; i < records.length(); i++)
-              records[i]->trace(trc);
-        }
+        JS::TraceEdge(trc, &cleanupCallback, "FinalizationRegistry cleanup callback");
+        if (cleanupJob.unbarrieredGet())
+            JS::TraceEdge(trc, &cleanupJob, "FinalizationRegistry cleanup job");
+        for (size_t i = 0; i < records.length(); i++)
+            records[i]->trace(trc);
     }
 
     bool appendRecord(FinalizationRecord* record) {
@@ -350,7 +317,7 @@ FinalizationRegistry_register_impl(JSContext* cx, const CallArgs& args)
         unregisterToken.setUndefined();
     }
 
-    FinalizationRecord* record = cx->new_<FinalizationRecord>(target, heldValue, unregisterToken, cx->options().weakRefs());
+    FinalizationRecord* record = cx->new_<FinalizationRecord>(target, heldValue, unregisterToken);
     if (!record)
         return false;
 
@@ -434,7 +401,7 @@ FinalizationRegistryObject::create(JSContext* cx, HandleObject cleanupCallback,
     if (!obj)
         return nullptr;
 
-    Data* data = cx->new_<Data>(cleanupCallback, cx->options().weakRefs());
+    Data* data = cx->new_<Data>(cleanupCallback);
     if (!data)
         return nullptr;
 

js/src/builtin/WeakRefObject.cpp

@@ -146,9 +146,9 @@ WeakRefObject::create(JSContext* cx, HandleValue target, HandleObject proto /* =
 
     Referent* data;
     if (target.isObject())
-        data = cx->new_<Referent>(&target.toObject(), cx->options().weakRefs());
+        data = cx->new_<Referent>(&target.toObject());
     else
-        data = cx->new_<Referent>(target.toSymbol(), cx->options().weakRefs());
+        data = cx->new_<Referent>(target.toSymbol());
 
     if (!data)
         return nullptr;
@@ -216,25 +216,18 @@ WeakRefObject::trace(JSTracer* trc, JSObject* obj)
             if (!target)
                 return;
 
-            if (!data->enabled || IsInsideNursery(target)) {
-                // Weak edges must be tenured; trace strongly while referent is in
-                // the nursery or if WeakRefs are disabled by pref.
+            if (IsInsideNursery(target)) {
+                // Weak edges must be tenured; trace strongly while referent is in the nursery.
                 TraceManuallyBarrieredEdge(trc, data->objectTarget.unsafeGet(),
-                                           "WeakRef strong referent");
+                                           "WeakRef nursery referent");
             } else {
                 TraceWeakEdge(trc, &data->objectTarget, "WeakRef referent");
             }
         } else {
             MOZ_ASSERT(data->isSymbol());
             JS::Symbol* target = data->symbolTarget.unbarrieredGet();
-            if (target) {
-                if (data->enabled) {
-                    TraceWeakEdge(trc, &data->symbolTarget, "WeakRef symbol referent");
-                } else {
-                    TraceManuallyBarrieredEdge(trc, data->symbolTarget.unsafeGet(),
-                                               "WeakRef strong symbol referent");
-                }
-            }
+            if (target)
+                TraceWeakEdge(trc, &data->symbolTarget, "WeakRef symbol referent");
         }
     }
 }

js/src/builtin/WeakRefObject.h

@@ -22,27 +22,17 @@ class WeakRefObject : public NativeObject
             Symbol
         };
 
-        explicit Referent(JSObject* obj, bool enabled)
-          : kind(Kind::Object)
-          , objectTarget(obj)
-          , symbolTarget(nullptr)
-          , enabled(enabled)
-        {}
-        
-        explicit Referent(JS::Symbol* sym, bool enabled)
-          : kind(Kind::Symbol)
-          , objectTarget(nullptr)
-          , symbolTarget(sym)
-          , enabled(enabled)
-        {}
-        
+        explicit Referent(JSObject* obj)
+          : kind(Kind::Object), objectTarget(obj), symbolTarget(nullptr) {}
+        explicit Referent(JS::Symbol* sym)
+          : kind(Kind::Symbol), objectTarget(nullptr), symbolTarget(sym) {}
+
         bool isObject() const { return kind == Kind::Object; }
         bool isSymbol() const { return kind == Kind::Symbol; }
 
         Kind kind;
         WeakRef<JSObject*> objectTarget;
         WeakRef<JS::Symbol*> symbolTarget;
-        bool enabled;
     };
 
     static const Class class_;

js/src/jsapi.h

@@ -999,8 +999,7 @@ class JS_PUBLIC_API(ContextOptions) {
         strictMode_(false),
         extraWarnings_(false),
         arrayProtoValues_(true),
-        streams_(false),
-        weakRefs_(false)
+        streams_(false)
     {
     }
 
@@ -1140,13 +1139,12 @@ class JS_PUBLIC_API(ContextOptions) {
         return *this;
     }
 
-    bool weakRefs() const { return weakRefs_; }
+    bool weakRefs() const { return true; }
     ContextOptions& setWeakRefs(bool flag) {
-        weakRefs_ = flag;
+        (void) flag;
         return *this;
     }
     ContextOptions& toggleWeakRefs() {
-        weakRefs_ = !weakRefs_;
         return *this;
     }
 
@@ -1167,7 +1165,6 @@ class JS_PUBLIC_API(ContextOptions) {
     bool extraWarnings_ : 1;
     bool arrayProtoValues_ : 1;
     bool streams_ : 1;
-    bool weakRefs_ : 1;
 };
 
 JS_PUBLIC_API(ContextOptions&)

js/xpconnect/src/XPCJSContext.cpp

@@ -1442,7 +1442,6 @@ ReloadPrefsCallback(const char* pref, void* data)
     bool extraWarnings = Preferences::GetBool(JS_OPTIONS_DOT_STR "strict");
     
     bool streams = Preferences::GetBool(JS_OPTIONS_DOT_STR "streams");
-    bool weakRefs = Preferences::GetBool(JS_OPTIONS_DOT_STR "weakrefs");
 
     bool unboxedObjects = Preferences::GetBool(JS_OPTIONS_DOT_STR "unboxed_objects");
     
@@ -1469,8 +1468,7 @@ ReloadPrefsCallback(const char* pref, void* data)
                              .setWerror(werror)
                              .setExtraWarnings(extraWarnings)
                              .setArrayProtoValues(arrayProtoValues)
-                             .setStreams(streams)
-                             .setWeakRefs(weakRefs);
+                             .setStreams(streams);
 
     JS_SetParallelParsingEnabled(cx, parallelParsing);
     JS_SetOffthreadIonCompilationEnabled(cx, offthreadIonCompilation);

modules/libpref/init/all.js

@@ -1314,9 +1314,6 @@ pref("javascript.options.dynamicImport", true);
 // Streams API
 pref("javascript.options.streams", true);
 
-// WeakRefs API
-pref("javascript.options.weakrefs", true);
-
 // advanced prefs
 pref("advanced.mailftp",                    false);
 pref("image.animation_mode",                "normal");