Bug 1048659 - Return the union we construct in a JS callback. r=peterv
authorAndrew McCreight <continuation@gmail.com>
Tue, 19 Aug 2014 08:41:26 -0700
changeset 200348 6673cfac7488f8674f6dcf1e08ebaabc3fa8387b
parent 200347 9f6ea1c97cc6b701275403669634073952dedde2
child 200349 e189b65bba07e2e96fdb43c2bc90095975e0cb24
push id47872
push useramccreight@mozilla.com
push dateTue, 19 Aug 2014 15:41:46 +0000
treeherdermozilla-inbound@6673cfac7488 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv
bugs1048659
milestone34.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1048659 - Return the union we construct in a JS callback. r=peterv
dom/bindings/Codegen.py
dom/bindings/test/TestExampleGen.webidl
dom/bindings/test/TestInterfaceJS.js
dom/bindings/test/TestJSImplGen.webidl
dom/bindings/test/mochitest.ini
dom/bindings/test/test_returnUnion.html
dom/webidl/TestInterfaceJS.webidl
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -4093,21 +4093,22 @@ def getJSToNativeConversionInfo(type, de
                                         dealWithOptional=isOptional,
                                         holderArgs=holderArgs)
 
     if type.isUnion():
         nullable = type.nullable()
         if nullable:
             type = type.inner
 
-        unionArgumentObj = "${declName}" if isMember else "${holderName}"
+        isOwningUnion = isMember or isCallbackReturnValue
+        unionArgumentObj = "${declName}" if isOwningUnion else "${holderName}"
         if nullable:
-            # If we're a member, we're a Nullable, which hasn't been told it has
+            # If we're owning, we're a Nullable, which hasn't been told it has
             # a value.  Otherwise we're an already-constructed Maybe.
-            unionArgumentObj += ".SetValue()" if isMember else ".ref()"
+            unionArgumentObj += ".SetValue()" if isOwningUnion else ".ref()"
 
         memberTypes = type.flatMemberTypes
         names = []
 
         interfaceMemberTypes = filter(lambda t: t.isNonCallbackInterface(), memberTypes)
         if len(interfaceMemberTypes) > 0:
             interfaceObject = []
             for memberType in interfaceMemberTypes:
