apply pylbrecht/servo/named.window.getter (closes #27952)

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

@@ -3290,6 +3290,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 "&[]",
@@ -5508,15 +5515,15 @@ class CGDOMJSProxyHandler_getOwnPropertyDescriptor(CGAbstractExternMethod):
                 attrs += " | JSPROP_READONLY"
             fillDescriptor = ("set_property_descriptor(\n"
                               "    MutableHandle::from_raw(desc),\n"
-                              "    result_root.handle(),\n"
+                              "    rval.handle(),\n"
                               "    (%s) as u32,\n"
                               "    &mut *is_none\n"
                               ");\n"
                               "return true;" % attrs)
             templateValues = {
-                'jsvalRef': 'result_root.handle_mut()',
+                'jsvalRef': 'rval.handle_mut()',
                 'successCode': fillDescriptor,
-                'pre': 'rooted!(in(*cx) let mut result_root = UndefinedValue());'
+                'pre': 'rooted!(in(*cx) let mut rval = UndefinedValue());'
             }
             get += ("if let Some(index) = index {\n"
                     + "    let this = UnwrapProxy(proxy);\n"
@@ -5524,8 +5531,7 @@ class CGDOMJSProxyHandler_getOwnPropertyDescriptor(CGAbstractExternMethod):
                     + CGIndenter(CGProxyIndexedGetter(self.descriptor, templateValues)).define() + "\n"
                     + "}\n")
 
-        namedGetter = self.descriptor.operations['NamedGetter']
+        if self.descriptor.supportsNamedProperties():
-        if namedGetter:
             attrs = []
             if not self.descriptor.interface.getExtendedAttribute("LegacyUnenumerableNamedProperties"):
                 attrs.append("JSPROP_ENUMERATE")
@@ -5537,15 +5543,15 @@ class CGDOMJSProxyHandler_getOwnPropertyDescriptor(CGAbstractExternMethod):
                 attrs = "0"
             fillDescriptor = ("set_property_descriptor(\n"
                               "    MutableHandle::from_raw(desc),\n"
-                              "    result_root.handle(),\n"
+                              "    rval.handle(),\n"
                               "    (%s) as u32,\n"
                               "    &mut *is_none\n"
                               ");\n"
                               "return true;" % attrs)
             templateValues = {
-                'jsvalRef': 'result_root.handle_mut()',
+                'jsvalRef': 'rval.handle_mut()',
                 'successCode': fillDescriptor,
-                'pre': 'rooted!(in(*cx) let mut result_root = UndefinedValue());'
+                'pre': 'rooted!(in(*cx) let mut rval = UndefinedValue());'
             }
 
             # See the similar-looking in CGDOMJSProxyHandler_get for the spec quote.
@@ -5638,7 +5644,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 id.is_string() || id.is_int() {\n"
                     + CGIndenter(CGProxyNamedGetter(self.descriptor)).define()
                     + "    if result.is_some() {\n"
@@ -5722,7 +5728,7 @@ class CGDOMJSProxyHandler_ownPropertyKeys(CGAbstractExternMethod):
                 }
                 """)
 
-        if self.descriptor.operations['NamedGetter']:
+        if self.descriptor.supportsNamedProperties():
             body += dedent(
                 """
                 for name in (*unwrapped_proxy).SupportedPropertyNames() {
@@ -5844,11 +5850,10 @@ class CGDOMJSProxyHandler_hasOwn(CGAbstractExternMethod):
                         + "    return true;\n"
                         + "}\n\n")
 
-        namedGetter = self.descriptor.operations['NamedGetter']
         condition = "id.is_string() || id.is_int()"
         if indexedGetter:
             condition = "index.is_none() && (%s)" % condition
-        if namedGetter:
+        if self.descriptor.supportsNamedProperties():
             named = """\
 if %s {
     let mut has_on_proto = false;
@@ -5943,8 +5948,7 @@ if !expando.is_null() {
         else:
             getIndexedOrExpando = getFromExpando + "\n"
 
-        namedGetter = self.descriptor.operations['NamedGetter']
+        if self.descriptor.supportsNamedProperties():
-        if namedGetter:
             condition = "id.is_string() || id.is_int()"
             # From step 1:
             #     If O supports indexed properties and P is an array index, then:
@@ -6214,7 +6218,7 @@ class CGInterfaceTrait(CGThing):
                                ),
                                rettype)
 
-            if descriptor.proxy:
+            if descriptor.proxy or descriptor.isGlobal():
                 for name, operation in descriptor.operations.items():
                     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:
@@ -369,6 +370,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

@@ -39,6 +39,7 @@ use js::jsapi::{DOMProxyShadowsResult, JSContext, JSObject, PropertyDescriptor};
 use js::jsapi::{GetWellKnownSymbol, SymbolCode};
 use js::jsapi::{JSErrNum, SetDOMProxyInformation};
 use js::jsid::SymbolId;
+use js::jsval::JSVal;
 use js::jsval::ObjectValue;
 use js::jsval::UndefinedValue;
 use js::rust::wrappers::JS_AlreadyHasOwnPropertyById;
@@ -193,7 +194,7 @@ 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 set_property_descriptor(
-    desc: MutableHandle<PropertyDescriptor>,
+    mut desc: MutableHandle<PropertyDescriptor>,
     value: HandleValue,
     attrs: u32,
     is_none: &mut bool,

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) {
@@ -2870,73 +2864,12 @@ impl Document {
             .for_each(|(_, context)| context.send_swap_chain_present());
     }
 
-    // https://html.spec.whatwg.org/multipage/#dom-tree-accessors:supported-property-names
+    pub fn id_map(&self) -> Ref<HashMap<Atom, Vec<Dom<Element>>>> {
-    // (This takes the filter as a method so the window named getter can use it too)
+        self.id_map.borrow()
-    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();
+    pub fn name_map(&self) -> Ref<HashMap<Atom, Vec<Dom<Element>>>> {
-
+        self.name_map.borrow()
-        // 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()
     }
 }
 
@@ -3869,79 +3802,16 @@ impl Document {
         )
     }
 
-    // https://html.spec.whatwg.org/multipage/#dom-tree-accessors:determine-the-value-of-a-named-property
+    pub fn get_elements_with_id(&self, id: &Atom) -> Ref<[Dom<Element>]> {
-    // Support method for steps 1-3:
+        Ref::map(self.id_map.borrow(), |map| {
-    // Count if there are 0, 1, or >1 elements that match the name.
+            map.get(id).map(|vec| &**vec).unwrap_or_default()
-    // (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);
 
-        // If nothing can possibly have the name, exit fast
+    pub fn get_elements_with_name(&self, name: &Atom) -> Ref<[Dom<Element>]> {
-        if id_vec.is_none() && name_vec.is_none() {
+        Ref::map(self.name_map.borrow(), |map| {
-            return ElementLookupResult::None;
+            map.get(name).map(|vec| &**vec).unwrap_or_default()
-        }
+        })
-
-        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,
-        }
     }
 
     #[allow(unrooted_must_root)]
@@ -4871,45 +4741,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)]
+        if name.is_empty() {
-        struct NamedElementFilter {
+            return None;
-            name: Atom,
         }
-        impl CollectionFilter for NamedElementFilter {
+        let name = Atom::from(name);
-            fn filter(&self, elem: &Element, _root: &Node) -> bool {
+
-                elem.upcast::<Node>().is_document_named_item(&self.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 };
+        #[derive(JSTraceable, MallocSizeOf)]
-        let collection = HTMLCollection::create(self.window(), self.upcast(), Box::new(filter));
+        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(),
@@ -4919,7 +4818,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
@@ -5394,3 +5347,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

@@ -526,9 +526,8 @@ impl EventTarget {
         let args = if is_error { ERROR_ARG_NAMES } else { ARG_NAMES };
 
         let cx = window.get_cx();
-        let options = unsafe {
+        let options =
-            CompileOptionsWrapper::new(*cx, &handler.url.to_string(), handler.line as u32)
+            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/node.rs

@@ -82,7 +82,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;
@@ -1242,34 +1241,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

@@ -45,8 +45,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};
@@ -1386,9 +1395,182 @@ impl WindowMethods for Window {
     fn IsSecureContext(&self) -> bool {
         self.upcast::<GlobalScope>().is_secure_context()
     }
+
+    // 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 document = self.Document();
+
+        // https://html.spec.whatwg.org/multipage/#document-tree-child-browsing-context-name-property-set
+        let iframes: Vec<_> = document
+            .iter_iframes()
+            .filter(|iframe| {
+                if let Some(window) = iframe.GetContentWindow() {
+                    return window.get_name() == name;
+                }
+                false
+            })
+            .collect();
+
+        let iframe_iter = iframes.iter().map(|iframe| iframe.upcast::<Element>());
+
+        let name = Atom::from(&*name);
+
+        // Step 1.
+        let elements_with_name = document.get_elements_with_name(&name);
+        let name_iter = elements_with_name
+            .iter()
+            .map(|element| &**element)
+            .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()
+            .map(|element| &**element)
+            .filter(|elem| is_named_element_with_id_attribute(elem));
+
+        // Step 2.
+        // TODO(pylbrecht): it would be great to just iterate over
+        // elements_with_id, but it seems document.get_elements_with_id()
+        // does not return HTMLIFrameElements. Why is that?
+        for elem in iframe_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 = iframe_iter.chain(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 document = self.Document();
+        let name_map = document.name_map();
+        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 = document.id_map();
+        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
@@ -2659,3 +2841,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

@@ -910,7 +910,7 @@ unsafe extern "C" fn getOwnPropertyDescriptor(
         window.to_jsval(cx, val.handle_mut());
         set_property_descriptor(
             MutableHandle::from_raw(desc),
-            val.handle().into(),
+            val.handle(),
             attrs,
             &mut *is_none,
         );

components/script/lib.rs

@@ -110,6 +110,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/window_named_properties.rs

@@ -0,0 +1,220 @@
+/* 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::set_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, MutableHandleIdVector, 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: Some(own_property_keys),
+            delete_: Some(delete),
+            enumerate: None,
+            getPrototypeIfOrdinary: None,
+            getPrototype: None,
+            setPrototype: None,
+            setImmutablePrototype: 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,
+            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>,
+    is_none: *mut bool,
+) -> 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 rval = UndefinedValue());
+        obj.to_jsval(*cx, rval.handle_mut());
+        set_property_descriptor(RustMutableHandle::from_raw(desc), rval.handle(), 0, &mut *is_none);
+    }
+    return true;
+}
+
+#[allow(unsafe_code)]
+unsafe extern "C" fn own_property_keys(
+    _cx: *mut JSContext,
+    _proxy: HandleObject,
+    _props: MutableHandleIdVector,
+) -> bool {
+    // FIXME(pylbrecht): dummy implementation
+    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\0" as *const _ as *const i8
+}
+
+// Maybe this should be a DOMJSClass. See https://bugzilla.mozilla.org/show_bug.cgi?id=787070
+#[allow(unsafe_code)]
+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: unsafe { &ProxyClassOps },
+    spec: ptr::null(),
+    ext: unsafe { &ProxyClassExtension },
+    oOps: unsafe { &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,
+            false,
+        ));
+        assert!(!properties_obj.get().is_null());
+        let mut succeeded = false;
+        assert!(JS_SetImmutablePrototype(
+            *cx,
+            properties_obj.handle().into_handle(),
+            &mut succeeded
+        ));
+        assert!(succeeded);
+    }
+}

tests/wpt/metadata/html/browsers/the-window-object/named-access-on-the-window-object/cross-global-npo.html.ini

@@ -1,4 +0,0 @@
-[cross-global-npo.html]
-  [Named access across globals]
-    expected: FAIL
-

tests/wpt/metadata/html/browsers/the-window-object/named-access-on-the-window-object/named-objects.html.ini

@@ -1,7 +1,5 @@
 [named-objects.html]
   type: testharness
-  [Check if the first nested browsing context is returned by window['c'\]]
-    expected: FAIL
 
   [Check if window['a'\] contains all a, applet, area, embed, form, img, and object elements, and their order]
     expected: FAIL
@@ -9,15 +7,5 @@
   [Check if window['fs'\] return the frameset element with name='fs']
     expected: FAIL
 
-  [Check if window['b'\] returns the elements with the id='b']
-    expected: FAIL
-
-  [Check if window['d'\] returns the element with id='d']
-    expected: FAIL
-
   [Check if window['a'\] contains all applet, embed, form, img, and object elements, and their order]
     expected: FAIL
-
-  [Check if window['a'\] contains all embed, form, img, and object elements, and their order]
-    expected: FAIL
-

tests/wpt/metadata/html/browsers/the-window-object/named-access-on-the-window-object/navigated-named-objects.window.js.ini

@@ -1,37 +1,9 @@
 [navigated-named-objects.window.html]
-  [Window's associated Document object is used for finding named objects (<iframe> via same-origin <iframe>)]
-    expected: FAIL
-
-  [Window's associated Document object is used for finding named objects (<object> via srcdoc <iframe>)]
-    expected: FAIL
-
-  [Window's associated Document object is used for finding named objects (<iframe> via cross-site <iframe>)]
-    expected: FAIL
-
-  [Window's associated Document object is used for finding named objects (<div> via same-origin <iframe>)]
-    expected: FAIL
-
   [Window's associated Document object is used for finding named objects (<object> with browsing ccontext via srcdoc <iframe)>]
     expected: FAIL
 
-  [Window's associated Document object is used for finding named objects (<div> via cross-site <iframe>)]
-    expected: FAIL
-
   [Window's associated Document object is used for finding named objects (<object> with browsing ccontext via same-origin <iframe)>]
     expected: FAIL
 
   [Window's associated Document object is used for finding named objects (<object> with browsing ccontext via cross-site <iframe)>]
     expected: FAIL
-
-  [Window's associated Document object is used for finding named objects (<div> via srcdoc <iframe>)]
-    expected: FAIL
-
-  [Window's associated Document object is used for finding named objects (<object> via same-origin <iframe>)]
-    expected: FAIL
-
-  [Window's associated Document object is used for finding named objects (<object> via cross-site <iframe>)]
-    expected: FAIL
-
-  [Window's associated Document object is used for finding named objects (<iframe> via srcdoc <iframe>)]
-    expected: FAIL
-

tests/wpt/metadata/html/browsers/the-window-object/named-access-on-the-window-object/prototype.html.ini

@@ -1,10 +0,0 @@
-[prototype.html]
-  [Property on EventTarget.prototype.]
-    expected: FAIL
-
-  [Property on window.]
-    expected: FAIL
-
-  [Property on Window.prototype.]
-    expected: FAIL
-

tests/wpt/metadata/html/browsers/the-window-object/named-access-on-the-window-object/window-null-names.html.ini

@@ -1,5 +0,0 @@
-[window-null-names.html]
-  type: testharness
-  [Named access with null characters]
-    expected: FAIL
-