Closed Bug 790975 Opened 8 years ago Closed 8 years ago

Support sequences as members of dictionaries in WebIDL

Categories

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

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: smaug, Assigned: bzbarsky)

References

Details

Attachments

(1 file)

I get the error when trying to convert MutationObserver to webidl
http://dvcs.w3.org/hg/domcore/raw-file/tip/Overview.html#mutationobserver
sequence<DOMString> attributeFilter; in the dictionary seems to cause the problem.
Blocks: 790978
Summary: TypeError("Can't handle unrooted sequences") → Support sequences as members of dictionaries in WebIDL
Yeah, indeed.  I punted on this when initially doing this stuff.

What the code comments say is:

            # XXXbz we probably _could_ handle this; we just have to be careful
            # with reallocation behavior for arrays.  In particular, if we have
            # a return value that's a sequence of dictionaries of sequences,
            # that will cause us to have an nsTArray containing objects with
            # nsAutoTArray members, which is a recipe for badness as the
            # outermost array is resized.

which is actually more about return values than arguments.  The typeerror there is for arguments.

I need to sit down and think through whether there are any problems with the arguments part of this.  Will try to do it tonight.
OK.  I remember what the issue was now.

Say we have IDL like so:

  dictionary Bar {
    sequence<DOMString> item;
  };

  interface Foo {
    void foo(sequence<Bar> arg);
  };

The binding code will allocate a dom::Sequence<Bar> (which is an auto TArray).  Then it will set the capacity to the desired one.  But Bar has an auto TArray member itself.  So this will end up memmoving Bar and memmoving its members, but auto TArrays are not memmove-safe).

I _think_ we can loosen the restriction here to allow a sequence to be a dictionary member, as long as we're not in the middle of an argument conversion for some other sequence already.  Or something.  Peter, thoughts?
Assignee: nobody → bzbarsky
Whiteboard: [need review]
Attachment #661586 - Flags: review?(peterv) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/17dcbe563c2b
Flags: in-testsuite+
Whiteboard: [need review]
https://hg.mozilla.org/mozilla-central/rev/17dcbe563c2b
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Depends on: 841404
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.