Bug 1648453 - Record incumbent global in FinalizationRegistry constructor and use when calling back into the JS engine to call callbacks r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 17 Jul 2020 17:34:33 +0000
changeset 540948 ffa51e994ee8520b287c1440c995fa1da8549041
parent 540947 9ca68899ab7cda055f5f105f7c03c8378ef8c683
child 540949 13ee873ffdff46a34eea8b50844f8aba99a1517f
push id37612
push userabutkovits@mozilla.com
push dateFri, 17 Jul 2020 21:28:41 +0000
treeherdermozilla-central@bd7c35b7e234 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1648453
milestone80.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1648453 - Record incumbent global in FinalizationRegistry constructor and use when calling back into the JS engine to call callbacks r=sfink This changes the way the the host triggers cleanup, from calling an API to calling a JS function. This is done so we can use the existing DOM infrastructure that handles setting up the incumbent global for us. Differential Revision: https://phabricator.services.mozilla.com/D83613
js/public/GCAPI.h
js/src/builtin/FinalizationRegistryObject.cpp
js/src/builtin/FinalizationRegistryObject.h
js/src/gc/FinalizationRegistry.cpp
js/src/gc/GC.cpp
js/src/gc/GCRuntime.h
js/src/jsapi.cpp
js/src/jsapi.h
js/src/shell/js.cpp
js/src/shell/jsshell.h
--- a/js/public/GCAPI.h
+++ b/js/public/GCAPI.h
@@ -399,24 +399,24 @@ typedef void (*JSFinalizeCallback)(JSFre
 
 typedef void (*JSWeakPointerZonesCallback)(JSContext* cx, void* data);
 
 typedef void (*JSWeakPointerCompartmentCallback)(JSContext* cx,
                                                  JS::Compartment* comp,
                                                  void* data);
 
 /*
- * This is called to tell the embedding that the FinalizationRegistry object
- * |registry| has cleanup work, and that then engine should be called back at an
- * appropriate later time to perform this cleanup.
+ * This is called to tell the embedding that a FinalizationRegistry object has
+ * cleanup work, and that the engine should be called back at an appropriate
+ * later time to perform this cleanup, by calling the function |doCleanup|.
  *
  * This callback must not do anything that could cause GC.
  */
-using JSHostCleanupFinalizationRegistryCallback = void (*)(JSObject* registry,
-                                                           void* data);
+using JSHostCleanupFinalizationRegistryCallback =
+    void (*)(JSFunction* doCleanup, JSObject* incumbentGlobal, void* data);
 
 /**
  * Each external string has a pointer to JSExternalStringCallbacks. Embedders
  * can use this to implement custom finalization or memory reporting behavior.
  */
 struct JSExternalStringCallbacks {
   /**
    * Finalizes external strings created by JS_NewExternalString. The finalizer
--- a/js/src/builtin/FinalizationRegistryObject.cpp
+++ b/js/src/builtin/FinalizationRegistryObject.cpp
@@ -290,27 +290,30 @@ const JSPropertySpec FinalizationRegistr
 bool FinalizationRegistryObject::construct(JSContext* cx, unsigned argc,
                                            Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
 
   if (!ThrowIfNotConstructing(cx, args, "FinalizationRegistry")) {
     return false;
   }
 
+  RootedObject proto(cx);
+  if (!GetPrototypeFromBuiltinConstructor(
+          cx, args, JSProto_FinalizationRegistry, &proto)) {
+    return false;
+  }
+
   RootedObject cleanupCallback(
       cx, ValueToCallable(cx, args.get(0), 1, NO_CONSTRUCT));
   if (!cleanupCallback) {
     return false;
   }
 
-  RootedObject proto(cx);
-  if (!GetPrototypeFromBuiltinConstructor(
-          cx, args, JSProto_FinalizationRegistry, &proto)) {
-    return false;
-  }
+  Rooted<GlobalObject*> incumbentGlobal(cx,
+                                        cx->runtime()->getIncumbentGlobal(cx));
 
   Rooted<UniquePtr<ObjectWeakMap>> registrations(
       cx, cx->make_unique<ObjectWeakMap>(cx));
   if (!registrations) {
     return false;
   }
 
   Rooted<UniquePtr<FinalizationRecordSet>> activeRecords(
@@ -320,32 +323,47 @@ bool FinalizationRegistryObject::constru
   }
 
   Rooted<UniquePtr<FinalizationRecordVector>> recordsToBeCleanedUp(
       cx, cx->make_unique<FinalizationRecordVector>(cx->zone()));
   if (!recordsToBeCleanedUp) {
     return false;
   }
 
+  HandlePropertyName funName = cx->names().empty;
+  RootedFunction doCleanupFunction(
+      cx, NewNativeFunction(cx, doCleanup, 0, funName,
+                            gc::AllocKind::FUNCTION_EXTENDED));
+  if (!doCleanupFunction) {
+    return false;
+  }
+
   FinalizationRegistryObject* registry =
       NewObjectWithClassProto<FinalizationRegistryObject>(cx, proto);
   if (!registry) {
     return false;
   }
 
   registry->initReservedSlot(CleanupCallbackSlot,
                              ObjectValue(*cleanupCallback));
+  registry->initReservedSlot(IncumbentGlobalSlot,
+                             ObjectValue(*incumbentGlobal));
   InitReservedSlot(registry, RegistrationsSlot, registrations.release(),
                    MemoryUse::FinalizationRegistryRegistrations);
   InitReservedSlot(registry, ActiveRecords, activeRecords.release(),
                    MemoryUse::FinalizationRegistryRecordSet);
   InitReservedSlot(registry, RecordsToBeCleanedUpSlot,
                    recordsToBeCleanedUp.release(),
                    MemoryUse::FinalizationRegistryRecordVector);
   registry->initReservedSlot(IsQueuedForCleanupSlot, BooleanValue(false));
+  registry->initReservedSlot(DoCleanupFunctionSlot,
+                             ObjectValue(*doCleanupFunction));
+
+  doCleanupFunction->setExtendedSlot(DoCleanupFunction_RegistrySlot,
+                                     ObjectValue(*registry));
 
   if (!cx->runtime()->gc.addFinalizationRegistry(cx, registry)) {
     return false;
   }
 
   args.rval().setObject(*registry);
   return true;
 }
@@ -413,24 +431,32 @@ void FinalizationRegistryObject::finaliz
   fop->delete_(obj, registry->registrations(),
                MemoryUse::FinalizationRegistryRegistrations);
   fop->delete_(obj, registry->activeRecords(),
                MemoryUse::FinalizationRegistryRecordSet);
   fop->delete_(obj, registry->recordsToBeCleanedUp(),
                MemoryUse::FinalizationRegistryRecordVector);
 }
 
-inline JSObject* FinalizationRegistryObject::cleanupCallback() const {
+JSObject* FinalizationRegistryObject::cleanupCallback() const {
   Value value = getReservedSlot(CleanupCallbackSlot);
   if (value.isUndefined()) {
     return nullptr;
   }
   return &value.toObject();
 }
 
+GlobalObject* FinalizationRegistryObject::incumbentGlobal() const {
+  Value value = getReservedSlot(IncumbentGlobalSlot);
+  if (value.isUndefined()) {
+    return nullptr;
+  }
+  return &value.toObject().as<GlobalObject>();
+}
+
 ObjectWeakMap* FinalizationRegistryObject::registrations() const {
   Value value = getReservedSlot(RegistrationsSlot);
   if (value.isUndefined()) {
     return nullptr;
   }
   return static_cast<ObjectWeakMap*>(value.toPrivate());
 }
 
@@ -450,16 +476,24 @@ FinalizationRecordVector* FinalizationRe
   }
   return static_cast<FinalizationRecordVector*>(value.toPrivate());
 }
 
 bool FinalizationRegistryObject::isQueuedForCleanup() const {
   return getReservedSlot(IsQueuedForCleanupSlot).toBoolean();
 }
 
+JSFunction* FinalizationRegistryObject::doCleanupFunction() const {
+  Value value = getReservedSlot(DoCleanupFunctionSlot);
+  if (value.isUndefined()) {
+    return nullptr;
+  }
+  return &value.toObject().as<JSFunction>();
+}
+
 void FinalizationRegistryObject::queueRecordToBeCleanedUp(
     FinalizationRecordObject* record) {
   AutoEnterOOMUnsafeRegion oomUnsafe;
   if (!recordsToBeCleanedUp()->append(record)) {
     oomUnsafe.crash("FinalizationRegistryObject::queueRecordsToBeCleanedUp");
   }
 }
 
@@ -765,16 +799,31 @@ bool FinalizationRegistryObject::cleanup
   if (!cleanupQueuedRecords(cx, registry, cleanupCallback)) {
     return false;
   }
 
   args.rval().setUndefined();
   return true;
 }
 
+/* static */
+bool FinalizationRegistryObject::doCleanup(JSContext* cx, unsigned argc,
+                                           Value* vp) {
+  CallArgs args = CallArgsFromVp(argc, vp);
+
+  RootedFunction callee(cx, &args.callee().as<JSFunction>());
+
+  Value value = callee->getExtendedSlot(DoCleanupFunction_RegistrySlot);
+  RootedFinalizationRegistryObject registry(
+      cx, &value.toObject().as<FinalizationRegistryObject>());
+
+  registry->setQueuedForCleanup(false);
+  return cleanupQueuedRecords(cx, registry);
+}
+
 // CleanupFinalizationRegistry ( finalizationRegistry [ , callback ] )
 // https://tc39.es/proposal-weakrefs/#sec-cleanup-finalization-registry
 /* static */
 bool FinalizationRegistryObject::cleanupQueuedRecords(
     JSContext* cx, HandleFinalizationRegistryObject registry,
     HandleObject callbackArg) {
   MOZ_ASSERT(cx->compartment() == registry->compartment());
 
--- a/js/src/builtin/FinalizationRegistryObject.h
+++ b/js/src/builtin/FinalizationRegistryObject.h
@@ -170,32 +170,40 @@ using FinalizationRecordVector =
 
 using FinalizationRecordSet =
     GCHashSet<HeapPtrObject, MovableCellHasher<HeapPtrObject>, ZoneAllocPolicy>;
 
 // The FinalizationRegistry object itself.
 class FinalizationRegistryObject : public NativeObject {
   enum {
     CleanupCallbackSlot = 0,
+    IncumbentGlobalSlot,
     RegistrationsSlot,
     ActiveRecords,
     RecordsToBeCleanedUpSlot,
     IsQueuedForCleanupSlot,
+    DoCleanupFunctionSlot,
     SlotCount
   };
 
+  enum DoCleanupFunctionSlots {
+    DoCleanupFunction_RegistrySlot = 0,
+  };
+
  public:
   static const JSClass class_;
   static const JSClass protoClass_;
 
   JSObject* cleanupCallback() const;
+  GlobalObject* incumbentGlobal() const;
   ObjectWeakMap* registrations() const;
   FinalizationRecordSet* activeRecords() const;
   FinalizationRecordVector* recordsToBeCleanedUp() const;
   bool isQueuedForCleanup() const;
+  JSFunction* doCleanupFunction() const;
 
   void queueRecordToBeCleanedUp(FinalizationRecordObject* record);
   void setQueuedForCleanup(bool value);
 
   void sweep();
 
   static bool unregisterRecord(FinalizationRecordObject* record);
 
@@ -219,15 +227,17 @@ class FinalizationRegistryObject : publi
                               HandleObject unregisterToken,
                               HandleFinalizationRecordObject record);
   static void removeRegistrationOnError(
       HandleFinalizationRegistryObject registry, HandleObject unregisterToken,
       HandleFinalizationRecordObject record);
 
   static bool preserveDOMWrapper(JSContext* cx, HandleObject obj);
 
+  static bool doCleanup(JSContext* cx, unsigned argc, Value* vp);
+
   static void trace(JSTracer* trc, JSObject* obj);
   static void finalize(JSFreeOp* fop, JSObject* obj);
 };
 
 }  // namespace js
 
 #endif /* builtin_FinalizationRegistryObject_h */
--- a/js/src/gc/FinalizationRegistry.cpp
+++ b/js/src/gc/FinalizationRegistry.cpp
@@ -113,19 +113,13 @@ void GCRuntime::sweepFinalizationRegistr
     }
   }
 }
 
 void GCRuntime::queueFinalizationRegistryForCleanup(
     FinalizationRegistryObject* registry) {
   // Prod the embedding to call us back later to run the finalization callbacks.
   if (!registry->isQueuedForCleanup()) {
-    callHostCleanupFinalizationRegistryCallback(registry);
+    callHostCleanupFinalizationRegistryCallback(registry->doCleanupFunction(),
+                                                registry->incumbentGlobal());
     registry->setQueuedForCleanup(true);
   }
 }
