style: Document the restyle hints code, and make it operate on TElement.

This removes the annoying constraint of having to provide the current state from
outside of the restyle hints code.

components/style/data.rs

@@ -295,10 +295,8 @@ impl RestyleData {
         }
 
         // Compute the hint.
-        let state = element.get_state();
         let mut hint = stylist.compute_restyle_hint(&element,
-                                                    self.snapshot.as_ref().unwrap(),
+                                                    self.snapshot.as_ref().unwrap());
-                                                    state);
 
         // If the hint includes a directive for later siblings, strip it out and
         // notify the caller to modify the base hint for future siblings.

components/style/restyle_hints.rs

@@ -4,13 +4,16 @@
 
 //! Restyle hints: an optimization to avoid unnecessarily matching selectors.
 
+#![deny(missing_docs)]
+
 use Atom;
+use dom::TElement;
 use element_state::*;
 #[cfg(feature = "gecko")]
 use gecko_bindings::structs::nsRestyleHint;
 #[cfg(feature = "servo")]
 use heapsize::HeapSizeOf;
-use selector_parser::{AttrValue, ElementExt, NonTSPseudoClass, Snapshot, SelectorImpl};
+use selector_parser::{AttrValue, NonTSPseudoClass, Snapshot, SelectorImpl};
 use selectors::{Element, MatchAttr};
 use selectors::matching::{MatchingReason, StyleRelations};
 use selectors::matching::matches_complex_selector;
