Initialize rooted dictionaries to a stable value before setting fields.

2 files changed, 58 insertions, 23 deletions

diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py
index aa58afe1913..05db89f3766 100644
--- a/components/script/dom/bindings/codegen/CodegenRust.py
+++ b/components/script/dom/bindings/codegen/CodegenRust.py
@@ -1045,12 +1045,12 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None,
             if defaultValue is None:
                 default = None
             elif isinstance(defaultValue, IDLNullValue):
-                default = "Heap::new(NullValue())"
+                default = "NullValue()"
             elif isinstance(defaultValue, IDLUndefinedValue):
-                default = "Heap::new(UndefinedValue())"
+                default = "UndefinedValue()"
             else:
                 raise TypeError("Can't handle non-null, non-undefined default value here")
-            return handleOptional("Heap::new(${val}.get())", declType, default)
+            return handleOptional("${val}.get()", declType, default)
 
         declType = CGGeneric("HandleValue")
 
@@ -1077,8 +1077,6 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None,
 
         if isMember in ("Dictionary", "Union"):
             declType = CGGeneric("Heap<*mut JSObject>")
-            templateBody = "Heap::new(%s)" % templateBody
-            default = "Heap::new(%s)" % default
         else:
             # TODO: Need to root somehow
             # https://github.com/servo/servo/issues/6382
