Backed out changeset 8fb0fa2dca36 (bug 1464428) for Reftest crashes on a CLOSED TREE

--HG--
extra : amend_source : 023931e140119e040f5ca1c5b24bc4def5b0c215

layout/style/ServoBindings.cpp

@@ -2889,12 +2889,12 @@ Gecko_ContentList_AppendAll(
 }
 
 const nsTArray<Element*>*
-Gecko_GetElementsWithId(const DocumentOrShadowRoot* aDocOrShadowRoot, nsAtom* aId)
+Gecko_GetElementsWithId(const nsIDocument* aDocument, nsAtom* aId)
 {
-  MOZ_ASSERT(aDocOrShadowRoot);
+  MOZ_ASSERT(aDocument);
   MOZ_ASSERT(aId);
 
-  return aDocOrShadowRoot->GetAllElementsForId(nsDependentAtomString(aId));
+  return aDocument->GetAllElementsForId(nsDependentAtomString(aId));
 }
 
 bool

layout/style/ServoBindings.h

@@ -709,7 +709,7 @@ void Gecko_ContentList_AppendAll(nsSimpleContentList* aContentList,
                                  size_t aLength);
 
 const nsTArray<mozilla::dom::Element*>* Gecko_GetElementsWithId(
-    const mozilla::dom::DocumentOrShadowRoot* aDocOrShadowRoot,
+    const nsIDocument* aDocument,
     nsAtom* aId);
 
 // Check the value of the given bool preference. The pref name needs to

layout/style/ServoBindings.toml

@@ -458,7 +458,6 @@ structs-types = [
     "mozilla::css::URLValue",
     "mozilla::css::URLValueData",
     "mozilla::dom::CallerType",
-    "mozilla::dom::DocumentOrShadowRoot",
     "mozilla::AnonymousCounterStyle",
     "mozilla::AtomArray",
     "mozilla::FontStretch",

servo/components/style/dom.rs

@@ -135,17 +135,14 @@ pub trait TDocument: Sized + Copy + Clone {
     fn quirks_mode(&self) -> QuirksMode;
 
     /// Get a list of elements with a given ID in this document, sorted by
-    /// tree position.
+    /// document position.
     ///
     /// Can return an error to signal that this list is not available, or also
     /// return an empty slice.
-    fn elements_with_id<'a>(
+    fn elements_with_id(
         &self,
         _id: &Atom,
-    ) -> Result<&'a [<Self::ConcreteNode as TNode>::ConcreteElement], ()>
-    where
-        Self: 'a,
-    {
+    ) -> Result<&[<Self::ConcreteNode as TNode>::ConcreteElement], ()> {
         Err(())
     }
 }
@@ -345,21 +342,6 @@ pub trait TShadowRoot: Sized + Copy + Clone + PartialEq {
     fn style_data<'a>(&self) -> &'a CascadeData
     where
         Self: 'a;
-
-    /// Get a list of elements with a given ID in this shadow root, sorted by
-    /// tree position.
-    ///
-    /// Can return an error to signal that this list is not available, or also
-    /// return an empty slice.
-    fn elements_with_id<'a>(
-        &self,
-        _id: &Atom,
-    ) -> Result<&'a [<Self::ConcreteNode as TNode>::ConcreteElement], ()>
-    where
-        Self: 'a,
-    {
-        Err(())
-    }
 }
 
 /// The element trait, the main abstraction the style crate acts over.

servo/components/style/dom_apis.rs

@@ -221,31 +221,14 @@ where
     }
 }
 
-/// Returns whether a given element connected to `root` is descendant of `root`.
+/// Returns whether a given element is descendant of a given `root` node.
 ///
 /// NOTE(emilio): if root == element, this returns false.
-fn connected_element_is_descendant_of<E>(element: E, root: E::ConcreteNode) -> bool
+fn element_is_descendant_of<E>(element: E, root: E::ConcreteNode) -> bool
 where
     E: TElement,
 {
-    // Optimize for when the root is a document or a shadow root and the element
-    // is connected to that root.
-    if root.as_document().is_some() {
-        debug_assert!(element.as_node().is_in_document(), "Not connected?");
-        debug_assert_eq!(
-            root,
-            root.owner_doc().as_node(),
-            "Where did this element come from?",
-        );
-        return true;
-    }
-
-    if root.as_shadow_root().is_some() {
-        debug_assert_eq!(
-            element.containing_shadow().unwrap().as_node(),
-            root,
-            "Not connected?"
-        );
+    if element.as_node().is_in_document() && root == root.owner_doc().as_node() {
         return true;
     }
 
@@ -261,33 +244,28 @@ where
 }
 
 /// Fast path for iterating over every element with a given id in the document
-/// or shadow root that `root` is connected to.
-fn fast_connected_elements_with_id<'a, N>(
-    root: N,
+/// that `root` is connected to.
+fn fast_connected_elements_with_id<'a, D>(
+    doc: &'a D,
+    root: D::ConcreteNode,
     id: &Atom,
     quirks_mode: QuirksMode,
-) -> Result<&'a [N::ConcreteElement], ()>
+) -> Result<&'a [<D::ConcreteNode as TNode>::ConcreteElement], ()>
 where
