Bug 1719795 part 5 - Change JSCLASS_PRIVATE_IS_NSISUPPORTS JSClasses to use a reserved slot instead. r=mccr8,jonco

This is a step towards removing object private slots.

Classes with JSCLASS_PRIVATE_IS_NSISUPPORTS now use JSCLASS_SLOT0_IS_NSISUPPORTS
instead. For most classes this means we need to add an extra reserved slot and remove
the private slot.

Global objects (SimpleGlobalObject and the XPConnect BackstagePass and Sandbox globals)
however can use the JSCLASS_GLOBAL_APPLICATION_SLOTS already there. These slots were
only used for WebIDL DOM globals until now.

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

Author: jandem

dom/bindings/SimpleGlobalObject.cpp

@@ -8,7 +8,7 @@
 
 #include "jsapi.h"
 #include "js/Class.h"
-#include "js/Object.h"  // JS::GetClass, JS::GetPrivate, JS::SetPrivate
+#include "js/Object.h"  // JS::GetClass, JS::GetObjectISupports, JS::SetObjectISupports
 
 #include "nsJSPrincipals.h"
 #include "nsThreadUtils.h"
@@ -76,9 +76,12 @@ static const JSClassOps SimpleGlobalClassOps = {
 static const js::ClassExtension SimpleGlobalClassExtension = {
     SimpleGlobal_moved};
 
+static_assert(JSCLASS_GLOBAL_APPLICATION_SLOTS > 0,
+              "Need at least one slot for JSCLASS_SLOT0_IS_NSISUPPORTS");
+
 const JSClass SimpleGlobalClass = {"",
-                                   JSCLASS_GLOBAL_FLAGS | JSCLASS_HAS_PRIVATE |
-                                       JSCLASS_PRIVATE_IS_NSISUPPORTS |
+                                   JSCLASS_GLOBAL_FLAGS |
+                                       JSCLASS_SLOT0_IS_NSISUPPORTS |
                                        JSCLASS_FOREGROUND_FINALIZE,
                                    &SimpleGlobalClassOps,
                                    JS_NULL_CLASS_SPEC,
@@ -88,7 +91,7 @@ const JSClass SimpleGlobalClass = {"",
 static SimpleGlobalObject* GetSimpleGlobal(JSObject* global) {
   MOZ_ASSERT(JS::GetClass(global) == &SimpleGlobalClass);
 
-  return static_cast<SimpleGlobalObject*>(JS::GetPrivate(global));
+  return JS::GetObjectISupports<SimpleGlobalObject>(global);
 }
 
 // static
@@ -139,7 +142,7 @@ JSObject* SimpleGlobalObject::Create(GlobalType globalType,
         new SimpleGlobalObject(global, globalType);
 
     // Pass on ownership of globalObject to |global|.
-    JS::SetPrivate(global, globalObject.forget().take());
+    JS::SetObjectISupports(global, globalObject.forget().take());
 
     if (proto.isObjectOrNull()) {
       JS::Rooted<JSObject*> protoObj(cx, proto.toObjectOrNull());

js/public/Class.h

@@ -500,8 +500,8 @@ static const uint32_t JSCLASS_DELAY_METADATA_BUILDER = 1 << 1;
 // disposal mechanism.
 static const uint32_t JSCLASS_IS_WRAPPED_NATIVE = 1 << 2;
 
-// Private is `nsISupports*`.
-static const uint32_t JSCLASS_PRIVATE_IS_NSISUPPORTS = 1 << 3;
+// First reserved slot is `PrivateValue(nsISupports*)` or `UndefinedValue`.
+static constexpr uint32_t JSCLASS_SLOT0_IS_NSISUPPORTS = 1 << 3;
 
 // Objects are DOM.
 static const uint32_t JSCLASS_IS_DOMJSCLASS = 1 << 4;
@@ -706,6 +706,8 @@ struct alignas(js::gc::JSClassAlignBytes) JSClass {
 
   bool isWrappedNative() const { return flags & JSCLASS_IS_WRAPPED_NATIVE; }
 
+  bool slot0IsISupports() const { return flags & JSCLASS_SLOT0_IS_NSISUPPORTS; }
+
   static size_t offsetOfFlags() { return offsetof(JSClass, flags); }
 
   // Internal / friend API accessors:

js/public/Object.h

@@ -112,6 +112,31 @@ inline void SetReservedSlot(JSObject* obj, size_t slot, const Value& value) {
   }
 }
 
+/**
+ * Helper function to get the pointer value (or nullptr if not set) from the
+ * object's first reserved slot. Must only be used for objects with a JSClass
+ * that has the JSCLASS_SLOT0_IS_NSISUPPORTS flag.
+ */
+template <typename T>
+inline T* GetObjectISupports(JSObject* obj) {
+  MOZ_ASSERT(GetClass(obj)->slot0IsISupports());
+  Value v = GetReservedSlot(obj, 0);
+  return v.isUndefined() ? nullptr : static_cast<T*>(v.toPrivate());
+}
+
+/**
+ * Helper function to store |PrivateValue(nsISupportsValue)| in the object's
+ * first reserved slot. Must only be used for objects with a JSClass that has
+ * the JSCLASS_SLOT0_IS_NSISUPPORTS flag.
+ *
+ * Note: the pointer is opaque to the JS engine (including the GC) so it's the
+ * embedding's responsibility to trace or free this value.
+ */
+inline void SetObjectISupports(JSObject* obj, void* nsISupportsValue) {
+  MOZ_ASSERT(GetClass(obj)->slot0IsISupports());
+  SetReservedSlot(obj, 0, PrivateValue(nsISupportsValue));
+}
+
 }  // namespace JS
 
 #endif  // js_public_Object_h

js/src/builtin/streams/ReadableStream.cpp

@@ -23,7 +23,7 @@
 #include "builtin/streams/ReadableStreamReader.h"  // js::CreateReadableStream{BYOB,Default}Reader, js::ForAuthorCodeBool
 #include "builtin/streams/WritableStream.h"  // js::WritableStream
 #include "js/CallArgs.h"                     // JS::CallArgs{,FromVp}
-#include "js/Class.h"  // JSCLASS_PRIVATE_IS_NSISUPPORTS, JSCLASS_HAS_PRIVATE, JS_NULL_CLASS_OPS
+#include "js/Class.h"  // JSCLASS_SLOT0_IS_NSISUPPORTS, JSCLASS_HAS_PRIVATE, JS_NULL_CLASS_OPS
 #include "js/Conversions.h"           // JS::ToBoolean
 #include "js/friend/ErrorMessages.h"  // js::GetErrorMessage, JSMSG_*
 #include "js/PropertySpec.h"  // JS{Function,Property}Spec, JS_FN, JS_PSG, JS_{FS,PS}_END
@@ -543,7 +543,7 @@ static bool FinishReadableStreamClassInit(JSContext* cx, Handle<JSObject*> ctor,
   // This function and everything below should be replaced with
   //
   // JS_STREAMS_CLASS_SPEC(ReadableStream, 0, SlotCount, 0,
-  //                       JSCLASS_PRIVATE_IS_NSISUPPORTS | JSCLASS_HAS_PRIVATE,
+  //                       JSCLASS_SLOT0_IS_NSISUPPORTS,
   //                       JS_NULL_CLASS_OPS);
   //
   // when "pipeTo" is always enabled.
@@ -575,7 +575,7 @@ const JSClass ReadableStream::class_ = {
     "ReadableStream",
     JSCLASS_HAS_RESERVED_SLOTS(ReadableStream::SlotCount) |
         JSCLASS_HAS_CACHED_PROTO(JSProto_ReadableStream) |
-        JSCLASS_PRIVATE_IS_NSISUPPORTS | JSCLASS_HAS_PRIVATE,
+        JSCLASS_SLOT0_IS_NSISUPPORTS,
     JS_NULL_CLASS_OPS, &ReadableStream::classSpec_};
 
 const JSClass ReadableStream::protoClass_ = {

js/src/builtin/streams/ReadableStream.h

@@ -43,6 +43,12 @@ class ReadableStream : public NativeObject {
    * stream's constructor and thus cannot be in a different compartment.
    */
   enum Slots {
+    /**
+     * Optional pointer to make the stream participate in Gecko's cycle
+     * collection. See also JSCLASS_SLOT0_IS_NSISUPPORTS.
+     */
+    Slot_ISupports,
+
     Slot_Controller,
     Slot_Reader,
     Slot_State,

js/src/builtin/streams/ReadableStreamOperations.cpp

@@ -135,7 +135,9 @@ using JS::Value;
     return nullptr;
   }
 
-  stream->setPrivate(nsISupportsObject_alreadyAddreffed);
+  static_assert(Slot_ISupports == 0,
+                "Must use right slot for JSCLASS_SLOT0_IS_NSISUPPORTS");
+  JS::SetObjectISupports(stream, nsISupportsObject_alreadyAddreffed);
 
   // Step 1: Set stream.[[state]] to "readable".
   stream->initStateBits(Readable);

js/src/builtin/streams/StreamAPI.cpp

@@ -26,7 +26,7 @@
 #include "js/experimental/TypedData.h"  // JS_GetArrayBufferViewData, JS_NewUint8Array
 #include "js/friend/ErrorMessages.h"  // js::GetErrorMessage, JSMSG_*
 #include "js/GCAPI.h"       // JS::AutoCheckCannotGC, JS::AutoSuppressGCAnalysis
-#include "js/Object.h"      // JS::SetPrivate
+#include "js/Object.h"      // JS::SetObjectISupports
 #include "js/RootingAPI.h"  // JS::{,Mutable}Handle, JS::Rooted
 #include "js/Stream.h"      // JS::ReadableStreamUnderlyingSource
 #include "js/Value.h"       // JS::{,Object,Undefined}Value
@@ -399,7 +399,7 @@ JS_PUBLIC_API bool JS::ReadableStreamUpdateDataAvailableFromSource(
 
 JS_PUBLIC_API void JS::ReadableStreamReleaseCCObject(JSObject* streamObj) {
   MOZ_ASSERT(JS::IsReadableStream(streamObj));
-  JS::SetPrivate(streamObj, nullptr);
+  JS::SetObjectISupports(streamObj, nullptr);
 }
 
 JS_PUBLIC_API bool JS::ReadableStreamTee(JSContext* cx,

js/src/builtin/streams/WritableStream.cpp

@@ -19,7 +19,7 @@
 #include "builtin/streams/WritableStreamDefaultWriter.h"  // js::CreateWritableStreamDefaultWriter
 #include "builtin/streams/WritableStreamOperations.h"  // js::WritableStream{Abort,Close{,QueuedOrInFlight}}
 #include "js/CallArgs.h"                               // JS::CallArgs{,FromVp}
-#include "js/Class.h"  // JS{Function,Property}Spec, JS_{FS,PS}_END, JSCLASS_PRIVATE_IS_NSISUPPORTS, JSCLASS_HAS_PRIVATE, JS_NULL_CLASS_OPS
+#include "js/Class.h"  // JS{Function,Property}Spec, JS_{FS,PS}_END, JSCLASS_SLOT0_IS_NSISUPPORTS, JSCLASS_HAS_PRIVATE, JS_NULL_CLASS_OPS
 #include "js/friend/ErrorMessages.h"  // js::GetErrorMessage, JSMSG_*
 #include "js/RealmOptions.h"          // JS::RealmCreationOptions
 #include "js/RootingAPI.h"            // JS::Handle, JS::Rooted
@@ -277,5 +277,4 @@ static const JSPropertySpec WritableStream_properties[] = {
     JS_PSG("locked", WritableStream_locked, 0), JS_PS_END};
 
 JS_STREAMS_CLASS_SPEC(WritableStream, 0, SlotCount, 0,
-                      JSCLASS_PRIVATE_IS_NSISUPPORTS | JSCLASS_HAS_PRIVATE,
-                      JS_NULL_CLASS_OPS);
+                      JSCLASS_SLOT0_IS_NSISUPPORTS, JS_NULL_CLASS_OPS);

js/src/builtin/streams/WritableStream.h

@@ -33,6 +33,12 @@ class WritableStreamDefaultWriter;
 class WritableStream : public NativeObject {
  public:
   enum Slots {
+    /**
+     * Optional pointer to make the stream participate in Gecko's cycle
+     * collection. See also JSCLASS_SLOT0_IS_NSISUPPORTS.
+     */
+    Slot_ISupports,
+
     /**
      * A WritableStream's associated controller is always created from under the
      * stream's constructor and thus cannot be in a different compartment.

js/src/builtin/streams/WritableStreamOperations.cpp

@@ -76,7 +76,9 @@ using JS::Value;
     return nullptr;
   }
 
-  stream->setPrivate(nsISupportsObject_alreadyAddreffed);
+  static_assert(Slot_ISupports == 0,
+                "Must use right slot for JSCLASS_SLOT0_IS_NSISUPPORTS");
+  JS::SetObjectISupports(stream, nsISupportsObject_alreadyAddreffed);
 
   stream->initWritableState();