Closed
Bug 830099
Opened 12 years ago
Closed 12 years ago
Don't generate dictionary code for workers when binding is used only in the main thread
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla21
People
(Reporter: smaug, Assigned: bzbarsky)
References
Details
(Keywords: dev-doc-complete)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
peterv
:
review+
|
Details | Diff | Splinter Review |
Seems like we generate worker code for dictionaries which are main thread only.
Either .conf is missing something, or we just generate the code always.
Assignee | ||
Comment 1•12 years ago
|
||
We always generate both worker and main-thread code for workers, yes. Same thing for callbacks (modulo WorkerOnly annotations), as I recall.
We could try to do something smarter, I guess. Maybe using code similar to that which generates includes and forward-declares...
Assignee | ||
Comment 2•12 years ago
|
||
So some thoughts on implementing this, so I can sleep:
The right approach seems to be to start at relevant descriptors (all main-thread ones or all worker ones, including callback interface descriptors for main-thread) and then go through all the types listed. For each type, if it's a union add all types in the union, if it's a callback add the return type and all argument types, if it's a dictionary add all member types. This is kinda like the callForEachType setup, except that only takes a subset of descriptors and expects to be handed the dictionaries and callbacks, whereas here we'd figure those bits out.
Then we see what callbacks and dictionaries are in that set of types, and only generate code for those.
Note that all that information will not depend on the WebIDL file, so only needs to be computed once. I wonder whether we can pickle it... need to check what we pickle right now.
Assignee | ||
Comment 3•12 years ago
|
||
Attachment #702139 -
Flags: review?(peterv)
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → bzbarsky
Whiteboard: [need review]
Assignee | ||
Comment 4•12 years ago
|
||
Attachment #704009 -
Flags: review?(peterv)
Assignee | ||
Updated•12 years ago
|
Attachment #702139 -
Attachment is obsolete: true
Attachment #702139 -
Flags: review?(peterv)
Assignee | ||
Updated•12 years ago
|
Attachment #704009 -
Attachment description: Merged on top of → Merged on top of bug 822470
Assignee | ||
Comment 5•12 years ago
|
||
If this lands as-is, I should remember to remove the docs for [WorkerOnly]
Keywords: dev-doc-needed
Updated•12 years ago
|
Attachment #704009 -
Flags: review?(peterv) → review+
Assignee | ||
Comment 6•12 years ago
|
||
Flags: in-testsuite-
Whiteboard: [need review]
Target Milestone: --- → mozilla21
Comment 8•12 years ago
|
||
Backed out because of build bustage:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2f6fa70c289a
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=32786d29daf8
Assignee | ||
Comment 9•12 years ago
|
||
Ah, yes, RTCIceServer C++ uses got added without any actual WebIDL using it...
Added it to the DummyBinding and relanded:
http://hg.mozilla.org/integration/mozilla-inbound/rev/b3cabb259af7
Assignee | ||
Comment 10•12 years ago
|
||
And a followup to fix the RTC stuff on Android: https://hg.mozilla.org/integration/mozilla-inbound/rev/c5238879470f
Comment 11•12 years ago
|
||
And backed them both out in https://hg.mozilla.org/integration/mozilla-inbound/rev/003632d51638 for https://tbpl.mozilla.org/php/getParsedLog.php?id=19224713&tree=Mozilla-Inbound bustage on b2g.
Assignee | ||
Comment 12•12 years ago
|
||
Gah. I'm so tired of all the goddamned ifdefed not-WebIDL-but-pretending crap. :(
Assignee | ||
Comment 13•12 years ago
|
||
Once more, with feeling, and a lot fewer ifdefs in our IDL: https://hg.mozilla.org/integration/mozilla-inbound/rev/2b9a689ac459
Assignee | ||
Comment 14•12 years ago
|
||
Er, I meant https://hg.mozilla.org/integration/mozilla-inbound/rev/e894c103775f
Can this now qualify for Most Confusing Bug Ever awards?
Comment 15•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•