Jan-Ivar Bruaroey [:jib] (needinfo? me) Reporter
	Description • 11 years ago

See info.txt for callstack, webidl and faulty generated code.

[11:20am] bz: This is totally a dictionary codegen bug
[11:21am] bz: I mean, it checks for null
[11:21am] bz: and sets it to null
[11:21am] bz: and then tries to wrap as an object

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 1 • 11 years ago

Thank you for catching this!

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 3 • 11 years ago

Comment on attachment 745192 [details] [diff] [review]
Fix the successCode in dictionary to-js conversions to actually work right, and document the requirements on successCode better.

s/succesCode/successCode/

Do we really need do-while for scoping? Wouldn't {} be enough?

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 4 • 11 years ago

> Do we really need do-while for scoping? 

Yes, that's what fixes the bug.  I need it not for the scoping but so the 'break' will do something useful.  Basically it converts this old codegen:


  {
    // scope for 'temp' and 'currentValue'
    JS::Value temp;
    const nsRefPtr<mozilla::dom::TestInterface>& currentValue = mSomeNullableInterface;
    if (!currentValue) {
      temp = JSVAL_NULL;
      if (!JS_DefinePropertyById(cx, obj, someNullableInterface_id, temp, nullptr, nullptr, JSPROP_ENUMERATE)) {
        return false;
      }
    }
    if (!WrapNewBindingObject(cx, parentObject, currentValue, &temp)) {
      MOZ_ASSERT(JS_IsExceptionPending(cx));
      return false;
    }
    if (!JS_DefinePropertyById(cx, obj, someNullableInterface_id, temp, nullptr, nullptr, JSPROP_ENUMERATE)) {
      return false;
    }
  }

into this:

  do {
    // block for our 'break' successCode and scope for 'temp' and 'currentValue'
    JS::Rooted<JS::Value> temp(cx);
    const nsRefPtr<mozilla::dom::TestInterface>& currentValue = mSomeNullableInterface;
    if (!currentValue) {
      temp = JSVAL_NULL;
      if (!JS_DefinePropertyById(cx, obj, someNullableInterface_id, temp, nullptr, nullptr, JSPROP_ENUMERATE)) {
        return false;
      }
      break;
    }
    if (!WrapNewBindingObject(cx, parentObject, currentValue, temp.address())) {
      MOZ_ASSERT(JS_IsExceptionPending(cx));
      return false;
    }
    if (!JS_DefinePropertyById(cx, obj, someNullableInterface_id, temp, nullptr, nullptr, JSPROP_ENUMERATE)) {
      return false;
    }
    break;
  } while(0);

which has quite different behavior if !currentValue.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 5 • 11 years ago

http://hg.mozilla.org/integration/mozilla-inbound/rev/47b7673201e5

	Ryan VanderMeulen [:RyanVM]
	Comment 6 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/47b7673201e5