Created attachment 640441 [details] [diff] [review] v0 Try run up at: https://tbpl.mozilla.org/?tree=Try&rev=ba57767df1f9
Comment on attachment 640441 [details] [diff] [review] v0 Review of attachment 640441 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/xpconnect/src/XPCMaps.cpp @@ +93,5 @@ > + if (JS_IsAboutToBeFinalized(wrapper->GetJSObjectPreserveColor())) > + dying->AppendElement(wrapper); > + } > + } > + wrapper = wrapper->GetNextWrapper(); This line should be inside the while loop. ::: js/xpconnect/src/XPCMaps.h @@ +604,5 @@ > typedef js::HashMap<JSObject *, JSObject *, js::PointerHasher<JSObject *, 3>, > js::SystemAllocPolicy> Map; > > public: > + static JSObject2JSObjectMap* newMap(int size) { Unrelated? @@ +664,5 @@ > } > > private: > JSObject2JSObjectMap() MOZ_DELETE; > + JSObject2JSObjectMap(int size) {} Should remove this constructor, then.
Created attachment 640653 [details] [diff] [review] v1: With fixes. (In reply to :Ms2ger from comment #1) > ::: js/xpconnect/src/XPCMaps.cpp > @@ +93,5 @@ > > + if (JS_IsAboutToBeFinalized(wrapper->GetJSObjectPreserveColor())) > > + dying->AppendElement(wrapper); > > + } > > + } > > + wrapper = wrapper->GetNextWrapper(); > > This line should be inside the while loop. Good catch. I made the mistake of trying to understand what this loop was doing instead of just copying it. > ::: js/xpconnect/src/XPCMaps.h > @@ +604,5 @@ > > typedef js::HashMap<JSObject *, JSObject *, js::PointerHasher<JSObject *, 3>, > > js::SystemAllocPolicy> Map; > > > > public: > > + static JSObject2JSObjectMap* newMap(int size) { > > Unrelated? A cleanup of the previous jsdhash I squashed. I figured while I was here I would clean this up a bit more. > @@ +664,5 @@ > > } > > > > private: > > JSObject2JSObjectMap() MOZ_DELETE; > > + JSObject2JSObjectMap(int size) {} > > Should remove this constructor, then. Was the whole point of the above change. Did it for the JSObject2WrappedJS but forgot it here for some reason.
Previous try run appears to have happened in the middle of complete infrastructure havoc: https://tbpl.mozilla.org/?tree=Try&rev=08c73baedbee
Comment on attachment 640653 [details] [diff] [review] v1: With fixes. Review of attachment 640653 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/xpconnect/src/XPCMaps.h @@ -74,5 @@ > > - ~JSObject2WrappedJSMap(); > -private: > - JSObject2WrappedJSMap(); // no implementation > - JSObject2WrappedJSMap(int size); Should the constructor stay private here to make sure consumers use newMap? @@ -668,5 @@ > } > } > > private: > - JSObject2JSObjectMap() MOZ_DELETE; Ditto here.
https://hg.mozilla.org/integration/mozilla-inbound/rev/ff15e729ec86
https://hg.mozilla.org/mozilla-central/rev/ff15e729ec86
or