@@ -5708,6 +5706,7 @@ def generate_imports(config, cgthings, descriptors, callbacks=None, dictionaries
         'dom::bindings::iterable::Iterable',
         'dom::bindings::iterable::IteratorType',
         'dom::bindings::js::JS',
+        'dom::bindings::js::OptionalHeapSetter',
         'dom::bindings::js::Root',
         'dom::bindings::js::RootedReference',
         'dom::bindings::namespace::NamespaceObjectClass',
@@ -6064,7 +6063,7 @@ class CGDictionary(CGThing):
     def impl(self):
         d = self.dictionary
         if d.parent:
-            initParent = ("parent: {\n"
+            initParent = ("{\n"
                           "    match try!(%s::%s::new(cx, val)) {\n"
                           "        ConversionResult::Success(v) => v,\n"
                           "        ConversionResult::Failure(error) => {\n"
@@ -6072,16 +6071,20 @@ class CGDictionary(CGThing):
                           "            return Err(());\n"
                           "        }\n"
                           "    }\n"
-                          "},\n" % (self.makeModuleName(d.parent),
-                                    self.makeClassName(d.parent)))
+                          "}" % (self.makeModuleName(d.parent),
+                                 self.makeClassName(d.parent)))
         else:
             initParent = ""
 
-        def memberInit(memberInfo):
+        def memberInit(memberInfo, isInitial):
             member, _ = memberInfo
             name = self.makeMemberName(member.identifier.name)
             conversion = self.getMemberConversion(memberInfo, member.type)
-            return CGGeneric("%s: %s,\n" % (name, conversion.define()))
+            if isInitial:
+                return CGGeneric("%s: %s,\n" % (name, conversion.define()))
+            if member.type.isAny() or member.type.isObject():
+                return CGGeneric("dictionary.%s.set(%s);\n" % (name, conversion.define()))
+            return CGGeneric("dictionary.%s = %s;\n" % (name, conversion.define()))
 
         def varInsert(varName, dictionaryName):
             insertion = ("rooted!(in(cx) let mut %s_js = UndefinedValue());\n"
@@ -6101,16 +6104,21 @@ class CGDictionary(CGThing):
                                       (name, name, varInsert(name, member.identifier.name).define()))
             return CGGeneric("%s\n" % insertion.define())
 
-        memberInits = CGList([memberInit(m) for m in self.memberInfo])
         memberInserts = CGList([memberInsert(m) for m in self.memberInfo])
 
-        actualType = self.makeClassName(d)
-        preInitial = ""
-        postInitial = ""
+        selfName = self.makeClassName(d)
         if self.membersNeedTracing():
-            actualType = "RootedTraceableBox<%s>" % actualType
-            preInitial = "RootedTraceableBox::new("
-            postInitial = ")"
+            actualType = "RootedTraceableBox<%s>" % selfName
+            preInitial = "let mut dictionary = RootedTraceableBox::new(%s::default());\n" % selfName
+            initParent = initParent = ("dictionary.parent = %s;\n" % initParent) if initParent else ""
+            memberInits = CGList([memberInit(m, False) for m in self.memberInfo])
+            postInitial = ""
+        else:
+            actualType = selfName
+            preInitial = "let dictionary = %s {\n" % selfName
+            postInitial = "};\n"
+            initParent = ("parent: %s,\n" % initParent) if initParent else ""
+            memberInits = CGList([memberInit(m, True) for m in self.memberInfo])
 
         return string.Template(
             "impl ${selfName} {\n"
@@ -6131,10 +6139,10 @@ class CGDictionary(CGThing):
             "            return Err(());\n"
             "        };\n"
             "        rooted!(in(cx) let object = object);\n"
-            "        let dictionary = ${preInitial}${selfName} {\n"
+            "${preInitial}"
             "${initParent}"
             "${initMembers}"
-            "        }${postInitial};\n"
+            "${postInitial}"
             "        Ok(ConversionResult::Success(dictionary))\n"
             "    }\n"
             "}\n"
@@ -6154,13 +6162,13 @@ class CGDictionary(CGThing):
             "        rval.set(ObjectOrNullValue(obj.get()))\n"
             "    }\n"
             "}\n").substitute({
-                "selfName": self.makeClassName(d),
+                "selfName": selfName,
                 "actualType": actualType,
                 "initParent": CGIndenter(CGGeneric(initParent), indentLevel=12).define(),
                 "initMembers": CGIndenter(memberInits, indentLevel=12).define(),
                 "insertMembers": CGIndenter(memberInserts, indentLevel=8).define(),
-                "preInitial": CGGeneric(preInitial).define(),
-                "postInitial": CGGeneric(postInitial).define(),
+                "preInitial": CGIndenter(CGGeneric(preInitial), indentLevel=12).define(),
+                "postInitial": CGIndenter(CGGeneric(postInitial), indentLevel=12).define(),
             })
 
     def membersNeedTracing(self):
diff --git a/components/script/dom/bindings/js.rs b/components/script/dom/bindings/js.rs
index 976665e4850..07dc4d74778 100644
--- a/components/script/dom/bindings/js.rs
+++ b/components/script/dom/bindings/js.rs
@@ -31,7 +31,8 @@ use dom::bindings::trace::JSTraceable;
 use dom::bindings::trace::trace_reflector;
 use dom::node::Node;
 use heapsize::HeapSizeOf;
-use js::jsapi::{JSObject, JSTracer};
+use js::jsapi::{JSObject, JSTracer, Heap};
+use js::rust::GCMethods;
 use mitochondria::OnceCell;
 use script_layout_interface::TrustedNodeAddress;
 use script_thread::STACK_ROOTS;
@@ -654,3 +655,29 @@ unsafe impl<T: DomObject> JSTraceable for Root<T> {
         // Already traced.
     }
 }
+
+/// Helper trait for safer manipulations of Option<Heap<T>> values.
+pub trait OptionalHeapSetter {
+    type Value;
+    /// Update this optional heap value with a new value.
+    fn set(&mut self, v: Option<Self::Value>);
+}
+
+impl<T: GCMethods + Copy> OptionalHeapSetter for Option<Heap<T>> where Heap<T>: Default {
+    type Value = T;
+    fn set(&mut self, v: Option<T>) {
+        let v = match v {
+            None => {
+                *self = None;
+                return;
+            }
+            Some(v) => v,
+        };
+
+        if self.is_none() {
+            *self = Some(Heap::default());
+        }
+
+        self.as_ref().unwrap().set(v);
+    }
+}