This is really just preparation. It reorganizes the HAS_MEMBER macros so you can have multiple of them in one struct (eg NativeHasMember<T>::GetParentObject/WrapObject/...), and for the others it uses ::value instead of ::Value, which is more in line with what mfbt does.

This involved moving the refcounting macros to some base classes (but they are never used as concrete classes).

By making sure that we null out the private if we create a binding object and binding initialization fails somewhere, we don't need the tookOwnership bool.

I wonder if we should actually spit out multiple example classes :-/.

Comment on attachment 8519939 [details] [diff] [review] Reorganize checks for members in BindingUtils.h. r=me

Comment on attachment 8519941 [details] [diff] [review] Add template to detect refcounted classes. Nice catches on the DOMMatrix/DOMPoint bits. >+ cgThings.append(CGGeneric(define="static_assert(IsRefcounted<NativeType>::value == IsRefcounted<%s::NativeType>::value,\n" You don't need the define= part, right? Just the string. r=me

Comment on attachment 8519942 [details] [diff] [review] Templatize deferred finalization. r=me. This C++ template stuff can get downright scary sometimes.

Comment on attachment 8519945 [details] [diff] [review] Make the WrapObject signature for non-refcounted objects the same as for refcounted objects. >+class BindingCreator Maybe ReflectorCreator? Or BindingJSObjectCreator? >+ ~BindingCreator() Newline before this line? >+ js::SetReservedOrProxyPrivateSlot(mObject, DOM_OBJECT_SLOT, >+ JS::PrivateValue(nullptr)); I think if you set it to JS::UndefinedValue() you won't need to add a null-check in AppendAndTake. >+ JS::Rooted<JSObject*>& >+ CreateObject(JSContext* aCx, const JSClass* aClass, Newline before these two lines? r=me, but it feels like a comment somewhere, perhaps before the BindingCreator (or whatever) clas definition, that explains how this works, would help a lot.

Comment on attachment 8519946 [details] [diff] [review] Make WrapNewBindingNonWrapperCachedObject work for owned objects. >+ static_assert(IsRefcounted<T>::value, "Don't pas owned classes in here."); "pass" >+ static_assert(!IsRefcounted<T>::value, "Only pas owned classes in here."); And here. >+ static_assert(IsRefcounted<T>::value, "Don't pas owned classes in here."); And here. r=me

Comment on attachment 8519947 [details] [diff] [review] Autodetect the right smart pointer for owned and refcounted objects. There's a stray bit of commit comment above the "#" bits there. >+struct StrongPtrMember Maybe StrongPtrForMember? >+ nsRefPtr<T>, nsAutoPtr<T>>::Type Ptr; And maybe call this Type instead of Ptr? So with this patch the return values of [NewObject] things that return owned objects need to be nsAutoPtr, right? That seems reasonable, as long as we update the documentation accordingly.

Comment on attachment 8519948 [details] [diff] [review] Make example code spit out a refcounted class always. r=me

Comment on attachment 8519949 [details] [diff] [review] Change a python check for a refcounted type to a static_assert. r=me

Comment on attachment 8519950 [details] [diff] [review] Don't condition wrapperCache on having a refcounted type. r=me

Comment on attachment 8519951 [details] [diff] [review] Remove the now unused nativeOwnership property. r=me. Please update the docs!

The rooting analysis complains about a hazard because the call to BindingJSObjectCreator::ForgetObject returns a JSObject*, and then the BindingJSObjectCreator destructor destroys a nsRefPtr. ForgetObject nulls out that nsRefPtr though, so it's not really an issue. We could annotate, but it might make more sense to take this patch, to make WrapObject take a MutableHandle instead of returning a raw pointer. I've avoided changing the nsWrapperCache::WrapObject signature for now, there's a lot of implementations and we're considering changing its signature anyway for bug 1117172 so we might want to do it there at the same time. The one thing I don't like is that now BindingJSObjectCreator keeps a JSObject* pointer to an object that's held alive by the MutableHandle in the Wrap functions, but nothing really enforces that. Not sure if we can do something elegant about that.

Comment on attachment 8546105 [details] [diff] [review] Make rooting analysis happy How is holding a JSObject* mReflector ok? How do we know it hasn't been moved by the time we get to ~BindingJSObjectCreator? @@ -3323,70 +3325,70 @@ class CGWrapGlobalMethod(CGAbstractMetho ... + if (!DefineProperties(aCx, aReflector, ${properties}, ${chromeProperties})) { return nullptr; return false? The rest looks good, but I would _really_ like to understand the story with mReflector before you land this.

(In reply to Boris Zbarsky [:bz] from comment #23) > How is holding a JSObject* mReflector ok? How do we know it hasn't been > moved by the time we get to ~BindingJSObjectCreator? I made it a JS::Rooted<JSObject*>. https://hg.mozilla.org/integration/mozilla-inbound/rev/c37e10cff765 https://hg.mozilla.org/integration/mozilla-inbound/rev/0f330a5dd346 https://hg.mozilla.org/integration/mozilla-inbound/rev/6005fd357342 https://hg.mozilla.org/integration/mozilla-inbound/rev/55c831086864 https://hg.mozilla.org/integration/mozilla-inbound/rev/899d8cd8c4e8 https://hg.mozilla.org/integration/mozilla-inbound/rev/d6e68ba4846d https://hg.mozilla.org/integration/mozilla-inbound/rev/47be69b84be5 https://hg.mozilla.org/integration/mozilla-inbound/rev/9ed945e9a8a5 https://hg.mozilla.org/integration/mozilla-inbound/rev/e2e64e751ece https://hg.mozilla.org/integration/mozilla-inbound/rev/676112a4f092 https://hg.mozilla.org/integration/mozilla-inbound/rev/4a067de94f22 https://hg.mozilla.org/integration/mozilla-inbound/rev/12dd1ad43923

Turns out this wasn't the cause of the problem for which it was backed out, so relanded: https://hg.mozilla.org/integration/mozilla-inbound/rev/fb38a3ad96ce https://hg.mozilla.org/integration/mozilla-inbound/rev/7941dd7c8866 https://hg.mozilla.org/integration/mozilla-inbound/rev/ecdac08e0897 https://hg.mozilla.org/integration/mozilla-inbound/rev/8e4a528db0d9 https://hg.mozilla.org/integration/mozilla-inbound/rev/f4f784f1d227 https://hg.mozilla.org/integration/mozilla-inbound/rev/a3b689b8d233 https://hg.mozilla.org/integration/mozilla-inbound/rev/ae57edae370e https://hg.mozilla.org/integration/mozilla-inbound/rev/c2bdf38c84f3 https://hg.mozilla.org/integration/mozilla-inbound/rev/d5557b9aee8f https://hg.mozilla.org/integration/mozilla-inbound/rev/fae280dc596d https://hg.mozilla.org/integration/mozilla-inbound/rev/b5be217117fa https://hg.mozilla.org/integration/mozilla-inbound/rev/3bff2bbe6677

https://hg.mozilla.org/mozilla-central/rev/fb38a3ad96ce https://hg.mozilla.org/mozilla-central/rev/7941dd7c8866 https://hg.mozilla.org/mozilla-central/rev/ecdac08e0897 https://hg.mozilla.org/mozilla-central/rev/8e4a528db0d9 https://hg.mozilla.org/mozilla-central/rev/f4f784f1d227 https://hg.mozilla.org/mozilla-central/rev/a3b689b8d233 https://hg.mozilla.org/mozilla-central/rev/ae57edae370e https://hg.mozilla.org/mozilla-central/rev/c2bdf38c84f3 https://hg.mozilla.org/mozilla-central/rev/d5557b9aee8f https://hg.mozilla.org/mozilla-central/rev/fae280dc596d https://hg.mozilla.org/mozilla-central/rev/b5be217117fa https://hg.mozilla.org/mozilla-central/rev/3bff2bbe6677

Peter, we need to document the new WrapObject setup, and probably update the example codegen. People on IRC are being confused...