Closed Bug 1225603 Opened 6 years ago Closed 6 years ago

Add ToJSValue overloads for Web IDL enums

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(1 file, 1 obsolete file)

We've talked about this on and off, and people are running into this with both Promises and iterables, so let's just do it.
Attachment #8688621 - Flags: review?(bkelly)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Jan-Ivar, can you look at the media/ bits, please
Attachment #8688687 - Flags: review?(jib)
Attachment #8688687 - Flags: review?(bkelly)
Attachment #8688621 - Attachment is obsolete: true
Attachment #8688621 - Flags: review?(bkelly)
Attachment #8688687 - Flags: review?(jib) → review+
One question, what runs signaling_unittests_standalone.cpp ?
Nm, that's existing code [1], but when I messed up one of the strings and ran

    ./obj-x86_64-apple-darwin12.2.1-debug/media/webrtc/signaling/test/signaling_unittests

I didn't see it get caught. It's been a while, how did you test failure?

http://mxr.mozilla.org/mozilla-central/source/testing/cppunittest.ini#109
Nm, figured it out (first time).

> ./obj-x86_64-apple-darwin12.2.1-debug/media/webrtc/signaling/test/standalone/signaling_unittests_standalone
> Hit MOZ_CRASH(Our tables are out of sync with the bindings) at /Users/Jan/moz/mozilla-central/media/webrtc/signaling/test/standalone/../signaling_unittests.cpp:4761
Comment on attachment 8688687 [details] [diff] [review]
Codegen ToJSValue overloadas for Web IDL enums

Ben's not going to get to this before he turns into a pumpkin.  Olli, do you mind looking at the dom/ changes?  Just ignore the media/ bits, for the sake of your sanity.  ;)
Attachment #8688687 - Flags: review?(bkelly) → review?(bugs)
Comment on attachment 8688687 [details] [diff] [review]
Codegen ToJSValue overloadas for Web IDL enums

>Bug 1225603.  Codegen ToJSValue overloadas for Web IDL enums.  r=bkelly,jib
overloadas?
>-            {
>-              // Scope for resultStr
>-              MOZ_ASSERT(uint32_t(${result}) < ArrayLength(${strings}));
>-              JSString* resultStr = JS_NewStringCopyN(cx, ${strings}[uint32_t(${result})].value, ${strings}[uint32_t(${result})].length);
>-              if (!resultStr) {
>-                $*{exceptionCode}
>-              }
>-              $*{setResultStr}
>-            }
>+            if (!ToJSValue(cx, ${result}, $${jsvalHandle})) {
took some time to figure out where jsvalHandle is coming from.
But the string is then used as a template, $${jsvalHandle} will be ${jsvalHandle} there...


As always with Codegen changes, I think it'd help if one gets an example of generated code ;)
Attachment #8688687 - Flags: review?(bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/94da01072e14
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.