For bug 784293, I need a rootable <JSObject*, JSOBject*> hashmap. This is the bug for implementing it.

And this is the patch implementing it. I hope it's not too ridiculously wrong.

Comment on attachment 690876 [details] [diff] [review] Implement AutoHashMapRooter and AutoObjectObjectHashMap. r= Review of attachment 690876 [details] [diff] [review]: ----------------------------------------------------------------- This looks just about perfect, modulo a couple nits. ::: js/src/gc/RootMarking.cpp @@ +532,5 @@ > } > > + case OBJOBJHASHMAP: { > + AutoObjectObjectHashMap::HashMapImpl &map = static_cast<AutoObjectObjectHashMap *>(this)->map; > + for (AutoObjectObjectHashMap::Range r = map.all(); !r.empty(); r.popFront()) { Make this an Enum instead of a Range so that we can handle key movement later without needing to update this type. @@ +533,5 @@ > > + case OBJOBJHASHMAP: { > + AutoObjectObjectHashMap::HashMapImpl &map = static_cast<AutoObjectObjectHashMap *>(this)->map; > + for (AutoObjectObjectHashMap::Range r = map.all(); !r.empty(); r.popFront()) { > + MarkObjectRoot(trc, (JSObject **) &r.front().key, "AutoObjectObjectHashMap key"); Store r.front().key before marking it and JS_ASSERT that the value is unchanged after marking. This will very quickly remind us to revisit this spot when we turn on moving GC. @@ +534,5 @@ > + case OBJOBJHASHMAP: { > + AutoObjectObjectHashMap::HashMapImpl &map = static_cast<AutoObjectObjectHashMap *>(this)->map; > + for (AutoObjectObjectHashMap::Range r = map.all(); !r.empty(); r.popFront()) { > + MarkObjectRoot(trc, (JSObject **) &r.front().key, "AutoObjectObjectHashMap key"); > + MarkObjectRoot(trc, (JSObject **) &r.front().value, "AutoObjectObjectHashMap value"); This explicit cast should not be necessary after you update the types. ::: js/src/jsapi.h @@ +1041,5 @@ > JS_ASSERT(this == *stackTop); > *stackTop = down; > } > > + /* Implemented in gc/RootMarking.cpp. */ \o/ Nice catch! @@ +1351,5 @@ > + typedef typename HashMapImpl::AddPtr AddPtr; > + > + bool init(uint32_t len = 16) { > + return map.init(len); > + //FIXME: make GC safe You don't need to MakeRangeGCSafe for heap storage structures, so it's safe to remove this note. ::: js/src/jscntxt.h @@ +2143,5 @@ > > JS_DECL_USE_GUARD_OBJECT_NOTIFIER > }; > > +class AutoObjectObjectHashMap : public AutoHashMapRooter<JSObject *, JSObject *> These GC thing pointers are stored on the Heap, so they need to be wrapped in one of the types in the HeapPtr<T> class hierarchy. The KeyType should be EncapsulatedPtrObject and the ValueType should RelocatablePtrObject. I can explain why on IRC if you are interested. @@ +2147,5 @@ > +class AutoObjectObjectHashMap : public AutoHashMapRooter<JSObject *, JSObject *> > +{ > + public: > + explicit AutoObjectObjectHashMap(JSContext *cx > + JS_GUARD_OBJECT_NOTIFIER_PARAM) Make JS_GUARD... line up with JSContext. @@ +2148,5 @@ > +{ > + public: > + explicit AutoObjectObjectHashMap(JSContext *cx > + JS_GUARD_OBJECT_NOTIFIER_PARAM) > + : AutoHashMapRooter<JSObject *, JSObject *>(cx, OBJOBJHASHMAP) 2 space indent before the :

Comment on attachment 691995 [details] [diff] [review] Implement AutoHashMapRooter and AutoObjectObjectHashMap. r= As discussed on IRC, I implemented your feedback and changed the Auto*Vectors to use raw* types. I think that a quick interdiff review should be sufficient.

Comment on attachment 691995 [details] [diff] [review] Implement AutoHashMapRooter and AutoObjectObjectHashMap. r= Review of attachment 691995 [details] [diff] [review]: ----------------------------------------------------------------- Looks good!