From 2fe6f395290a82d0570f019e55d7e8e251bc7b65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 25 May 2018 16:56:41 +0200 Subject: [PATCH] Bug 1464428: Optimize QuerySelector in shadow trees. r=xidorn Pretty much the same setup we have for document. We have the awkwardness of having to check containing shadow manually for ShadowRoot because it's not available in TNode (and making it available added a bit more complexity that wasn't worth it IMO). MozReview-Commit-ID: CqOh0sLHf6o --- layout/style/ServoBindings.cpp | 6 +-- layout/style/ServoBindings.h | 2 +- layout/style/ServoBindings.toml | 1 + servo/components/style/dom.rs | 24 ++++++++-- servo/components/style/dom_apis.rs | 62 ++++++++++++++++--------- servo/components/style/gecko/wrapper.rs | 56 +++++++++++++++------- 6 files changed, 105 insertions(+), 46 deletions(-) diff --git a/layout/style/ServoBindings.cpp b/layout/style/ServoBindings.cpp index bbefcfb75124..9e3fc34cfdc7 100644 --- a/layout/style/ServoBindings.cpp +++ b/layout/style/ServoBindings.cpp @@ -2892,12 +2892,12 @@ Gecko_ContentList_AppendAll( } const nsTArray* -Gecko_GetElementsWithId(const nsIDocument* aDocument, nsAtom* aId) +Gecko_GetElementsWithId(const DocumentOrShadowRoot* aDocOrShadowRoot, nsAtom* aId) { - MOZ_ASSERT(aDocument); + MOZ_ASSERT(aDocOrShadowRoot); MOZ_ASSERT(aId); - return aDocument->GetAllElementsForId(nsDependentAtomString(aId)); + return aDocOrShadowRoot->GetAllElementsForId(nsDependentAtomString(aId)); } bool diff --git a/layout/style/ServoBindings.h b/layout/style/ServoBindings.h index 8a99931f5c74..386b81f27085 100644 --- a/layout/style/ServoBindings.h +++ b/layout/style/ServoBindings.h @@ -709,7 +709,7 @@ void Gecko_ContentList_AppendAll(nsSimpleContentList* aContentList, size_t aLength); const nsTArray* Gecko_GetElementsWithId( - const nsIDocument* aDocument, + const mozilla::dom::DocumentOrShadowRoot* aDocOrShadowRoot, nsAtom* aId); // Check the value of the given bool preference. The pref name needs to diff --git a/layout/style/ServoBindings.toml b/layout/style/ServoBindings.toml index 11270bf3db8b..262011b414ec 100644 --- a/layout/style/ServoBindings.toml +++ b/layout/style/ServoBindings.toml @@ -458,6 +458,7 @@ structs-types = [ "mozilla::css::URLValue", "mozilla::css::URLValueData", "mozilla::dom::CallerType", + "mozilla::dom::DocumentOrShadowRoot", "mozilla::AnonymousCounterStyle", "mozilla::AtomArray", "mozilla::FontStretch", diff --git a/servo/components/style/dom.rs b/servo/components/style/dom.rs index bee4d88329ef..0e5da1542d74 100644 --- a/servo/components/style/dom.rs +++ b/servo/components/style/dom.rs @@ -135,14 +135,17 @@ 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 - /// document position. + /// tree position. /// /// Can return an error to signal that this list is not available, or also /// return an empty slice. - fn elements_with_id( + fn elements_with_id<'a>( &self, _id: &Atom, - ) -> Result<&[::ConcreteElement], ()> { + ) -> Result<&'a [::ConcreteElement], ()> + where + Self: 'a, + { Err(()) } } @@ -342,6 +345,21 @@ 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 [::ConcreteElement], ()> + where + Self: 'a, + { + Err(()) + } } /// The element trait, the main abstraction the style crate acts over. diff --git a/servo/components/style/dom_apis.rs b/servo/components/style/dom_apis.rs index 2da4b476c938..393fb6e119e4 100644 --- a/servo/components/style/dom_apis.rs +++ b/servo/components/style/dom_apis.rs @@ -221,14 +221,31 @@ where } } -/// Returns whether a given element is descendant of a given `root` node. +/// Returns whether a given element connected to `root` is descendant of `root`. /// /// NOTE(emilio): if root == element, this returns false. -fn element_is_descendant_of(element: E, root: E::ConcreteNode) -> bool +fn connected_element_is_descendant_of(element: E, root: E::ConcreteNode) -> bool where E: TElement, { - if element.as_node().is_in_document() && root == root.owner_doc().as_node() { + // 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?" + ); return true; } @@ -244,28 +261,33 @@ where } /// Fast path for iterating over every element with a given id in the document -/// that `root` is connected to. -fn fast_connected_elements_with_id<'a, D>( - doc: &'a D, - root: D::ConcreteNode, +/// or shadow root that `root` is connected to. +fn fast_connected_elements_with_id<'a, N>( + root: N, id: &Atom, quirks_mode: QuirksMode, -) -> Result<&'a [::ConcreteElement], ()> +) -> Result<&'a [N::ConcreteElement], ()> where - D: TDocument, + N: TNode + 'a, { - 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 Err(()); + if root.is_in_document() { + return root.owner_doc().elements_with_id(id); } - doc.elements_with_id(id) + 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(()) } /// Collects elements with a given id under `root`, that pass `filter`. @@ -280,8 +302,7 @@ fn collect_elements_with_id( Q: SelectorQuery, F: FnMut(E) -> bool, { - let doc = root.owner_doc(); - let elements = match fast_connected_elements_with_id(&doc, root, id, quirks_mode) { + let elements = match fast_connected_elements_with_id(root, id, quirks_mode) { Ok(elements) => elements, Err(()) => { let case_sensitivity = quirks_mode.classes_and_ids_case_sensitivity(); @@ -297,7 +318,7 @@ fn collect_elements_with_id( 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 !element_is_descendant_of(*element, root) { + if !connected_element_is_descendant_of(*element, root) { continue; } @@ -405,9 +426,8 @@ where return Ok(()); } - let doc = root.owner_doc(); - let elements = fast_connected_elements_with_id(&doc, root, id, quirks_mode)?; - + let elements = + fast_connected_elements_with_id(root, id, quirks_mode)?; if elements.is_empty() { return Ok(()); } @@ -432,7 +452,7 @@ where // // Give up on trying to optimize based on this id and // keep walking our selector. - if !element_is_descendant_of(*element, root) { + if !connected_element_is_descendant_of(*element, root) { continue 'component_loop; } diff --git a/servo/components/style/gecko/wrapper.rs b/servo/components/style/gecko/wrapper.rs index 7a565ec469ed..9149b48e7199 100644 --- a/servo/components/style/gecko/wrapper.rs +++ b/servo/components/style/gecko/wrapper.rs @@ -87,6 +87,30 @@ 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); @@ -109,24 +133,12 @@ impl<'ld> TDocument for GeckoDocument<'ld> { self.0.mCompatMode.into() } - 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)) - } + #[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)) } } @@ -176,6 +188,14 @@ 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.