-    N: TNode + 'a,
+    D: TDocument,
 {
+    debug_assert_eq!(root.owner_doc().as_node(), doc.as_node());
+
     let case_sensitivity = quirks_mode.classes_and_ids_case_sensitivity();
     if case_sensitivity != CaseSensitivity::CaseSensitive {
         return Err(());
     }
 
-    if root.is_in_document() {
-        return root.owner_doc().elements_with_id(id);
+    if !root.is_in_document() {
+        return Err(());
     }
 
-    if let Some(shadow) = root.as_shadow_root() {
-        return shadow.elements_with_id(id);
-    }
-
-    if let Some(shadow) = root.as_element().and_then(|e| e.containing_shadow()) {
-        return shadow.elements_with_id(id);
-    }
-
-    Err(())
+    doc.elements_with_id(id)
 }
 
 /// Collects elements with a given id under `root`, that pass `filter`.
@@ -302,7 +280,8 @@ fn collect_elements_with_id<E, Q, F>(
     Q: SelectorQuery<E>,
     F: FnMut(E) -> bool,
 {
-    let elements = match fast_connected_elements_with_id(root, id, quirks_mode) {
+    let doc = root.owner_doc();
+    let elements = match fast_connected_elements_with_id(&doc, root, id, quirks_mode) {
         Ok(elements) => elements,
         Err(()) => {
             let case_sensitivity = quirks_mode.classes_and_ids_case_sensitivity();
@@ -318,7 +297,7 @@ fn collect_elements_with_id<E, Q, F>(
     for element in elements {
         // If the element is not an actual descendant of the root, even though
         // it's connected, we don't really care about it.
-        if !connected_element_is_descendant_of(*element, root) {
+        if !element_is_descendant_of(*element, root) {
             continue;
         }
 
@@ -426,8 +405,9 @@ where
                         return Ok(());
                     }
 
-                    let elements =
-                        fast_connected_elements_with_id(root, id, quirks_mode)?;
+                    let doc = root.owner_doc();
+                    let elements = fast_connected_elements_with_id(&doc, root, id, quirks_mode)?;
+
                     if elements.is_empty() {
                         return Ok(());
                     }
@@ -452,7 +432,7 @@ where
                         //
                         // Give up on trying to optimize based on this id and
                         // keep walking our selector.
-                        if !connected_element_is_descendant_of(*element, root) {
+                        if !element_is_descendant_of(*element, root) {
                             continue 'component_loop;
                         }
 

servo/components/style/gecko/wrapper.rs

@@ -87,30 +87,6 @@ use std::ptr;
 use string_cache::{Atom, Namespace, WeakAtom, WeakNamespace};
 use stylist::CascadeData;
 
-
-fn elements_with_id<'a, 'le>(
-    root: &'a structs::DocumentOrShadowRoot,
-    id: &Atom,
-) -> &'a [GeckoElement<'le>] {
-    unsafe {
-        let array = bindings::Gecko_GetElementsWithId(root, id.as_ptr());
-        if array.is_null() {
-            return &[];
-        }
-
-        let elements: &[*mut RawGeckoElement] = &**array;
-
-        // NOTE(emilio): We rely on the in-memory representation of
-        // GeckoElement<'ld> and *mut RawGeckoElement being the same.
-        #[allow(dead_code)]
-        unsafe fn static_assert() {
-            mem::transmute::<*mut RawGeckoElement, GeckoElement<'static>>(0xbadc0de as *mut _);
-        }
-
-        mem::transmute(elements)
-    }
-}
-
 /// A simple wrapper over `nsIDocument`.
 #[derive(Clone, Copy)]
 pub struct GeckoDocument<'ld>(pub &'ld structs::nsIDocument);
@@ -133,12 +109,24 @@ impl<'ld> TDocument for GeckoDocument<'ld> {
         self.0.mCompatMode.into()
     }
 
-    #[inline]
-    fn elements_with_id<'a>(&self, id: &Atom) -> Result<&'a [GeckoElement<'ld>], ()>
-    where
-        Self: 'a,
-    {
-        Ok(elements_with_id(&self.0._base_1, id))
+    fn elements_with_id(&self, id: &Atom) -> Result<&[GeckoElement<'ld>], ()> {
+        unsafe {
+            let array = bindings::Gecko_GetElementsWithId(self.0, id.as_ptr());
+            if array.is_null() {
+                return Ok(&[]);
+            }
+
+            let elements: &[*mut RawGeckoElement] = &**array;
+
+            // NOTE(emilio): We rely on the in-memory representation of
+            // GeckoElement<'ld> and *mut RawGeckoElement being the same.
+            #[allow(dead_code)]
+            unsafe fn static_assert() {
+                mem::transmute::<*mut RawGeckoElement, GeckoElement<'static>>(0xbadc0de as *mut _);
+            }
+
+            Ok(mem::transmute(elements))
+        }
     }
 }
 
@@ -188,14 +176,6 @@ impl<'lr> TShadowRoot for GeckoShadowRoot<'lr> {
 
         &author_styles.data
     }
-
-    #[inline]
-    fn elements_with_id<'a>(&self, id: &Atom) -> Result<&'a [GeckoElement<'lr>], ()>
-    where
-        Self: 'a,
-    {
-        Ok(elements_with_id(&self.0._base_1, id))
-    }
 }
 
 /// A simple wrapper over a non-null Gecko node (`nsINode`) pointer.