From 6b1b8bbee83a090b67dbccb83ac527052559a5a6 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Tue, 6 Jun 2017 16:36:25 -0500 Subject: [PATCH 1/2] Log element during selector matching MozReview-Commit-ID: D8eFyRCy5BR --- Cargo.lock | 1 + components/script/dom/element.rs | 6 ++++++ components/selectors/Cargo.toml | 1 + components/selectors/lib.rs | 1 + components/selectors/matching.rs | 5 ++++- components/selectors/parser.rs | 10 ++++++++++ components/selectors/tree.rs | 3 ++- components/style/restyle_hints.rs | 10 ++++++++++ 8 files changed, 35 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 50bfbccb10e..492f284e075 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2518,6 +2518,7 @@ dependencies = [ "bitflags 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", "cssparser 0.13.7 (registry+https://github.com/rust-lang/crates.io-index)", "fnv 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)", + "log 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)", "matches 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)", "phf 0.7.21 (registry+https://github.com/rust-lang/crates.io-index)", "phf_codegen 0.7.21 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index f875986edd2..c0d4b6b700a 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -151,6 +151,12 @@ impl fmt::Debug for Element { } } +impl fmt::Debug for Root { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + (**self).fmt(f) + } +} + #[derive(PartialEq, HeapSizeOf)] pub enum ElementCreator { ParserCreated(u64), diff --git a/components/selectors/Cargo.toml b/components/selectors/Cargo.toml index 15aba3d1811..90ff6ecaa17 100644 --- a/components/selectors/Cargo.toml +++ b/components/selectors/Cargo.toml @@ -25,6 +25,7 @@ gecko_like_types = [] bitflags = "0.7" matches = "0.1" cssparser = "0.13.7" +log = "0.3" fnv = "1.0" phf = "0.7.18" precomputed-hash = "0.1" diff --git a/components/selectors/lib.rs b/components/selectors/lib.rs index a6f4acd3d84..130475271fc 100644 --- a/components/selectors/lib.rs +++ b/components/selectors/lib.rs @@ -4,6 +4,7 @@ #[macro_use] extern crate bitflags; #[macro_use] extern crate cssparser; +#[macro_use] extern crate log; #[macro_use] extern crate matches; extern crate fnv; extern crate phf; diff --git a/components/selectors/matching.rs b/components/selectors/matching.rs index 3c269427421..84ddde2b261 100644 --- a/components/selectors/matching.rs +++ b/components/selectors/matching.rs @@ -198,7 +198,7 @@ fn may_match(hashes: &AncestorHashes, /// and `is_unvisited` are based on relevant link state of only the current /// complex selector being matched (not the global relevant link status for all /// selectors in `MatchingContext`). -#[derive(PartialEq, Eq, Copy, Clone)] +#[derive(PartialEq, Eq, Copy, Clone, Debug)] pub enum RelevantLinkStatus { /// Looking for a possible relevant link. This is the initial mode when /// matching a selector. @@ -429,6 +429,9 @@ fn matches_complex_selector_internal(mut selector_iter: SelectorIter Iterator for SelectorIter<'a, Impl> { } } +impl<'a, Impl: SelectorImpl> fmt::Debug for SelectorIter<'a, Impl> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let iter = self.iter.clone().rev(); + for component in iter { + component.to_css(f)? + } + Ok(()) + } +} + /// An iterator over all simple selectors belonging to ancestors. pub struct AncestorIter<'a, Impl: 'a + SelectorImpl>(SelectorIter<'a, Impl>); impl<'a, Impl: 'a + SelectorImpl> AncestorIter<'a, Impl> { diff --git a/components/selectors/tree.rs b/components/selectors/tree.rs index 6122f22490f..93b59dbb4c5 100644 --- a/components/selectors/tree.rs +++ b/components/selectors/tree.rs @@ -8,8 +8,9 @@ use attr::{AttrSelectorOperation, NamespaceConstraint}; use matching::{ElementSelectorFlags, MatchingContext, RelevantLinkStatus}; use parser::SelectorImpl; +use std::fmt::Debug; -pub trait Element: Sized { +pub trait Element: Sized + Debug { type Impl: SelectorImpl; fn parent_element(&self) -> Option; diff --git a/components/style/restyle_hints.rs b/components/style/restyle_hints.rs index 53e33608967..44eec13f13c 100644 --- a/components/style/restyle_hints.rs +++ b/components/style/restyle_hints.rs @@ -29,6 +29,7 @@ use smallvec::SmallVec; use std::cell::Cell; use std::clone::Clone; use std::cmp; +use std::fmt; /// When the ElementState of an element (like IN_HOVER_STATE) changes, /// certain pseudo-classes (like :hover) may require us to restyle that @@ -607,6 +608,15 @@ impl<'a, E> ElementWrapper<'a, E> } } +impl<'a, E> fmt::Debug for ElementWrapper<'a, E> + where E: TElement, +{ + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + // Ignore other fields for now, can change later if needed. + self.element.fmt(f) + } +} + #[cfg(feature = "gecko")] fn dir_selector_to_state(s: &[u16]) -> ElementState { // Jump through some hoops to deal with our Box<[u16]> thing. From 2c8866e919f864e989eeacf73b515fbb4325111d Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Tue, 6 Jun 2017 17:04:03 -0500 Subject: [PATCH 2/2] Stop looking for relevant links once found Once we've encountered a relevant link, stop looking for potential relevant link. A relevant link is only the nearest ancestor link, so it is incorrect to keep looking once one is found. This only matters for the somewhat unusual case of nested links. MozReview-Commit-ID: LiXsUGWfxg3 --- components/selectors/matching.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/components/selectors/matching.rs b/components/selectors/matching.rs index 84ddde2b261..0ab6c7e29e0 100644 --- a/components/selectors/matching.rs +++ b/components/selectors/matching.rs @@ -225,8 +225,10 @@ impl RelevantLinkStatus { -> RelevantLinkStatus where E: Element, { + // If a relevant link was previously found, we no longer want to look + // for links. Only the nearest ancestor link is considered relevant. if *self != RelevantLinkStatus::Looking { - return *self + return RelevantLinkStatus::NotLooking } if !element.is_link() { @@ -407,7 +409,7 @@ pub fn matches_complex_selector(complex_selector: &Selector, match matches_complex_selector_internal(iter, element, context, - RelevantLinkStatus::Looking, + &mut RelevantLinkStatus::Looking, flags_setter) { SelectorMatchingResult::Matched => true, _ => false @@ -417,13 +419,13 @@ pub fn matches_complex_selector(complex_selector: &Selector, fn matches_complex_selector_internal(mut selector_iter: SelectorIter, element: &E, context: &mut MatchingContext, - relevant_link: RelevantLinkStatus, + relevant_link: &mut RelevantLinkStatus, flags_setter: &mut F) -> SelectorMatchingResult where E: Element, F: FnMut(&E, ElementSelectorFlags), { - let mut relevant_link = relevant_link.examine_potential_link(element, context); + *relevant_link = relevant_link.examine_potential_link(element, context); let matches_all_simple_selectors = selector_iter.all(|simple| { matches_simple_selector(simple, element, context, &relevant_link, flags_setter) @@ -449,7 +451,7 @@ fn matches_complex_selector_internal(mut selector_iter: SelectorIter { // Only ancestor combinators are allowed while looking for // relevant links, so switch to not looking. - relevant_link = RelevantLinkStatus::NotLooking; + *relevant_link = RelevantLinkStatus::NotLooking; (element.prev_sibling_element(), SelectorMatchingResult::NotMatchedAndRestartFromClosestDescendant) }