-
-bool GCRuntime::cleanupQueuedFinalizationRegistry(
-    JSContext* cx, HandleFinalizationRegistryObject registry) {
-  registry->setQueuedForCleanup(false);
-  bool ok = FinalizationRegistryObject::cleanupQueuedRecords(cx, registry);
-  return ok;
-}
--- a/js/src/gc/GC.cpp
+++ b/js/src/gc/GC.cpp
@@ -1586,21 +1586,21 @@ void GCRuntime::callFinalizeCallbacks(JS
 }
 
 void GCRuntime::setHostCleanupFinalizationRegistryCallback(
     JSHostCleanupFinalizationRegistryCallback callback, void* data) {
   hostCleanupFinalizationRegistryCallback.ref() = {callback, data};
 }
 
 void GCRuntime::callHostCleanupFinalizationRegistryCallback(
-    FinalizationRegistryObject* registry) {
+    JSFunction* doCleanup, GlobalObject* incumbentGlobal) {
   JS::AutoSuppressGCAnalysis nogc;
   const auto& callback = hostCleanupFinalizationRegistryCallback.ref();
   if (callback.op) {
-    callback.op(registry, callback.data);
+    callback.op(doCleanup, incumbentGlobal, callback.data);
   }
 }
 
 bool GCRuntime::addWeakPointerZonesCallback(JSWeakPointerZonesCallback callback,
                                             void* data) {
   return updateWeakPointerZonesCallbacks.ref().append(
       Callback<JSWeakPointerZonesCallback>(callback, data));
 }
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -416,17 +416,17 @@ class GCRuntime {
   void setObjectsTenuredCallback(JSObjectsTenuredCallback callback, void* data);
   void callObjectsTenuredCallback();
   MOZ_MUST_USE bool addFinalizeCallback(JSFinalizeCallback callback,
                                         void* data);
   void removeFinalizeCallback(JSFinalizeCallback func);
   void setHostCleanupFinalizationRegistryCallback(
       JSHostCleanupFinalizationRegistryCallback callback, void* data);
   void callHostCleanupFinalizationRegistryCallback(
-      FinalizationRegistryObject* registry);
+      JSFunction* doCleanup, GlobalObject* incumbentGlobal);
   MOZ_MUST_USE bool addWeakPointerZonesCallback(
       JSWeakPointerZonesCallback callback, void* data);
   void removeWeakPointerZonesCallback(JSWeakPointerZonesCallback callback);
   MOZ_MUST_USE bool addWeakPointerCompartmentCallback(
       JSWeakPointerCompartmentCallback callback, void* data);
   void removeWeakPointerCompartmentCallback(
       JSWeakPointerCompartmentCallback callback);
   JS::GCSliceCallback setSliceCallback(JS::GCSliceCallback callback);
