Be more conservative about safety of dictionary and union values.

Mark dictionaries containing GC values as must_root, and wrap them in
RootedTraceableBox in automatically-generated APIs. To accommodate
union variants that are now flagged as unsafe, add RootedTraceableBox
to union variants that need to be rooted, rather than wrapping the
entire union value.

5 files changed, 47 insertions, 17 deletions

diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py
index 208d4b1d923..4d31da25efe 100644
--- a/components/script/dom/bindings/codegen/CodegenRust.py
+++ b/components/script/dom/bindings/codegen/CodegenRust.py
@@ -723,9 +723,6 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None,
         if type.nullable():
             declType = CGWrapper(declType, pre="Option<", post=" >")
 
-        if isMember != "Dictionary" and type_needs_tracing(type):
-            declType = CGTemplatedType("RootedTraceableBox", declType)
-
         templateBody = ("match FromJSValConvertible::from_jsval(cx, ${val}, ()) {\n"
                         "    Ok(ConversionResult::Success(value)) => value,\n"
                         "    Ok(ConversionResult::Failure(error)) => {\n"
@@ -1427,6 +1424,8 @@ def getRetvalDeclarationForType(returnType, descriptorProvider):
         nullable = returnType.nullable()
         dictName = returnType.inner.name if nullable else returnType.name
         result = CGGeneric(dictName)
+        if type_needs_tracing(returnType):
+            result = CGWrapper(result, pre="RootedTraceableBox<", post=">")
         if nullable:
             result = CGWrapper(result, pre="Option<", post=">")
         return result
@@ -2262,6 +2261,7 @@ def UnionTypes(descriptors, dictionaries, callbacks, typedefs, config):
         'dom::bindings::str::ByteString',
         'dom::bindings::str::DOMString',
         'dom::bindings::str::USVString',
+        'dom::bindings::trace::RootedTraceableBox',
         'dom::types::*',
         'js::error::throw_type_error',
         'js::jsapi::HandleValue',
@@ -4132,15 +4132,23 @@ class CGUnionStruct(CGThing):
         self.type = type
         self.descriptorProvider = descriptorProvider
 
+    def membersNeedTracing(self):
+        for t in self.type.flatMemberTypes:
+            if type_needs_tracing(t):
+                return True
+        return False
+
     def define(self):
-        templateVars = map(lambda t: getUnionTypeTemplateVars(t, self.descriptorProvider),
+        templateVars = map(lambda t: (getUnionTypeTemplateVars(t, self.descriptorProvider),
+                                      type_needs_tracing(t)),
                            self.type.flatMemberTypes)
         enumValues = [
-            "    %s(%s)," % (v["name"], v["typeName"]) for v in templateVars
+            "    %s(%s)," % (v["name"], "RootedTraceableBox<%s>" % v["typeName"] if trace else v["typeName"])
+            for (v, trace) in templateVars
         ]
         enumConversions = [
             "            %s::%s(ref inner) => inner.to_jsval(cx, rval),"
-            % (self.type, v["name"]) for v in templateVars
+            % (self.type, v["name"]) for (v, _) in templateVars
         ]
         return ("""\
 #[derive(JSTraceable)]
@@ -4167,6 +4175,12 @@ class CGUnionConversionStruct(CGThing):
         self.type = type
         self.descriptorProvider = descriptorProvider
 
+    def membersNeedTracing(self):
+        for t in self.type.flatMemberTypes:
+            if type_needs_tracing(t):
+                return True
+        return False
+
     def from_jsval(self):
         memberTypes = self.type.flatMemberTypes
         names = []
@@ -4310,7 +4324,10 @@ class CGUnionConversionStruct(CGThing):
 
     def try_method(self, t):
         templateVars = getUnionTypeTemplateVars(t, self.descriptorProvider)
-        returnType = "Result<Option<%s>, ()>" % templateVars["typeName"]
+        actualType = templateVars["typeName"]
+        if type_needs_tracing(t):
+            actualType = "RootedTraceableBox<%s>" % actualType
+        returnType = "Result<Option<%s>, ()>" % actualType
         jsConversion = templateVars["jsConversion"]
 
         # Any code to convert to Object is unused, since we're already converting
@@ -6022,13 +6039,17 @@ class CGDictionary(CGThing):
                        (self.makeMemberName(m[0].identifier.name), self.getMemberType(m))
                        for m in self.memberInfo]
 
+        mustRoot = "#[must_root]\n" if self.membersNeedTracing() else ""
+
         return (string.Template(
                 "#[derive(JSTraceable)]\n"
+                "${mustRoot}" +
                 "pub struct ${selfName} {\n" +
                 "${inheritance}" +
                 "\n".join(memberDecls) + "\n" +
                 "}").substitute({"selfName": self.makeClassName(d),
-                                 "inheritance": inheritance}))
+                                 "inheritance": inheritance,
+                                 "mustRoot": mustRoot}))
 
     def impl(self):
         d = self.dictionary
@@ -6120,6 +6141,12 @@ class CGDictionary(CGThing):
                 "insertMembers": CGIndenter(memberInserts, indentLevel=8).define(),
             })
 
+    def membersNeedTracing(self):
+        for member, _ in self.memberInfo:
+            if type_needs_tracing(member.type):
+                return True
+        return False
+
     @staticmethod
     def makeDictionaryName(dictionary):
         return dictionary.identifier.name
diff --git a/components/script/dom/bindings/iterable.rs b/components/script/dom/bindings/iterable.rs
index 03fac1cb27f..39c681c7d8b 100644
--- a/components/script/dom/bindings/iterable.rs
+++ b/components/script/dom/bindings/iterable.rs
@@ -12,7 +12,7 @@ use dom::bindings::codegen::Bindings::IterableIteratorBinding::IterableKeyOrValu
 use dom::bindings::error::Fallible;
 use dom::bindings::js::{JS, Root};
 use dom::bindings::reflector::{DomObject, Reflector, reflect_dom_object};
-use dom::bindings::trace::JSTraceable;
+use dom::bindings::trace::{JSTraceable, RootedTraceableBox};
 use dom::globalscope::GlobalScope;
 use dom_struct::dom_struct;
 use js::conversions::ToJSValConvertible;
@@ -115,7 +115,7 @@ fn dict_return(cx: *mut JSContext,
                result: MutableHandleObject,
                done: bool,
                value: HandleValue) -> Fallible<()> {
-    let mut dict = unsafe { IterableKeyOrValueResult::empty(cx) };
+    let mut dict = RootedTraceableBox::new(unsafe { IterableKeyOrValueResult::empty(cx) });
     dict.done = done;
     dict.value.set(value.get());
     rooted!(in(cx) let mut dict_value = UndefinedValue());
@@ -130,7 +130,7 @@ fn key_and_value_return(cx: *mut JSContext,
                         result: MutableHandleObject,
                         key: HandleValue,
                         value: HandleValue) -> Fallible<()> {
-    let mut dict = unsafe { IterableKeyAndValueResult::empty(cx) };
+    let mut dict = RootedTraceableBox::new(unsafe { IterableKeyAndValueResult::empty(cx) });
     dict.done = false;
     dict.value = Some(vec![Heap::new(key.get()), Heap::new(value.get())]);
     rooted!(in(cx) let mut dict_value = UndefinedValue());
diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs
index ab2b20b97c8..797ee356bd8 100644
--- a/components/script/dom/bindings/trace.rs
+++ b/components/script/dom/bindings/trace.rs
@@ -745,6 +745,7 @@ impl<'a, T: JSTraceable + 'static> Drop for RootedTraceable<'a, T> {
 /// If you have GC things like *mut JSObject or JSVal, use rooted!.
 /// If you have an arbitrary number of DomObjects to root, use rooted_vec!.
 /// If you know what you're doing, use this.
+#[allow_unrooted_interior]
 pub struct RootedTraceableBox<T: 'static + JSTraceable> {
     ptr: *mut T,
 }
diff --git a/components/script/dom/filereader.rs b/components/script/dom/filereader.rs
index 3ee3c146b75..370643bfa44 100644
--- a/components/script/dom/filereader.rs
+++ b/components/script/dom/filereader.rs
@@ -13,6 +13,7 @@ use dom::bindings::js::{MutNullableJS, Root};
 use dom::bindings::refcounted::Trusted;
 use dom::bindings::reflector::{DomObject, reflect_dom_object};
 use dom::bindings::str::DOMString;
+use dom::bindings::trace::RootedTraceableBox;
 use dom::blob::Blob;
 use dom::domexception::{DOMErrorName, DOMException};
 use dom::event::{Event, EventBubbles, EventCancelable};
@@ -338,7 +339,8 @@ impl FileReaderMethods for FileReader {
             FileReaderResult::String(ref string) =>
                 StringOrObject::String(string.clone()),
             FileReaderResult::ArrayBuffer(ref arr_buffer) => {
-                StringOrObject::Object(Heap::new((*arr_buffer.ptr.get()).to_object()))
+                StringOrObject::Object(RootedTraceableBox::new(
+                    Heap::new((*arr_buffer.ptr.get()).to_object())))
             }
         })
     }
diff --git a/components/script/dom/testbinding.rs b/components/script/dom/testbinding.rs
index 1a56a77a64f..266497da83e 100644
--- a/components/script/dom/testbinding.rs
+++ b/components/script/dom/testbinding.rs
@@ -338,8 +338,8 @@ impl TestBindingMethods for TestBinding {
         Some(ByteStringOrLong::ByteString(ByteString::new(vec!())))
     }
     fn ReceiveNullableSequence(&self) -> Option<Vec<i32>> { Some(vec![1]) }
-    fn ReceiveTestDictionaryWithSuccessOnKeyword(&self) -> TestDictionary {
-        TestDictionary {
+    fn ReceiveTestDictionaryWithSuccessOnKeyword(&self) -> RootedTraceableBox<TestDictionary> {
+        RootedTraceableBox::new(TestDictionary {
             anyValue: Heap::new(NullValue()),
             booleanValue: None,
             byteValue: None,
@@ -401,7 +401,7 @@ impl TestBindingMethods for TestBinding {
             usvstringValue: None,
             nonRequiredNullable: None,
             nonRequiredNullable2: Some(None), // null
-        }
+        })
     }
 
     fn DictMatchesPassedValues(&self, arg: RootedTraceableBox<TestDictionary>) -> bool {
@@ -436,9 +436,9 @@ impl TestBindingMethods for TestBinding {
     fn PassUnion6(&self, _: UnsignedLongOrBoolean) {}
     fn PassUnion7(&self, _: StringSequenceOrUnsignedLong) {}
     fn PassUnion8(&self, _: ByteStringSequenceOrLong) {}
-    fn PassUnion9(&self, _: RootedTraceableBox<UnionTypes::TestDictionaryOrLong>) {}
+    fn PassUnion9(&self, _: UnionTypes::TestDictionaryOrLong) {}
     #[allow(unsafe_code)]
-    unsafe fn PassUnion10(&self, _: *mut JSContext, _: RootedTraceableBox<UnionTypes::StringOrObject>) {}
+    unsafe fn PassUnion10(&self, _: *mut JSContext, _: UnionTypes::StringOrObject) {}
     fn PassUnionWithTypedef(&self, _: DocumentOrTestTypedef) {}
     fn PassUnionWithTypedef2(&self, _: LongSequenceOrTestTypedef) {}
     #[allow(unsafe_code)]