Bug 1037106 - Use UniquePtr to manage ownership of the Debugger C++ class instance created for a new Debugger object, until it can be stored in the Debugger object's private slot (and owned by it). r=jimb, r=luke

--HG--
extra : rebase_source : 0f283bdd22e0e1d320f12b60e9fe98b5a606aa8d

js/public/Utility.h

@@ -170,6 +170,9 @@ static inline void js_free(void* p)
 #define JS_NEW_BODY(allocator, t, parms)                                       \
     void *memory = allocator(sizeof(t));                                       \
     return memory ? new(memory) t parms : nullptr;
+#define JS_MAKE_BODY(newname, T, parms)                                        \
+    T *ptr = newname<T> parms;                                                 \
+    return mozilla::UniquePtr<T, JS::DeletePolicy<T>>(ptr);
 
 /*
  * Given a class which should provide 'new' methods, add
@@ -324,6 +327,187 @@ static inline void js_free(void* p)
                      mozilla::Forward<P12>(p12)))\
     }\
 
+/*
+ * Given a class which should provide 'make' methods, add
+ * JS_DECLARE_MAKE_METHODS (see JSContext for a usage example).  This method
+ * is functionally the same as JS_DECLARE_NEW_METHODS: it just declares methods
+ * that return mozilla::UniquePtr instances that will singly-manage ownership
+ * of the created object.  This adds makes with up to 12 parameters.  Add more
+ * versions below if you need more than 12 parameters.
+ *
+ * Note: Do not add a ; at the end of a use of JS_DECLARE_MAKE_METHODS,
+ * or the build will break.
+ */
+#define JS_DECLARE_MAKE_METHODS(MAKENAME, NEWNAME, QUALIFIERS)\
+    template <class T> \
+    QUALIFIERS \
+    mozilla::UniquePtr<T, JS::DeletePolicy<T>> \
+    MAKENAME() MOZ_HEAP_ALLOCATOR {\
+        JS_MAKE_BODY(NEWNAME, T, ())\
+    }\
+\
+    template <class T, class P1> \
+    QUALIFIERS \
+    mozilla::UniquePtr<T, JS::DeletePolicy<T>> \
+    MAKENAME(P1 &&p1) MOZ_HEAP_ALLOCATOR {\
+        JS_MAKE_BODY(NEWNAME, T,\
+                    (mozilla::Forward<P1>(p1)))\
+    }\
+\
+    template <class T, class P1, class P2> \
+    QUALIFIERS \
+mozilla::UniquePtr<T, JS::DeletePolicy<T>> \
+    MAKENAME(P1 &&p1, P2 &&p2) MOZ_HEAP_ALLOCATOR {\
+        JS_MAKE_BODY(NEWNAME, T,\
+                    (mozilla::Forward<P1>(p1),\
+                     mozilla::Forward<P2>(p2)))\
+    }\
+\
+    template <class T, class P1, class P2, class P3> \
+    QUALIFIERS \
+    mozilla::UniquePtr<T, JS::DeletePolicy<T>> \
+    MAKENAME(P1 &&p1, P2 &&p2, P3 &&p3) MOZ_HEAP_ALLOCATOR {\
+        JS_MAKE_BODY(NEWNAME, T,\
+                    (mozilla::Forward<P1>(p1),\
+                     mozilla::Forward<P2>(p2),\
+                     mozilla::Forward<P3>(p3)))\
+    }\
+\
+    template <class T, class P1, class P2, class P3, class P4> \
+    QUALIFIERS \
+    mozilla::UniquePtr<T, JS::DeletePolicy<T>> \
+    MAKENAME(P1 &&p1, P2 &&p2, P3 &&p3, P4 &&p4) MOZ_HEAP_ALLOCATOR {\
+        JS_MAKE_BODY(NEWNAME, T,\
+                    (mozilla::Forward<P1>(p1),\
+                     mozilla::Forward<P2>(p2),\
+                     mozilla::Forward<P3>(p3),\
+                     mozilla::Forward<P4>(p4)))\
+    }\
+\
+    template <class T, class P1, class P2, class P3, class P4, class P5> \
+    QUALIFIERS \
+    mozilla::UniquePtr<T, JS::DeletePolicy<T>> \
+    MAKENAME(P1 &&p1, P2 &&p2, P3 &&p3, P4 &&p4, P5 &&p5) MOZ_HEAP_ALLOCATOR {\
+        JS_MAKE_BODY(NEWNAME, T,\
+                    (mozilla::Forward<P1>(p1),\
+                     mozilla::Forward<P2>(p2),\
+                     mozilla::Forward<P3>(p3),\
+                     mozilla::Forward<P4>(p4),\
+                     mozilla::Forward<P5>(p5)))\
+    }\
+\
+    template <class T, class P1, class P2, class P3, class P4, class P5, class P6> \
+    QUALIFIERS \
+    mozilla::UniquePtr<T, JS::DeletePolicy<T>> \
+    MAKENAME(P1 &&p1, P2 &&p2, P3 &&p3, P4 &&p4, P5 &&p5, P6 &&p6) MOZ_HEAP_ALLOCATOR {\
+        JS_MAKE_BODY(NEWNAME, T,\
+                    (mozilla::Forward<P1>(p1),\
+                     mozilla::Forward<P2>(p2),\
+                     mozilla::Forward<P3>(p3),\
+                     mozilla::Forward<P4>(p4),\
+                     mozilla::Forward<P5>(p5),\
+                     mozilla::Forward<P6>(p6)))\
+    }\
+\
+    template <class T, class P1, class P2, class P3, class P4, class P5, class P6, class P7> \
+    QUALIFIERS \
+    mozilla::UniquePtr<T, JS::DeletePolicy<T>> \
+    MAKENAME(P1 &&p1, P2 &&p2, P3 &&p3, P4 &&p4, P5 &&p5, P6 &&p6, P7 &&p7) MOZ_HEAP_ALLOCATOR {\
+        JS_MAKE_BODY(NEWNAME, T,\
+                    (mozilla::Forward<P1>(p1),\
+                     mozilla::Forward<P2>(p2),\
+                     mozilla::Forward<P3>(p3),\
+                     mozilla::Forward<P4>(p4),\
+                     mozilla::Forward<P5>(p5),\
+                     mozilla::Forward<P6>(p6),\
+                     mozilla::Forward<P7>(p7)))\
+    }\
+\
+    template <class T, class P1, class P2, class P3, class P4, class P5, class P6, class P7, class P8> \
+    QUALIFIERS \
+    mozilla::UniquePtr<T, JS::DeletePolicy<T>> \
+    MAKENAME(P1 &&p1, P2 &&p2, P3 &&p3, P4 &&p4, P5 &&p5, P6 &&p6, P7 &&p7, P8 &&p8) MOZ_HEAP_ALLOCATOR {\
+        JS_MAKE_BODY(NEWNAME, T,\
+                    (mozilla::Forward<P1>(p1),\
+                     mozilla::Forward<P2>(p2),\
+                     mozilla::Forward<P3>(p3),\
+                     mozilla::Forward<P4>(p4),\
+                     mozilla::Forward<P5>(p5),\
+                     mozilla::Forward<P6>(p6),\
+                     mozilla::Forward<P7>(p7),\
+                     mozilla::Forward<P8>(p8)))\
+    }\
+\
+    template <class T, class P1, class P2, class P3, class P4, class P5, class P6, class P7, class P8, class P9> \
+    QUALIFIERS \
+    mozilla::UniquePtr<T, JS::DeletePolicy<T>> \
+    MAKENAME(P1 &&p1, P2 &&p2, P3 &&p3, P4 &&p4, P5 &&p5, P6 &&p6, P7 &&p7, P8 &&p8, P9 &&p9) MOZ_HEAP_ALLOCATOR {\
+        JS_MAKE_BODY(NEWNAME, T,\
+                    (mozilla::Forward<P1>(p1),\
+                     mozilla::Forward<P2>(p2),\
+                     mozilla::Forward<P3>(p3),\
+                     mozilla::Forward<P4>(p4),\
+                     mozilla::Forward<P5>(p5),\
+                     mozilla::Forward<P6>(p6),\
+                     mozilla::Forward<P7>(p7),\
+                     mozilla::Forward<P8>(p8),\
+                     mozilla::Forward<P9>(p9)))\
+    }\
+\
+    template <class T, class P1, class P2, class P3, class P4, class P5, class P6, class P7, class P8, class P9, class P10> \
+    QUALIFIERS \
+    mozilla::UniquePtr<T, JS::DeletePolicy<T>> \
+    MAKENAME(P1 &&p1, P2 &&p2, P3 &&p3, P4 &&p4, P5 &&p5, P6 &&p6, P7 &&p7, P8 &&p8, P9 &&p9, P10 &&p10) MOZ_HEAP_ALLOCATOR {\
+        JS_MAKE_BODY(NEWNAME, T,\
+                    (mozilla::Forward<P1>(p1),\
+                     mozilla::Forward<P2>(p2),\
+                     mozilla::Forward<P3>(p3),\
+                     mozilla::Forward<P4>(p4),\
+                     mozilla::Forward<P5>(p5),\
+                     mozilla::Forward<P6>(p6),\
+                     mozilla::Forward<P7>(p7),\
+                     mozilla::Forward<P8>(p8),\
+                     mozilla::Forward<P9>(p9),\
+                     mozilla::Forward<P10>(p10)))\
+    }\
+\
+    template <class T, class P1, class P2, class P3, class P4, class P5, class P6, class P7, class P8, class P9, class P10, class P11> \
+    QUALIFIERS \
+    mozilla::UniquePtr<T, JS::DeletePolicy<T>> \
+    MAKENAME(P1 &&p1, P2 &&p2, P3 &&p3, P4 &&p4, P5 &&p5, P6 &&p6, P7 &&p7, P8 &&p8, P9 &&p9, P10 &&p10, P11 &&p11) MOZ_HEAP_ALLOCATOR {\
+        JS_MAKE_BODY(NEWNAME, T,\
+                    (mozilla::Forward<P1>(p1),\
+                     mozilla::Forward<P2>(p2),\
+                     mozilla::Forward<P3>(p3),\
+                     mozilla::Forward<P4>(p4),\
+                     mozilla::Forward<P5>(p5),\
+                     mozilla::Forward<P6>(p6),\
+                     mozilla::Forward<P7>(p7),\
+                     mozilla::Forward<P8>(p8),\
+                     mozilla::Forward<P9>(p9),\
+                     mozilla::Forward<P10>(p10),\
+                     mozilla::Forward<P11>(p11)))\
+    }\
+\
+    template <class T, class P1, class P2, class P3, class P4, class P5, class P6, class P7, class P8, class P9, class P10, class P11, class P12> \
+    QUALIFIERS \
+    mozilla::UniquePtr<T, JS::DeletePolicy<T>> \
+    MAKENAME(P1 &&p1, P2 &&p2, P3 &&p3, P4 &&p4, P5 &&p5, P6 &&p6, P7 &&p7, P8 &&p8, P9 &&p9, P10 &&p10, P11 &&p11, P12 &&p12) MOZ_HEAP_ALLOCATOR {\
+        JS_MAKE_BODY(NEWNAME, T,\
+                    (mozilla::Forward<P1>(p1),\
+                     mozilla::Forward<P2>(p2),\
+                     mozilla::Forward<P3>(p3),\
+                     mozilla::Forward<P4>(p4),\
+                     mozilla::Forward<P5>(p5),\
+                     mozilla::Forward<P6>(p6),\
+                     mozilla::Forward<P7>(p7),\
+                     mozilla::Forward<P8>(p8),\
+                     mozilla::Forward<P9>(p9),\
+                     mozilla::Forward<P10>(p10),\
+                     mozilla::Forward<P11>(p11),\
+                     mozilla::Forward<P12>(p12)))\
+    }\
+
 JS_DECLARE_NEW_METHODS(js_new, js_malloc, static MOZ_ALWAYS_INLINE)
 
 template <class T>