@@ -434,18 +434,16 @@ class GCRuntime {
       JS::GCNurseryCollectionCallback callback);
   JS::DoCycleCollectionCallback setDoCycleCollectionCallback(
       JS::DoCycleCollectionCallback callback);
 
   bool addFinalizationRegistry(JSContext* cx,
                                FinalizationRegistryObject* registry);
   bool registerWithFinalizationRegistry(JSContext* cx, HandleObject target,
                                         HandleObject record);
-  bool cleanupQueuedFinalizationRegistry(
-      JSContext* cx, Handle<FinalizationRegistryObject*> registry);
 
   void setFullCompartmentChecks(bool enable);
 
   JS::Zone* getCurrentSweepGroup() { return currentSweepGroup; }
   unsigned getCurrentSweepGroupIndex() {
     return state() == State::Sweep ? sweepGroupIndex : 0;
   }
 
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -1391,25 +1391,16 @@ JS_PUBLIC_API void JS_RemoveFinalizeCall
 }
 
 JS_PUBLIC_API void JS::SetHostCleanupFinalizationRegistryCallback(
     JSContext* cx, JSHostCleanupFinalizationRegistryCallback cb, void* data) {
   AssertHeapIsIdle();
   cx->runtime()->gc.setHostCleanupFinalizationRegistryCallback(cb, data);
 }
 