@@ -4287,17 +4288,17 @@ def getJSToNativeConversionInfo(type, de
             }
             """,
             exceptionCode=exceptionCode,
             desc=firstCap(sourceDescription),
             names=", ".join(names)))
 
         templateBody = CGWrapper(CGIndenter(CGList([templateBody, throw])), pre="{\n", post="}\n")
 
-        typeName = CGUnionStruct.unionTypeDecl(type, isMember)
+        typeName = CGUnionStruct.unionTypeDecl(type, isOwningUnion)
         argumentTypeName = typeName + "Argument"
         if nullable:
             typeName = "Nullable<" + typeName + " >"
 
         def handleNull(templateBody, setToNullVar, extraConditionForNull=""):
             nullTest = "%s${val}.isNullOrUndefined()" % extraConditionForNull
             return CGIfElseWrapper(nullTest,
                                    CGGeneric("%s.SetNull();\n" % setToNullVar),
@@ -4310,48 +4311,54 @@ def getJSToNativeConversionInfo(type, de
                 assert defaultValue.type == type
                 extraConditionForNull = "!(${haveValue}) || "
             else:
                 extraConditionForNull = ""
             templateBody = handleNull(templateBody, unionArgumentObj,
                                       extraConditionForNull=extraConditionForNull)
 
         declType = CGGeneric(typeName)
-        if isMember:
+        if isOwningUnion:
             holderType = None
         else:
             holderType = CGGeneric(argumentTypeName)
             if nullable:
                 holderType = CGTemplatedType("Maybe", holderType)
 
         # If we're isOptional and not nullable the normal optional handling will
         # handle lazy construction of our holder.  If we're nullable and not
-        # isMember we do it all by hand because we do not want our holder
-        # constructed if we're null.  But if we're isMember we don't have a
+        # owning we do it all by hand because we do not want our holder
+        # constructed if we're null.  But if we're owning we don't have a
         # holder anyway, so we can do the normal Optional codepath.
         declLoc = "${declName}"
         constructDecl = None
         if nullable:
-            if isOptional and not isMember:
+            if isOptional and not isOwningUnion:
                 holderArgs = "${declName}.Value().SetValue()"
                 declType = CGTemplatedType("Optional", declType)
                 constructDecl = CGGeneric("${declName}.Construct();\n")
                 declLoc = "${declName}.Value()"
             else:
                 holderArgs = "${declName}.SetValue()"
             if holderType is not None:
                 constructHolder = CGGeneric("${holderName}.emplace(%s);\n" % holderArgs)
             else:
                 constructHolder = None
             # Don't need to pass those args when the holder is being constructed
             holderArgs = None
         else:
             holderArgs = "${declName}"
             constructHolder = None
 
+        if not isMember and isCallbackReturnValue:
+            declType = CGWrapper(declType, post="&")
+            declArgs = "aRetVal"
+        else:
+            declArgs = None
+
         if defaultValue and not isinstance(defaultValue, IDLNullValue):
             tag = defaultValue.type.tag()
 
             if tag in numericSuffixes or tag is IDLType.Tags.bool:
                 defaultStr = getHandleDefault(defaultValue)
                 value = declLoc + (".Value()" if nullable else "")
                 name = getUnionMemberName(defaultValue.type)
                 default = CGGeneric("%s.RawSetAs%s() = %s;\n" %
@@ -4382,36 +4389,37 @@ def getJSToNativeConversionInfo(type, de
             else:
                 extraConditionForNull = ""
             templateBody = handleNull(templateBody, declLoc,
                                       extraConditionForNull=extraConditionForNull)
         elif (not type.hasNullableType and defaultValue and
               isinstance(defaultValue, IDLNullValue)):
             assert type.hasDictionaryType
             assert defaultValue.type.isDictionary()
-            if not isMember and typeNeedsRooting(defaultValue.type):
+            if not isOwningUnion and typeNeedsRooting(defaultValue.type):
                 ctorArgs = "cx"
             else:
                 ctorArgs = ""
             initDictionaryWithNull = CGIfWrapper(
                 CGGeneric("return false;\n"),
                 ('!%s.RawSetAs%s(%s).Init(cx, JS::NullHandleValue, "Member of %s")'
                  % (declLoc, getUnionMemberName(defaultValue.type),
                     ctorArgs, type)))
             templateBody = CGIfElseWrapper("!(${haveValue})",
                                            initDictionaryWithNull,
                                            templateBody)
 
         templateBody = CGList([constructDecl, templateBody])
 
         return JSToNativeConversionInfo(templateBody.define(),
                                         declType=declType,
+                                        declArgs=declArgs,
                                         holderType=holderType,
                                         holderArgs=holderArgs,
-                                        dealWithOptional=isOptional and (not nullable or isMember))
+                                        dealWithOptional=isOptional and (not nullable or isOwningUnion))
 
     if type.isGeckoInterface():
         assert not isEnforceRange and not isClamp
 
         descriptor = descriptorProvider.getDescriptor(
             type.unroll().inner.identifier.name)
 
         if descriptor.nativeType == 'JSObject':
--- a/dom/bindings/test/TestExampleGen.webidl
+++ b/dom/bindings/test/TestExampleGen.webidl
@@ -486,25 +486,25 @@ interface TestExampleInterface {
   void passVariadicUnion((CanvasPattern or CanvasGradient)... arg);
 
   void passSequenceOfNullableUnions(sequence<(CanvasPattern or CanvasGradient)?> arg);
   void passVariadicNullableUnion((CanvasPattern or CanvasGradient)?... arg);
   void passMozMapOfUnions(MozMap<(CanvasPattern or CanvasGradient)> arg);
   // XXXbz no move constructor on some unions
   // void passMozMapOfUnions2(MozMap<(object or long)> arg);
 
-  //(CanvasPattern or CanvasGradient) receiveUnion();
-  //(object or long) receiveUnion2();
-  //(CanvasPattern? or CanvasGradient) receiveUnionContainingNull();
-  //(CanvasPattern or CanvasGradient)? receiveNullableUnion();
-  //(object or long)? receiveNullableUnion2();
+  (CanvasPattern or CanvasGradient) receiveUnion();
+  (object or long) receiveUnion2();
+  (CanvasPattern? or CanvasGradient) receiveUnionContainingNull();
+  (CanvasPattern or CanvasGradient)? receiveNullableUnion();
+  (object or long)? receiveNullableUnion2();
 
-  //attribute (CanvasPattern or CanvasGradient) writableUnion;
-  //attribute (CanvasPattern? or CanvasGradient) writableUnionContainingNull;
-  //attribute (CanvasPattern or CanvasGradient)? writableNullableUnion;
+  attribute (CanvasPattern or CanvasGradient) writableUnion;
+  attribute (CanvasPattern? or CanvasGradient) writableUnionContainingNull;
+  attribute (CanvasPattern or CanvasGradient)? writableNullableUnion;
 
   // Date types
   void passDate(Date arg);
   void passNullableDate(Date? arg);
   void passOptionalDate(optional Date arg);
   void passOptionalNullableDate(optional Date? arg);
   void passOptionalNullableDateWithDefaultValue(optional Date? arg = null);
   void passDateSequence(sequence<Date> arg);
--- a/dom/bindings/test/TestInterfaceJS.js
+++ b/dom/bindings/test/TestInterfaceJS.js
@@ -42,12 +42,17 @@ TestInterfaceJS.prototype = {
   set anyAttr(val) { checkGlobal(val); this._anyAttr = val; },
   get objectAttr() { return this._objectAttr; },
   set objectAttr(val) { checkGlobal(val); this._objectAttr = val; },
   pingPongAny: function(any) { checkGlobal(any); return any; },
   pingPongObject: function(obj) { checkGlobal(obj); return obj; },
 
   getCallerPrincipal: function() { return Cu.getWebIDLCallerPrincipal().origin; },
 
-  convertSVS: function(svs) { return svs; }
+  convertSVS: function(svs) { return svs; },
+
+  pingPongUnion: function(x) { return x; },
+  pingPongUnionContainingNull: function(x) { return x; },
+  pingPongNullableUnion: function(x) { return x; },
+  returnBadUnion: function(x) { return 3; }
 };
 
 this.NSGetFactory = XPCOMUtils.generateNSGetFactory([TestInterfaceJS])
--- a/dom/bindings/test/TestJSImplGen.webidl
+++ b/dom/bindings/test/TestJSImplGen.webidl
@@ -510,25 +510,25 @@ interface TestJSImplInterface {
   void passVariadicUnion((CanvasPattern or CanvasGradient)... arg);
 
   void passSequenceOfNullableUnions(sequence<(CanvasPattern or CanvasGradient)?> arg);
   void passVariadicNullableUnion((CanvasPattern or CanvasGradient)?... arg);
   void passMozMapOfUnions(MozMap<(CanvasPattern or CanvasGradient)> arg);
   // XXXbz no move constructor on some unions
   // void passMozMapOfUnions2(MozMap<(object or long)> arg);
 
-  //(CanvasPattern or CanvasGradient) receiveUnion();
-  //(object or long) receiveUnion2();
-  //(CanvasPattern? or CanvasGradient) receiveUnionContainingNull();
-  //(CanvasPattern or CanvasGradient)? receiveNullableUnion();
-  //(object or long)? receiveNullableUnion2();
+  (CanvasPattern or CanvasGradient) receiveUnion();
+  (object or long) receiveUnion2();
+  (CanvasPattern? or CanvasGradient) receiveUnionContainingNull();
+  (CanvasPattern or CanvasGradient)? receiveNullableUnion();
+  (object or long)? receiveNullableUnion2();
 
-  //attribute (CanvasPattern or CanvasGradient) writableUnion;
-  //attribute (CanvasPattern? or CanvasGradient) writableUnionContainingNull;
-  //attribute (CanvasPattern or CanvasGradient)? writableNullableUnion;
+  attribute (CanvasPattern or CanvasGradient) writableUnion;
+  attribute (CanvasPattern? or CanvasGradient) writableUnionContainingNull;
+  attribute (CanvasPattern or CanvasGradient)? writableNullableUnion;
 
   // Date types
   void passDate(Date arg);
   void passNullableDate(Date? arg);
   void passOptionalDate(optional Date arg);
   void passOptionalNullableDate(optional Date? arg);
   void passOptionalNullableDateWithDefaultValue(optional Date? arg = null);
   void passDateSequence(sequence<Date> arg);
--- a/dom/bindings/test/mochitest.ini
+++ b/dom/bindings/test/mochitest.ini
@@ -35,15 +35,17 @@ skip-if = true
 [test_exceptions_from_jsimplemented.html]
 skip-if = (toolkit == 'gonk' && debug) #debug-only failure; bug 926547
 [test_lenientThis.html]
 [test_lookupGetter.html]
 [test_namedNoIndexed.html]
 [test_named_getter_enumerability.html]
 [test_Object.prototype_props.html]
 [test_queryInterface.html]
+[test_returnUnion.html]
+skip-if = debug == false
 [test_scalarvaluestring.html]
 skip-if = debug == false
 [test_sequence_wrapping.html]
 [test_setWithNamedGetterNoNamedSetter.html]
 [test_throwing_method_noDCE.html]
 [test_treat_non_object_as_null.html]
 [test_traceProtos.html]
new file mode 100644
--- /dev/null
+++ b/dom/bindings/test/test_returnUnion.html
@@ -0,0 +1,59 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1048659
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 1048659</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+  <script type="application/javascript">
+
+  /** Test for returning unions from JS-implemented WebIDL. **/
+  SimpleTest.waitForExplicitFinish();
+  SpecialPowers.pushPrefEnv({set: [['dom.expose_test_interfaces', true]]}, go);
+
+  function go() {
+    var t = new TestInterfaceJS();
+    var t2 = new TestInterfaceJS();
+
+    is(t.pingPongUnion(t2), t2, "ping pong union for left case should be identity");
+    is(t.pingPongUnion(12), 12, "ping pong union for right case should be identity");
+
+    is(t.pingPongUnionContainingNull("this is not a string"), "this is not a string",
+      "ping pong union containing union for left case should be identity");
+    is(t.pingPongUnionContainingNull(null), null,
+      "ping pong union containing null for right case null should be identity");
+    is(t.pingPongUnionContainingNull(t2), t2,
+      "ping pong union containing null for right case should be identity");
+
+    is(t.pingPongNullableUnion(t2), t2, "ping pong nullable union for left case should be identity");
+    is(t.pingPongNullableUnion(12), 12, "ping pong nullable union for right case should be identity");
+    is(t.pingPongNullableUnion(null), null, "ping pong nullable union for null case should be identity");
+
+    var rejectedBadUnion = false;
+    var result = null;
+    try {
+      result = t.returnBadUnion();
+    } catch (e) {
+      rejectedBadUnion = true;
+    }
+    is(result, null, "bad union should not set a value for result");
+    ok(rejectedBadUnion, "bad union should throw an exception");
+
+    SimpleTest.finish();
+  }
+
+  </script>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1048659">Mozilla Bug 1048659</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+
+</div>
+<pre id="test">
+</pre>
+</body>
+</html>
--- a/dom/webidl/TestInterfaceJS.webidl
+++ b/dom/webidl/TestInterfaceJS.webidl
@@ -14,9 +14,14 @@ interface TestInterfaceJS {
   attribute object objectAttr;
   any pingPongAny(any arg);
   object pingPongObject(any obj);
 
   // For testing bug 968335.
   DOMString getCallerPrincipal();
 
   DOMString convertSVS(ScalarValueString svs);
+
+  (TestInterfaceJS or long) pingPongUnion((TestInterfaceJS or long) something);
+  (DOMString or TestInterfaceJS?) pingPongUnionContainingNull((TestInterfaceJS? or DOMString) something);
+  (TestInterfaceJS or long)? pingPongNullableUnion((TestInterfaceJS or long)? something);
+  (Location or TestInterfaceJS) returnBadUnion();
 };