919268 - Add codegen for worker-only WebIDL callbacks

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Jan Varga [:janv]]

I had to do this for sync IDB.

Comment 1

[Jan Varga [:janv]]

Attachment: patch

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 808262 [details] [diff] [review]
patch

This seems like the opposite of what we _want_ to be doing, in general, which is getting rid of worker-specific stuff.  Especially the EventHandler changes here are totally backwards and will prevent sane event targets on workers (which is a _very_ short-term goal here).  So we're not going to be doing that.

I assume the basic issue is that IDB*Sync interfaces have to marked as "worker" interfaces because of the existing EventTarget breakage or something?  Are there any other reason they need to be marked as "worker" interfaces?

If we posit that they do need to be thus marked, how about we only generate worker-descriptored callbacks for those callbacks for which we did not generate a mainthread-descriptored one?  That will avoid breaking EventHandler, at least.

Comment 3

[Jan Varga [:janv]]

Well there were two options and I clearly chose the bad one :)
When I added:
cgthings.extend(CGCallbackFunction(c, config.getDescriptorProvider(True)) 
                for c in workerCallbacks)

it generated slightly different EventHandlerNonNull bindings (nsDOMEvent vs JSObject, I'll attach the diff)
So I thought that I needed to keep the "Workers" suffix

I don't think IDBTransacionCallback and IDBVersionChangeCallback need to use the suffix.

Comment 4

[Jan Varga [:janv]]

Attachment: EventHandlerBinding.diff

Comment 5

[Boris Zbarsky [:bzbarsky]]

> it generated slightly different EventHandlerNonNull bindings (nsDOMEvent vs JSObject

Right.  But the latter is supposed to go away ASAP, and would be unused in the meantime.

Comment 6

[Jan Varga [:janv]]

(In reply to Boris Zbarsky [:bz] from comment #5)
> > it generated slightly different EventHandlerNonNull bindings (nsDOMEvent vs JSObject
> 
> Right.  But the latter is supposed to go away ASAP, and would be unused in
> the meantime.

ok, so I'll try the other option to get rid of the "Workers" suffix

Comment 7

[Jan Varga [:janv]]

Attachment: interdiff

Boris, something like this ?
I'll submit a new patch for m-c, if you like the approach.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 809299 [details] [diff] [review]
interdiff

Much better, thanks.

Comment 9

[Jan Varga [:janv]]

Attachment: final patch

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 809330 [details] [diff] [review]
final patch

r=me

Comment 11

[Jan Varga [:janv]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3b16cca767c4

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/3b16cca767c4