1134112 - MOZ_ASSERT(mType == eUninitialized) in OwningStringOrStringSequenceOrConstrainDOMStringParameters

Status: RESOLVED FIXED

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

Description

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

Attachment: lldb mochitest output

STR:
- Build with first two patches in Bug 1119335.
- mochitest dom/media/tests/mochitest/test_getUserMedia_constraints.html

See attached mochitest output and try [1]. From what I can tell, there's just binding code involved. Assert seems provoked by JS call mozGetUserMedia({ video: { facingMode:{ exact:'left' } }) with this new webidl union:

  typedef (DOMString or sequence<DOMString> or ConstrainDOMStringParameters) ConstrainDOMString;

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=ac146fa48426

Comment 1

[Boris Zbarsky [:bzbarsky]]

Hmm.  So the problem here is that we have a non-fast dictionary (e.g. a dictionary in a another dictionary or in a sequence) which has a member which is a union between a sequence and a dictionary.  Simple testcase:

  dictionary Foo {
  };
  dictionary Bar {
    (sequence<long> or Foo) m;
  };
  interface Baz {
    void func(sequence<Bar> arg);
  }

and then invoking:

  func([{ m: {} }])

What happens here is that when we'd doing the outermost sequence we append a Bar to the array.  This runs the Bar constructor (note: not the FastBar constructor), which does an Init() on Bar with a null value, so init to default values.

Unfortunately, the "m" member of Bar is a union with a default value (null, because it contains a dictionary).  So this initializes the union to contain the dictionary.

Then we go ahead and init Bar _again_ with the value we really plan to use.  This tries to init the union to a type different from the one it contains, but using the RawSetAs* version of the type setter, which asserts.

So.  Ways to fix this bug:

1)  Change the sequence-of-dictionaries case to call the non-initing dictionary constructor somehow.  Also, change dictionaries with dictionary members to call the non-initing constructor of their members when their own non-initing constructor is called.  I _think_ this should be sufficient.

2)  Change dictionary init of union members to always uninit the union before trying to init it, just to be safe.

I'd guess approach #2 is safer, while approach #1 is likely a bit faster and the part about not initing members if we're not supposed to init is less suprising.  Of course maybe we want to do both...

Comment 2

[Boris Zbarsky [:bzbarsky]]

Or more precisely change dictionary init of union members with default values to always uninit.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Approach #1 would not have worked, because it fails if you have a union containing a dictionary containing a union containing a dictionary.  When the outer union is initialized as dictionary, it default-constructs the outer dictionary, which then inits it, which initializes the inner union.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. When doing fast-init of a Web IDL dictionary, do fast-init of its dictionary members and its ancestors too

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. When doing dictionary init, if we have a non-optional union member just go ahead and uninit it before we try initing it. That way even if it's already been initialized (e.g. because we default-initialized at some point) things will still work co

Comment 6

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

Great, works! Thanks!

How good are the chances of this being reviewed and landed today or tomorrow? I was really hoping to land Bug 1119335 in 38. If this is not possible, then I should plan to compromise on the syntax a bit.

Comment 7

[Boris Zbarsky [:bzbarsky]]

   https://hg.mozilla.org/integration/mozilla-inbound/rev/18cb3176175c
   https://hg.mozilla.org/integration/mozilla-inbound/rev/2be7a90b66b9

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/18cb3176175c
https://hg.mozilla.org/mozilla-central/rev/2be7a90b66b9