Bug 1048659 - Return the union we construct in a JS callback. r=peterv

dom/bindings/Codegen.py

@@ -4098,11 +4098,12 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None,
         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 = []
@@ -4292,7 +4293,7 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None,
 
         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 + " >"
@@ -4315,7 +4316,7 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None,
                                       extraConditionForNull=extraConditionForNull)
 
         declType = CGGeneric(typeName)
-        if isMember:
+        if isOwningUnion:
             holderType = None
         else:
             holderType = CGGeneric(argumentTypeName)
@@ -4324,13 +4325,13 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None,
 
         # 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")
@@ -4347,6 +4348,12 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None,
             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()
 
@@ -4387,7 +4394,7 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None,
               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 = ""
@@ -4404,9 +4411,10 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None,
 
         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

dom/bindings/test/TestExampleGen.webidl

@@ -491,15 +491,15 @@ interface TestExampleInterface {
   // 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);

dom/bindings/test/TestInterfaceJS.js

@@ -47,7 +47,12 @@ TestInterfaceJS.prototype = {
 
   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])

dom/bindings/test/TestJSImplGen.webidl

@@ -515,15 +515,15 @@ interface TestJSImplInterface {
   // 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);

dom/bindings/test/mochitest.ini

@@ -40,6 +40,8 @@ skip-if = (toolkit == 'gonk' && debug) #debug-only failure; bug 926547
 [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]

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>

dom/webidl/TestInterfaceJS.webidl

@@ -19,4 +19,9 @@ interface TestInterfaceJS {
   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();
 };