870219 - Add a way for JS-implemented WebIDL objects to sanely do event handlers

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Without having to reinvent the wheel, that is.

Comment 1

[Boris Zbarsky [:bzbarsky]]

With the upcoming patch, implementing something like this:

  attribute EventHandler onaddstream;

would look like this:

  get onaddstream() {
    return this.__DOM_IMPL__.getEventHandler("addstream");
  },
  set onaddstream(handler) {
    return this.__DOM_IMPL__.setEventHandler("addstream", handler);
  },

which is not as cool as if we autogenerated it in the binding code, but better than nothing...

Of course the other option is we could just special-case event handlers in the binding codegen: for things inheriting from EventTarget, attributes of type EventHandler, named on*, just generate calls into the superclass...  Not sure whether it's worth the bother.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Add a way for chrome JS that's implementing a WebIDL interface to get/set event handlers sanely on content objects.

Comment 3

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/52f646b2055c

Comment 4

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Any reason you called it setEventHandler rather than setEventListener?

Comment 5

[Boris Zbarsky [:bzbarsky]]

Yes they're different things.  An event listener is something that gets invoked when an event fires.  An event handler is a callback that a very special kinds of event listener can call...

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

addEventListener handles event listeners, onfoo properties are event handlers.

Comment 7

[Boris Zbarsky [:bzbarsky]]

And backed out: https://hg.mozilla.org/integration/mozilla-inbound/rev/10cb7bc4cc58

The problem is that worker code is crashing at startup with:

  ###!!! ASSERTION: Wrong thread!: '!NS_IsMainThread()', file ../../../dom/workers/WorkerPrivate.cpp, line 4257

with the stack showing JS running under the component loader and doing:

mozilla::dom::EventTargetBinding_workers::CreateInterfaceObjects(JSContext*, JS::Handle<JSObject*>, JSObject**)+0x00000063 [/builds/slave/m-in-osx64-d-00000000000000000/build/obj-firefox/dist/FirefoxNightlyDebug.app/Contents/MacOS/XUL +0x01882423]
mozilla::dom::workers::ResolveWorkerClasses(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, unsigned int, JS::MutableHandle<JSObject*>)+0x000001BF [/builds/slave/m-in-osx64-d-00000000000000000/build/obj-firefox/dist/FirefoxNightlyDebug.app/Contents/MacOS/XUL +0x00D1679F]
BackstagePass::NewResolve(nsIXPConnectWrappedNative*, JSContext*, JSObject*, jsid, unsigned int, JSObject**, bool*)+0x00000086 [/builds/slave/m-in-osx64-d-00000000000000000/build/obj-firefox/dist/FirefoxNightlyDebug.app/Contents/MacOS/XUL +0x010FAC86]

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Fix the chrome-scope checking for workers to work even if we incorrectly set up worker interface objects on the main thread.

Comment 9

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 747539 [details] [diff] [review]
part 1.  Fix the chrome-scope checking for workers to work even if we incorrectly set up worker interface objects on the main thread.

Are we guaranteed that nsThreadUtils.h is included somewhere?

Comment 10

[Boris Zbarsky [:bzbarsky]]

> Are we guaranteed that nsThreadUtils.h is included somewhere?

I'll add it to the list of binding includes, just in case.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Relanded:

   https://hg.mozilla.org/integration/mozilla-inbound/rev/096e054032bf
   https://hg.mozilla.org/integration/mozilla-inbound/rev/3e42977d6d77

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/096e054032bf
https://hg.mozilla.org/mozilla-central/rev/3e42977d6d77