Bug 1572782 - Remove js::FreeOp and make JSFreeOp opaque in public API r=tcampbell?

Merge js::FreeOp and JSFreeOp, but alias the former to the latter while we fix uses.

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

--HG--
extra : moz-landing-system : lando

dom/bindings/Codegen.py

@@ -1764,7 +1764,7 @@ def finalizeHook(descriptor, hookName, freeOp, obj):
     if descriptor.wrapperCache:
         finalize += "ClearWrapper(self, self, %s);\n" % obj
     if descriptor.isGlobal():
-        finalize += "mozilla::dom::FinalizeGlobal(CastToJSFreeOp(%s), %s);\n" % (freeOp, obj)
+        finalize += "mozilla::dom::FinalizeGlobal(js::CastToJSFreeOp(%s), %s);\n" % (freeOp, obj)
     finalize += fill(
         """
         if (size_t mallocBytes = BindingJSObjectMallocBytes(self)) {

js/public/Class.h

@@ -25,13 +25,11 @@
  */
 
 struct JSAtomState;
-struct JSFreeOp;
 struct JSFunctionSpec;
 
 namespace js {
 
 struct Class;
-class FreeOp;
 class Shape;
 
 // This is equal to JSFunction::class_.  Use it in places where you don't want

js/public/GCAPI.h

@@ -19,8 +19,6 @@
 #include "js/UniquePtr.h"
 #include "js/Utility.h"
 
-struct JSFreeOp;
-
 #ifdef JS_BROKEN_GCC_ATTRIBUTE_WARNING
 #  pragma GCC diagnostic push
 #  pragma GCC diagnostic ignored "-Wattributes"

js/public/MemoryFunctions.h

@@ -16,22 +16,10 @@
 #include "jstypes.h"  // JS_PUBLIC_API
 
 struct JSContext;
+class JSFreeOp;
 class JSObject;
 struct JSRuntime;
 
-struct JSFreeOp {
- protected:
-  JSRuntime* runtime_;
-
-  explicit JSFreeOp(JSRuntime* rt) : runtime_(rt) {}
-
- public:
-  JSRuntime* runtime() const {
-    MOZ_ASSERT(runtime_);
-    return runtime_;
-  }
-};
-
 extern JS_PUBLIC_API void* JS_malloc(JSContext* cx, size_t nbytes);
 
 extern JS_PUBLIC_API void* JS_realloc(JSContext* cx, void* p, size_t oldBytes,

js/public/TypeDecls.h

@@ -27,13 +27,14 @@ typedef uint8_t jsbytecode;
 class JSAtom;
 struct JSContext;
 class JSFunction;
+class JSFreeOp;
 class JSObject;
 struct JSRuntime;
 class JSScript;
 class JSString;
-struct JSFreeOp;
 
 namespace js {
+using FreeOp = JSFreeOp;
 class TempAllocPolicy;
 };  // namespace js
 

js/src/builtin/ModuleObject.cpp

@@ -679,9 +679,8 @@ void ModuleNamespaceObject::ProxyHandler::trace(JSTracer* trc,
   }
 }
 
-void ModuleNamespaceObject::ProxyHandler::finalize(JSFreeOp* fopArg,
+void ModuleNamespaceObject::ProxyHandler::finalize(JSFreeOp* fop,
                                                    JSObject* proxy) const {
-  FreeOp* fop = FreeOp::get(fopArg);
   auto& self = proxy->as<ModuleNamespaceObject>();
 
   if (self.hasBindings()) {

js/src/builtin/intl/Collator.h

@@ -17,7 +17,6 @@
 
 namespace js {
 
-class FreeOp;
 class GlobalObject;
 
 /******************** Collator ********************/

js/src/builtin/intl/DateTimeFormat.h

@@ -17,7 +17,6 @@
 
 namespace js {
 
-class FreeOp;
 class GlobalObject;
 
 class DateTimeFormatObject : public NativeObject {

js/src/builtin/intl/NumberFormat.h

@@ -24,7 +24,6 @@ struct UNumberFormatter;
 namespace js {
 
 class ArrayObject;
-class FreeOp;
 
 class NumberFormatObject : public NativeObject {
  public:

js/src/builtin/intl/PluralRules.h

@@ -20,8 +20,6 @@ struct UPluralRules;
 
 namespace js {
 
-class FreeOp;
-
 class PluralRulesObject : public NativeObject {
  public:
   static const Class class_;

js/src/builtin/intl/RelativeTimeFormat.h

@@ -17,8 +17,6 @@
 
 namespace js {
 
-class FreeOp;
-
 class RelativeTimeFormatObject : public NativeObject {
  public:
   static const Class class_;

js/src/ctypes/CTypes.cpp

@@ -4365,9 +4365,8 @@ static void FinalizeFFIType(JSFreeOp* fop, JSObject* obj, const Value& slot,
                             size_t elementCount) {
   ffi_type* ffiType = static_cast<ffi_type*>(slot.toPrivate());
   size_t size = elementCount * sizeof(ffi_type*);
-  FreeOp::get(fop)->free_(obj, ffiType->elements, size,
-                          MemoryUse::CTypeFFITypeElements);
-  FreeOp::get(fop)->delete_(obj, ffiType, MemoryUse::CTypeFFIType);
+  fop->free_(obj, ffiType->elements, size, MemoryUse::CTypeFFITypeElements);
+  fop->delete_(obj, ffiType, MemoryUse::CTypeFFIType);
 }
 
 void CType::Finalize(JSFreeOp* fop, JSObject* obj) {
@@ -4384,7 +4383,7 @@ void CType::Finalize(JSFreeOp* fop, JSObject* obj) {
       slot = JS_GetReservedSlot(obj, SLOT_FNINFO);
       if (!slot.isUndefined()) {
         auto fninfo = static_cast<FunctionInfo*>(slot.toPrivate());
-        FreeOp::get(fop)->delete_(obj, fninfo, MemoryUse::CTypeFunctionInfo);
+        fop->delete_(obj, fninfo, MemoryUse::CTypeFunctionInfo);
       }
       break;
     }
@@ -4397,7 +4396,7 @@ void CType::Finalize(JSFreeOp* fop, JSObject* obj) {
       if (!slot.isUndefined()) {
         auto info = static_cast<FieldInfoHash*>(slot.toPrivate());
         fieldCount = info->count();
-        FreeOp::get(fop)->delete_(obj, info, MemoryUse::CTypeFieldInfo);
+        fop->delete_(obj, info, MemoryUse::CTypeFieldInfo);
       }
 
       // Free the ffi_type info.
@@ -7268,7 +7267,7 @@ void CClosure::Finalize(JSFreeOp* fop, JSObject* obj) {
   }
 
   ClosureInfo* cinfo = static_cast<ClosureInfo*>(slot.toPrivate());
-  FreeOp::get(fop)->delete_(obj, cinfo, MemoryUse::CClosureInfo);
+  fop->delete_(obj, cinfo, MemoryUse::CClosureInfo);
 }
 
 void CClosure::ClosureStub(ffi_cif* cif, void* result, void** args,
@@ -7532,9 +7531,9 @@ void CData::Finalize(JSFreeOp* fop, JSObject* obj) {
   if (owns) {
     JSObject* typeObj = &JS_GetReservedSlot(obj, SLOT_CTYPE).toObject();
     size_t size = CType::GetSize(typeObj);
-    FreeOp::get(fop)->free_(obj, *buffer, size, MemoryUse::CDataBuffer);
+    fop->free_(obj, *buffer, size, MemoryUse::CDataBuffer);
   }
-  FreeOp::get(fop)->delete_(obj, buffer, MemoryUse::CDataBufferPtr);
+  fop->delete_(obj, buffer, MemoryUse::CDataBufferPtr);
 }
 
 JSObject* CData::GetCType(JSObject* dataObj) {
@@ -8463,7 +8462,7 @@ void Int64Base::Finalize(JSFreeOp* fop, JSObject* obj) {
   }
 
   uint64_t* buffer = static_cast<uint64_t*>(slot.toPrivate());
-  FreeOp::get(fop)->delete_(obj, buffer, MemoryUse::CTypesInt64);
+  fop->delete_(obj, buffer, MemoryUse::CTypesInt64);
 }
 
 uint64_t Int64Base::GetInt(JSObject* obj) {

js/src/gc/ArenaList.h

@@ -18,7 +18,6 @@
 
 namespace js {
 
-class FreeOp;
 class Nursery;
 class TenuringTracer;
 

js/src/gc/FreeOp-inl.h

@@ -12,51 +12,45 @@
 #include "gc/ZoneAllocator.h"
 #include "js/RefCounted.h"
 
-namespace js {
-
-inline void FreeOp::free_(gc::Cell* cell, void* p, size_t nbytes,
-                          MemoryUse use) {
+inline void JSFreeOp::free_(Cell* cell, void* p, size_t nbytes, MemoryUse use) {
   if (p) {
     removeCellMemory(cell, nbytes, use);
     js_free(p);
   }
 }
 
-inline void FreeOp::freeLater(gc::Cell* cell, void* p, size_t nbytes,
-                              MemoryUse use) {
+inline void JSFreeOp::freeLater(Cell* cell, void* p, size_t nbytes,
+                                MemoryUse use) {
   if (p) {
     removeCellMemory(cell, nbytes, use);
     queueForFreeLater(p);
   }
 }
 
-inline void FreeOp::queueForFreeLater(void* p) {
-  // Default FreeOps are not constructed on the stack, and will hold onto the
+inline void JSFreeOp::queueForFreeLater(void* p) {
+  // Default JSFreeOps are not constructed on the stack, and will hold onto the
   // pointers to free indefinitely.
   MOZ_ASSERT(!isDefaultFreeOp());
 
   // It's not safe to free this allocation immediately, so we must crash if we
   // can't append to the list.
-  AutoEnterOOMUnsafeRegion oomUnsafe;
+  js::AutoEnterOOMUnsafeRegion oomUnsafe;
   if (!freeLaterList.append(p)) {
-    oomUnsafe.crash("FreeOp::freeLater");
+    oomUnsafe.crash("JSFreeOp::freeLater");
   }
 }
 
 template <class T>
-inline void FreeOp::release(gc::Cell* cell, T* p, size_t nbytes,
-                            MemoryUse use) {
+inline void JSFreeOp::release(Cell* cell, T* p, size_t nbytes, MemoryUse use) {
   if (p) {
     removeCellMemory(cell, nbytes, use);
     p->Release();
   }
 }
 
-inline void FreeOp::removeCellMemory(gc::Cell* cell, size_t nbytes,
-                                     MemoryUse use) {
+inline void JSFreeOp::removeCellMemory(Cell* cell, size_t nbytes,
+                                       MemoryUse use) {
   RemoveCellMemory(cell, nbytes, use, isCollecting());
 }
 
-}  // namespace js
-
-#endif  // gc_FreeOp_inl_h
+#endif  // gc_JSFreeOp_inl_h

js/src/gc/FreeOp.h

@@ -19,10 +19,10 @@
 struct JSRuntime;
 
 namespace js {
-
 namespace gc {
 class AutoSetThreadIsPerformingGC;
 }  // namespace gc
+}  // namespace js
 
 /*
  * A FreeOp can do one thing: free memory. For convenience, it has delete_
@@ -31,19 +31,32 @@ class AutoSetThreadIsPerformingGC;
  * FreeOp is passed to finalizers and other sweep-phase hooks so that we do not
  * need to pass a JSContext to those hooks.
  */
-class FreeOp : public JSFreeOp {
-  Vector<void*, 0, SystemAllocPolicy> freeLaterList;
-  jit::JitPoisonRangeVector jitPoisonRanges;
+class JSFreeOp {
+  using Cell = js::gc::Cell;
+  using MemoryUse = js::MemoryUse;
+
+  JSRuntime* runtime_;
+
+  // We may accumulate a set of deferred free operations to be performed when
+  // the JSFreeOp is destroyed. This only applies to non-default JSFreeOps that
+  // are stack allocated and used during GC sweeping.
+  js::Vector<void*, 0, js::SystemAllocPolicy> freeLaterList;
+
+  js::jit::JitPoisonRangeVector jitPoisonRanges;
+
   const bool isDefault;
   bool isCollecting_;
 
-  friend class gc::AutoSetThreadIsPerformingGC;
+  friend class js::gc::AutoSetThreadIsPerformingGC;
 
  public:
-  static FreeOp* get(JSFreeOp* fop) { return static_cast<FreeOp*>(fop); }
+  explicit JSFreeOp(JSRuntime* maybeRuntime, bool isDefault = false);
+  ~JSFreeOp();
 
-  explicit FreeOp(JSRuntime* maybeRuntime, bool isDefault = false);
-  ~FreeOp();
+  JSRuntime* runtime() const {
+    MOZ_ASSERT(runtime_);
+    return runtime_;
+  }
 
   bool onMainThread() const { return runtime_ != nullptr; }
 
@@ -65,7 +78,7 @@ class FreeOp : public JSFreeOp {
   // The memory should have been associated with the GC thing using
   // js::InitReservedSlot or js::InitObjectPrivate, or possibly
   // js::AddCellMemory.
-  void free_(gc::Cell* cell, void* p, size_t nbytes, MemoryUse use);
+  void free_(Cell* cell, void* p, size_t nbytes, MemoryUse use);
 
   // Deprecated. Where possible, memory should be tracked against the owning GC
   // thing by calling js::AddCellMemory and the memory freed with freeLater()
@@ -81,9 +94,9 @@ class FreeOp : public JSFreeOp {
   //
   // This is used to ensure that copy-on-write object elements are not freed
   // until all objects that refer to them have been finalized.
-  void freeLater(gc::Cell* cell, void* p, size_t nbytes, MemoryUse use);
+  void freeLater(Cell* cell, void* p, size_t nbytes, MemoryUse use);
 
-  bool appendJitPoisonRange(const jit::JitPoisonRange& range) {
+  bool appendJitPoisonRange(const js::jit::JitPoisonRange& range) {
     // FreeOps other than the defaultFreeOp() are constructed on the stack,
     // and won't hold onto the pointers to free indefinitely.
     MOZ_ASSERT(!isDefaultFreeOp());
@@ -109,7 +122,7 @@ class FreeOp : public JSFreeOp {
   // js::InitReservedSlot or js::InitObjectPrivate, or possibly
   // js::AddCellMemory.
   template <class T>
-  void delete_(gc::Cell* cell, T* p, MemoryUse use) {
+  void delete_(Cell* cell, T* p, MemoryUse use) {
     delete_(cell, p, sizeof(T), use);
   }
 
@@ -120,7 +133,7 @@ class FreeOp : public JSFreeOp {
   // js::InitReservedSlot or js::InitObjectPrivate, or possibly
   // js::AddCellMemory.
   template <class T>
-  void delete_(gc::Cell* cell, T* p, size_t nbytes, MemoryUse use) {
+  void delete_(Cell* cell, T* p, size_t nbytes, MemoryUse use) {
     if (p) {
       p->~T();
       free_(cell, p, nbytes, use);
@@ -139,7 +152,7 @@ class FreeOp : public JSFreeOp {
   // each zone. If this is the case then some other form of accounting would be
   // more appropriate.
   template <class T>
-  void release(gc::Cell* cell, T* p, MemoryUse use) {
+  void release(Cell* cell, T* p, MemoryUse use) {
     release(cell, p, sizeof(T), use);
   }
 
@@ -150,16 +163,18 @@ class FreeOp : public JSFreeOp {
   // js::InitReservedSlot or js::InitObjectPrivate, or possibly
   // js::AddCellMemory.
   template <class T>
-  void release(gc::Cell* cell, T* p, size_t nbytes, MemoryUse use);
+  void release(Cell* cell, T* p, size_t nbytes, MemoryUse use);
 
   // Update the memory accounting for a GC for memory freed by some other
   // method.
-  void removeCellMemory(gc::Cell* cell, size_t nbytes, MemoryUse use);
+  void removeCellMemory(Cell* cell, size_t nbytes, MemoryUse use);
 
  private:
   void queueForFreeLater(void* p);
 };
 
+namespace js {
+using FreeOp = JSFreeOp;
 }  // namespace js
 
 #endif  // gc_FreeOp_h

js/src/gc/GC.h

@@ -25,7 +25,6 @@ namespace js {
 
 class AccessorShape;
 class FatInlineAtom;
-class FreeOp;
 class NormalAtom;
 
 class Nursery;

js/src/gc/Heap.h

@@ -21,7 +21,6 @@ namespace js {
 
 class AutoLockGC;
 class AutoLockGCBgAlloc;
-class FreeOp;
 class NurseryDecommitTask;
 
 namespace gc {

js/src/jsapi.cpp

@@ -1146,7 +1146,7 @@ JS_PUBLIC_API void* JS_string_realloc(JSContext* cx, void* p, size_t oldBytes,
 JS_PUBLIC_API void JS_string_free(JSContext* cx, void* p) { return js_free(p); }
 
 JS_PUBLIC_API void JS_freeop(JSFreeOp* fop, void* p) {
-  return FreeOp::get(fop)->freeUntracked(p);
+  return fop->freeUntracked(p);
 }
 
 JS_PUBLIC_API void JS::AddAssociatedMemory(JSObject* obj, size_t nbytes,

js/src/jsapi.h

@@ -52,7 +52,6 @@
 
 /************************************************************************/
 
-struct JSFreeOp;
 struct JSFunctionSpec;
 struct JSPropertySpec;
 

js/src/vm/Runtime.cpp

@@ -567,12 +567,12 @@ void JSRuntime::traceSharedIntlData(JSTracer* trc) {
   sharedIntlData.ref().trace(trc);
 }
 
-FreeOp::FreeOp(JSRuntime* maybeRuntime, bool isDefault)
-    : JSFreeOp(maybeRuntime), isDefault(isDefault), isCollecting_(!isDefault) {
+JSFreeOp::JSFreeOp(JSRuntime* maybeRuntime, bool isDefault)
+    : runtime_(maybeRuntime), isDefault(isDefault), isCollecting_(!isDefault) {
   MOZ_ASSERT_IF(maybeRuntime, CurrentThreadCanAccessRuntime(maybeRuntime));
 }
 
-FreeOp::~FreeOp() {
+JSFreeOp::~JSFreeOp() {
   for (size_t i = 0; i < freeLaterList.length(); i++) {
     freeUntracked(freeLaterList[i]);
   }