From 1c8cd3045bd430249449b299df769b248575524d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Tue, 30 Jan 2018 16:35:13 +0100
Subject: [PATCH 1/3] script: Partially implement the window named getter.

There's tons of layout tests that depend on this, and I keep forgetting Servo
doesn't support it :)
---
 components/script/dom/document.rs           | 129 +++++++++++---------
 components/script/dom/element.rs            |   4 +
 components/script/dom/webidls/Window.webidl |   2 +-
 components/script/dom/window.rs             |  58 ++++++++-
 4 files changed, 131 insertions(+), 62 deletions(-)

diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs
index 436d4e71d1a8..66adea442eec 100644
--- a/components/script/dom/document.rs
+++ b/components/script/dom/document.rs
@@ -368,6 +368,56 @@ pub struct Document {
     throw_on_dynamic_markup_insertion_counter: Cell<u64>,
 }
 
+// https://html.spec.whatwg.org/multipage/#dom-document-nameditem-filter
+fn filter_by_name(name: &Atom, element: &Element) -> bool {
+    // TODO: Handle <embed>, <iframe> and <object>.
+    if element.is::<HTMLFormElement>() || element.is::<HTMLImageElement>() {
+        if let Some(attr) = element.get_attribute(&ns!(), &local_name!("name")) {
+            if attr.value().as_atom() == name {
+                return true;
+            }
+        }
+    }
+
+    if element.is::<HTMLImageElement>() &&
+        element.get_id().as_ref().map_or(false, |id| id == name)
+    {
+        return true;
+    }
+
+    false
+}
+
+/// The filter for the named getter HTMLCollection that must be returned from
+/// the document.
+#[derive(JSTraceable, MallocSizeOf)]
+struct NamedGetterFilter {
+    name: Atom,
+}
+
+impl CollectionFilter for NamedGetterFilter {
+    fn filter(&self, elem: &Element, _root: &Node) -> bool {
+        filter_by_name(&self.name, elem)
+    }
+}
+
+/// The filter for the named getter HTMLCollection that must be returned from
+/// the window (not the document).
+#[derive(JSTraceable, MallocSizeOf)]
+pub struct WindowNamedGetterFilter {
+    pub name: Atom,
+}
+
+impl CollectionFilter for WindowNamedGetterFilter {
+    fn filter(&self, element: &Element, _: &Node) -> bool {
+        if filter_by_name(&self.name, element) {
+            return true;
+        }
+
+        element.get_id().as_ref().map_or(false, |id| id == &self.name)
+    }
+}
+
 #[derive(JSTraceable, MallocSizeOf)]
 struct ImagesFilter;
 impl CollectionFilter for ImagesFilter {
@@ -3555,72 +3605,27 @@ impl DocumentMethods for Document {
     #[allow(unsafe_code)]
     // https://html.spec.whatwg.org/multipage/#dom-tree-accessors:dom-document-nameditem-filter
     unsafe fn NamedGetter(&self, _cx: *mut JSContext, name: DOMString) -> Option<NonNull<JSObject>> {
-        #[derive(JSTraceable, MallocSizeOf)]
-        struct NamedElementFilter {
-            name: Atom,
-        }
-        impl CollectionFilter for NamedElementFilter {
-            fn filter(&self, elem: &Element, _root: &Node) -> bool {
-                filter_by_name(&self.name, elem.upcast())
-            }
-        }
-        // https://html.spec.whatwg.org/multipage/#dom-document-nameditem-filter
-        fn filter_by_name(name: &Atom, node: &Node) -> bool {
-            let html_elem_type = match node.type_id() {
-                NodeTypeId::Element(ElementTypeId::HTMLElement(type_)) => type_,
-                _ => return false,
-            };
-            let elem = match node.downcast::<Element>() {
-                Some(elem) => elem,
-                None => return false,
-            };
-            match html_elem_type {
-                HTMLElementTypeId::HTMLFormElement => {
-                    match elem.get_attribute(&ns!(), &local_name!("name")) {
-                        Some(ref attr) => attr.value().as_atom() == name,
-                        None => false,
-                    }
-                },
-                HTMLElementTypeId::HTMLImageElement => {
-                    match elem.get_attribute(&ns!(), &local_name!("name")) {
-                        Some(ref attr) => {
-                            if attr.value().as_atom() == name {
-                                true
-                            } else {
-                                match elem.get_attribute(&ns!(), &local_name!("id")) {
-                                    Some(ref attr) => attr.value().as_atom() == name,
-                                    None => false,
-                                }
-                            }
-                        },
-                        None => false,
-                    }
-                },
-                // TODO: Handle <embed>, <iframe> and <object>.
-                _ => false,
-            }
-        }
         let name = Atom::from(name);
+        let filter = NamedGetterFilter { name };
+
         let root = self.upcast::<Node>();
         {
             // Step 1.
-            let mut elements = root.traverse_preorder()
-                                   .filter(|node| filter_by_name(&name, &node))
-                                   .peekable();
-            if let Some(first) = elements.next() {
-                if elements.peek().is_none() {
-                    // TODO: Step 2.
-                    // Step 3.
-                    return Some(NonNull::new_unchecked(first.reflector().get_jsobject().get()));
-                }
-            } else {
-                return None;
+            let mut elements = root
+                .traverse_preorder()
+                .filter_map(DomRoot::downcast::<Element>)
+                .filter(|element| filter.filter(&element, &root));
+
+            let first_element = elements.next()?;
+            if elements.next().is_none() {
+                // TODO: Step 2.
+                // Step 3.
+                return Some(NonNull::new_unchecked(
+                    first_element.reflector().get_jsobject().get()
+                ));
             }
         }
         // Step 4.
-        let filter = NamedElementFilter {
-            name: name,
-        };
         let collection = HTMLCollection::create(self.window(), root, Box::new(filter));
         Some(NonNull::new_unchecked(collection.reflector().get_jsobject().get()))
     }
@@ -3628,6 +3633,12 @@ impl DocumentMethods for Document {
     // https://html.spec.whatwg.org/multipage/#dom-tree-accessors:supported-property-names
     fn SupportedPropertyNames(&self) -> Vec<DOMString> {
         // FIXME: unimplemented (https://github.com/servo/servo/issues/7273)
+        //
+        // We really need a HashMap from name to element, much like `id_map` for
+        // this.
+        //
+        // Chances are that if you implement this, you can also implement most
+        // of Window::SupportedPropertyNames.
         vec![]
     }
 
diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs
index ad050be3dda3..672cc2b37b44 100644
--- a/components/script/dom/element.rs
+++ b/components/script/dom/element.rs
@@ -303,6 +303,10 @@ impl Element {
         *self.is.borrow_mut() = Some(is);
     }
 
+    pub fn get_id(&self) -> Ref<Option<Atom>> {
+        self.id_attribute.borrow()
+    }
+
     pub fn get_is(&self) -> Option<LocalName> {
         self.is.borrow().clone()
     }
diff --git a/components/script/dom/webidls/Window.webidl b/components/script/dom/webidls/Window.webidl
index 0ec0489937b8..de5c4a26888c 100644
--- a/components/script/dom/webidls/Window.webidl
+++ b/components/script/dom/webidls/Window.webidl
@@ -46,7 +46,7 @@
   //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;
diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs
index e9055f54ea0b..a662b1561c9d 100644
--- a/components/script/dom/window.rs
+++ b/components/script/dom/window.rs
@@ -30,11 +30,12 @@ use dom::bluetooth::BluetoothExtraPermissionData;
 use dom::crypto::Crypto;
 use dom::cssstyledeclaration::{CSSModificationAccess, CSSStyleDeclaration, CSSStyleOwner};
 use dom::customelementregistry::CustomElementRegistry;
-use dom::document::{AnimationFrameCallback, Document};
+use dom::document::{AnimationFrameCallback, Document, WindowNamedGetterFilter};
 use dom::element::Element;
 use dom::event::Event;
 use dom::globalscope::GlobalScope;
 use dom::history::History;
+use dom::htmlcollection::{CollectionFilter, HTMLCollection};
 use dom::htmliframeelement::build_mozbrowser_custom_event;
 use dom::location::Location;
 use dom::mediaquerylist::{MediaQueryList, WeakMediaQueryListVec};
@@ -54,7 +55,7 @@ use euclid::{Point2D, Vector2D, Rect, Size2D};
 use fetch;
 use ipc_channel::ipc::{self, IpcSender};
 use ipc_channel::router::ROUTER;
-use js::jsapi::{HandleObject, HandleValue, JSAutoCompartment, JSContext};
+use js::jsapi::{HandleObject, HandleValue, JSAutoCompartment, JSContext, JSObject};
 use js::jsapi::{JS_GC, JS_GetRuntime};
 use js::jsval::UndefinedValue;
 use layout_image::fetch_image_for_layout;
@@ -82,6 +83,7 @@ use script_traits::{TimerSchedulerMsg, UntrustedNodeAddress, WindowSizeData, Win
 use script_traits::webdriver_msg::{WebDriverJSError, WebDriverJSResult};
 use selectors::attr::CaseSensitivity;
 use servo_arc;
+use servo_atoms::Atom;
 use servo_config::opts;
 use servo_config::prefs::PREFS;
 use servo_geometry::{f32_rect_to_au_rect, MaxRect};
@@ -95,6 +97,7 @@ use std::env;
 use std::fs;
 use std::io::{Write, stderr, stdout};
 use std::mem;
+use std::ptr::NonNull;
 use std::rc::Rc;
 use std::sync::{Arc, Mutex};
 use std::sync::atomic::{AtomicBool, Ordering};
@@ -842,6 +845,56 @@ impl WindowMethods for Window {
                                  CSSModificationAccess::Readonly)
     }
 
+    // https://html.spec.whatwg.org/multipage/window-object.html#named-access-on-the-window-object
+    fn SupportedPropertyNames(&self) -> Vec<DOMString> {
+        // FIXME: unimplemented (https://github.com/servo/servo/issues/7273).
+        //
+        // See also Document::SupportedPropertyNames.
+        vec![]
+    }
+
+    #[allow(unsafe_code)]
+    // https://html.spec.whatwg.org/multipage/#named-access-on-the-window-object
+    unsafe fn NamedGetter(
+        &self,
+        cx: *mut JSContext,
+        name: DOMString,
+    ) -> Option<NonNull<JSObject>> {
+        // TODO: Return child windows, like iframes and such. When this is
+        // fixed, please implement IndexedGetter properly too.
+        let document = self.Document();
+        if !document.is_html_document() {
+            return None;
+        }
+
+        // TODO(emilio): There is a missing fast-path here for when we know
+        // there aren't any named items with a given `name`. In that case, we
+        // can return `document.get_element_by_id(name)` (unless there are many
+        // of those elements), which is much faster.
+        let name = Atom::from(name);
+
+        let filter = WindowNamedGetterFilter { name };
+
+        let root = document.upcast();
+        {
+            let mut named_elements = document.upcast::<Node>()
+                .traverse_preorder()
+                .filter_map(DomRoot::downcast::<Element>)
+                .filter(|element| filter.filter(&element, &root));
+
+            let first_element = named_elements.next()?;
+            if named_elements.next().is_none() {
+                return Some(NonNull::new_unchecked(
+                    first_element.reflector().get_jsobject().get()
+                ));
+            }
+        }
+
+        let collection =
+            HTMLCollection::create(self, root, Box::new(filter));
+        Some(NonNull::new_unchecked(collection.reflector().get_jsobject().get()))
+    }
+
     // https://drafts.csswg.org/cssom-view/#dom-window-innerheight
     //TODO Include Scrollbar
     fn InnerHeight(&self) -> i32 {
@@ -1609,6 +1662,7 @@ impl Window {
 
     // https://html.spec.whatwg.org/multipage/#accessing-other-browsing-contexts
     pub fn IndexedGetter(&self, _index: u32, _found: &mut bool) -> Option<DomRoot<Window>> {
+        // TODO: When this is fixed, also implement NamedGetter properly.
         None
     }
 

From 5f410dc6e94ee8b9afe9c61daffb639b9e784cf5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Sun, 4 Feb 2018 20:34:09 +0100
Subject: [PATCH 2/3] dom/bindings: Minimal cleanup.

---
 components/script/dom/bindings/codegen/CodegenRust.py | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py
index d2d090f83502..d56984780269 100644
--- a/components/script/dom/bindings/codegen/CodegenRust.py
+++ b/components/script/dom/bindings/codegen/CodegenRust.py
@@ -4899,8 +4899,7 @@ def getBody(self):
                     CGIndenter(CGProxyIndexedGetter(self.descriptor, templateValues)).define() + "\n" +
                     "}\n")
 
-        namedGetter = self.descriptor.operations['NamedGetter']
-        if namedGetter:
+        if self.descriptor.operations['NamedGetter']:
             attrs = []
             if not self.descriptor.interface.getExtendedAttribute("LegacyUnenumerableNamedProperties"):
                 attrs.append("JSPROP_ENUMERATE")
@@ -5152,11 +5151,10 @@ def getBody(self):
         else:
             indexed = ""
 
-        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.operations['NamedGetter']:
             named = """\
 if %s {
     let mut has_on_proto = false;
@@ -5237,8 +5235,7 @@ def getBody(self):
         else:
             getIndexedOrExpando = getFromExpando + "\n"
 
-        namedGetter = self.descriptor.operations['NamedGetter']
-        if namedGetter:
+        if self.descriptor.operations['NamedGetter']:
             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:
@@ -5896,7 +5893,7 @@ def reexportedName(name):
                 elif m.getExtendedAttribute("Replaceable"):
                     cgThings.append(CGSpecializedReplaceableSetter(descriptor, m))
 
-                if (not m.isStatic() and not descriptor.interface.isCallback()):
+                if not m.isStatic() and not descriptor.interface.isCallback():
                     cgThings.append(CGMemberJITInfo(descriptor, m))
 
         if descriptor.concrete:

From c098d21acb01ccc580a37bd2793350bf5bf398c1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Sun, 4 Feb 2018 20:38:07 +0100
Subject: [PATCH 3/3] Add codegen to hook into the window prototype.

---
 .../dom/bindings/codegen/CodegenRust.py       | 23 ++++++++++++++++++-
 .../dom/bindings/codegen/Configuration.py     | 13 ++++++++++-
 components/script/dom/window.rs               | 11 +++++++--
 3 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py
index d56984780269..dd6d535e5ec8 100644
--- a/components/script/dom/bindings/codegen/CodegenRust.py
+++ b/components/script/dom/bindings/codegen/CodegenRust.py
@@ -2834,8 +2834,19 @@ def definition_body(self):
         else:
             proto_properties = properties
 
+
         code.append(CGGeneric("""
 rooted!(in(cx) let mut prototype = ptr::null_mut::<JSObject>());
+"""))
+
+        if self.descriptor.hasNamedPropertiesObject():
+            assert self.descriptor.isGlobal()
+            assert not self.haveUnscopables
+            code.append(CGGeneric("""
+dom::types::%s::CreateNamedPropertiesObject(cx, prototype.handle_mut());
+""" % name))
+        else:
+            code.append(CGGeneric("""
 create_interface_prototype_object(cx,
                                   prototype_proto.handle(),
                                   &PrototypeClass,
@@ -2844,6 +2855,9 @@ def definition_body(self):
                                   %(consts)s,
                                   %(unscopables)s,
                                   prototype.handle_mut());
+""" % proto_properties))
+
+        code.append(CGGeneric("""
 assert!(!prototype.is_null());
 assert!((*cache)[PrototypeList::ID::%(id)s as usize].is_null());
 (*cache)[PrototypeList::ID::%(id)s as usize] = prototype.get();
@@ -5523,7 +5537,7 @@ def members():
                             rettype = "ErrorResult"
                         yield name, attribute_arguments(typeNeedsCx(m.type, False), m.type), rettype
 
-            if descriptor.proxy:
+            if descriptor.proxy or descriptor.hasNamedPropertiesObject():
                 for name, operation in descriptor.operations.iteritems():
                     if not operation or operation.isStringifier():
                         continue
@@ -5558,6 +5572,12 @@ def contains_unsafe_arg(arguments):
             return reduce((lambda x, y: x or y[1] == '*mut JSContext'), arguments, False)
 
         methods = []
+
+        if descriptor.hasNamedPropertiesObject():
+            methods.append(CGGeneric("""
+unsafe fn CreateNamedPropertiesObject(cx: *mut JSContext, proto: MutableHandleObject);
+"""))
+
         for name, arguments, rettype in members():
             arguments = list(arguments)
             methods.append(CGGeneric("%sfn %s(&self%s) -> %s;\n" % (
@@ -5856,6 +5876,7 @@ def reexportedName(name):
         cgThings = []
 
         unscopableNames = []
+
         for m in descriptor.interface.members:
             if (m.isMethod() and
                     (not m.isIdentifierLess() or m == descriptor.operations["Stringifier"])):
diff --git a/components/script/dom/bindings/codegen/Configuration.py b/components/script/dom/bindings/codegen/Configuration.py
index 0ab27151bb6e..17aa0efde2e7 100644
--- a/components/script/dom/bindings/codegen/Configuration.py
+++ b/components/script/dom/bindings/codegen/Configuration.py
@@ -267,8 +267,10 @@ def addOperation(operation, m):
                         continue
 
                     def addIndexedOrNamedOperation(operation, m):
-                        self.proxy = True
+                        if not self.isGlobal():
+                            self.proxy = True
                         if m.isIndexed():
+                            assert not self.isGlobal()
                             operation = 'Indexed' + operation
                         else:
                             assert m.isNamed()
@@ -355,6 +357,15 @@ def binaryNameFor(self, name):
     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:
diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs
index a662b1561c9d..79b0f9fe4531 100644
--- a/components/script/dom/window.rs
+++ b/components/script/dom/window.rs
@@ -55,7 +55,7 @@ use euclid::{Point2D, Vector2D, Rect, Size2D};
 use fetch;
 use ipc_channel::ipc::{self, IpcSender};
 use ipc_channel::router::ROUTER;
-use js::jsapi::{HandleObject, HandleValue, JSAutoCompartment, JSContext, JSObject};
+use js::jsapi::{HandleObject, HandleValue, MutableHandleObject, JSAutoCompartment, JSContext, JSObject};
 use js::jsapi::{JS_GC, JS_GetRuntime};
 use js::jsval::UndefinedValue;
 use layout_image::fetch_image_for_layout;
@@ -845,7 +845,14 @@ impl WindowMethods for Window {
                                  CSSModificationAccess::Readonly)
     }
 
-    // https://html.spec.whatwg.org/multipage/window-object.html#named-access-on-the-window-object
+    // https://heycam.github.io/webidl/#named-properties-object
+    // https://html.spec.whatwg.org/multipage/#named-access-on-the-window-object
+    #[allow(unsafe_code)]
+    unsafe fn CreateNamedPropertiesObject(_cx: *mut JSContext, _proto: MutableHandleObject) {
+        unimplemented!("Wow!")
+    }
+
+    // https://html.spec.whatwg.org/multipage/#named-access-on-the-window-object
     fn SupportedPropertyNames(&self) -> Vec<DOMString> {
         // FIXME: unimplemented (https://github.com/servo/servo/issues/7273).
         //