WIP

This removes Document::look_up_named_elements which was not a good abstraction
over the Document and Window named getters.

Cargo.lock

@@ -3650,8 +3650,8 @@ dependencies = [
 
 [[package]]
 name = "mozjs"
-version = "0.13.0"
-source = "git+https://github.com/servo/rust-mozjs#5a50c377a4e1f55abc1286b74100a4cc35adc789"
+version = "0.14.1"
+source = "git+https://github.com/servo/rust-mozjs#2028017734e8c274d0e6a16d21526eecd75089ba"
 dependencies = [
  "cc",
  "lazy_static",
@@ -3663,8 +3663,8 @@ dependencies = [
 
 [[package]]
 name = "mozjs_sys"
-version = "0.68.1"
-source = "git+https://github.com/servo/mozjs?rev=aa97a46f277823a0e66017fea820ac3a9d987711#aa97a46f277823a0e66017fea820ac3a9d987711"
+version = "0.68.2"
+source = "git+https://github.com/servo/mozjs?rev=a40b829f0fefa06a743f006260c7facebcfa27aa#a40b829f0fefa06a743f006260c7facebcfa27aa"
 dependencies = [
  "bindgen",
  "cc",

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

@@ -2778,6 +2778,8 @@ rooted!(in(*cx) let obj = NewProxyObject(
     handler,
     Handle::from_raw(UndefinedHandleValue),
     proto.get(),
+    ptr::null(),
+    false,
 ));
 assert!(!obj.is_null());
 SetProxyReservedSlot(
@@ -3133,6 +3135,13 @@ rooted!(in(*cx) let mut prototype_proto = ptr::null_mut::<JSObject>());
 %s;
 assert!(!prototype_proto.is_null());""" % getPrototypeProto)]
 
+        if self.descriptor.hasNamedPropertiesObject():
+            assert not self.haveUnscopables
+            code.append(CGGeneric("""\
+rooted!(in(*cx) let mut prototype_proto_proto = prototype_proto.get());
+dom::types::%s::create_named_properties_object(cx, prototype_proto_proto.handle(), prototype_proto.handle_mut());
+assert!(!prototype_proto.is_null());""" % name))
+
         properties = {
             "id": name,
             "unscopables": "unscopable_names" if self.haveUnscopables else "&[]",
@@ -5300,13 +5309,12 @@ class CGDOMJSProxyHandler_getOwnPropertyDescriptor(CGAbstractExternMethod):
             attrs = "JSPROP_ENUMERATE"
             if self.descriptor.operations['IndexedSetter'] is None:
                 attrs += " | JSPROP_READONLY"
-            fillDescriptor = ("desc.value = result_root.get();\n"
-                              "fill_property_descriptor(MutableHandle::from_raw(desc), proxy.get(), (%s) as u32);\n"
-                              "return true;" % attrs)
+            fill = ("fill_property_descriptor(MutableHandle::from_raw(desc), proxy.get(), rval.get(), (%s) as u32);\n"
+                    "return true;" % attrs)
             templateValues = {
-                'jsvalRef': 'result_root.handle_mut()',
-                'successCode': fillDescriptor,
-                'pre': 'rooted!(in(*cx) let mut result_root = UndefinedValue());'
+                'jsvalRef': 'rval.handle_mut()',
+                'successCode': fill,
+                'pre': 'rooted!(in(*cx) let mut rval = UndefinedValue());'
             }
             get += ("if let Some(index) = index {\n"
                     + "    let this = UnwrapProxy(proxy);\n"
@@ -5314,8 +5322,7 @@ class CGDOMJSProxyHandler_getOwnPropertyDescriptor(CGAbstractExternMethod):
                     + CGIndenter(CGProxyIndexedGetter(self.descriptor, templateValues)).define() + "\n"
                     + "}\n")
 
-        namedGetter = self.descriptor.operations['NamedGetter']
-        if namedGetter:
+        if self.descriptor.supportsNamedProperties():
             attrs = []
             if not self.descriptor.interface.getExtendedAttribute("LegacyUnenumerableNamedProperties"):
                 attrs.append("JSPROP_ENUMERATE")
@@ -5325,13 +5332,12 @@ class CGDOMJSProxyHandler_getOwnPropertyDescriptor(CGAbstractExternMethod):
                 attrs = " | ".join(attrs)
             else:
                 attrs = "0"
-            fillDescriptor = ("desc.value = result_root.get();\n"
-                              "fill_property_descriptor(MutableHandle::from_raw(desc), proxy.get(), (%s) as u32);\n"
-                              "return true;" % attrs)
+            fill = ("fill_property_descriptor(MutableHandle::from_raw(desc), proxy.get(), rval.get(), (%s) as u32);\n"
+                    "return true;" % attrs)
             templateValues = {
-                'jsvalRef': 'result_root.handle_mut()',
-                'successCode': fillDescriptor,
-                'pre': 'rooted!(in(*cx) let mut result_root = UndefinedValue());'
+                'jsvalRef': 'rval.handle_mut()',
+                'successCode': fill,
+                'pre': 'rooted!(in(*cx) let mut rval = UndefinedValue());'
             }
 
             # See the similar-looking in CGDOMJSProxyHandler_get for the spec quote.
@@ -5414,7 +5420,7 @@ class CGDOMJSProxyHandler_defineProperty(CGAbstractExternMethod):
                     + CGIndenter(CGProxyNamedSetter(self.descriptor)).define()
                     + "    return (*opresult).succeed();\n"
                     + "}\n")
-        elif self.descriptor.operations['NamedGetter']:
+        elif self.descriptor.supportsNamedProperties():
             set += ("if RUST_JSID_IS_STRING(id) || RUST_JSID_IS_INT(id) {\n"
                     + CGIndenter(CGProxyNamedGetter(self.descriptor)).define()
                     + "    if result.is_some() {\n"
@@ -5475,7 +5481,7 @@ class CGDOMJSProxyHandler_ownPropertyKeys(CGAbstractExternMethod):
                 }
                 """)
 
-        if self.descriptor.operations['NamedGetter']:
+        if self.descriptor.supportsNamedProperties():
             body += dedent(
                 """
                 for name in (*unwrapped_proxy).SupportedPropertyNames() {
@@ -5572,11 +5578,10 @@ class CGDOMJSProxyHandler_hasOwn(CGAbstractExternMethod):
                         + "    return true;\n"
                         + "}\n\n")
 
-        namedGetter = self.descriptor.operations['NamedGetter']
         condition = "RUST_JSID_IS_STRING(id) || RUST_JSID_IS_INT(id)"
         if indexedGetter:
             condition = "index.is_none() && (%s)" % condition
-        if namedGetter:
+        if self.descriptor.supportsNamedProperties():
             named = """\
 if %s {
     let mut has_on_proto = false;
@@ -5659,8 +5664,7 @@ if !expando.is_null() {
         else:
             getIndexedOrExpando = getFromExpando + "\n"
 
-        namedGetter = self.descriptor.operations['NamedGetter']
-        if namedGetter:
+        if self.descriptor.supportsNamedProperties():
             condition = "RUST_JSID_IS_STRING(id) || RUST_JSID_IS_INT(id)"
             # From step 1:
             #     If O supports indexed properties and P is an array index, then:
@@ -5908,7 +5912,7 @@ class CGInterfaceTrait(CGThing):
                                ),
                                rettype)
 
-            if descriptor.proxy:
+            if descriptor.proxy or descriptor.isGlobal():
                 for name, operation in descriptor.operations.iteritems():
                     if not operation or operation.isStringifier():
                         continue

components/script/dom/bindings/codegen/Configuration.py

@@ -280,7 +280,8 @@ class Descriptor(DescriptorProvider):
                         continue
 
                     def addIndexedOrNamedOperation(operation, m):
-                        self.proxy = True
+                        if not self.isGlobal():
+                            self.proxy = True
                         if m.isIndexed():
                             operation = 'Indexed' + operation
                         else:
@@ -366,6 +367,15 @@ class Descriptor(DescriptorProvider):
     def internalNameFor(self, name):
         return self._internalNames.get(name, name)
 
+    def hasNamedPropertiesObject(self):
+        if self.interface.isExternal():
+            return False
+
+        return self.isGlobal() and self.supportsNamedProperties()
+
+    def supportsNamedProperties(self):
+        return self.operations['NamedGetter'] is not None
+
     def getExtendedAttributes(self, member, getter=False, setter=False):
         def maybeAppendInfallibleToAttrs(attrs, throws):
             if throws is None:

components/script/dom/bindings/proxyhandler.rs

@@ -19,6 +19,7 @@ use js::jsapi::MutableHandleObject as RawMutableHandleObject;
 use js::jsapi::ObjectOpResult;
 use js::jsapi::{DOMProxyShadowsResult, JSContext, JSObject, PropertyDescriptor};
 use js::jsapi::{JSErrNum, SetDOMProxyInformation};
+use js::jsval::JSVal;
 use js::jsval::ObjectValue;
 use js::jsval::UndefinedValue;
 use js::rust::wrappers::JS_AlreadyHasOwnPropertyById;
@@ -167,12 +168,14 @@ pub unsafe fn ensure_expando_object(
 
 /// Set the property descriptor's object to `obj` and set it to enumerable,
 /// and writable if `readonly` is true.
-pub fn fill_property_descriptor(
+pub unsafe fn fill_property_descriptor(
     mut desc: MutableHandle<PropertyDescriptor>,
     obj: *mut JSObject,
+    v: JSVal,
     attrs: u32,
 ) {
     desc.obj = obj;
+    desc.value = v;
     desc.attrs = attrs;
     desc.getter = None;
     desc.setter = None;

components/script/dom/document.rs

@@ -464,12 +464,6 @@ impl CollectionFilter for AnchorsFilter {
     }
 }
 
-enum ElementLookupResult {
-    None,
-    One(DomRoot<Element>),
-    Many,
-}
-
 #[allow(non_snake_case)]
 impl Document {
     pub fn note_node_with_dirty_descendants(&self, node: &Node) {
@@ -2852,75 +2846,6 @@ impl Document {
             .drain()
             .for_each(|(_, context)| context.send_swap_chain_present());
     }
-
-    // https://html.spec.whatwg.org/multipage/#dom-tree-accessors:supported-property-names
-    // (This takes the filter as a method so the window named getter can use it too)
-    pub fn supported_property_names_impl(
-        &self,
-        nameditem_filter: fn(&Node, &Atom) -> bool,
-    ) -> Vec<DOMString> {
-        // The tricky part here is making sure we return the names in
-        // tree order, without just resorting to a full tree walkthrough.
-
-        let mut first_elements_with_name: HashMap<&Atom, &Dom<Element>> = HashMap::new();
-
-        // Get the first-in-tree-order element for each name in the name_map
-        let name_map = self.name_map.borrow();
-        name_map.iter().for_each(|(name, value)| {
-            if let Some(first) = value
-                .iter()
-                .find(|n| nameditem_filter((***n).upcast::<Node>(), &name))
-            {
-                first_elements_with_name.insert(name, first);
-            }
-        });
-
-        // Get the first-in-tree-order element for each name in the id_map;
-        // if we already had one from the name_map, figure out which of
-        // the two is first.
-        let id_map = self.id_map.borrow();
-        id_map.iter().for_each(|(name, value)| {
-            if let Some(first) = value
-                .iter()
-                .find(|n| nameditem_filter((***n).upcast::<Node>(), &name))
-            {
-                match first_elements_with_name.get(&name) {
-                    None => {
-                        first_elements_with_name.insert(name, first);
-                    },
-                    Some(el) => {
-                        if *el != first && first.upcast::<Node>().is_before(el.upcast::<Node>()) {
-                            first_elements_with_name.insert(name, first);
-                        }
-                    },
-                }
-            }
-        });
-
-        // first_elements_with_name now has our supported property names
-        // as keys, and the elements to order on as values.
-        let mut sortable_vec: Vec<(&Atom, &Dom<Element>)> = first_elements_with_name
-            .iter()
-            .map(|(k, v)| (*k, *v))
-            .collect();
-        sortable_vec.sort_unstable_by(|a, b| {
-            if a.1 == b.1 {
-                // This can happen if an img has an id different from its name,
-                // spec does not say which string to put first.
-                a.0.cmp(&b.0)
-            } else if a.1.upcast::<Node>().is_before(b.1.upcast::<Node>()) {
-                Ordering::Less
-            } else {
-                Ordering::Greater
-            }
-        });
-
-        // And now that they're sorted, we can return the keys
-        sortable_vec
-            .iter()
-            .map(|(k, _v)| DOMString::from(&***k))
-            .collect()
-    }
 }
 
 fn is_character_value_key(key: &Key) -> bool {
@@ -3846,79 +3771,16 @@ impl Document {
         )
     }
 
-    // https://html.spec.whatwg.org/multipage/#dom-tree-accessors:determine-the-value-of-a-named-property
-    // Support method for steps 1-3:
-    // Count if there are 0, 1, or >1 elements that match the name.
-    // (This takes the filter as a method so the window named getter can use it too)
-    fn look_up_named_elements(
-        &self,
-        name: &Atom,
-        nameditem_filter: fn(&Node, &Atom) -> bool,
-    ) -> ElementLookupResult {
-        // We might match because of either id==name or name==name, so there
-        // are two sets of nodes to look through, but we don't need a
-        // full tree traversal.
-        let id_map = self.id_map.borrow();
-        let name_map = self.name_map.borrow();
-        let id_vec = id_map.get(&name);
-        let name_vec = name_map.get(&name);
+    pub fn get_elements_with_id(&self, id: &Atom) -> Ref<[Dom<Element>]> {
+        Ref::map(self.id_map.borrow(), |map| {
+            map.get(id).map(|vec| &**vec).unwrap_or_default()
+        })
+    }
 
-        // If nothing can possibly have the name, exit fast
-        if id_vec.is_none() && name_vec.is_none() {
-            return ElementLookupResult::None;
-        }
-
-        let one_from_id_map = if let Some(id_vec) = id_vec {
-            let mut elements = id_vec
-                .iter()
-                .filter(|n| nameditem_filter((***n).upcast::<Node>(), &name))
-                .peekable();
-            if let Some(first) = elements.next() {
-                if elements.peek().is_none() {
-                    Some(first)
-                } else {
-                    return ElementLookupResult::Many;
-                }
-            } else {
-                None
-            }
-        } else {
-            None
-        };
-
-        let one_from_name_map = if let Some(name_vec) = name_vec {
-            let mut elements = name_vec
-                .iter()
-                .filter(|n| nameditem_filter((***n).upcast::<Node>(), &name))
-                .peekable();
-            if let Some(first) = elements.next() {
-                if elements.peek().is_none() {
-                    Some(first)
-                } else {
-                    return ElementLookupResult::Many;
-                }
-            } else {
-                None
-            }
-        } else {
-            None
-        };
-
-        // We now have two elements, or one element, or the same
-        // element twice, or no elements.
-        match (one_from_id_map, one_from_name_map) {
-            (Some(one), None) | (None, Some(one)) => {
-                ElementLookupResult::One(DomRoot::from_ref(&one))
-            },
-            (Some(one), Some(other)) => {
-                if one == other {
-                    ElementLookupResult::One(DomRoot::from_ref(&one))
-                } else {
-                    ElementLookupResult::Many
-                }
-            },
-            (None, None) => ElementLookupResult::None,
-        }
+    pub fn get_elements_with_name(&self, name: &Atom) -> Ref<[Dom<Element>]> {
+        Ref::map(self.name_map.borrow(), |map| {
+            map.get(name).map(|vec| &**vec).unwrap_or_default()
+        })
     }
 
     #[allow(unrooted_must_root)]
@@ -4843,45 +4705,74 @@ impl DocumentMethods for Document {
     #[allow(unsafe_code)]
     // https://html.spec.whatwg.org/multipage/#dom-tree-accessors:dom-document-nameditem-filter
     fn NamedGetter(&self, _cx: JSContext, name: DOMString) -> Option<NonNull<JSObject>> {
-        #[derive(JSTraceable, MallocSizeOf)]
-        struct NamedElementFilter {
-            name: Atom,
+        if name.is_empty() {
+            return None;
         }
-        impl CollectionFilter for NamedElementFilter {
-            fn filter(&self, elem: &Element, _root: &Node) -> bool {
-                elem.upcast::<Node>().is_document_named_item(&self.name)
+        let name = Atom::from(name);
+
+        // Step 1.
+        let elements_with_name = self.get_elements_with_name(&name);
+        let name_iter = elements_with_name
+            .iter()
+            .filter(|elem| is_named_element_with_name_attribute(elem));
+        let elements_with_id = self.get_elements_with_id(&name);
+        let id_iter = elements_with_id
+            .iter()
+            .filter(|elem| is_named_element_with_id_attribute(elem));
+        let mut elements = name_iter.chain(id_iter);
+
+        let first = elements.next()?;
+
+        if elements.next().is_none() {
+            // Step 2.
+            if let Some(nested_window_proxy) = first
+                .downcast::<HTMLIFrameElement>()
+                .and_then(|iframe| iframe.GetContentWindow())
+            {
+                unsafe {
+                    return Some(NonNull::new_unchecked(
+                        nested_window_proxy.reflector().get_jsobject().get(),
+                    ));
+                }
+            }
+
+            // Step 3.
+            unsafe {
+                return Some(NonNull::new_unchecked(
+                    first.reflector().get_jsobject().get(),
+                ));
             }
         }
 
-        let name = Atom::from(name);
-
-        match self.look_up_named_elements(&name, Node::is_document_named_item) {
-            ElementLookupResult::None => {
-                return None;
-            },
-            ElementLookupResult::One(element) => {
-                if let Some(nested_proxy) = element
-                    .downcast::<HTMLIFrameElement>()
-                    .and_then(|iframe| iframe.GetContentWindow())
-                {
-                    unsafe {
-                        return Some(NonNull::new_unchecked(
-                            nested_proxy.reflector().get_jsobject().get(),
-                        ));
-                    }
-                }
-                unsafe {
-                    return Some(NonNull::new_unchecked(
-                        element.reflector().get_jsobject().get(),
-                    ));
-                }
-            },
-            ElementLookupResult::Many => {},
-        };
-
         // Step 4.
-        let filter = NamedElementFilter { name: name };
-        let collection = HTMLCollection::create(self.window(), self.upcast(), Box::new(filter));
+        #[derive(JSTraceable, MallocSizeOf)]
+        struct DocumentNamedGetter {
+            name: Atom,
+        }
+        impl CollectionFilter for DocumentNamedGetter {
+            fn filter(&self, elem: &Element, _root: &Node) -> bool {
+                let type_ = match elem.upcast::<Node>().type_id() {
+                    NodeTypeId::Element(ElementTypeId::HTMLElement(type_)) => type_,
+                    _ => return false,
+                };
+                match type_ {
+                    HTMLElementTypeId::HTMLFormElement | HTMLElementTypeId::HTMLIFrameElement => {
+                        elem.get_name().as_ref() == Some(&self.name)
+                    },
+                    HTMLElementTypeId::HTMLImageElement => elem.get_name().map_or(false, |name| {
+                        name == *self.name ||
+                            !name.is_empty() && elem.get_id().as_ref() == Some(&self.name)
+                    }),
+                    // TODO: Handle exposed objects.
+                    _ => false,
+                }
+            }
+        }
+        let collection = HTMLCollection::create(
+            self.window(),
+            self.upcast(),
+            Box::new(DocumentNamedGetter { name }),
+        );
         unsafe {
             Some(NonNull::new_unchecked(
                 collection.reflector().get_jsobject().get(),
@@ -4891,7 +4782,61 @@ impl DocumentMethods for Document {
 
     // https://html.spec.whatwg.org/multipage/#dom-tree-accessors:supported-property-names
     fn SupportedPropertyNames(&self) -> Vec<DOMString> {
-        self.supported_property_names_impl(Node::is_document_named_item)
+        let mut names_with_first_named_element_map: HashMap<&Atom, &Element> = HashMap::new();
+
+        let name_map = self.name_map.borrow();
+        for (name, elements) in &*name_map {
+            if name.is_empty() {
+                continue;
+            }
+            let mut name_iter = elements
+                .iter()
+                .filter(|elem| is_named_element_with_name_attribute(elem));
+            if let Some(first) = name_iter.next() {
+                names_with_first_named_element_map.insert(name, first);
+            }
+        }
+        let id_map = self.id_map.borrow();
+        for (id, elements) in &*id_map {
+            if id.is_empty() {
+                continue;
+            }
+            let mut id_iter = elements
+                .iter()
+                .filter(|elem| is_named_element_with_id_attribute(elem));
+            if let Some(first) = id_iter.next() {
+                match names_with_first_named_element_map.entry(id) {
+                    Vacant(entry) => drop(entry.insert(first)),
+                    Occupied(mut entry) => {
+                        if first.upcast::<Node>().is_before(entry.get().upcast()) {
+                            *entry.get_mut() = first;
+                        }
+                    },
+                }
+            }
+        }
+
+        let mut names_with_first_named_element_vec: Vec<(&Atom, &Element)> =
+            names_with_first_named_element_map
+                .iter()
+                .map(|(k, v)| (*k, *v))
+                .collect();
+        names_with_first_named_element_vec.sort_unstable_by(|a, b| {
+            if a.1 == b.1 {
+                // This can happen if an img has an id different from its name,
+                // spec does not say which string to put first.
+                a.0.cmp(&b.0)
+            } else if a.1.upcast::<Node>().is_before(b.1.upcast::<Node>()) {
+                Ordering::Less
+            } else {
+                Ordering::Greater
+            }
+        });
+
+        names_with_first_named_element_vec
+            .iter()
+            .map(|(k, _v)| DOMString::from(&***k))
+            .collect()
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-document-clear
@@ -5366,3 +5311,22 @@ pub enum ReflowTriggerCondition {
     PendingRestyles,
     PaintPostponed,
 }
+
+fn is_named_element_with_name_attribute(elem: &Element) -> bool {
+    let type_ = match elem.upcast::<Node>().type_id() {
+        NodeTypeId::Element(ElementTypeId::HTMLElement(type_)) => type_,
+        _ => return false,
+    };
+    match type_ {
+        HTMLElementTypeId::HTMLFormElement |
+        HTMLElementTypeId::HTMLIFrameElement |
+        HTMLElementTypeId::HTMLImageElement => true,
+        // TODO: Handle exposed objects.
+        _ => false,
+    }
+}
+
+fn is_named_element_with_id_attribute(elem: &Element) -> bool {
+    // TODO: Handle exposed objects.
+    elem.is::<HTMLImageElement>() && elem.get_name().map_or(false, |name| !name.is_empty())
+}

components/script/dom/eventtarget.rs

@@ -511,7 +511,6 @@ impl EventTarget {
 
         // Step 3.9
 
-        let url_serialized = CString::new(handler.url.to_string()).unwrap();
         let name = CString::new(format!("on{}", &**ty)).unwrap();
 
         // Step 3.9, subsection ParameterList
@@ -527,9 +526,8 @@ impl EventTarget {
         let args = if is_error { ERROR_ARG_NAMES } else { ARG_NAMES };
 
         let cx = window.get_cx();
-        let options = unsafe {
-            CompileOptionsWrapper::new(*cx, url_serialized.as_ptr(), handler.line as u32)
-        };
+        let options =
+            unsafe { CompileOptionsWrapper::new(*cx, handler.url.as_str(), handler.line as u32) };
 
         // Step 3.9, subsection Scope steps 1-6
         let scopechain = RootedObjectVectorWrapper::new(*cx);

components/script/dom/globalscope.rs

@@ -118,7 +118,6 @@ use std::borrow::Cow;
 use std::cell::Cell;
 use std::collections::hash_map::Entry;
 use std::collections::{HashMap, VecDeque};
-use std::ffi::CString;
 use std::mem;
 use std::ops::Index;
 use std::rc::Rc;
@@ -2586,7 +2585,6 @@ impl GlobalScope {
             self.time_profiler_chan().clone(),
             || {
                 let cx = self.get_cx();
-                let filename = CString::new(filename).unwrap();
 
                 let ar = enter_realm(&*self);
 
@@ -2596,8 +2594,7 @@ impl GlobalScope {
                     rooted!(in(*cx) let mut compiled_script = std::ptr::null_mut::<JSScript>());
                     match code {
                         SourceCode::Text(text_code) => {
-                            let options =
-                                CompileOptionsWrapper::new(*cx, filename.as_ptr(), line_number);
+                            let options = CompileOptionsWrapper::new(*cx, filename, line_number);
 
                             debug!("compiling dom string");
                             compiled_script.set(Compile1(

components/script/dom/htmlscriptelement.rs

@@ -58,7 +58,6 @@ use servo_config::pref;
 use servo_url::ImmutableOrigin;
 use servo_url::ServoUrl;
 use std::cell::Cell;
-use std::ffi::CString;
 use std::fs::{create_dir_all, read_to_string, File};
 use std::io::{Read, Seek, Write};
 use std::mem::replace;
@@ -424,8 +423,7 @@ impl FetchResponseListener for ClassicContext {
         let cx = global.get_cx();
         let _ar = enter_realm(&*global);
 
-        let final_url_c_str = CString::new(final_url.as_str()).unwrap();
-        let options = unsafe { CompileOptionsWrapper::new(*cx, final_url_c_str.as_ptr(), 1) };
+        let options = unsafe { CompileOptionsWrapper::new(*cx, final_url.as_str(), 1) };
 
         let can_compile_off_thread = pref!(dom.script.asynch) &&
             unsafe { CanCompileOffThread(*cx, options.ptr as *const _, source_text.len()) };

components/script/dom/node.rs

@@ -83,7 +83,6 @@ use script_traits::UntrustedNodeAddress;
 use selectors::matching::{matches_selector_list, MatchingContext, MatchingMode};
 use selectors::parser::SelectorList;
 use servo_arc::Arc;
-use servo_atoms::Atom;
 use servo_url::ServoUrl;
 use smallvec::SmallVec;
 use std::borrow::Cow;
@@ -1230,34 +1229,6 @@ impl Node {
         }
     }
 
-    // https://html.spec.whatwg.org/multipage/#dom-document-nameditem-filter
-    pub fn is_document_named_item(&self, name: &Atom) -> bool {
-        let html_elem_type = match self.type_id() {
-            NodeTypeId::Element(ElementTypeId::HTMLElement(type_)) => type_,
-            _ => return false,
-        };
-        let elem = self
-            .downcast::<Element>()
-            .expect("Node with an Element::HTMLElement NodeTypeID must be an Element");
-        match html_elem_type {
-            HTMLElementTypeId::HTMLFormElement | HTMLElementTypeId::HTMLIFrameElement => {
-                elem.get_name().map_or(false, |n| n == *name)
-            },
-            HTMLElementTypeId::HTMLImageElement =>
-            // Images can match by id, but only when their name is non-empty.
-            {
-                elem.get_name().map_or(false, |n| {
-                    n == *name || elem.get_id().map_or(false, |i| i == *name)
-                })
-            },
-            // TODO: Handle <embed> and <object>; these depend on
-            // whether the element is "exposed", a concept which
-            // doesn't fully make sense until embed/object behaviors
-            // are actually implemented.
-            _ => false,
-        }
-    }
-
     pub fn is_styled(&self) -> bool {
         self.style_and_layout_data.borrow().is_some()
     }

components/script/dom/webidls/Window.webidl

@@ -44,8 +44,7 @@
                              optional DOMString features = "");
   //getter WindowProxy (unsigned long index);
 
-  // https://github.com/servo/servo/issues/14453
-  // getter object (DOMString name);
+  getter object (DOMString name);
 
   // the user agent
   readonly attribute Navigator navigator;

components/script/dom/window.rs

@@ -6,6 +6,7 @@ use crate::dom::bindings::cell::{DomRefCell, Ref};
 use crate::dom::bindings::codegen::Bindings::DocumentBinding::{
     DocumentMethods, DocumentReadyState,
 };
+use crate::dom::bindings::codegen::Bindings::HTMLIFrameElementBinding::HTMLIFrameElementMethods;
 use crate::dom::bindings::codegen::Bindings::HistoryBinding::HistoryBinding::HistoryMethods;
 use crate::dom::bindings::codegen::Bindings::ImageBitmapBinding::{
     ImageBitmapOptions, ImageBitmapSource,
@@ -19,7 +20,7 @@ use crate::dom::bindings::codegen::Bindings::WindowBinding::{
 use crate::dom::bindings::codegen::Bindings::WindowBinding::{ScrollBehavior, ScrollToOptions};
 use crate::dom::bindings::codegen::UnionTypes::{RequestOrUSVString, StringOrFunction};
 use crate::dom::bindings::error::{Error, ErrorResult, Fallible};
-use crate::dom::bindings::inheritance::Castable;
+use crate::dom::bindings::inheritance::{Castable, ElementTypeId, HTMLElementTypeId, NodeTypeId};
 use crate::dom::bindings::num::Finite;
 use crate::dom::bindings::refcounted::Trusted;
 use crate::dom::bindings::reflector::DomObject;
@@ -40,6 +41,8 @@ use crate::dom::eventtarget::EventTarget;
 use crate::dom::globalscope::GlobalScope;
 use crate::dom::hashchangeevent::HashChangeEvent;
 use crate::dom::history::History;
+use crate::dom::htmlcollection::{CollectionFilter, HTMLCollection};
+use crate::dom::htmliframeelement::HTMLIFrameElement;
 use crate::dom::identityhub::Identities;
 use crate::dom::location::Location;
 use crate::dom::mediaquerylist::{MediaQueryList, MediaQueryListMatchState};
@@ -71,6 +74,7 @@ use crate::task_manager::TaskManager;
 use crate::task_source::{TaskSource, TaskSourceName};
 use crate::timers::{IsInterval, TimerCallback};
 use crate::webdriver_handlers::jsval_to_webdriver;
+use crate::window_named_properties;
 use app_units::Au;
 use backtrace::Backtrace;
 use base64;
@@ -94,7 +98,9 @@ use js::jsapi::{GCReason, StackFormat, JS_GC};
 use js::jsval::UndefinedValue;
 use js::jsval::{JSVal, NullValue};
 use js::rust::wrappers::JS_DefineProperty;
-use js::rust::{CustomAutoRooter, CustomAutoRooterGuard, HandleValue};
+use js::rust::{
+    CustomAutoRooter, CustomAutoRooterGuard, HandleObject, HandleValue, MutableHandleObject,
+};
 use media::WindowGLContext;
 use msg::constellation_msg::{BrowsingContextId, PipelineId};
 use net_traits::image_cache::{ImageCache, ImageResponder, ImageResponse};
@@ -120,17 +126,20 @@ use script_traits::{
 use script_traits::{TimerSchedulerMsg, WebrenderIpcSender, WindowSizeData, WindowSizeType};
 use selectors::attr::CaseSensitivity;
 use servo_arc::Arc as ServoArc;
+use servo_atoms::Atom;
 use servo_geometry::{f32_rect_to_au_rect, MaxRect};
 use servo_url::{ImmutableOrigin, MutableOrigin, ServoUrl};
 use std::borrow::Cow;
 use std::borrow::ToOwned;
 use std::cell::Cell;
+use std::cmp;
 use std::collections::hash_map::Entry;
 use std::collections::{HashMap, HashSet};
 use std::default::Default;
 use std::env;
 use std::io::{stderr, stdout, Write};
 use std::mem;
+use std::ptr::NonNull;
 use std::rc::Rc;
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::{Arc, Mutex};
@@ -1381,9 +1390,164 @@ impl WindowMethods for Window {
         }
         rval.get()
     }
+
+    // https://html.spec.whatwg.org/multipage/#named-access-on-the-window-object
+    #[allow(unsafe_code)]
+    fn NamedGetter(&self, _cx: JSContext, name: DOMString) -> Option<NonNull<JSObject>> {
+        if name.is_empty() {
+            return None;
+        }
+        let name = Atom::from(name);
+        let document = self.Document();
+
+        // TODO: Handle the document-tree child browsing context name property set.
+
+        // Step 1.
+        let elements_with_name = document.get_elements_with_name(&name);
+        let name_iter = elements_with_name
+            .iter()
+            .filter(|elem| is_named_element_with_name_attribute(elem));
+        let elements_with_id = document.get_elements_with_id(&name);
+        let id_iter = elements_with_id
+            .iter()
+            .filter(|elem| is_named_element_with_id_attribute(elem));
+
+        // Step 2.
+        for elem in id_iter.clone() {
+            if let Some(nested_window_proxy) = elem
+                .downcast::<HTMLIFrameElement>()
+                .and_then(|iframe| iframe.GetContentWindow())
+            {
+                unsafe {
+                    return Some(NonNull::new_unchecked(
+                        nested_window_proxy.reflector().get_jsobject().get(),
+                    ));
+                }
+            }
+        }
+
+        let mut elements = name_iter.chain(id_iter);
+
+        let first = elements.next()?;
+
+        if elements.next().is_none() {
+            // Step 3.
+            unsafe {
+                return Some(NonNull::new_unchecked(
+                    first.reflector().get_jsobject().get(),
+                ));
+            }
+        }
+
+        // Step 4.
+        #[derive(JSTraceable, MallocSizeOf)]
+        struct WindowNamedGetter {
+            name: Atom,
+        }
+        impl CollectionFilter for WindowNamedGetter {
+            fn filter(&self, elem: &Element, _root: &Node) -> bool {
+                let type_ = match elem.upcast::<Node>().type_id() {
+                    NodeTypeId::Element(ElementTypeId::HTMLElement(type_)) => type_,
+                    _ => return false,
+                };
+                if elem.get_id().as_ref() == Some(&self.name) {
+                    return true;
+                }
+                match type_ {
+                    HTMLElementTypeId::HTMLEmbedElement |
+                    HTMLElementTypeId::HTMLFormElement |
+                    HTMLElementTypeId::HTMLImageElement |
+                    HTMLElementTypeId::HTMLObjectElement => {
+                        elem.get_name().as_ref() == Some(&self.name)
+                    },
+                    _ => false,
+                }
+            }
+        }
+        let collection = HTMLCollection::create(
+            self,
+            document.upcast(),
+            Box::new(WindowNamedGetter { name }),
+        );
+        unsafe {
+            Some(NonNull::new_unchecked(
+                collection.reflector().get_jsobject().get(),
+            ))
+        }
+    }
+
+    // https://html.spec.whatwg.org/multipage/#dom-tree-accessors:supported-property-names
+    fn SupportedPropertyNames(&self) -> Vec<DOMString> {
+        let mut names_with_first_named_element_map: HashMap<&Atom, &Element> = HashMap::new();
+
+        let name_map = self.name_map.borrow();
+        for (name, elements) in &*name_map {
+            if name.is_empty() {
+                continue;
+            }
+            let mut name_iter = elements
+                .iter()
+                .filter(|elem| is_named_element_with_name_attribute(elem));
+            if let Some(first) = name_iter.next() {
+                names_with_first_named_element_map.insert(name, first);
+            }
+        }
+        let id_map = self.id_map.borrow();
+        for (id, elements) in &*id_map {
+            if id.is_empty() {
+                continue;
+            }
+            let mut id_iter = elements
+                .iter()
+                .filter(|elem| is_named_element_with_id_attribute(elem));
+            if let Some(first) = id_iter.next() {
+                match names_with_first_named_element_map.entry(id) {
+                    Entry::Vacant(entry) => drop(entry.insert(first)),
+                    Entry::Occupied(mut entry) => {
+                        if first.upcast::<Node>().is_before(entry.get().upcast()) {
+                            *entry.get_mut() = first;
+                        }
+                    },
+                }
+            }
+        }
+
+        let mut names_with_first_named_element_vec: Vec<(&Atom, &Element)> =
+            names_with_first_named_element_map
+                .iter()
+                .map(|(k, v)| (*k, *v))
+                .collect();
+        names_with_first_named_element_vec.sort_unstable_by(|a, b| {
+            if a.1 == b.1 {
+                // This can happen if an img has an id different from its name,
+                // spec does not say which string to put first.
+                a.0.cmp(&b.0)
+            } else if a.1.upcast::<Node>().is_before(b.1.upcast::<Node>()) {
+                cmp::Ordering::Less
+            } else {
+                cmp::Ordering::Greater
+            }
+        });
+
+        names_with_first_named_element_vec
+            .iter()
+            .map(|(k, _v)| DOMString::from(&***k))
+            .collect()
+    }
 }
 
 impl Window {
+    // https://heycam.github.io/webidl/#named-properties-object
+    // https://html.spec.whatwg.org/multipage/#named-access-on-the-window-object
+    #[allow(unsafe_code)]
+    pub fn create_named_properties_object(
+        cx: JSContext,
+        proto: HandleObject,
+        object: MutableHandleObject,
+    ) {
+        window_named_properties::create(cx, proto, object)
+    }
+
     pub(crate) fn set_current_event(&self, event: Option<&Event>) -> Option<DomRoot<Event>> {
         let current = self
             .current_event
@@ -2654,3 +2818,21 @@ impl ParseErrorReporter for CSSErrorReporter {
             ));
     }
 }
+
+fn is_named_element_with_name_attribute(elem: &Element) -> bool {
+    let type_ = match elem.upcast::<Node>().type_id() {
+        NodeTypeId::Element(ElementTypeId::HTMLElement(type_)) => type_,
+        _ => return false,
+    };
+    match type_ {
+        HTMLElementTypeId::HTMLEmbedElement |
+        HTMLElementTypeId::HTMLFormElement |
+        HTMLElementTypeId::HTMLImageElement |
+        HTMLElementTypeId::HTMLObjectElement => true,
+        _ => false,
+    }
+}
+
+fn is_named_element_with_id_attribute(elem: &Element) -> bool {
+    elem.is_html_element()
+}

components/script/dom/windowproxy.rs

@@ -906,8 +906,7 @@ unsafe extern "C" fn getOwnPropertyDescriptor(
     if let Some((window, attrs)) = window {
         rooted!(in(cx) let mut val = UndefinedValue());
         window.to_jsval(cx, val.handle_mut());
-        desc.value = val.get();
-        fill_property_descriptor(MutableHandle::from_raw(desc), proxy.get(), attrs);
+        fill_property_descriptor(MutableHandle::from_raw(desc), proxy.get(), val.get(), attrs);
         return true;
     }
 

components/script/lib.rs

@@ -111,6 +111,8 @@ mod timers;
 mod unpremultiplytable;
 #[warn(deprecated)]
 mod webdriver_handlers;
+#[warn(deprecated)]
+mod window_named_properties;
 
 pub use init::init;
 pub use script_runtime::JSEngineSetup;

components/script/script_module.rs

@@ -70,7 +70,6 @@ use net_traits::{FetchResponseListener, NetworkError};
 use net_traits::{ResourceFetchTiming, ResourceTimingType};
 use servo_url::ServoUrl;
 use std::collections::{HashMap, HashSet};
-use std::ffi;
 use std::mem;
 use std::rc::Rc;
 use std::str::FromStr;
@@ -420,12 +419,10 @@ impl ModuleTree {
         url: ServoUrl,
         options: ScriptFetchOptions,
     ) -> Result<ModuleObject, RethrowError> {
-        let url_cstr = ffi::CString::new(url.as_str().as_bytes()).unwrap();
-
         let _ac = JSAutoRealm::new(*global.get_cx(), *global.reflector().get_jsobject());
 
         let compile_options =
-            unsafe { CompileOptionsWrapper::new(*global.get_cx(), url_cstr.as_ptr(), 1) };
+            unsafe { CompileOptionsWrapper::new(*global.get_cx(), url.as_str(), 1) };
 
         unsafe {
             rooted!(in(*global.get_cx()) let mut module_script = CompileModuleDontInflate(

components/script/window_named_properties.rs

@@ -0,0 +1,205 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+use crate::dom::bindings::codegen::Bindings::WindowBinding::WindowMethods;
+use crate::dom::bindings::proxyhandler::fill_property_descriptor;
+use crate::dom::bindings::root::Root;
+use crate::dom::bindings::utils::has_property_on_prototype;
+use crate::dom::globalscope::GlobalScope;
+use crate::dom::window::Window;
+use crate::js::conversions::ToJSValConvertible;
+use crate::script_runtime::JSContext as SafeJSContext;
+use js::conversions::jsstr_to_string;
+use js::error::throw_type_error;
+use js::glue::RUST_JSID_TO_STRING;
+use js::glue::{CreateProxyHandler, NewProxyObject, ProxyTraps, RUST_JSID_IS_STRING};
+use js::jsapi::JS_SetImmutablePrototype;
+use js::jsapi::{Handle, HandleObject, JSClass, JSContext, JSErrNum, UndefinedHandleValue};
+use js::jsapi::{
+    HandleId, JSClass_NON_NATIVE, MutableHandle, ObjectOpResult, PropertyDescriptor,
+    ProxyClassExtension, ProxyClassOps, ProxyObjectOps, JSCLASS_DELAY_METADATA_BUILDER,
+    JSCLASS_IS_PROXY, JSCLASS_RESERVED_SLOTS_MASK, JSCLASS_RESERVED_SLOTS_SHIFT,
+};
+use js::jsval::UndefinedValue;
+use js::rust::IntoHandle;
+use js::rust::{Handle as RustHandle, MutableHandle as RustMutableHandle};
+use js::rust::{HandleObject as RustHandleObject, MutableHandleObject as RustMutableHandleObject};
+use libc;
+use std::ptr;
+
+struct SyncWrapper(*const libc::c_void);
+#[allow(unsafe_code)]
+unsafe impl Sync for SyncWrapper {}
+
+lazy_static! {
+    static ref HANDLER: SyncWrapper = {
+        let traps = ProxyTraps {
+            enter: None,
+            getOwnPropertyDescriptor: Some(get_own_property_descriptor),
+            defineProperty: Some(define_property),
+            ownPropertyKeys: None,
+            delete_: Some(delete),
+            enumerate: None,
+            getPrototypeIfOrdinary: None,
+            preventExtensions: Some(prevent_extensions),
+            isExtensible: Some(is_extensible),
+            has: None,
+            get: None,
+            set: None,
+            call: None,
+            construct: None,
+            hasOwn: None,
+            getOwnEnumerablePropertyKeys: None,
+            nativeCall: None,
+            hasInstance: None,
+            objectClassIs: None,
+            className: Some(class_name),
+            fun_toString: None,
+            boxedValue_unbox: None,
+            defaultValue: None,
+            trace: None,
+            finalize: None,
+            objectMoved: None,
+            isCallable: None,
+            isConstructor: None,
+        };
+
+        #[allow(unsafe_code)]
+        unsafe {
+            SyncWrapper(CreateProxyHandler(&traps, ptr::null()))
+        }
+    };
+}
+
+#[allow(unsafe_code)]
+unsafe extern "C" fn get_own_property_descriptor(
+    cx: *mut JSContext,
+    proxy: HandleObject,
+    id: HandleId,
+    desc: MutableHandle<PropertyDescriptor>,
+) -> bool {
+    let cx = SafeJSContext::from_ptr(cx);
+    if !RUST_JSID_IS_STRING(id) {
+        // Nothing to do if we're resolving a non-string property.
+        return true;
+    }
+
+    let mut found = false;
+    if !has_property_on_prototype(
+        *cx,
+        RustHandle::from_raw(proxy),
+        RustHandle::from_raw(id),
+        &mut found,
+    ) {
+        return false;
+    }
+    if found {
+        return true;
+    }
+
+    let s = jsstr_to_string(*cx, RUST_JSID_TO_STRING(id));
+    if s.is_empty() {
+        return true;
+    }
+
+    let window = Root::downcast::<Window>(GlobalScope::from_object(proxy.get()))
+        .expect("global is not a window");
+    if let Some(obj) = window.NamedGetter(cx, s.into()) {
+        rooted!(in(*cx) let mut val = UndefinedValue());
+        obj.to_jsval(*cx, val.handle_mut());
+        fill_property_descriptor(RustMutableHandle::from_raw(desc), proxy.get(), val.get(), 0);
+    }
+    return true;
+}
+
+#[allow(unsafe_code)]
+unsafe extern "C" fn define_property(
+    cx: *mut JSContext,
+    _proxy: HandleObject,
+    _id: HandleId,
+    _desc: Handle<PropertyDescriptor>,
+    _result: *mut ObjectOpResult,
+) -> bool {
+    throw_type_error(
+        cx,
+        "Not allowed to define a property on the named properties object.",
+    );
+    false
+}
+
+#[allow(unsafe_code)]
+unsafe extern "C" fn delete(
+    _cx: *mut JSContext,
+    _proxy: HandleObject,
+    _id: HandleId,
+    result: *mut ObjectOpResult,
+) -> bool {
+    (*result).code_ = JSErrNum::JSMSG_CANT_DELETE_WINDOW_NAMED_PROPERTY as usize;
+    true
+}
+
+#[allow(unsafe_code)]
+unsafe extern "C" fn prevent_extensions(
+    _cx: *mut JSContext,
+    _proxy: HandleObject,
+    result: *mut ObjectOpResult,
+) -> bool {
+    (*result).code_ = JSErrNum::JSMSG_CANT_PREVENT_EXTENSIONS as usize;
+    true
+}
+
+#[allow(unsafe_code)]
+unsafe extern "C" fn is_extensible(
+    _cx: *mut JSContext,
+    _proxy: HandleObject,
+    extensible: *mut bool,
+) -> bool {
+    *extensible = true;
+    true
+}
+
+#[allow(unsafe_code)]
+unsafe extern "C" fn class_name(_cx: *mut JSContext, _proxy: HandleObject) -> *const i8 {
+    &b"WindowProperties" as *const _ as *const i8
+}
+
+// Maybe this should be a DOMJSClass. See https://bugzilla.mozilla.org/show_bug.cgi?id=787070
+static CLASS: JSClass = JSClass {
+    name: b"WindowProperties\0" as *const u8 as *const libc::c_char,
+    flags: JSClass_NON_NATIVE |
+        JSCLASS_IS_PROXY |
+        JSCLASS_DELAY_METADATA_BUILDER |
+        ((1 & JSCLASS_RESERVED_SLOTS_MASK) << JSCLASS_RESERVED_SLOTS_SHIFT), /* JSCLASS_HAS_RESERVED_SLOTS(1) */
+    cOps: &ProxyClassOps,
+    spec: ptr::null(),
+    ext: &ProxyClassExtension,
+    oOps: &ProxyObjectOps,
+};
+
+#[allow(unsafe_code)]
+pub fn create(
+    cx: SafeJSContext,
+    proto: RustHandleObject,
+    mut properties_obj: RustMutableHandleObject,
+) {
+    unsafe {
+        properties_obj.set(NewProxyObject(
+            *cx,
+            HANDLER.0,
+            UndefinedHandleValue,
+            proto.get(),
+            // TODO: pass proper clasp
+            &CLASS,
+            true,
+        ));
+        assert!(!properties_obj.get().is_null());
+        let mut succeeded = false;
+        assert!(JS_SetImmutablePrototype(
+            *cx,
+            properties_obj.handle().into_handle(),
+            &mut succeeded
+        ));
+        assert!(succeeded);
+    }
+}