-JS_PUBLIC_API bool JS::CleanupQueuedFinalizationRegistry(
-    JSContext* cx, HandleObject registry) {
-  AssertHeapIsIdle();
-  CHECK_THREAD(cx);
-  cx->check(registry);
-  return cx->runtime()->gc.cleanupQueuedFinalizationRegistry(
-      cx, registry.as<FinalizationRegistryObject>());
-}
-
 JS_PUBLIC_API void JS::ClearKeptObjects(JSContext* cx) {
   gc::GCRuntime* gc = &cx->runtime()->gc;
 
   for (ZonesIter zone(gc, ZoneSelector::WithAtoms); !zone.done(); zone.next()) {
     zone->clearKeptObjects();
   }
 }
 
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -3064,23 +3064,16 @@ extern JS_PUBLIC_API bool BuildStackStri
 extern JS_PUBLIC_API bool IsMaybeWrappedSavedFrame(JSObject* obj);
 
 /**
  * Return true iff the given object is a SavedFrame object and not the
  * SavedFrame.prototype object.
  */
 extern JS_PUBLIC_API bool IsUnwrappedSavedFrame(JSObject* obj);
 
-/**
- * Clean up a finalization registry in response to the engine calling the
- * HostCleanupFinalizationRegistry callback.
- */
-extern JS_PUBLIC_API bool CleanupQueuedFinalizationRegistry(
-    JSContext* cx, HandleObject registry);
-
 } /* namespace JS */
 
 namespace js {
 
 /**
  * Hint that we expect a crash. Currently, the only thing that cares is the
  * breakpad injector, which (if loaded) will suppress minidump generation.
  */
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -653,17 +653,17 @@ ShellContext::ShellContext(JSContext* cx
       unhandledRejectedPromises(cx),
       watchdogLock(mutexid::ShellContextWatchdog),
       exitCode(0),
       quitting(false),
       readLineBufPos(0),
       errFilePtr(nullptr),
       outFilePtr(nullptr),
       offThreadMonitor(mutexid::ShellOffThreadState),
-      finalizationRegistriesToCleanUp(cx) {}
+      finalizationRegistryCleanupCallbacks(cx) {}
 
 ShellContext::~ShellContext() { MOZ_ASSERT(offThreadJobs.empty()); }
 
 ShellContext* js::shell::GetShellContext(JSContext* cx) {
   ShellContext* sc = static_cast<ShellContext*>(JS_GetContextPrivate(cx));
   MOZ_ASSERT(sc);
   return sc;
 }
@@ -975,46 +975,51 @@ static MOZ_MUST_USE bool RunModule(JSCon
   path = ResolvePath(cx, path, RootRelative);
   if (!path) {
     return false;
   }
 
   return sc->moduleLoader->loadRootModule(cx, path);
 }
 
-static void ShellCleanupFinalizationRegistryCallback(JSObject* registry,
+static void ShellCleanupFinalizationRegistryCallback(JSFunction* doCleanup,
+                                                     JSObject* incumbentGlobal,
                                                      void* data) {
   // In the browser this queues a task. Shell jobs correspond to microtasks so
-  // we arrange for cleanup to happen after all jobs/microtasks have run.
+  // we arrange for cleanup to happen after all jobs/microtasks have run. The
+  // incumbent global is ignored in the shell.
+
   auto sc = static_cast<ShellContext*>(data);
   AutoEnterOOMUnsafeRegion oomUnsafe;
-  if (!sc->finalizationRegistriesToCleanUp.append(registry)) {
+  if (!sc->finalizationRegistryCleanupCallbacks.append(doCleanup)) {
     oomUnsafe.crash("ShellCleanupFinalizationRegistryCallback");
   }
 }
 
 // Run any FinalizationRegistry cleanup tasks and return whether any ran.
 static bool MaybeRunFinalizationRegistryCleanupTasks(JSContext* cx) {
   ShellContext* sc = GetShellContext(cx);
   MOZ_ASSERT(!sc->quitting);
 
-  Rooted<ShellContext::ObjectVector> registries(cx);
-  std::swap(registries.get(), sc->finalizationRegistriesToCleanUp.get());
+  Rooted<ShellContext::FunctionVector> callbacks(cx);
+  std::swap(callbacks.get(), sc->finalizationRegistryCleanupCallbacks.get());
 
   bool ranTasks = false;
 
-  RootedObject registry(cx);
-  for (const auto& r : registries) {
-    registry = r;
-
-    AutoRealm ar(cx, registry);
+  RootedFunction callback(cx);
+  for (JSFunction* f : callbacks) {
+    callback = f;
+
+    AutoRealm ar(cx, f);
 
     {
       AutoReportException are(cx);
-      mozilla::Unused << JS::CleanupQueuedFinalizationRegistry(cx, registry);
+      RootedValue unused(cx);
+      mozilla::Unused << JS_CallFunction(cx, nullptr, callback,
+                                         HandleValueArray::empty(), &unused);
     }
 
     ranTasks = true;
 
     if (sc->quitting) {
       break;
     }
   }
--- a/js/src/shell/jsshell.h
+++ b/js/src/shell/jsshell.h
@@ -246,18 +246,18 @@ struct ShellContext {
 
   UniquePtr<MarkBitObservers> markObservers;
 
   // Off-thread parse state.
   js::Monitor offThreadMonitor;
   Vector<OffThreadJob*, 0, SystemAllocPolicy> offThreadJobs;
 
   // Queued finalization registry cleanup jobs.
-  using ObjectVector = GCVector<JSObject*, 0, SystemAllocPolicy>;
-  JS::PersistentRooted<ObjectVector> finalizationRegistriesToCleanUp;
+  using FunctionVector = GCVector<JSFunction*, 0, SystemAllocPolicy>;
+  JS::PersistentRooted<FunctionVector> finalizationRegistryCleanupCallbacks;
 };
 
 extern ShellContext* GetShellContext(JSContext* cx);
 
 extern MOZ_MUST_USE bool PrintStackTrace(JSContext* cx,
                                          JS::Handle<JSObject*> stackObj);
 
 extern JSObject* CreateScriptPrivate(JSContext* cx,