@@ -18,13 +21,14 @@ use selectors::parser::{AttrSelector, Combinator, ComplexSelector, SimpleSelecto
 use std::clone::Clone;
 use std::sync::Arc;
 
-/// When the ElementState of an element (like IN_HOVER_STATE) changes, certain
-/// pseudo-classes (like :hover) may require us to restyle that element, its
-/// siblings, and/or its descendants. Similarly, when various attributes of an
-/// element change, we may also need to restyle things with id, class, and
-/// attribute selectors. Doing this conservatively is expensive, and so we use
-/// RestyleHints to short-circuit work we know is unnecessary.
 bitflags! {
+    /// When the ElementState of an element (like IN_HOVER_STATE) changes,
+    /// certain pseudo-classes (like :hover) may require us to restyle that
+    /// element, its siblings, and/or its descendants. Similarly, when various
+    /// attributes of an element change, we may also need to restyle things with
+    /// id, class, and attribute selectors. Doing this conservatively is
+    /// expensive, and so we use RestyleHints to short-circuit work we know is
+    /// unnecessary.
     pub flags RestyleHint: u32 {
         #[doc = "Rerun selector matching on the element."]
         const RESTYLE_SELF = 0x01,
@@ -99,26 +103,28 @@ pub trait ElementSnapshot : Sized + MatchAttr<Impl=SelectorImpl> {
 }
 
 struct ElementWrapper<'a, E>
-    where E: ElementExt
+    where E: TElement,
 {
     element: E,
     snapshot: Option<&'a Snapshot>,
 }
 
 impl<'a, E> ElementWrapper<'a, E>
-    where E: ElementExt
+    where E: TElement,
 {
+    /// Trivially constructs an `ElementWrapper` without a snapshot.
     pub fn new(el: E) -> ElementWrapper<'a, E> {
         ElementWrapper { element: el, snapshot: None }
     }
 
+    /// Trivially constructs an `ElementWrapper` with a snapshot.
     pub fn new_with_snapshot(el: E, snapshot: &'a Snapshot) -> ElementWrapper<'a, E> {
         ElementWrapper { element: el, snapshot: Some(snapshot) }
     }
 }
 
 impl<'a, E> MatchAttr for ElementWrapper<'a, E>
-    where E: ElementExt,
+    where E: TElement,
 {
     type Impl = SelectorImpl;
 
@@ -202,7 +208,7 @@ impl<'a, E> MatchAttr for ElementWrapper<'a, E>
 }
 
 impl<'a, E> Element for ElementWrapper<'a, E>
-    where E: ElementExt<Impl=SelectorImpl>
+    where E: TElement,
 {
     fn match_non_ts_pseudo_class(&self, pseudo_class: NonTSPseudoClass) -> bool {
         let flag = SelectorImpl::pseudo_class_state_flag(&pseudo_class);
@@ -393,6 +399,7 @@ impl DependencySet {
         }
     }
 
+    /// Create an empty `DependencySet`.
     pub fn new() -> Self {
         DependencySet {
             state_deps: vec![],
@@ -401,10 +408,13 @@ impl DependencySet {
         }
     }
 
+    /// Return the total number of dependencies that this set contains.
     pub fn len(&self) -> usize {
         self.common_deps.len() + self.attr_deps.len() + self.state_deps.len()
     }
 
+    /// Create the needed dependencies that a given selector creates, and add
+    /// them to the set.
     pub fn note_selector(&mut self, selector: &Arc<ComplexSelector<SelectorImpl>>) {
         let mut cur = selector;
         let mut combinator: Option<Combinator> = None;
@@ -434,18 +444,22 @@ impl DependencySet {
         }
     }
 
+    /// Clear this dependency set.
     pub fn clear(&mut self) {
         self.common_deps.clear();
         self.attr_deps.clear();
         self.state_deps.clear();
     }
 
-    pub fn compute_hint<E>(&self, el: &E,
+    /// Compute a restyle hint given an element and a snapshot, per the rules
-                           snapshot: &Snapshot,
+    /// explained in the rest of the documentation.
-                           current_state: ElementState)
+    pub fn compute_hint<E>(&self,
+                           el: &E,
+                           snapshot: &Snapshot)
                            -> RestyleHint
-        where E: ElementExt + Clone
+        where E: TElement + Clone,
     {
+        let current_state = el.get_state();
         let state_changes = snapshot.state()
                                     .map_or_else(ElementState::empty, |old_state| current_state ^ old_state);
         let attrs_changed = snapshot.has_attrs();
@@ -483,7 +497,7 @@ impl DependencySet {
                                state_changes: &ElementState,
                                attrs_changed: bool,
                                hint: &mut RestyleHint)
-        where E: ElementExt
+        where E: TElement,
     {
         if hint.is_all() {
             return;

components/style/selector_parser.rs

@@ -4,6 +4,8 @@
 
 //! The pseudo-classes and pseudo-elements supported by the style system.
 
+#![deny(missing_docs)]
+
 use cssparser::Parser as CssParser;
 use matching::{common_style_affecting_attributes, CommonStyleAffectingAttributeMode};
 use selectors::Element;
@@ -11,6 +13,8 @@ use selectors::parser::{AttrSelector, SelectorList};
 use std::fmt::Debug;
 use stylesheets::{Origin, Namespaces};
 
+/// A convenient alias for the type that represents an attribute value used for
+/// selector parser implementation.
 pub type AttrValue = <SelectorImpl as ::selectors::SelectorImpl>::AttrValue;
 
 #[cfg(feature = "servo")]
@@ -31,19 +35,30 @@ pub use servo::restyle_damage::ServoRestyleDamage as RestyleDamage;
 #[cfg(feature = "gecko")]
 pub use gecko::restyle_damage::GeckoRestyleDamage as RestyleDamage;
 
+/// A type that represents the previous computed values needed for restyle
+/// damage calculation.
 #[cfg(feature = "servo")]
 pub type PreExistingComputedValues = ::std::sync::Arc<::properties::ServoComputedValues>;
 
+/// A type that represents the previous computed values needed for restyle
+/// damage calculation.
 #[cfg(feature = "gecko")]
 pub type PreExistingComputedValues = ::gecko_bindings::structs::nsStyleContext;
 
+/// Servo's selector parser.
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub struct SelectorParser<'a> {
+    /// The origin of the stylesheet we're parsing.
     pub stylesheet_origin: Origin,
+    /// The namespace set of the stylesheet.
     pub namespaces: &'a Namespaces,
 }
 
 impl<'a> SelectorParser<'a> {
+    /// Parse a selector list with an author origin and without taking into
+    /// account namespaces.
+    ///
+    /// This is used for some DOM APIs like `querySelector`.
     pub fn parse_author_origin_no_namespace(input: &str)
                                             -> Result<SelectorList<SelectorImpl>, ()> {
         let namespaces = Namespaces::default();
@@ -54,68 +69,79 @@ impl<'a> SelectorParser<'a> {
         SelectorList::parse(&parser, &mut CssParser::new(input))
     }
 
+    /// Whether we're parsing selectors in a user-agent stylesheet.
     pub fn in_user_agent_stylesheet(&self) -> bool {
         matches!(self.stylesheet_origin, Origin::UserAgent)
     }
 }
 
-/// This function determines if a pseudo-element is eagerly cascaded or not.
+/// This enumeration determines if a pseudo-element is eagerly cascaded or not.
 ///
-/// Eagerly cascaded pseudo-elements are "normal" pseudo-elements (i.e.
+/// If you're implementing a public selector for `Servo` that the end-user might
-/// `::before` and `::after`). They inherit styles normally as another
+/// customize, then you probably need to make it eager.
-/// selector would do, and they're part of the cascade.
-///
-/// Lazy pseudo-elements are affected by selector matching, but they're only
-/// computed when needed, and not before. They're useful for general
-/// pseudo-elements that are not very common.
-///
-/// Note that in Servo lazy pseudo-elements are restricted to a subset of
-/// selectors, so you can't use it for public pseudo-elements. This is not the
-/// case with Gecko though.
-///
-/// Precomputed ones skip the cascade process entirely, mostly as an
-/// optimisation since they are private pseudo-elements (like
-/// `::-servo-details-content`).
-///
-/// This pseudo-elements are resolved on the fly using *only* global rules
-/// (rules of the form `*|*`), and applying them to the parent style.
-///
-/// If you're implementing a public selector that the end-user might customize,
-/// then you probably need to make it eager.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum PseudoElementCascadeType {
+    /// Eagerly cascaded pseudo-elements are "normal" pseudo-elements (i.e.
+    /// `::before` and `::after`). They inherit styles normally as another
+    /// selector would do, and they're computed as part of the cascade.
     Eager,
+    /// Lazy pseudo-elements are affected by selector matching, but they're only
+    /// computed when needed, and not before. They're useful for general
+    /// pseudo-elements that are not very common.
+    ///
+    /// Note that in Servo lazy pseudo-elements are restricted to a subset of
+    /// selectors, so you can't use it for public pseudo-elements. This is not
+    /// the case with Gecko though.
     Lazy,
+    /// Precomputed pseudo-elements skip the cascade process entirely, mostly as
+    /// an optimisation since they are private pseudo-elements (like
+    /// `::-servo-details-content`).
+    ///
+    /// This pseudo-elements are resolved on the fly using *only* global rules
+    /// (rules of the form `*|*`), and applying them to the parent style.
     Precomputed,
 }
 
 impl PseudoElementCascadeType {
+    /// Simple accessor to check whether the cascade type is eager.
     #[inline]
     pub fn is_eager(&self) -> bool {
         *self == PseudoElementCascadeType::Eager
     }
 
+    /// Simple accessor to check whether the cascade type is lazy.
     #[inline]
     pub fn is_lazy(&self) -> bool {
         *self == PseudoElementCascadeType::Lazy
     }
 
+    /// Simple accessor to check whether the cascade type is precomputed.
     #[inline]
     pub fn is_precomputed(&self) -> bool {
         *self == PseudoElementCascadeType::Precomputed
     }
 }
 
+/// An extension to rust-selector's `Element` trait.
 pub trait ElementExt: Element<Impl=SelectorImpl> + Debug {
+    /// Whether this element is a `link`.
     fn is_link(&self) -> bool;
 
+    /// Whether this element should match user and author rules.
+    ///
+    /// We use this for Native Anonymous Content in Gecko.
     fn matches_user_and_author_rules(&self) -> bool;
 }
 
 impl SelectorImpl {
+    /// A helper to traverse each eagerly cascaded pseudo-element, executing
+    /// `fun` on it.
+    ///
+    /// TODO(emilio): We can optimize this for Gecko using the pseudo-element
+    /// macro, and we should consider doing that for Servo too.
     #[inline]
     pub fn each_eagerly_cascaded_pseudo_element<F>(mut fun: F)
-        where F: FnMut(PseudoElement)
+        where F: FnMut(PseudoElement),
     {
         Self::each_pseudo_element(|pseudo| {
             if Self::pseudo_element_cascade_type(&pseudo).is_eager() {
@@ -124,9 +150,14 @@ impl SelectorImpl {
         })
     }
 
+    /// A helper to traverse each precomputed pseudo-element, executing `fun` on
+    /// it.
+    ///
+    /// The optimization comment in `each_eagerly_cascaded_pseudo_element` also
+    /// applies here.
     #[inline]
     pub fn each_precomputed_pseudo_element<F>(mut fun: F)
-        where F: FnMut(PseudoElement)
+        where F: FnMut(PseudoElement),
     {
         Self::each_pseudo_element(|pseudo| {
             if Self::pseudo_element_cascade_type(&pseudo).is_precomputed() {
@@ -136,6 +167,11 @@ impl SelectorImpl {
     }
 }
 
+/// Checks whether we can share style if an element matches a given
+/// attribute-selector that checks for existence (`[attr_name]`) easily.
+///
+/// We could do the same thing that we do for sibling rules and keep optimizing
+/// these common attributes, but we'd have to measure how common it is.
 pub fn attr_exists_selector_is_shareable(attr_selector: &AttrSelector<SelectorImpl>) -> bool {
     // NB(pcwalton): If you update this, remember to update the corresponding list in
     // `can_share_style_with()` as well.
@@ -147,6 +183,12 @@ pub fn attr_exists_selector_is_shareable(attr_selector: &AttrSelector<SelectorIm
     })
 }
 
+/// Checks whether we can share style if an element matches a given
+/// attribute-selector that checks for equality (`[attr_name="attr_value"]`)
+/// easily.
+///
+/// We could do the same thing that we do for sibling rules and keep optimizing
+/// these common attributes, but we'd have to measure how common it is.
 pub fn attr_equals_selector_is_shareable(attr_selector: &AttrSelector<SelectorImpl>,
                                          value: &AttrValue) -> bool {
     // FIXME(pcwalton): Remove once we start actually supporting RTL text. This is in

components/style/stylist.rs

@@ -8,8 +8,7 @@
 
 use {Atom, LocalName};
 use data::ComputedStyle;
-use dom::PresentationalHintsSynthetizer;
+use dom::{PresentationalHintsSynthetizer, TElement};
-use element_state::*;
 use error_reporting::StdoutErrorReporter;
 use keyframes::KeyframesAnimation;
 use media_queries::{Device, MediaType};
@@ -662,16 +661,11 @@ impl Stylist {
     /// restyle we need to do.
     pub fn compute_restyle_hint<E>(&self,
                                    element: &E,
-                                   snapshot: &Snapshot,
+                                   snapshot: &Snapshot)
-                                   // NB: We need to pass current_state as an argument because
-                                   // selectors::Element doesn't provide access to ElementState
-                                   // directly, and computing it from the ElementState would be
-                                   // more expensive than getting it directly from the caller.
-                                   current_state: ElementState)
                                    -> RestyleHint
-        where E: ElementExt + Clone,
+        where E: TElement,
     {
-        self.state_deps.compute_hint(element, snapshot, current_state)
+        self.state_deps.compute_hint(element, snapshot)
     }
 }