Like bug 779048, but callback interfaces; I can reuse a lot of the work bug 779048 did here, I think. I started poking at this a bit last night. Patches hopefully to come later this week.

Once this lands, I'll do followups for EventListener (which will take a bit of work, I would guess) and NodeFilter (mostly an issue for the Document WebIDL bindings; I'd rather not make them depend on this code or vice versa).

One other note: Right now we just create a new CallbackInterface object each time we convert JS to C++ here, unlike XPConnect, which looks for an existing XPCWrappedJS. I would prefer to keep the new behavior and add a way to ask whether two CallbackInterface objects wrap the same JS object for cases like EventListener that actually care about that sort of thing. Any objections?

Sounds ok, although I assume it will make ELM a tiny bit more complicated. Will you wrap make wrapper which can deal with EventListenerCallbacks and nsIDOMEventListeners, and make nsListenerStruct::mListener to keep such wrapper alive? And the wrapper would have Equals()?

Comment on attachment 693776 [details] [diff] [review] part 6. Hook up callback interface codegen. Review of attachment 693776 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/bindings/CallbackInterface.cpp @@ +14,5 @@ > +bool > +CallbackInterface::GetCallableProperty(JSContext* cx, const char* aPropName, > + JS::Value* aCallable) > +{ > + if (!JS_GetProperty(cx, mCallback, "doSomething", aCallable)) { doSomething?

Er, copy-paste fail! That used to be codegenned, and then I factored it out into a function. Let me fix that!

> Will you wrap make wrapper which can deal with EventListenerCallbacks and > nsIDOMEventListeners, and make nsListenerStruct::mListener to keep such wrapper alive? > And the wrapper would have Equals()? That's a good question. I guess I do need to handle compares between nsIDOMEventListener (coming from XPConnect) and EventListener (coming from WebIDL). Though if we hook up EventTarget to quickstubs, will we really need that? I guess Window would still be coming through xpconnect... This is sounding annoying. ;)

Well, I was thinking just EventListener comparisons. How would you check in ELM whether some existing EventListener is the same as the one being set? ELM needs support EventListeners and nsIDOMEventListeners and would be nice to not have different code paths for WebIDL and XPCOM stuff.

Sure. For just EventListener, I plan to have a SameObject method or something (need to bikeshed the naming) per comment 9. The question is whether we'll also need a method to check whether an EventListener and an nsIDOMEventListener wrap the same JS object...

I filed bug 829708 for Splinter not understanding part 1.

Comment on attachment 693772 [details] [diff] [review] part 2. Create a CallbackInterface helper class. Review of attachment 693772 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/bindings/CallbackFunction.h @@ +9,4 @@ > * > + * This class implements common functionality like lifetime management, > + * initialization with the callback object, and setup of the call environment. > + * Subclasses corresponding to particular callback function types should s/function/object/ or s/function/interface/?

Comment on attachment 693774 [details] [diff] [review] part 4. Expose the concept of "single operation interface" in the WebIDL data model. Review of attachment 693774 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/bindings/parser/tests/test_callback_interface.py @@ +91,5 @@ > + for (i, iface) in enumerate(results): > + harness.check(iface.isSingleOperationInterface(), i < 4, > + "Interface %s should be a single operation interface" % > + iface.identifier.name) > + Trailing whitespace.

> s/function/object/ or s/function/interface/? >Trailing whitespace. Fixed.

Peter, thanks for the reviews! https://hg.mozilla.org/integration/mozilla-inbound/rev/39d1a2977442 https://hg.mozilla.org/integration/mozilla-inbound/rev/d03fc64bbe76 https://hg.mozilla.org/integration/mozilla-inbound/rev/e57b8e4373d4 https://hg.mozilla.org/integration/mozilla-inbound/rev/c8b54141ddd7 https://hg.mozilla.org/integration/mozilla-inbound/rev/fdad64f0f20d https://hg.mozilla.org/integration/mozilla-inbound/rev/02a297c64232 https://hg.mozilla.org/integration/mozilla-inbound/rev/5517fe5d3fda Still need to document.

https://hg.mozilla.org/mozilla-central/rev/39d1a2977442 https://hg.mozilla.org/mozilla-central/rev/d03fc64bbe76 https://hg.mozilla.org/mozilla-central/rev/e57b8e4373d4 https://hg.mozilla.org/mozilla-central/rev/c8b54141ddd7 https://hg.mozilla.org/mozilla-central/rev/fdad64f0f20d https://hg.mozilla.org/mozilla-central/rev/02a297c64232 https://hg.mozilla.org/mozilla-central/rev/5517fe5d3fda

Documented at https://developer.mozilla.org/en-US/docs/Mozilla/WebIDL_bindings#Interface_types