Closed Bug 798187 Opened 9 years ago Closed 9 years ago

WebIDL bindings: add support for dictionary return values


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

Not set





(Reporter: bjacob, Assigned: bzbarsky)


(Keywords: dev-doc-complete)


(3 files)

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

For example, in WebGL, getContextAttributes() is supposed to return a dictionary.
Attachment #668551 - Flags: review?(peterv)
Attachment #668551 - Flags: review?(bjacob)
Whiteboard: [need review]
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.
Attachment #668551 - Flags: review?(bjacob) → review+
Comment on attachment 668550 [details] [diff] [review]
part 1.  Add support for dictionary return values.

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

::: dom/bindings/
@@ +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().
Attachment #668550 - Flags: review?(peterv) → review+
Attachment #668551 - Flags: review?(peterv) → review+
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 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.
Attachment #673941 - Flags: review?(peterv) → review+
Documented that we have this now.
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.