798187 - WebIDL bindings: add support for dictionary return values

Status: RESOLVED FIXED

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

Description

[Benoit Jacob [:bjacob] (mostly away)]

It would be nice to be able to return a dictionary.

For example, in WebGL, getContextAttributes() is supposed to return a dictionary.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Add support for dictionary return values.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Switch WebGLContextAttributes to being a dictionary.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Try push: https://tbpl.mozilla.org/?tree=Try&rev=ac40d85f77bc

Comment 4

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 668551 [details] [diff] [review]
part 2.  Switch WebGLContextAttributes to being a dictionary.

Review of attachment 668551 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/canvas/src/WebGLContext.cpp
@@ +876,5 @@
>      if (!IsContextStable())
>      {
> +        // XXXbz spec says we should still return our attributes in
> +        // this case!  Should we store them all in mOptions?
> +        return;

Yes, probably; in my local patch implementing attributes as an interface, I was refactoring WebGLContextOptions into WebGLContextAttributes.

Comment 5

[Peter Van der Beken [:peterv]]

Comment on attachment 668550 [details] [diff] [review]
part 1.  Add support for dictionary return values.

Review of attachment 668550 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bindings/Codegen.py
@@ +5405,5 @@
>                            m in d.members) + "\n"
> +                "};\n"
> +                "struct ${selfName}Initializer : public ${selfName} {\n"
> +                "  ${selfName}Initializer() {\n"
> +                "    // Safe to pass a null context if we pass a null value\n"

It is not safe right now, because we try to init the JS ids, but we don't actually need them. I'd make Init deal with a null JSContext by skipping the id initing, but only after asserting that the value is JS::NullValue().

Comment 6

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/085bffb67692
https://hg.mozilla.org/integration/mozilla-inbound/rev/0b6a823a8cfe

Comment 7

[Boris Zbarsky [:bzbarsky]]

Man, I suck.  I was pretty sure I'd run the webgl tests on this, but I had not.  The code that returns dictionaries never returns the new objects, causing test fails.

Patch coming up to fix that.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: followup. Actually return our newly-created object from dictionary ToObject.

Comment 9

[Peter Van der Beken [:peterv]]

Comment on attachment 673941 [details] [diff] [review]
followup.  Actually return our newly-created object from dictionary ToObject.

As you mentioned, might be slightly better to do this in the "no parent" path.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Followups:

https://hg.mozilla.org/integration/mozilla-inbound/rev/c2a70bac7627
https://hg.mozilla.org/integration/mozilla-inbound/rev/12e833d6f152

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/085bffb67692
https://hg.mozilla.org/mozilla-central/rev/0b6a823a8cfe
https://hg.mozilla.org/mozilla-central/rev/c2a70bac7627
https://hg.mozilla.org/mozilla-central/rev/12e833d6f152

Comment 12

[Boris Zbarsky [:bzbarsky]]

Documented that we have this now.