837339 - python assertion in webidl compiler on nullable dictionary

Status: RESOLVED FIXED

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

Description

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: RTCConfiguration.webidl

This webidl file causes the python assertion below:

/* -*- Mode: IDL; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/* This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this file,
 * You can obtain one at http://mozilla.org/MPL/2.0/.
 */

dictionary RTCConfiguration {
    sequence<RTCIceServer> iceServers;
};

dictionary RTCIceServer {
    DOMString  url;
    DOMString? credential = null;
};

dictionary MediaConstraints {
    sequence<SupportedConstraints> optionals;
    SupportedConstraints? mandatory = null;
};

dictionary SupportedConstraints {
    boolean OfferToReceiveVideo = false;
    boolean OfferToReceiveAudio = false;
    boolean MozDontOfferDataChannel = false;
};

The offending line is this one:
    SupportedConstraints? mandatory = null;

Output:
Traceback (most recent call last):
  File "/Users/Jan/moz/mozilla-central/config/pythonpath.py", line 56, in <module>
    main(sys.argv[1:])
  File "/Users/Jan/moz/mozilla-central/config/pythonpath.py", line 48, in main
    execfile(script, frozenglobals)
  File "/Users/Jan/moz/mozilla-central/dom/bindings/BindingGen.py", line 66, in <module>
    main()
  File "/Users/Jan/moz/mozilla-central/dom/bindings/BindingGen.py", line 59, in main
    generate_binding_header(config, outputPrefix, webIDLFile);
  File "/Users/Jan/moz/mozilla-central/dom/bindings/BindingGen.py", line 17, in generate_binding_header
    root = CGBindingRoot(config, outputprefix, webidlfile)
  File "/Users/Jan/moz/mozilla-central/dom/bindings/Codegen.py", line 7006, in __init__
    for d in sortDictionaries(mainDictionaries)])
  File "/Users/Jan/moz/mozilla-central/dom/bindings/Codegen.py", line 6522, in __init__
    for member in dictionary.members ]
  File "/Users/Jan/moz/mozilla-central/dom/bindings/Codegen.py", line 2885, in getJSToNativeConversionTemplate
    assert not type.nullable()
AssertionError
make[6]: *** [RTCConfigurationBinding.h] Error 1
make[6]: *** Waiting for unfinished jobs....
make[5]: *** [bindings_export] Error 2
make[4]: *** [export_tier_platform] Error 2
make[3]: *** [tier_platform] Error 2
make[2]: *** [default] Error 2
make[1]: *** [realbuild] Error 2
make: *** [build] Error 2
localhost:~/moz/mozilla-central Jan$

Comment 1

[Boris Zbarsky [:bzbarsky]]

Cameron, this is the thing I mailed you about.  If we need to support this, I'll add support, but if the intent is that WebIDL still disallow that I'd need to add a corresponding check in the parser.  Let me know which it is.  The current setup with the parser producing a data model the codegen doesn't expect is not good.

Comment 2

[Cameron McCormack (:heycam)]

If I'm remembering correctly this is the revision 1.570 change:

  http://dev.w3.org/cvsweb/2006/webapi/WebIDL/Overview.xml

Before that change, dictionaries could not be nullable when used as a dictionary member, and although the main purpose of the change was to relax the restriction on nullable dictionaries for return values, the dictionary member change was an opportunistic one since there didn't seem to be any apparent reason to disallow that.

The reason for disallowing nullable dictionaries as operation arguments was due to the fact that null will already get converted into an empty dictionary (or one with its default dictionary member values filled in), so we did not want to encourage specs to have null be treated differently from that (i.e., it should always be the case that passing { } means the same thing as passing null).

From that reasoning I think we should re-disallow dictionary members from having nullable dictionary types.

Comment 3

[Cameron McCormack (:heycam)]

Although for the same reason as allowing operations to return a nullable dictionary type, maybe dictionary members should be allowed to be nullable when part of a return value.  That is a bit more complicated though.  Let's just try disallowing nullable dictionary-typed dictionary members again and see how it goes.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Sounds good to me.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Re-disallow nullable dictionary members inside dictionaries.

Comment 6

[Boris Zbarsky [:bzbarsky]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/7dc016fc39bf

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7dc016fc39bf