Bug 1149397 - JS::ubi::Node::edges should return a mozilla::UniquePtr; r=jimb

js/public/UbiNode.h

@@ -13,6 +13,7 @@
 #include "mozilla/Maybe.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/Move.h"
+#include "mozilla/UniquePtr.h"
 
 #include "jspubtd.h"
 
@@ -92,7 +93,7 @@
 // represented by a "rope", a structure that points to the two original
 // strings.
 //
-
+//
 // We intend to use ubi::Node to write tools that report memory usage, so it's
 // important that ubi::Node accurately portray how much memory nodes consume.
 // Thus, for example, when data that apparently belongs to multiple nodes is
@@ -141,11 +142,23 @@
 namespace JS {
 namespace ubi {
 
-using mozilla::Maybe;
-
 class Edge;
 class EdgeRange;
 
+}
+}
+
+namespace mozilla {
+template<>
+class DefaultDelete<JS::ubi::EdgeRange> : public JS::DeletePolicy<JS::ubi::EdgeRange> { };
+}
+
+namespace JS {
+namespace ubi {
+
+using mozilla::Maybe;
+using mozilla::UniquePtr;
+
 // The base class implemented by each ubi::Node referent type. Subclasses must
 // not add data members to this class.
 class Base {
@@ -188,13 +201,11 @@ class Base {
     virtual size_t size(mozilla::MallocSizeOf mallocSizeof) const { return 0; }
 
     // Return an EdgeRange that initially contains all the referent's outgoing
-    // edges. The EdgeRange should be freed with 'js_delete'. (You could use
-    // ScopedDJSeletePtr<EdgeRange> to manage it.) On OOM, report an exception
-    // on |cx| and return nullptr.
+    // edges. The caller takes ownership of the EdgeRange.
     //
     // If wantNames is true, compute names for edges. Doing so can be expensive
     // in time and memory.
-    virtual EdgeRange* edges(JSContext* cx, bool wantNames) const = 0;
+    virtual UniquePtr<EdgeRange> edges(JSContext* cx, bool wantNames) const = 0;
 
     // Return the Zone to which this node's referent belongs, or nullptr if the
     // referent is not of a type allocated in SpiderMonkey Zones.
@@ -341,7 +352,7 @@ class Node {
         return base()->size(mallocSizeof);
     }
 
-    EdgeRange* edges(JSContext* cx, bool wantNames = true) const {
+    UniquePtr<EdgeRange> edges(JSContext* cx, bool wantNames = true) const {
         return base()->edges(cx, wantNames);
     }
 
@@ -521,7 +532,7 @@ class MOZ_STACK_CLASS RootList {
 
 template<>
 struct Concrete<RootList> : public Base {
-    EdgeRange* edges(JSContext* cx, bool wantNames) const override;
+    UniquePtr<EdgeRange> edges(JSContext* cx, bool wantNames) const override;
     const char16_t* typeName() const override { return concreteTypeName; }
 
   protected:
@@ -538,7 +549,7 @@ struct Concrete<RootList> : public Base {
 template<typename Referent>
 class TracerConcrete : public Base {
     const char16_t* typeName() const override { return concreteTypeName; }
-    EdgeRange* edges(JSContext*, bool wantNames) const override;
+    UniquePtr<EdgeRange> edges(JSContext*, bool wantNames) const override;
     JS::Zone* zone() const override;
 
   protected:
@@ -591,7 +602,7 @@ template<>
 class Concrete<void> : public Base {
     const char16_t* typeName() const override;
     size_t size(mozilla::MallocSizeOf mallocSizeOf) const override;
-    EdgeRange* edges(JSContext* cx, bool wantNames) const override;
+    UniquePtr<EdgeRange> edges(JSContext* cx, bool wantNames) const override;
     JS::Zone* zone() const override;
     JSCompartment* compartment() const override;
 

js/public/UbiNodeTraverse.h

@@ -126,7 +126,7 @@ struct BreadthFirst {
             pending.popFront();
 
             // Get a range containing all origin's outgoing edges.
-            js::ScopedJSDeletePtr<EdgeRange> range(origin.edges(cx, wantNames));
+            auto range = origin.edges(cx, wantNames);
             if (!range)
                 return false;
 

js/src/vm/UbiNode.cpp

@@ -9,7 +9,6 @@
 #include "mozilla/Assertions.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/Scoped.h"
-#include "mozilla/UniquePtr.h"
 
 #include "jscntxt.h"
 #include "jsobj.h"
@@ -32,6 +31,7 @@
 #include "vm/Debugger-inl.h"
 
 using mozilla::Some;
+using mozilla::UniquePtr;
 using JS::HandleValue;
 using JS::Value;
 using JS::ZoneSet;
@@ -46,10 +46,14 @@ using JS::ubi::TracerConcreteWithCompartment;
 
 // All operations on null ubi::Nodes crash.
 const char16_t* Concrete<void>::typeName() const          { MOZ_CRASH("null ubi::Node"); }
-EdgeRange* Concrete<void>::edges(JSContext*, bool) const { MOZ_CRASH("null ubi::Node"); }
 JS::Zone* Concrete<void>::zone() const                    { MOZ_CRASH("null ubi::Node"); }
 JSCompartment* Concrete<void>::compartment() const        { MOZ_CRASH("null ubi::Node"); }
 
+UniquePtr<EdgeRange>
+Concrete<void>::edges(JSContext*, bool) const {
+    MOZ_CRASH("null ubi::Node");
+}
+
 size_t
 Concrete<void>::size(mozilla::MallocSizeOf mallocSizeof) const
 {
@@ -203,16 +207,17 @@ TracerConcrete<Referent>::zone() const
 }
 
 template<typename Referent>
-EdgeRange*
+UniquePtr<EdgeRange>
 TracerConcrete<Referent>::edges(JSContext* cx, bool wantNames) const {
-    js::ScopedJSDeletePtr<SimpleEdgeRange> r(js_new<SimpleEdgeRange>(cx));
-    if (!r)
+    UniquePtr<SimpleEdgeRange, JS::DeletePolicy<SimpleEdgeRange>> range(
+      cx->new_<SimpleEdgeRange>(cx));
+    if (!range)
         return nullptr;
 
-    if (!r->init(cx, ptr, ::js::gc::MapTypeToTraceKind<Referent>::kind, wantNames))
+    if (!range->init(cx, ptr, ::js::gc::MapTypeToTraceKind<Referent>::kind, wantNames))
         return nullptr;
 
-    return r.forget();
+    return UniquePtr<EdgeRange>(range.release());
 }
 
 template<typename Referent>
@@ -347,7 +352,7 @@ RootList::addRoot(Node node, const char16_t* edgeName)
     MOZ_ASSERT(noGC.isSome());
     MOZ_ASSERT_IF(wantNames, edgeName);
 
-    mozilla::UniquePtr<char16_t[], JS::FreePolicy> name;
+    UniquePtr<char16_t[], JS::FreePolicy> name;
     if (edgeName) {
         name = DuplicateString(cx, edgeName);
         if (!name)
@@ -379,10 +384,10 @@ class PreComputedEdgeRange : public EdgeRange {
 
 const char16_t Concrete<RootList>::concreteTypeName[] = MOZ_UTF16("RootList");
 
-EdgeRange*
+UniquePtr<EdgeRange>
 Concrete<RootList>::edges(JSContext* cx, bool wantNames) const {
     MOZ_ASSERT_IF(wantNames, get().wantNames);
-    return js_new<PreComputedEdgeRange>(cx, get().edges);
+    return UniquePtr<EdgeRange>(cx->new_<PreComputedEdgeRange>(cx, get().edges));
 }
 
 } // namespace ubi