js/src/vm/Debugger.cpp

@@ -12,6 +12,7 @@
 #include "jsnum.h"
 #include "jsobj.h"
 #include "jswrapper.h"
+
 #include "frontend/BytecodeCompiler.h"
 #include "gc/Marking.h"
 #include "jit/BaselineJIT.h"
@@ -19,10 +20,12 @@
 #include "vm/ArgumentsObject.h"
 #include "vm/DebuggerMemory.h"
 #include "vm/WrapperObject.h"
+
 #include "jsgcinlines.h"
 #include "jsobjinlines.h"
 #include "jsopcodeinlines.h"
 #include "jsscriptinlines.h"
+
 #include "vm/ObjectImpl-inl.h"
 #include "vm/Stack-inl.h"
 
@@ -2246,21 +2249,18 @@ Debugger::construct(JSContext *cx, unsigned argc, Value *vp)
     obj->setReservedSlot(JSSLOT_DEBUG_MEMORY_INSTANCE, ObjectValue(*memory));
 
     /* Construct the underlying C++ object. */
-    Debugger *dbg = cx->new_<Debugger>(cx, obj.get());
+    auto dbg = cx->make_unique<Debugger>(cx, obj.get());
-    if (!dbg)
+    if (!dbg || !dbg->init(cx))
         return false;
-    if (!dbg->init(cx)) {
+
-        js_delete(dbg);
+    Debugger *debugger = dbg.release();
-        return false;
+    obj->setPrivate(debugger); // owns the released pointer
-    }
-    obj->setPrivate(dbg);
-    /* Now the JSObject owns the js::Debugger instance, so we needn't delete it. */
 
     /* Add the initial debuggees, if any. */
     for (unsigned i = 0; i < args.length(); i++) {
         Rooted<GlobalObject*>
             debuggee(cx, &args[i].toObject().as<ProxyObject>().private_().toObject().global());
-        if (!dbg->addDebuggeeGlobal(cx, debuggee))
+        if (!debugger->addDebuggeeGlobal(cx, debuggee))
             return false;
     }
 

js/src/vm/MallocProvider.h

@@ -133,6 +133,7 @@ struct MallocProvider
     }
 
     JS_DECLARE_NEW_METHODS(new_, malloc_, MOZ_ALWAYS_INLINE)
+    JS_DECLARE_MAKE_METHODS(make_unique, new_, MOZ_ALWAYS_INLINE)
 };
 
 } /* namespace js */