Address review comments. Fix union conversion result mistakes introduced in refactoring.

src/components/script/dom/bindings/codegen/CodegenRust.py

@@ -856,9 +856,7 @@ for (uint32_t i = 0; i < length; ++i) {
                           "  return 0;\n"
                           "}\n"
                           "if !done {\n"
-                          "  //XXXjdm throw exception\n"
+                          "  return throw_not_in_union(cx, \"%s\");\n"
-                          "  //return ThrowErrorMessage(cx, MSG_NOT_IN_UNION, \"%s\");\n"
-                          "  return 0;"
                           "}" % ", ".join(names))
         templateBody = CGWrapper(CGIndenter(CGList([templateBody, throw], "\n")), pre="{\n", post="\n}")
 
@@ -869,9 +867,9 @@ for (uint32_t i = 0; i < length; ++i) {
         nonConstDecl = "${declName}"
 
         def handleNull(templateBody, setToNullVar, extraConditionForNull=""):
-            null = CGGeneric("if %sRUST_JSVAL_IS_NULL(${val}) != 0 || %sRUST_JSVAL_IS_VOID(${val}) != 0 {\n"
+            null = CGGeneric("if %s(RUST_JSVAL_IS_NULL(${val}) != 0 || RUST_JSVAL_IS_VOID(${val}) != 0) {\n"
                              "  %s = None;\n"
-                             "}" % (extraConditionForNull, extraConditionForNull, setToNullVar))
+                             "}" % (extraConditionForNull, setToNullVar))
             templateBody = CGWrapper(CGIndenter(templateBody), pre="{\n", post="\n}")
             return CGList([null, templateBody], " else ")
 
@@ -3782,11 +3780,7 @@ def getUnionTypeTemplateVars(type, descriptorProvider):
         type, descriptorProvider, failureCode=tryNextCode,
         isDefinitelyObject=True, isOptional=type.nullable(), preSuccess="e" + name + "(", postSuccess=")")
 
-    # This is ugly, but UnionMember needs to call a constructor with no
-    # arguments so the type can't be const.
     structType = declType.define()
-    if structType.startswith("const "):
-        structType = structType[6:]
     externalType = getUnionAccessorSignatureType(type, descriptorProvider).define()
 
     if type.isObject():
@@ -3804,7 +3798,7 @@ def getUnionTypeTemplateVars(type, descriptorProvider):
             )
         jsConversion = CGWrapper(CGGeneric(jsConversion),
                                  post="\n"
-                                      "return Ok(true);")
+                                      "return Ok(false);")
         setter = CGWrapper(CGIndenter(jsConversion),
                            pre="pub fn TrySetTo" + name + "(&mut self, cx: *JSContext, value: JSVal, pvalue: *JSVal) -> Result<bool,()> {\n",
                            post="\n"
@@ -3850,8 +3844,6 @@ class CGUnionStruct(CGThing):
 
         destructorTemplate = """  fn Destroy${name}(&mut self) {
     assert!(Is${name}(), "Wrong type!");
-    //mValue.m${name}.Destroy();
-    //mType = eUninitialized;
     *self.mUnion = None;
   }"""
         destructors = mapTemplate(destructorTemplate, templateVars)

src/components/script/dom/bindings/utils.rs

@@ -879,6 +879,15 @@ pub fn throw_method_failed_with_details<T>(cx: *JSContext,
     return 0;
 }
 
+pub fn throw_not_in_union(cx: *JSContext, names: &'static str) -> JSBool {
+    assert!(unsafe { JS_IsExceptionPending(cx) } == 0);
+    let message = format!("argument could not be converted to any of: {}", names);
+    message.with_c_str(|string| {
+        unsafe { ReportError(cx, string) };
+    });
+    return 0;
+}
+
 /// Execute arbitrary code with the JS GC enabled, then disable it afterwards.
 pub fn with_gc_enabled<R>(cx: *JSContext, f: || -> R) -> R {
     unsafe {

src/test/html/content/harness.js

@@ -37,6 +37,24 @@ function is_function(val, name) {
   starts_with(String(val), "function " + name + "(");
 }
 
+function should_throw(f) {
+  try {
+    f();
+    _fail("operation should have thrown but did not");
+  } catch (x) {
+    _pass("operation successfully threw an exception", x.toString());
+  }
+}
+
+function should_not_throw(f) {
+  try {
+    f();
+    _pass("operation did not throw an exception");
+  } catch (x) {
+    _fail("operation should have not thrown", x.toString());
+  }
+}
+
 var _test_complete = false;
 var _test_timeout = 10000; //10 seconds
 function finish() {

src/test/html/content/test_exception.html

@@ -1,11 +1,6 @@
 <!doctype html>
 <script src="harness.js"></script>
 <script>
-try {
+should_throw(function() { document.createElement("1foo") });
-  document.createElement("1foo");
-  is(true, false, "No exception thrown");
-} catch (e) {
-  is(true, true, "Exception caught");
-}
 finish();
 </script>

src/test/html/content/test_union.html

@@ -3,31 +3,22 @@
 <script src="harness.js"></script>
 <select id="sel"></select>
 <script>
-  var sel = document.getElementById('sel');
-  var opt = document.createElement('option');
-  sel.add(opt);
-  var optgroup = document.createElement('optgroup');
-  sel.add(optgroup);
   var div = document.createElement('div');
-  sel.add(opt, div);
+  var optgroup = document.createElement('optgroup');
-  sel.add(optgroup, div);
+  var sel = document.getElementById('sel');
-  sel.add(opt, 5);
-  sel.add(optgroup, 5);
-  is(true, true, "should not have thrown any exceptions");
 
-  try {
+  should_not_throw(function() {
-    self.add(div);
+    var opt = document.createElement('option');
-    is(true, false, "should have thrown");
+    sel.add(opt);
-  } catch (x) {
+    sel.add(optgroup);
-    is(true, true, "should have thrown");
+    sel.add(opt, div);
-  }
+    sel.add(optgroup, div);
+    sel.add(opt, 5);
+    sel.add(optgroup, 5);
+  });
 
-  try {
+  should_throw(function() { sel.add(div) });
-    self.add(optgroup, "");
+  should_throw(function() { sel.add(optgroup, function() {}) });
-    is(true, false, "should have thrown");
-  } catch (x) {
-    is(true, true, "should have thrown");
-  }
 
   finish();
 </script>