Auto merge of #14819 - emilio:no-missing-docs, r=Manishearth

Document most of the remaining parts of the style system, make docs-by-default for the whole style system

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14819)
<!-- Reviewable:end -->

components/layout_thread/lib.rs

@@ -1054,27 +1054,24 @@ impl LayoutThread {
         let device = Device::new(MediaType::Screen, initial_viewport);
         Arc::get_mut(&mut rw_data.stylist).unwrap().set_device(device, &data.document_stylesheets);
 
-        let constraints = rw_data.stylist.viewport_constraints().clone();
+        self.viewport_size =
-        self.viewport_size = match constraints {
+            rw_data.stylist.viewport_constraints().map_or(current_screen_size, |constraints| {
-            Some(ref constraints) => {
                 debug!("Viewport constraints: {:?}", constraints);
 
                 // other rules are evaluated against the actual viewport
                 Size2D::new(Au::from_f32_px(constraints.size.width),
                             Au::from_f32_px(constraints.size.height))
-            }
+            });
-            None => current_screen_size,
-        };
 
         // Handle conditions where the entire flow tree is invalid.
         let mut needs_dirtying = false;
 
         let viewport_size_changed = self.viewport_size != old_viewport_size;
         if viewport_size_changed {
-            if let Some(constraints) = constraints {
+            if let Some(constraints) = rw_data.stylist.viewport_constraints() {
                 // let the constellation know about the viewport constraints
                 rw_data.constellation_chan
-                       .send(ConstellationMsg::ViewportConstrained(self.id, constraints))
+                       .send(ConstellationMsg::ViewportConstrained(self.id, constraints.clone()))
                        .unwrap();
             }
             if data.document_stylesheets.iter().any(|sheet| sheet.dirty_on_viewport_size_change()) {

components/style/context.rs

@@ -3,6 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! The context within which style is calculated.
+#![deny(missing_docs)]
 
 use animation::Animation;
 use app_units::Au;
@@ -24,6 +25,7 @@ pub struct ThreadLocalStyleContextCreationInfo {
 }
 
 impl ThreadLocalStyleContextCreationInfo {
+    /// Trivially constructs a `ThreadLocalStyleContextCreationInfo`.
     pub fn new(animations_sender: Sender<Animation>) -> Self {
         ThreadLocalStyleContextCreationInfo {
             new_animations_sender: animations_sender,
@@ -31,14 +33,24 @@ impl ThreadLocalStyleContextCreationInfo {
     }
 }
 
+/// Which quirks mode is this document in.
+///
+/// See: https://quirks.spec.whatwg.org/
 #[derive(PartialEq, Eq, Copy, Clone, Hash, Debug)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub enum QuirksMode {
+    /// Quirks mode.
     Quirks,
+    /// Limited quirks mode.
     LimitedQuirks,
+    /// No quirks mode.
     NoQuirks,
 }
 
+/// A shared style context.
+///
+/// There's exactly one of these during a given restyle traversal, and it's
+/// shared among the worker threads.
 pub struct SharedStyleContext {
     /// The current viewport size.
     pub viewport_size: Size2D<Au>,
@@ -72,8 +84,15 @@ pub struct SharedStyleContext {
     pub quirks_mode: QuirksMode,
 }
 
+/// A thread-local style context.
+///
+/// This context contains data that needs to be used during restyling, but is
+/// not required to be unique among worker threads, so we create one per worker
+/// thread in order to be able to mutate it without locking.
 pub struct ThreadLocalStyleContext<E: TElement> {
+    /// A cache to share style among siblings.
     pub style_sharing_candidate_cache: StyleSharingCandidateCache<E>,
+    /// The bloom filter used to fast-reject selector-matching.
     pub bloom_filter: StyleBloom<E>,
     /// A channel on which new animations that have been triggered by style
     /// recalculation can be sent.
@@ -81,6 +100,7 @@ pub struct ThreadLocalStyleContext<E: TElement> {
 }
 
 impl<E: TElement> ThreadLocalStyleContext<E> {
+    /// Create a new `ThreadLocalStyleContext` from a shared one.
     pub fn new(shared: &SharedStyleContext) -> Self {
         ThreadLocalStyleContext {
             style_sharing_candidate_cache: StyleSharingCandidateCache::new(),
@@ -90,8 +110,12 @@ impl<E: TElement> ThreadLocalStyleContext<E> {
     }
 }
 
+/// A `StyleContext` is just a simple container for a immutable reference to a
+/// shared style context, and a mutable reference to a local one.
 pub struct StyleContext<'a, E: TElement + 'a> {
+    /// The shared style context reference.
     pub shared: &'a SharedStyleContext,
+    /// The thread-local style context (mutable) reference.
     pub thread_local: &'a mut ThreadLocalStyleContext<E>,
 }
 

components/style/custom_properties.rs

@@ -17,10 +17,14 @@ use std::fmt;
 use std::sync::Arc;
 use style_traits::ToCss;
 
-// Does not include the `--` prefix
+/// A custom property name is just an `Atom`.
+///
+/// Note that this does not include the `--` prefix
 pub type Name = Atom;
 
-// https://drafts.csswg.org/css-variables/#typedef-custom-property-name
+/// Parse a custom property name.
+///
+/// https://drafts.csswg.org/css-variables/#typedef-custom-property-name
 pub fn parse_name(s: &str) -> Result<&str, ()> {
     if s.starts_with("--") {
         Ok(&s[2..])
@@ -29,6 +33,10 @@ pub fn parse_name(s: &str) -> Result<&str, ()> {
     }
 }
 
+/// A specified value for a custom property is just a set of tokens.
+///
+/// We preserve the original CSS for serialization, and also the variable
+/// references to other custom property names.
 #[derive(Clone, PartialEq, Debug)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub struct SpecifiedValue {
@@ -47,6 +55,7 @@ impl ::values::HasViewportPercentage for SpecifiedValue {
     }
 }
 
+/// This struct is a cheap borrowed version of a `SpecifiedValue`.
 pub struct BorrowedSpecifiedValue<'a> {
     css: &'a str,
     first_token_type: TokenSerializationType,
@@ -54,6 +63,8 @@ pub struct BorrowedSpecifiedValue<'a> {
     references: Option<&'a HashSet<Name>>,
 }
 
+/// A computed value is just a set of tokens as well, until we resolve variables
+/// properly.
 #[derive(Clone, Debug)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub struct ComputedValue {
@@ -63,17 +74,23 @@ pub struct ComputedValue {
 }
 
 impl ToCss for SpecifiedValue {
-    fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
+    fn to_css<W>(&self, dest: &mut W) -> fmt::Result
+        where W: fmt::Write,
+    {
         dest.write_str(&self.css)
     }
 }
 
 impl ToCss for ComputedValue {
-    fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
+    fn to_css<W>(&self, dest: &mut W) -> fmt::Result
+        where W: fmt::Write,
+    {
         dest.write_str(&self.css)
     }
 }
 
+/// A map from CSS variable names to CSS variable computed values, used for
+/// resolving.
 pub type ComputedValuesMap = HashMap<Name, ComputedValue>;
 
 impl ComputedValue {
@@ -172,8 +189,8 @@ fn parse_declaration_value<'i, 't>
     })
 }
 
-/// Like parse_declaration_value,
+/// Like parse_declaration_value, but accept `!` and `;` since they are only
-/// but accept `!` and `;` since they are only invalid at the top level
+/// invalid at the top level
 fn parse_declaration_value_block(input: &mut Parser,
                                  references: &mut Option<HashSet<Name>>,
                                  missing_closing_characters: &mut String)
@@ -275,11 +292,10 @@ fn parse_declaration_value_block(input: &mut Parser,
         };
 
         token_start = input.position();
-        token = if let Ok(token) = input.next_including_whitespace_and_comments() {
+        token = match input.next_including_whitespace_and_comments() {
-            token
+            Ok(token) => token,
-        } else {
+            Err(..) => return Ok((first_token_type, last_token_type)),
-            return Ok((first_token_type, last_token_type))
+        };
-        }
     }
 }
 
@@ -306,51 +322,59 @@ fn parse_var_function<'i, 't>(input: &mut Parser<'i, 't>,
     Ok(())
 }
 
-/// Add one custom property declaration to a map,
+/// Add one custom property declaration to a map, unless another with the same
-/// unless another with the same name was already there.
+/// name was already there.
 pub fn cascade<'a>(custom_properties: &mut Option<HashMap<&'a Name, BorrowedSpecifiedValue<'a>>>,
                    inherited: &'a Option<Arc<HashMap<Name, ComputedValue>>>,
                    seen: &mut HashSet<&'a Name>,
                    name: &'a Name,
                    specified_value: &'a DeclaredValue<SpecifiedValue>) {
-    let was_not_already_present = seen.insert(name);
+    let was_already_present = !seen.insert(name);
-    if was_not_already_present {
+    if was_already_present {
-        let map = match *custom_properties {
+        return;
-            Some(ref mut map) => map,
+    }
-            None => {
+
-                *custom_properties = Some(match *inherited {
+    let map = match *custom_properties {
-                    Some(ref inherited) => inherited.iter().map(|(key, inherited_value)| {
+        Some(ref mut map) => map,
-                        (key, BorrowedSpecifiedValue {
+        None => {
-                            css: &inherited_value.css,
+            *custom_properties = Some(match *inherited {
-                            first_token_type: inherited_value.first_token_type,
+                Some(ref inherited) => inherited.iter().map(|(key, inherited_value)| {
-                            last_token_type: inherited_value.last_token_type,
+                    (key, BorrowedSpecifiedValue {
-                            references: None
+                        css: &inherited_value.css,
-                        })
+                        first_token_type: inherited_value.first_token_type,
-                    }).collect(),
+                        last_token_type: inherited_value.last_token_type,
-                    None => HashMap::new(),
+                        references: None
-                });
+                    })
-                custom_properties.as_mut().unwrap()
+                }).collect(),
-            }
+                None => HashMap::new(),
-        };
+            });
-        match *specified_value {
+            custom_properties.as_mut().unwrap()
-            DeclaredValue::Value(ref specified_value) => {
-                map.insert(name, BorrowedSpecifiedValue {
-                    css: &specified_value.css,
-                    first_token_type: specified_value.first_token_type,
-                    last_token_type: specified_value.last_token_type,
-                    references: Some(&specified_value.references),
-                });
-            },
-            DeclaredValue::WithVariables { .. } => unreachable!(),
-            DeclaredValue::Initial => {
-                map.remove(&name);
-            }
-            DeclaredValue::Unset | // Custom properties are inherited by default.
-            DeclaredValue::Inherit => {}  // The inherited value is what we already have.
         }
+    };
+    match *specified_value {
+        DeclaredValue::Value(ref specified_value) => {
+            map.insert(name, BorrowedSpecifiedValue {
+                css: &specified_value.css,
+                first_token_type: specified_value.first_token_type,
+                last_token_type: specified_value.last_token_type,
+                references: Some(&specified_value.references),
+            });
+        },
+        DeclaredValue::WithVariables { .. } => unreachable!(),
+        DeclaredValue::Initial => {
+            map.remove(&name);
+        }
+        DeclaredValue::Unset | // Custom properties are inherited by default.
+        DeclaredValue::Inherit => {}  // The inherited value is what we already have.
     }
 }
 
+/// Returns the final map of applicable custom properties.
+///
+/// If there was any specified property, we've created a new map and now we need
+/// to remove any potential cycles, and wrap it in an arc.
+///
+/// Otherwise, just use the inherited custom properties map.
 pub fn finish_cascade(specified_values_map: Option<HashMap<&Name, BorrowedSpecifiedValue>>,
                       inherited: &Option<Arc<HashMap<Name, ComputedValue>>>)
                       -> Option<Arc<HashMap<Name, ComputedValue>>> {
@@ -363,7 +387,9 @@ pub fn finish_cascade(specified_values_map: Option<HashMap<&Name, BorrowedSpecif
 }
 
 /// https://drafts.csswg.org/css-variables/#cycles
-/// The initial value of a custom property is represented by this property not being in the map.
+///
+/// The initial value of a custom property is represented by this property not
+/// being in the map.
 fn remove_cycles(map: &mut HashMap<&Name, BorrowedSpecifiedValue>) {
     let mut to_remove = HashSet::new();
     {
@@ -514,10 +540,9 @@ fn substitute_block<F>(input: &mut Parser,
             });
             set_position_at_next_iteration = false;
         }
-        let token = if let Ok(token) = next {
+        let token = match next {
-            token
+            Ok(token) => token,
-        } else {
+            Err(..) => break,
-            break
         };
         match token {
             Token::Function(ref name) if name.eq_ignore_ascii_case("var") => {

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/font_face.rs

@@ -6,6 +6,8 @@
 //!
 //! [ff]: https://drafts.csswg.org/css-fonts/#at-font-face-rule
 
+#![deny(missing_docs)]
+
 use computed_values::font_family::FontFamily;
 use cssparser::{AtRuleParser, DeclarationListParser, DeclarationParser, Parser};
 use parser::{ParserContext, log_css_error, Parse};
@@ -15,15 +17,20 @@ use std::iter;
 use style_traits::ToCss;
 use values::specified::url::SpecifiedUrl;
 
+/// A source for a font-face rule.
 #[derive(Clone, Debug, PartialEq, Eq)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf, Deserialize, Serialize))]
 pub enum Source {
+    /// A `url()` source.
     Url(UrlSource),
+    /// A `local()` source.
     Local(FontFamily),
 }
 
 impl ToCss for Source {
-    fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
+    fn to_css<W>(&self, dest: &mut W) -> fmt::Result
+        where W: fmt::Write,
+    {
         match *self {
             Source::Url(ref url) => {
                 try!(dest.write_str("url(\""));
@@ -38,29 +45,45 @@ impl ToCss for Source {
     }
 }
 
+/// A `UrlSource` represents a font-face source that has been specified with a
+/// `url()` function.
+///
+/// https://drafts.csswg.org/css-fonts/#src-desc
 #[derive(Clone, Debug, PartialEq, Eq)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf, Deserialize, Serialize))]
 pub struct UrlSource {
+    /// The specified url.
     pub url: SpecifiedUrl,
+    /// The format hints specified with the `format()` function.
     pub format_hints: Vec<String>,
 }
 
 impl ToCss for UrlSource {
-    fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
+    fn to_css<W>(&self, dest: &mut W) -> fmt::Result
+        where W: fmt::Write,
+    {
         dest.write_str(self.url.as_str())
     }
 }
 
+/// A `@font-face` rule.
+///
+/// https://drafts.csswg.org/css-fonts/#font-face-rule
 #[derive(Debug, PartialEq, Eq)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub struct FontFaceRule {
+    /// The font family specified with the `font-family` property declaration.
     pub family: FontFamily,
+    /// The list of sources specified with the different `src` property
+    /// declarations.
     pub sources: Vec<Source>,
 }
 
 impl ToCss for FontFaceRule {
     // Serialization of FontFaceRule is not specced.
-    fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
+    fn to_css<W>(&self, dest: &mut W) -> fmt::Result
+        where W: fmt::Write,
+    {
         try!(dest.write_str("@font-face { font-family: "));
         try!(self.family.to_css(dest));
         try!(dest.write_str(";"));
@@ -80,6 +103,9 @@ impl ToCss for FontFaceRule {
     }
 }
 
+/// Parse the block inside a `@font-face` rule.
+///
+/// Note that the prelude parsing code lives in the `stylesheets` module.
 pub fn parse_font_face_block(context: &ParserContext, input: &mut Parser)
                              -> Result<FontFaceRule, ()> {
     let mut family = None;
@@ -112,6 +138,7 @@ pub fn parse_font_face_block(context: &ParserContext, input: &mut Parser)
     }
 }
 
+/// A list of effective sources that we send over through IPC to the font cache.
 #[derive(Clone, Debug)]
 #[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
 pub struct EffectiveSources(Vec<Source>);

components/style/gecko/conversions.rs

@@ -9,7 +9,7 @@
 #![allow(unsafe_code)]
 
 use app_units::Au;
-use gecko::values::{convert_rgba_to_nscolor, StyleCoordHelpers};
+use gecko::values::convert_rgba_to_nscolor;
 use gecko_bindings::bindings::{Gecko_CreateGradient, Gecko_SetGradientImageValue, Gecko_SetUrlImageValue};
 use gecko_bindings::bindings::{RawServoStyleSheet, RawServoDeclarationBlock, RawServoStyleRule, RawServoImportRule};
 use gecko_bindings::bindings::{ServoComputedValues, ServoCssRules};
@@ -99,6 +99,7 @@ impl From<LengthOrPercentage> for nsStyleCoord_CalcValue {
 }
 
 impl LengthOrPercentageOrAuto {
+    /// Convert this value in an appropriate `nsStyleCoord::CalcValue`.
     pub fn to_calc_value(&self) -> Option<nsStyleCoord_CalcValue> {
         match *self {
             LengthOrPercentageOrAuto::Length(au) => {
@@ -132,6 +133,7 @@ impl From<nsStyleCoord_CalcValue> for LengthOrPercentage {
 }
 
 impl nsStyleImage {
+    /// Set a given Servo `Image` value into this `nsStyleImage`.
     pub fn set(&mut self, image: Image, with_url: bool, cacheable: &mut bool) {
         match image {
             Image::Gradient(gradient) => {
@@ -154,6 +156,9 @@ impl nsStyleImage {
                 // the applicable declarations cache is not per document, but
                 // global, and the imgRequestProxy objects we store in the style
                 // structs don't like to be tracked by more than one document.
+                //
+                // FIXME(emilio): With the scoped TLS thing this is no longer
+                // true, remove this line in a follow-up!
                 *cacheable = false;
             },
             _ => (),
@@ -326,6 +331,8 @@ impl nsStyleImage {
 }
 
 pub mod basic_shape {
+    //! Conversions from and to CSS shape representations.
+
     use euclid::size::Size2D;
     use gecko::values::GeckoStyleCoordConvertible;
     use gecko_bindings::structs;
@@ -418,6 +425,7 @@ pub mod basic_shape {
     // Can't be a From impl since we need to set an existing
     // nsStyleCorners, not create a new one
     impl BorderRadius {
+        /// Set this `BorderRadius` into a given `nsStyleCoord`.
         pub fn set_corners(&self, other: &mut nsStyleCorners) {
             let mut set_corner = |field: &BorderRadiusSize, index| {
                 field.0.width.to_gecko_style_coord(&mut other.data_at_mut(index));

components/style/gecko/data.rs

@@ -2,6 +2,8 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+//! Data needed to style a Gecko document.
+
 use animation::Animation;
 use atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut};
 use dom::OpaqueNode;
@@ -21,6 +23,8 @@ use style_traits::ViewportPx;
 use stylesheets::Stylesheet;
 use stylist::Stylist;
 
+/// The container for data that a Servo-backed Gecko document needs to style
+/// itself.
 pub struct PerDocumentStyleDataImpl {
     /// Rule processor.
     pub stylist: Arc<Stylist>,
@@ -32,20 +36,33 @@ pub struct PerDocumentStyleDataImpl {
     pub stylesheets_changed: bool,
 
     // FIXME(bholley): Hook these up to something.
+    /// Unused. Will go away when we actually implement transitions and
+    /// animations properly.
     pub new_animations_sender: Sender<Animation>,
+    /// Unused. Will go away when we actually implement transitions and
+    /// animations properly.
     pub new_animations_receiver: Receiver<Animation>,
+    /// Unused. Will go away when we actually implement transitions and
+    /// animations properly.
     pub running_animations: Arc<RwLock<HashMap<OpaqueNode, Vec<Animation>>>>,
+    /// Unused. Will go away when we actually implement transitions and
+    /// animations properly.
     pub expired_animations: Arc<RwLock<HashMap<OpaqueNode, Vec<Animation>>>>,
 
-    // FIXME(bholley): This shouldn't be per-document.
+    /// The worker thread pool.
+    /// FIXME(bholley): This shouldn't be per-document.
     pub work_queue: Option<rayon::ThreadPool>,
 
+    /// The number of threads of the work queue.
     pub num_threads: usize,
 }
 
+/// The data itself is an `AtomicRefCell`, which guarantees the proper semantics
+/// and unexpected races while trying to mutate it.
 pub struct PerDocumentStyleData(AtomicRefCell<PerDocumentStyleDataImpl>);
 
 lazy_static! {
+    /// The number of layout threads, computed statically.
     pub static ref NUM_THREADS: usize = {
         match env::var("STYLO_THREADS").map(|s| s.parse::<usize>().expect("invalid STYLO_THREADS")) {
             Ok(num) => num,
@@ -55,6 +72,7 @@ lazy_static! {
 }
 
 impl PerDocumentStyleData {
+    /// Create a dummy `PerDocumentStyleData`.
     pub fn new() -> Self {
         // FIXME(bholley): Real window size.
         let window_size: TypedSize2D<f32, ViewportPx> = TypedSize2D::new(800.0, 600.0);
@@ -81,16 +99,19 @@ impl PerDocumentStyleData {
         }))
     }
 
+    /// Get an immutable reference to this style data.
     pub fn borrow(&self) -> AtomicRef<PerDocumentStyleDataImpl> {
         self.0.borrow()
     }
 
+    /// Get an mutable reference to this style data.
     pub fn borrow_mut(&self) -> AtomicRefMut<PerDocumentStyleDataImpl> {
         self.0.borrow_mut()
     }
 }
 
 impl PerDocumentStyleDataImpl {
+    /// Recreate the style data if the stylesheets have changed.
     pub fn flush_stylesheets(&mut self) {
         // The stylist wants to be flushed if either the stylesheets change or the
         // device dimensions change. When we add support for media queries, we'll

components/style/gecko/mod.rs

@@ -2,6 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+//! Gecko-specific style-system bits.
 
 pub mod data;
 pub mod restyle_damage;

components/style/gecko/restyle_damage.rs

@@ -2,6 +2,8 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+//! Gecko's restyle damage computation (aka change hints, aka `nsChangeHint`).
+
 use gecko_bindings::bindings;
 use gecko_bindings::structs;
 use gecko_bindings::structs::{nsChangeHint, nsStyleContext};
@@ -10,28 +12,42 @@ use properties::ComputedValues;
 use std::ops::{BitOr, BitOrAssign};
 use std::sync::Arc;
 
+/// The representation of Gecko's restyle damage is just a wrapper over
+/// `nsChangeHint`.
 #[derive(Clone, Copy, Debug, PartialEq)]
 pub struct GeckoRestyleDamage(nsChangeHint);
 
 impl GeckoRestyleDamage {
+    /// Trivially construct a new `GeckoRestyleDamage`.
     pub fn new(raw: nsChangeHint) -> Self {
         GeckoRestyleDamage(raw)
     }
 
+    /// Get the inner change hint for this damage.
     pub fn as_change_hint(&self) -> nsChangeHint {
         self.0
     }
 
+    /// Get an empty change hint, that is (`nsChangeHint(0)`).
     pub fn empty() -> Self {
         GeckoRestyleDamage(nsChangeHint(0))
     }
 
+    /// Returns whether this restyle damage represents the empty damage.
     pub fn is_empty(&self) -> bool {
         self.0 == nsChangeHint(0)
     }
 
+    /// Computes a change hint given an old style (in the form of a
+    /// `nsStyleContext`, and a new style (in the form of `ComputedValues`).
+    ///
+    /// Note that we could in theory just get two `ComputedValues` here and diff
+    /// them, but Gecko has an interesting optimization when they mark accessed
+    /// structs, so they effectively only diff structs that have ever been
+    /// accessed from layout.
     pub fn compute(source: &nsStyleContext,
                    new_style: &Arc<ComputedValues>) -> Self {
+        // TODO(emilio): Const-ify this?
         let context = source as *const nsStyleContext as *mut nsStyleContext;
         let hint = unsafe {
             bindings::Gecko_CalcStyleDifference(context,
@@ -40,6 +56,8 @@ impl GeckoRestyleDamage {
         GeckoRestyleDamage(hint)
     }
 
+    /// Get a restyle damage that represents the maximum action to be taken
+    /// (rebuild and reflow).
     pub fn rebuild_and_reflow() -> Self {
         GeckoRestyleDamage(structs::nsChangeHint_nsChangeHint_ReconstructFrame)
     }

components/style/gecko/selector_parser.rs

@@ -2,6 +2,8 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+//! Gecko-specific bits for selector-parsing.
+
 use cssparser::ToCss;
 use element_state::ElementState;
 use selector_parser::{SelectorParser, PseudoElementCascadeType};
@@ -11,11 +13,16 @@ use std::borrow::Cow;
 use std::fmt;
 use string_cache::{Atom, Namespace, WeakAtom, WeakNamespace};
 
-/// NOTE: The boolean field represents whether this element is an anonymous box.
+/// A representation of a CSS pseudo-element.
 ///
-/// This is just for convenience, instead of recomputing it. Also, note that
+/// In Gecko, we represent pseudo-elements as plain `Atom`s.
-/// Atom is always a static atom, so if space is a concern, we can use the
+///
-/// raw pointer and use the lower bit to represent it without space overhead.
+/// The boolean field represents whether this element is an anonymous box. This
+/// is just for convenience, instead of recomputing it.
+///
+/// Also, note that the `Atom` member is always a static atom, so if space is a
+/// concern, we can use the raw pointer and use the lower bit to represent it
+/// without space overhead.
 ///
 /// FIXME(emilio): we know all these atoms are static. Patches are starting to
 /// pile up, but a further potential optimisation is generating bindings without
@@ -32,16 +39,22 @@ use string_cache::{Atom, Namespace, WeakAtom, WeakNamespace};
 pub struct PseudoElement(Atom, bool);
 
 impl PseudoElement {
+    /// Get the pseudo-element as an atom.
     #[inline]
     pub fn as_atom(&self) -> &Atom {
         &self.0
     }
 
+    /// Whether this pseudo-element is an anonymous box.
     #[inline]
     fn is_anon_box(&self) -> bool {
         self.1
     }
 
+    /// Construct a pseudo-element from an `Atom`, receiving whether it is also
+    /// an anonymous box, and don't check it on release builds.
+    ///
+    /// On debug builds we assert it's the result we expect.
     #[inline]
     pub fn from_atom_unchecked(atom: Atom, is_anon_box: bool) -> Self {
         if cfg!(debug_assertions) {
@@ -73,6 +86,13 @@ impl PseudoElement {
         None
     }
 
+    /// Constructs an atom from a string of text, and whether we're in a
+    /// user-agent stylesheet.
+    ///
+    /// If we're not in a user-agent stylesheet, we will never parse anonymous
+    /// box pseudo-elements.
+    ///
+    /// Returns `None` if the pseudo-element is not recognised.
     #[inline]
     fn from_slice(s: &str, in_ua_stylesheet: bool) -> Option<Self> {
         use std::ascii::AsciiExt;
@@ -102,20 +122,36 @@ impl ToCss for PseudoElement {
     }
 }
 
+/// Our representation of a non tree-structural pseudo-class.
+///
+/// FIXME(emilio): Find a way to autogenerate this.
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
 pub enum NonTSPseudoClass {
+    /// :any-link
     AnyLink,
+    /// :link
     Link,
+    /// :visited
     Visited,
+    /// :active
     Active,
+    /// :focus
     Focus,
+    /// :fullscreen
     Fullscreen,
+    /// :hover
     Hover,
+    /// :enabled
     Enabled,
+    /// :disabled
     Disabled,
+    /// :checked
     Checked,
+    /// :indeterminate
     Indeterminate,
+    /// :read-write
     ReadWrite,
+    /// :read-only
     ReadOnly,
 }
 
@@ -141,6 +177,7 @@ impl ToCss for NonTSPseudoClass {
 }
 
 impl NonTSPseudoClass {
+    /// Get the state flag associated with a pseudo-class, if any.
     pub fn state_flag(&self) -> ElementState {
         use element_state::*;
         use self::NonTSPseudoClass::*;
@@ -162,6 +199,7 @@ impl NonTSPseudoClass {
     }
 }
 
+/// The dummy struct we use to implement our selector parsing.
 #[derive(Clone, Debug, PartialEq)]
 pub struct SelectorImpl;
 
@@ -231,6 +269,13 @@ impl<'a> ::selectors::Parser for SelectorParser<'a> {
 
 impl SelectorImpl {
     #[inline]
+    /// Returns the kind of cascade type that a given pseudo is going to use.
+    ///
+    /// In Gecko we only compute ::before and ::after eagerly. We save the rules
+    /// for anonymous boxes separately, so we resolve them as precomputed
+    /// pseudos.
+    ///
+    /// We resolve the others lazily, see `Servo_ResolvePseudoStyle`.
     pub fn pseudo_element_cascade_type(pseudo: &PseudoElement) -> PseudoElementCascadeType {
         if Self::pseudo_is_before_or_after(pseudo) {
             return PseudoElementCascadeType::Eager
@@ -244,8 +289,9 @@ impl SelectorImpl {
     }
 
     #[inline]
+    /// Executes a function for each pseudo-element.
     pub fn each_pseudo_element<F>(mut fun: F)
-        where F: FnMut(PseudoElement)
+        where F: FnMut(PseudoElement),
     {
         macro_rules! pseudo_element {
             ($pseudo_str_with_colon:expr, $atom:expr, $is_anon_box:expr) => {{
@@ -257,12 +303,15 @@ impl SelectorImpl {
     }
 
     #[inline]
+    /// Returns whether the given pseudo-element is `::before` or `::after`.
     pub fn pseudo_is_before_or_after(pseudo: &PseudoElement) -> bool {
         *pseudo.as_atom() == atom!(":before") ||
         *pseudo.as_atom() == atom!(":after")
     }
 
     #[inline]
+    /// Returns the relevant state flag for a given non-tree-structural
+    /// pseudo-class.
     pub fn pseudo_class_state_flag(pc: &NonTSPseudoClass) -> ElementState {
         pc.state_flag()
     }

components/style/gecko/snapshot.rs

@@ -2,6 +2,9 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+//! A gecko snapshot, that stores the element attributes and state before they
+//! change in order to properly calculate restyle hints.
+
 use element_state::ElementState;
 use gecko::snapshot_helpers;
 use gecko::wrapper::{AttrSelectorHelpers, GeckoElement};
@@ -14,6 +17,10 @@ use selectors::parser::AttrSelector;
 use std::ptr;
 use string_cache::Atom;
 
+/// A snapshot of a Gecko element.
+///
+/// This is really a Gecko type (see `ServoElementSnapshot.h` in Gecko) we wrap
+/// here.
 #[derive(Debug)]
 pub struct GeckoElementSnapshot(bindings::ServoElementSnapshotOwned);
 
@@ -30,14 +37,17 @@ impl Drop for GeckoElementSnapshot {
 }
 
 impl GeckoElementSnapshot {
+    /// Create a new snapshot of the given element.
     pub fn new<'le>(el: GeckoElement<'le>) -> Self {
         unsafe { GeckoElementSnapshot(bindings::Gecko_CreateElementSnapshot(el.0)) }
     }
 
+    /// Get a mutable reference to the snapshot.
     pub fn borrow_mut_raw(&mut self) -> bindings::ServoElementSnapshotBorrowedMut {
         &mut *self.0
     }
 
+    /// Get the pointer to the actual snapshot.
     pub fn ptr(&self) -> *const ServoElementSnapshot {
         &*self.0
     }
@@ -152,7 +162,6 @@ impl ElementSnapshot for GeckoElementSnapshot {
         }
     }
 
-    // TODO: share logic with Element::{has_class, each_class}?
     fn has_class(&self, name: &Atom) -> bool {
         snapshot_helpers::has_class(self.ptr(),
                                     name,

components/style/gecko/snapshot_helpers.rs

@@ -8,8 +8,12 @@ use gecko_bindings::structs::nsIAtom;
 use std::{ptr, slice};
 use string_cache::Atom;
 
+/// A function that, given an element of type `T`, allows you to get a single
+/// class or a class list.
 pub type ClassOrClassList<T> = unsafe extern fn (T, *mut *mut nsIAtom, *mut *mut *mut nsIAtom) -> u32;
 
+/// Given an item `T`, a class name, and a getter function, return whether that
+/// element has the class that `name` represents.
 pub fn has_class<T>(item: T,
                     name: &Atom,
                     getter: ClassOrClassList<T>) -> bool
@@ -30,6 +34,8 @@ pub fn has_class<T>(item: T,
 }
 
 
+/// Given an item, a callback, and a getter, execute `callback` for each class
+/// this `item` has.
 pub fn each_class<F, T>(item: T,
                         mut callback: F,
                         getter: ClassOrClassList<T>)

components/style/gecko/traversal.rs

@@ -2,6 +2,8 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+//! Gecko-specific bits for the styling DOM traversal.
+
 use atomic_refcell::AtomicRefCell;
 use context::{SharedStyleContext, StyleContext, ThreadLocalStyleContext};
 use data::ElementData;
@@ -9,11 +11,14 @@ use dom::{NodeInfo, TNode};
 use gecko::wrapper::{GeckoElement, GeckoNode};
 use traversal::{DomTraversal, PerLevelTraversalData, recalc_style_at};
 
+/// This is the simple struct that Gecko uses to encapsulate a DOM traversal for
+/// styling.
 pub struct RecalcStyleOnly {
     shared: SharedStyleContext,
 }
 
 impl RecalcStyleOnly {
+    /// Create a `RecalcStyleOnly` traversal from a `SharedStyleContext`.
     pub fn new(shared: SharedStyleContext) -> Self {
         RecalcStyleOnly {
             shared: shared,

components/style/gecko/values.rs

@@ -4,6 +4,8 @@
 
 #![allow(unsafe_code)]
 
+//! Different kind of helpers to interact with Gecko values.
+
 use app_units::Au;
 use cssparser::RGBA;
 use gecko_bindings::structs::{nsStyleCoord, StyleShapeRadius};
@@ -14,22 +16,22 @@ use values::computed::{Angle, LengthOrPercentageOrNone, Number};
 use values::computed::{LengthOrPercentage, LengthOrPercentageOrAuto};
 use values::computed::basic_shape::ShapeRadius;
 
-pub trait StyleCoordHelpers {
+/// A trait that defines an interface to convert from and to `nsStyleCoord`s.
-    fn set<T: GeckoStyleCoordConvertible>(&mut self, val: T);
+pub trait GeckoStyleCoordConvertible : Sized {
+    /// Convert this to a `nsStyleCoord`.
+    fn to_gecko_style_coord<T: CoordDataMut>(&self, coord: &mut T);
+    /// Given a `nsStyleCoord`, try to get a value of this type..
+    fn from_gecko_style_coord<T: CoordData>(coord: &T) -> Option<Self>;
 }
 
-impl StyleCoordHelpers for nsStyleCoord {
+impl nsStyleCoord {
     #[inline]
-    fn set<T: GeckoStyleCoordConvertible>(&mut self, val: T) {
+    /// Set this `nsStyleCoord` value to `val`.
+    pub fn set<T: GeckoStyleCoordConvertible>(&mut self, val: T) {
         val.to_gecko_style_coord(self);
     }
 }
 
-pub trait GeckoStyleCoordConvertible : Sized {
-    fn to_gecko_style_coord<T: CoordDataMut>(&self, coord: &mut T);
-    fn from_gecko_style_coord<T: CoordData>(coord: &T) -> Option<Self>;
-}
-
 impl<A: GeckoStyleCoordConvertible, B: GeckoStyleCoordConvertible> GeckoStyleCoordConvertible for Either<A, B> {
     fn to_gecko_style_coord<T: CoordDataMut>(&self, coord: &mut T) {
         match *self {
@@ -221,6 +223,7 @@ impl GeckoStyleCoordConvertible for None_ {
     }
 }
 
+/// Convert a given RGBA value to `nscolor`.
 pub fn convert_rgba_to_nscolor(rgba: &RGBA) -> u32 {
     (((rgba.alpha * 255.0).round() as u32) << 24) |
     (((rgba.blue  * 255.0).round() as u32) << 16) |
@@ -228,6 +231,7 @@ pub fn convert_rgba_to_nscolor(rgba: &RGBA) -> u32 {
      ((rgba.red   * 255.0).round() as u32)
 }
 
+/// Convert a given `nscolor` to a Servo RGBA value.
 pub fn convert_nscolor_to_rgba(color: u32) -> RGBA {
     RGBA {
         red:    ((color        & 0xff) as f32) / 255.0,
@@ -237,11 +241,11 @@ pub fn convert_nscolor_to_rgba(color: u32) -> RGBA {
     }
 }
 
+/// Round `width` down to the nearest device pixel, but any non-zero value that
+/// would round down to zero is clamped to 1 device pixel.  Used for storing
+/// computed values of border-*-width and outline-width.
 #[inline]
 pub fn round_border_to_device_pixels(width: Au, au_per_device_px: Au) -> Au {
-    // Round width down to the nearest device pixel, but any non-zero value that
-    // would round down to zero is clamped to 1 device pixel.  Used for storing
-    // computed values of border-*-width and outline-width.
     if width == Au(0) {
         Au(0)
     } else {

components/style/gecko/wrapper.rs

@@ -4,6 +4,15 @@
 
 #![allow(unsafe_code)]
 
+//! Wrapper definitions on top of Gecko types in order to be used in the style
+//! system.
+//!
+//! This really follows the Servo pattern in
+//! `components/script/layout_wrapper.rs`.
+//!
+//! This theoretically should live in its own crate, but now it lives in the
+//! style system it's kind of pointless in the Stylo case, and only Servo forces
+//! the separation between the style system implementation and everything else.
 
 use atomic_refcell::AtomicRefCell;
 use data::ElementData;
@@ -42,11 +51,14 @@ use std::sync::Arc;
 use string_cache::{Atom, Namespace, WeakAtom, WeakNamespace};
 use stylist::ApplicableDeclarationBlock;
 
-// Important: We don't currently refcount the DOM, because the wrapper lifetime
+/// A simple wrapper over a non-null Gecko node (`nsINode`) pointer.
-// magic guarantees that our LayoutFoo references won't outlive the root, and
+///
-// we don't mutate any of the references on the Gecko side during restyle. We
+/// Important: We don't currently refcount the DOM, because the wrapper lifetime
-// could implement refcounting if need be (at a potentially non-trivial
+/// magic guarantees that our LayoutFoo references won't outlive the root, and
-// performance cost) by implementing Drop and making LayoutFoo non-Copy.
+/// we don't mutate any of the references on the Gecko side during restyle.
+///
+/// We could implement refcounting if need be (at a potentially non-trivial
+/// performance cost) by implementing Drop and making LayoutFoo non-Copy.
 #[derive(Clone, Copy)]
 pub struct GeckoNode<'ln>(pub &'ln RawGeckoNode);
 
@@ -173,11 +185,23 @@ impl<'ln> TNode for GeckoNode<'ln> {
     unsafe fn set_dirty_on_viewport_size_changed(&self) {}
 }
 
-// We generally iterate children by traversing the siblings of the first child
+/// A wrapper on top of two kind of iterators, depending on the parent being
-// like Servo does. However, for nodes with anonymous children, we use a custom
+/// iterated.
-// (heavier-weight) Gecko-implemented iterator.
+///
+/// We generally iterate children by traversing the light-tree siblings of the
+/// first child like Servo does.
+///
+/// However, for nodes with anonymous children, we use a custom (heavier-weight)
+/// Gecko-implemented iterator.
+///
+/// FIXME(emilio): If we take into account shadow DOM, we're going to need the
+/// flat tree pretty much always. We can try to optimize the case where there's
+/// no shadow root sibling, probably.
 pub enum GeckoChildrenIterator<'a> {
+    /// A simple iterator that tracks the current node being iterated and
+    /// replaces it with the next sibling when requested.
     Current(Option<GeckoNode<'a>>),
+    /// A Gecko-implemented iterator we need to drop appropriately.
     GeckoIterator(bindings::StyleChildrenIteratorOwned),
 }
 
@@ -207,6 +231,7 @@ impl<'a> Iterator for GeckoChildrenIterator<'a> {
     }
 }
 
+/// A simple wrapper over a non-null Gecko `Element` pointer.
 #[derive(Clone, Copy)]
 pub struct GeckoElement<'le>(pub &'le RawGeckoElement);
 
@@ -221,6 +246,7 @@ impl<'le> fmt::Debug for GeckoElement<'le> {
 }
 
 impl<'le> GeckoElement<'le> {
+    /// Parse the style attribute of an element.
     pub fn parse_style_attribute(value: &str) -> PropertyDeclarationBlock {
         // FIXME(bholley): Real base URL and error reporter.
         let base_url = &*DUMMY_BASE_URL;
@@ -250,6 +276,7 @@ impl<'le> GeckoElement<'le> {
         unsafe { Gecko_UnsetNodeFlags(self.as_node().0, flags) }
     }
 
+    /// Clear the element data for a given element.
     pub fn clear_data(&self) {
         let ptr = self.0.mServoData.get();
         if !ptr.is_null() {
@@ -264,7 +291,11 @@ impl<'le> GeckoElement<'le> {
         }
     }
 
-    // Only safe to call with exclusive access to the element.
+    /// Ensures the element has data, returning the existing data or allocating
+    /// it.
+    ///
+    /// Only safe to call with exclusive access to the element, given otherwise
+    /// it could race to allocate and leak.
     pub unsafe fn ensure_data(&self) -> &AtomicRefCell<ElementData> {
         match self.get_data() {
             Some(x) => x,
@@ -284,6 +315,9 @@ impl<'le> GeckoElement<'le> {
 }
 
 lazy_static! {
+    /// A dummy base url in order to get it where we don't have any available.
+    ///
+    /// We need to get rid of this sooner than later.
     pub static ref DUMMY_BASE_URL: ServoUrl = {
         ServoUrl::parse("http://www.example.org").unwrap()
     };
@@ -387,7 +421,7 @@ impl<'le> PartialEq for GeckoElement<'le> {
 
 impl<'le> PresentationalHintsSynthetizer for GeckoElement<'le> {
     fn synthesize_presentational_hints_for_legacy_attributes<V>(&self, _hints: &mut V)
-        where V: Push<ApplicableDeclarationBlock>
+        where V: Push<ApplicableDeclarationBlock>,
     {
         // FIXME(bholley) - Need to implement this.
     }
@@ -522,8 +556,12 @@ impl<'le> ::selectors::Element for GeckoElement<'le> {
     }
 }
 
+/// A few helpers to help with attribute selectors and snapshotting.
 pub trait AttrSelectorHelpers {
+    /// Returns the namespace of the selector, or null otherwise.
     fn ns_or_null(&self) -> *mut nsIAtom;
+    /// Returns the proper selector name depending on whether the requesting
+    /// element is an HTML element in an HTML document or not.
     fn select_name(&self, is_html_element_in_html_document: bool) -> *mut nsIAtom;
 }
 

components/style/gecko_bindings/mod.rs

@@ -2,7 +2,9 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-#[allow(dead_code, improper_ctypes, non_camel_case_types)]
+//! Gecko's C++ bindings, along with some rust helpers to ease its use.
+
+#[allow(dead_code, improper_ctypes, non_camel_case_types, missing_docs)]
 pub mod bindings {
     include!(concat!(env!("OUT_DIR"), "/gecko/bindings.rs"));
 }
@@ -11,7 +13,7 @@ pub mod bindings {
 // foreign structs to have `PhantomData`. We should remove this once the lint
 // ignores this case.
 
-#[allow(dead_code, improper_ctypes, non_camel_case_types, non_snake_case, non_upper_case_globals)]
+#[allow(dead_code, improper_ctypes, non_camel_case_types, non_snake_case, non_upper_case_globals, missing_docs)]
 pub mod structs {
     cfg_if! {
         if #[cfg(debug_assertions)] {

components/style/gecko_bindings/sugar/mod.rs

@@ -2,6 +2,8 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+//! Rust sugar and convenience methods for Gecko types.
+
 mod ns_com_ptr;
 mod ns_css_shadow_array;
 mod ns_style_auto_array;

components/style/gecko_bindings/sugar/ns_com_ptr.rs

@@ -1,15 +1,20 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+//! Little helpers for `nsCOMPtr`.
+
 use gecko_bindings::structs::nsCOMPtr;
 
 impl<T> nsCOMPtr<T> {
+    /// Get this pointer as a raw pointer.
     #[cfg(debug_assertions)]
     #[inline]
     pub fn raw(&self) -> *mut T {
         self.mRawPtr
     }
 
+    /// Get this pointer as a raw pointer.
     #[cfg(not(debug_assertions))]
     #[inline]
     pub fn raw(&self) -> *mut T {

components/style/gecko_bindings/sugar/ns_css_shadow_array.rs

@@ -2,6 +2,8 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+//! Rust helpers for Gecko's `nsCSSShadowArray`.
+
 use gecko_bindings::bindings::Gecko_AddRefCSSShadowArrayArbitraryThread;
 use gecko_bindings::bindings::Gecko_NewCSSShadowArray;
 use gecko_bindings::bindings::Gecko_ReleaseCSSShadowArrayArbitraryThread;
@@ -10,6 +12,7 @@ use std::{ptr, slice};
 use std::ops::{Deref, DerefMut};
 
 impl RefPtr<nsCSSShadowArray> {
+    /// Replaces the current `nsCSSShadowArray` with a new one of len `len`.
     pub fn replace_with_new(&mut self, len: u32) {
         unsafe {
             if !self.mRawPtr.is_null() {
@@ -23,6 +26,12 @@ impl RefPtr<nsCSSShadowArray> {
             }
         }
     }
+
+    /// Sets the value to other `nsCSSShadowArray`, bumping and decreasing
+    /// refcounts as needed.
+    ///
+    /// TODO(emilio): Seems like this could move to `refptr.rs`, and be more
+    /// generic.
     pub fn copy_from(&mut self, other: &Self) {
         unsafe {
             if !self.mRawPtr.is_null() {

components/style/gecko_bindings/sugar/ns_style_auto_array.rs

@@ -2,21 +2,27 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+//! Rust helpers for Gecko's `nsStyleAutoArray`.
+
 use gecko_bindings::structs::nsStyleAutoArray;
 use std::iter::{once, Chain, Once, IntoIterator};
 use std::slice::{Iter, IterMut};
 
 impl<T> nsStyleAutoArray<T> {
+    /// Mutably iterate over the array elements.
     pub fn iter_mut(&mut self) -> Chain<Once<&mut T>, IterMut<T>> {
         once(&mut self.mFirstElement).chain(self.mOtherElements.iter_mut())
     }
+
+    /// Iterate over the array elements.
     pub fn iter(&self) -> Chain<Once<&T>, Iter<T>> {
         once(&self.mFirstElement).chain(self.mOtherElements.iter())
     }
 
-    // Note that often structs containing autoarrays will have
+    /// Returns the length of the array.
-    // additional member fields that contain the length, which must be kept
+    ///
-    // in sync
+    /// Note that often structs containing autoarrays will have additional
+    /// member fields that contain the length, which must be kept in sync.
     pub fn len(&self) -> usize {
         1 + self.mOtherElements.len()
     }

components/style/gecko_bindings/sugar/ns_style_coord.rs

@@ -2,6 +2,8 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+//! Rust helpers for Gecko's `nsStyleCoord`.
+
 use gecko_bindings::bindings::{Gecko_ResetStyleCoord, Gecko_SetStyleCoordCalcValue, Gecko_AddRefCalcArbitraryThread};
 use gecko_bindings::structs::{nsStyleCoord_Calc, nsStyleUnit, nsStyleUnion, nsStyleCoord, nsStyleSides, nsStyleCorners};
 use gecko_bindings::structs::{nsStyleCoord_CalcValue, nscoord};
@@ -9,6 +11,7 @@ use std::mem;
 
 impl nsStyleCoord {
     #[inline]
+    /// Get a `null` nsStyleCoord.
     pub fn null() -> Self {
         // Can't construct directly because it has private fields
         let mut coord: Self = unsafe { mem::zeroed() };
@@ -41,6 +44,7 @@ impl CoordDataMut for nsStyleCoord {
 }
 
 impl nsStyleCoord_CalcValue {
+    /// Create an "empty" CalcValue (whose value is `0`).
     pub fn new() -> Self {
         nsStyleCoord_CalcValue {
             mLength: 0,
@@ -51,6 +55,8 @@ impl nsStyleCoord_CalcValue {
 }
 
 impl nsStyleSides {
+    /// Immutably get the `nsStyleCoord`-like object representing the side at
+    /// index `index`.
     #[inline]
     pub fn data_at(&self, index: usize) -> SidesData {
         SidesData {
@@ -58,6 +64,9 @@ impl nsStyleSides {
             index: index,
         }
     }
+
+    /// Mutably get the `nsStyleCoord`-like object representing the side at
+    /// index `index`.
     #[inline]
     pub fn data_at_mut(&mut self, index: usize) -> SidesDataMut {
         SidesDataMut {
@@ -67,10 +76,15 @@ impl nsStyleSides {
     }
 }
 
+/// A `nsStyleCoord`-like object on top of an immutable reference to
+/// `nsStyleSides`.
 pub struct SidesData<'a> {
     sides: &'a nsStyleSides,
     index: usize,
 }
+
+/// A `nsStyleCoord`-like object on top of an mutable reference to
+/// `nsStyleSides`.
 pub struct SidesDataMut<'a> {
     sides: &'a mut nsStyleSides,
     index: usize,
@@ -113,6 +127,8 @@ impl<'a> CoordDataMut for SidesDataMut<'a> {
 }
 
 impl nsStyleCorners {
+    /// Get a `nsStyleCoord` like object representing the given index's value
+    /// and unit.
     #[inline]
     pub fn data_at(&self, index: usize) -> CornersData {
         CornersData {
@@ -120,6 +136,9 @@ impl nsStyleCorners {
             index: index,
         }
     }
+
+    /// Get a `nsStyleCoord` like object representing the mutable given index's
+    /// value and unit.
     #[inline]
     pub fn data_at_mut(&mut self, index: usize) -> CornersDataMut {
         CornersDataMut {
@@ -129,10 +148,13 @@ impl nsStyleCorners {
     }
 }
 
+/// A `nsStyleCoord`-like struct on top of `nsStyleCorners`.
 pub struct CornersData<'a> {
     corners: &'a nsStyleCorners,
     index: usize,
 }
+
+/// A `nsStyleCoord`-like struct on top of a mutable `nsStyleCorners` reference.
 pub struct CornersDataMut<'a> {
     corners: &'a mut nsStyleCorners,
     index: usize,
@@ -172,36 +194,56 @@ impl<'a> CoordDataMut for CornersDataMut<'a> {
 
 #[derive(Copy, Clone)]
 /// Enum representing the tagged union that is CoordData.
-/// In release mode this should never actually exist in the code,
+///
-/// and will be optimized out by threading matches and inlining.
+/// In release mode this should never actually exist in the code, and will be
+/// optimized out by threading matches and inlining.
 pub enum CoordDataValue {
+    /// eStyleUnit_Null
     Null,
+    /// eStyleUnit_Normal
     Normal,
+    /// eStyleUnit_Auto
     Auto,
+    /// eStyleUnit_None
     None,
+    /// eStyleUnit_Percent
     Percent(f32),
+    /// eStyleUnit_Factor
     Factor(f32),
+    /// eStyleUnit_Degree
     Degree(f32),
+    /// eStyleUnit_Grad
     Grad(f32),
+    /// eStyleUnit_Radian
     Radian(f32),
+    /// eStyleUnit_Turn
     Turn(f32),
+    /// eStyleUnit_FlexFraction
     FlexFraction(f32),
+    /// eStyleUnit_Coord
     Coord(nscoord),
+    /// eStyleUnit_Integer
     Integer(i32),
+    /// eStyleUnit_Enumerated
     Enumerated(u32),
+    /// eStyleUnit_Calc
     Calc(nsStyleCoord_CalcValue),
 }
 
 
+/// A trait to abstract on top of a mutable `nsStyleCoord`-like object.
 pub trait CoordDataMut : CoordData {
-    // This can't be two methods since we can't mutably borrow twice
+    /// Get mutably the unit and the union.
-    /// This is unsafe since it's possible to modify
+    ///
-    /// the unit without changing the union
+    /// This is unsafe since it's possible to modify the unit without changing
+    /// the union.
+    ///
+    /// NB: This can't be two methods since we can't mutably borrow twice
     unsafe fn values_mut(&mut self) -> (&mut nsStyleUnit, &mut nsStyleUnion);
 
-    /// Clean up any resources used by the union
+    /// Clean up any resources used by the union.
-    /// Currently, this only happens if the nsStyleUnit
+    ///
-    /// is a Calc
+    /// Currently, this only happens if the nsStyleUnit is a Calc.
     #[inline]
     fn reset(&mut self) {
         unsafe {
@@ -213,27 +255,26 @@ pub trait CoordDataMut : CoordData {
     }
 
     #[inline]
+    /// Copies the unit and value from another `CoordData` type.
     fn copy_from<T: CoordData>(&mut self, other: &T) {
         unsafe {
             self.reset();
-            {
+            self.copy_from_unchecked(other);
-                let (unit, union) = self.values_mut();
-                *unit = other.unit();
-                *union = other.union();
-            }
             self.addref_if_calc();
         }
     }
 
     #[inline]
+    /// Copies the unit and value from another `CoordData` type without checking
+    /// the type of the value (so refcounted values like calc may leak).
     unsafe fn copy_from_unchecked<T: CoordData>(&mut self, other: &T) {
-            let (unit, union) = self.values_mut();
+        let (unit, union) = self.values_mut();
-            *unit = other.unit();
+        *unit = other.unit();
-            *union = other.union();
+        *union = other.union();
     }
 
-    /// Useful for initializing uninits
+    /// Useful for initializing uninits, given that `set_value` may segfault on
-    /// (set_value may segfault on uninits)
+    /// uninits.
     fn leaky_set_null(&mut self) {
         use gecko_bindings::structs::nsStyleUnit::*;
         unsafe {
@@ -244,6 +285,7 @@ pub trait CoordDataMut : CoordData {
     }
 
     #[inline(always)]
+    /// Sets the inner value.
     fn set_value(&mut self, value: CoordDataValue) {
         use gecko_bindings::structs::nsStyleUnit::*;
         use self::CoordDataValue::*;
@@ -316,12 +358,16 @@ pub trait CoordDataMut : CoordData {
     }
 
     #[inline]
+    /// Gets the `Calc` value mutably, asserts in debug builds if the unit is
+    /// not `Calc`.
     unsafe fn as_calc_mut(&mut self) -> &mut nsStyleCoord_Calc {
         debug_assert!(self.unit() == nsStyleUnit::eStyleUnit_Calc);
         &mut *(*self.union().mPointer.as_mut() as *mut nsStyleCoord_Calc)
     }
 
     #[inline]
+    /// Does what it promises, if the unit is `calc`, it bumps the reference
+    /// count _of the calc expression_.
     fn addref_if_calc(&mut self) {
         unsafe {
             if self.unit() == nsStyleUnit::eStyleUnit_Calc {
@@ -330,12 +376,16 @@ pub trait CoordDataMut : CoordData {
         }
     }
 }
+/// A trait to abstract on top of a `nsStyleCoord`-like object.
 pub trait CoordData {
+    /// Get the unit of this object.
     fn unit(&self) -> nsStyleUnit;
+    /// Get the `nsStyleUnion` for this object.
     fn union(&self) -> nsStyleUnion;
 
 
     #[inline(always)]
+    /// Get the appropriate value for this object.
     fn as_value(&self) -> CoordDataValue {
         use gecko_bindings::structs::nsStyleUnit::*;
         use self::CoordDataValue::*;
@@ -390,6 +440,7 @@ pub trait CoordData {
 
 
     #[inline]
+    /// Pretend the inner value is a calc expression, and obtain it.
     unsafe fn as_calc(&self) -> &nsStyleCoord_Calc {
         debug_assert!(self.unit() == nsStyleUnit::eStyleUnit_Calc);
         &*(*self.union().mPointer.as_ref() as *const nsStyleCoord_Calc)

components/style/gecko_bindings/sugar/ns_t_array.rs

@@ -2,6 +2,8 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+//! Rust helpers for Gecko's nsTArray.
+
 use gecko_bindings::bindings;
 use gecko_bindings::structs::{nsTArray, nsTArrayHeader};
 use std::mem;
@@ -78,10 +80,12 @@ impl<T> nsTArray<T> {
         unsafe { self.clear() }
     }
 
-    // unsafe because the array may contain uninits
+    /// Resize and set the length of the array to `len`.
-    // This will not call constructors, either manually
+    ///
-    // add bindings or run the typed ensurecapacity call
+    /// unsafe because the array may contain uninitialized members.
-    // on the gecko side
+    ///
+    /// This will not call constructors, if you need that, either manually add
+    /// bindings or run the typed `EnsureCapacity` call on the gecko side.
     pub unsafe fn set_len(&mut self, len: u32) {
         // this can leak
         debug_assert!(len >= self.len() as u32);

components/style/gecko_bindings/sugar/ownership.rs

@@ -2,50 +2,52 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+//! Helpers for different FFI pointer kinds that Gecko's FFI layer uses.
+
 use std::marker::PhantomData;
 use std::mem::{forget, transmute};
 use std::ops::{Deref, DerefMut};
 use std::ptr;
 use std::sync::Arc;
 
-/// Indicates that a given Servo type has a corresponding
+/// Indicates that a given Servo type has a corresponding Gecko FFI type.
-/// Gecko FFI type
-/// The correspondence is not defined at this stage,
-/// use HasArcFFI or similar traits to define it
 pub unsafe trait HasFFI : Sized {
+    /// The corresponding Gecko type that this rust type represents.
+    ///
+    /// See the examples in `components/style/gecko/conversions.rs`.
     type FFIType: Sized;
 }
 
-/// Indicates that a given Servo type has the same layout
+/// Indicates that a given Servo type has the same layout as the corresponding
-/// as the corresponding HasFFI::FFIType type
+/// `HasFFI::FFIType` type.
 pub unsafe trait HasSimpleFFI : HasFFI {
     #[inline]
-    /// Given a Servo-side reference, converts it to an
+    /// Given a Servo-side reference, converts it to an FFI-safe reference which
-    /// FFI-safe reference which can be passed to Gecko
+    /// can be passed to Gecko.
     ///
     /// &ServoType -> &GeckoType
     fn as_ffi(&self) -> &Self::FFIType {
         unsafe { transmute(self) }
     }
     #[inline]
-    /// Given a Servo-side mutable reference, converts it to an
+    /// Given a Servo-side mutable reference, converts it to an FFI-safe mutable
-    /// FFI-safe mutable reference which can be passed to Gecko
+    /// reference which can be passed to Gecko.
     ///
     /// &mut ServoType -> &mut GeckoType
     fn as_ffi_mut(&mut self) -> &mut Self::FFIType {
         unsafe { transmute(self) }
     }
     #[inline]
-    /// Given an FFI-safe reference obtained from Gecko
+    /// Given an FFI-safe reference obtained from Gecko converts it to a
-    /// converts it to a Servo-side reference
+    /// Servo-side reference.
     ///
     /// &GeckoType -> &ServoType
     fn from_ffi(ffi: &Self::FFIType) -> &Self {
         unsafe { transmute(ffi) }
     }
     #[inline]
-    /// Given an FFI-safe mutable reference obtained from Gecko
+    /// Given an FFI-safe mutable reference obtained from Gecko converts it to a
-    /// converts it to a Servo-side mutable reference
+    /// Servo-side mutable reference.
     ///
     /// &mut GeckoType -> &mut ServoType
     fn from_ffi_mut(ffi: &mut Self::FFIType) -> &mut Self {
@@ -57,6 +59,9 @@ pub unsafe trait HasSimpleFFI : HasFFI {
 /// as a Box
 pub unsafe trait HasBoxFFI : HasSimpleFFI {
     #[inline]
+    /// Converts a borrowed Arc to a borrowed FFI reference.
+    ///
+    /// &Arc<ServoType> -> &GeckoType
     fn into_ffi(self: Box<Self>) -> Owned<Self::FFIType> {
         unsafe { transmute(self) }
     }
@@ -64,14 +69,15 @@ pub unsafe trait HasBoxFFI : HasSimpleFFI {
 
 /// Helper trait for conversions between FFI Strong/Borrowed types and Arcs
 ///
-/// Should be implemented by types which are passed over FFI as Arcs
+/// Should be implemented by types which are passed over FFI as Arcs via Strong
-/// via Strong and Borrowed
+/// and Borrowed.
 ///
-/// In this case, the FFIType is the rough equivalent of ArcInner<Self>
+/// In this case, the FFIType is the rough equivalent of ArcInner<Self>.
 pub unsafe trait HasArcFFI : HasFFI {
     // these methods can't be on Borrowed because it leads to an unspecified
     // impl parameter
-    /// Artificially increments the refcount of a (possibly null) borrowed Arc over FFI.
+    /// Artificially increments the refcount of a (possibly null) borrowed Arc
+    /// over FFI.
     unsafe fn addref_opt(ptr: Option<&Self::FFIType>) {
         forget(Self::arc_from_borrowed(&ptr).clone())
     }
@@ -109,6 +115,10 @@ pub unsafe trait HasArcFFI : HasFFI {
         }
     }
 
+    #[inline]
+    /// Converts a borrowed Arc to a borrowed FFI reference.
+    ///
+    /// &Arc<ServoType> -> &GeckoType
     fn arc_as_borrowed<'a>(arc: &'a Arc<Self>) -> &'a &Self::FFIType {
         unsafe {
             transmute::<&Arc<Self>, &&Self::FFIType>(arc)
@@ -116,6 +126,9 @@ pub unsafe trait HasArcFFI : HasFFI {
     }
 
     #[inline]
+    /// Converts a borrowed nullable FFI reference to a borrowed Arc.
+    ///
+    /// &GeckoType -> &Arc<ServoType>
     fn arc_from_borrowed<'a>(ptr: &'a Option<&Self::FFIType>) -> Option<&'a Arc<Self>> {
         unsafe {
             if let Some(ref reference) = *ptr {
@@ -129,27 +142,32 @@ pub unsafe trait HasArcFFI : HasFFI {
 
 #[repr(C)]
 /// Gecko-FFI-safe Arc (T is an ArcInner).
+///
 /// This can be null.
+///
 /// Leaks on drop. Please don't drop this.
-/// TODO: Add destructor bomb once drop flags are gone
+pub struct Strong<GeckoType> {
-pub struct Strong<T> {
+    ptr: *const GeckoType,
-    ptr: *const T,
+    _marker: PhantomData<GeckoType>,
-    _marker: PhantomData<T>,
 }
 
-impl<T> Strong<T> {
+impl<GeckoType> Strong<GeckoType> {
     #[inline]
+    /// Returns whether this reference is null.
     pub fn is_null(&self) -> bool {
-        self.ptr == ptr::null()
+        self.ptr.is_null()
     }
 
     #[inline]
-    /// Given a non-null strong FFI reference,
+    /// Given a non-null strong FFI reference, converts it into a servo-side
-    /// converts it into a servo-side Arc
+    /// Arc.
+    ///
     /// Panics on null.
     ///
     /// Strong<GeckoType> -> Arc<ServoType>
-    pub fn into_arc<U>(self) -> Arc<U> where U: HasArcFFI<FFIType = T> {
+    pub fn into_arc<ServoType>(self) -> Arc<ServoType>
+        where ServoType: HasArcFFI<FFIType = GeckoType>,
+    {
         self.into_arc_opt().unwrap()
     }
 
@@ -159,7 +177,9 @@ impl<T> Strong<T> {
     /// Returns None on null.
     ///
     /// Strong<GeckoType> -> Arc<ServoType>
-    pub fn into_arc_opt<U>(self) -> Option<Arc<U>> where U: HasArcFFI<FFIType = T> {
+    pub fn into_arc_opt<ServoType>(self) -> Option<Arc<ServoType>>
+        where ServoType: HasArcFFI<FFIType = GeckoType>,
+    {
         if self.is_null() {
             None
         } else {
@@ -168,12 +188,15 @@ impl<T> Strong<T> {
     }
 
     #[inline]
-    /// Given a reference to a strong FFI reference,
+    /// Given a reference to a strong FFI reference, converts it to a reference
-    /// converts it to a reference to a servo-side Arc
+    /// to a servo-side Arc.
+    ///
     /// Returns None on null.
     ///
     /// Strong<GeckoType> -> Arc<ServoType>
-    pub fn as_arc_opt<U>(&self) -> Option<&Arc<U>> where U: HasArcFFI<FFIType = T> {
+    pub fn as_arc_opt<ServoType>(&self) -> Option<&Arc<ServoType>>
+        where ServoType: HasArcFFI<FFIType = GeckoType>,
+    {
         if self.is_null() {
             None
         } else {
@@ -182,30 +205,42 @@ impl<T> Strong<T> {
     }
 
     #[inline]
-    /// Produces a null strong FFI reference
+    /// Produces a null strong FFI reference.
     pub fn null() -> Self {
-        unsafe { transmute(ptr::null::<T>()) }
+        unsafe { transmute(ptr::null::<GeckoType>()) }
     }
 }
 
+/// A few helpers implemented on top of Arc<ServoType> to make it more
+/// comfortable to use and write safe code with.
 pub unsafe trait FFIArcHelpers {
+    /// The Rust FFI type that we're implementing methods for.
     type Inner: HasArcFFI;
+
     /// Converts an Arc into a strong FFI reference.
     ///
     /// Arc<ServoType> -> Strong<GeckoType>
     fn into_strong(self) -> Strong<<Self::Inner as HasFFI>::FFIType>;
+
     /// Produces a (nullable) borrowed FFI reference by borrowing an Arc.
     ///
     /// &Arc<ServoType> -> Option<&GeckoType>
+    ///
+    /// FIXME(emilio): What's the point of the nullability? Arc should be
+    /// non-null, right?
+    ///
+    /// Then the `arc_as_borrowed` method can go away.
     fn as_borrowed_opt(&self) -> Option<&<Self::Inner as HasFFI>::FFIType>;
 }
 
 unsafe impl<T: HasArcFFI> FFIArcHelpers for Arc<T> {
     type Inner = T;
+
     #[inline]
     fn into_strong(self) -> Strong<T::FFIType> {
         unsafe { transmute(self) }
     }
+
     #[inline]
     fn as_borrowed_opt(&self) -> Option<&T::FFIType> {
         let borrowedptr = self as *const Arc<T> as *const Option<&T::FFIType>;
@@ -215,51 +250,62 @@ unsafe impl<T: HasArcFFI> FFIArcHelpers for Arc<T> {
 
 #[repr(C)]
 #[derive(Debug)]
-/// Gecko-FFI-safe owned pointer
+/// Gecko-FFI-safe owned pointer.
-/// Cannot be null
+///
-/// Leaks on drop. Please don't drop this.
+/// Cannot be null, and leaks on drop, so needs to be converted into a rust-side
-pub struct Owned<T> {
+/// `Box` before.
-    ptr: *mut T,
+pub struct Owned<GeckoType> {
-    _marker: PhantomData<T>,
+    ptr: *mut GeckoType,
+    _marker: PhantomData<GeckoType>,
 }
 
-impl<T> Owned<T> {
+impl<GeckoType> Owned<GeckoType> {
-    /// Owned<GeckoType> -> Box<ServoType>
+    /// Gets this `Owned` type as a `Box<ServoType>`.
-    pub fn into_box<U>(self) -> Box<U> where U: HasBoxFFI<FFIType = T> {
+    pub fn into_box<ServoType>(self) -> Box<ServoType>
+        where ServoType: HasBoxFFI<FFIType = GeckoType>,
+    {
         unsafe { transmute(self) }
     }
-    pub fn maybe(self) -> OwnedOrNull<T> {
+
+    /// Converts this instance to a (non-null) instance of `OwnedOrNull`.
+    pub fn maybe(self) -> OwnedOrNull<GeckoType> {
         unsafe { transmute(self) }
     }
 }
 
-impl<T> Deref for Owned<T> {
+impl<GeckoType> Deref for Owned<GeckoType> {
-    type Target = T;
+    type Target = GeckoType;
-    fn deref(&self) -> &T {
+    fn deref(&self) -> &GeckoType {
         unsafe { &*self.ptr }
     }
 }
 
-impl<T> DerefMut for Owned<T> {
+impl<GeckoType> DerefMut for Owned<GeckoType> {
-    fn deref_mut(&mut self) -> &mut T {
+    fn deref_mut(&mut self) -> &mut GeckoType {
         unsafe { &mut *self.ptr }
     }
 }
 
 #[repr(C)]
-/// Gecko-FFI-safe owned pointer
+/// Gecko-FFI-safe owned pointer.
-/// Can be null
+///
-pub struct OwnedOrNull<T> {
+/// Can be null, and just as `Owned` leaks on `Drop`.
-    ptr: *mut T,
+pub struct OwnedOrNull<GeckoType> {
-    _marker: PhantomData<T>,
+    ptr: *mut GeckoType,
+    _marker: PhantomData<GeckoType>,
 }
 
-impl<T> OwnedOrNull<T> {
+impl<GeckoType> OwnedOrNull<GeckoType> {
+    /// Returns whether this pointer is null.
+    #[inline]
     pub fn is_null(&self) -> bool {
-        self.ptr == ptr::null_mut()
+        self.ptr.is_null()
     }
-    /// OwnedOrNull<GeckoType> -> Option<Box<ServoType>>
+
-    pub fn into_box_opt<U>(self) -> Option<Box<U>> where U: HasBoxFFI<FFIType = T> {
+    /// Returns an owned pointer if this is non-null, and `None` otherwise.
+    pub fn into_box_opt<ServoType>(self) -> Option<Box<ServoType>>
+        where ServoType: HasBoxFFI<FFIType = GeckoType>,
+    {
         if self.is_null() {
             None
         } else {
@@ -267,8 +313,8 @@ impl<T> OwnedOrNull<T> {
         }
     }
 
-    /// OwnedOrNull<GeckoType> -> Option<Owned<GeckoType>>
+    /// Returns an `Owned<GeckoType>` if non-null, `None` otherwise.
-    pub fn into_owned_opt(self) -> Option<Owned<T>> {
+    pub fn into_owned_opt(self) -> Option<Owned<GeckoType>> {
         if self.is_null() {
             None
         } else {
@@ -276,11 +322,15 @@ impl<T> OwnedOrNull<T> {
         }
     }
 
-    pub fn borrow(&self) -> Option<&T> {
+    /// Gets a immutable reference to the underlying Gecko type, or `None` if
+    /// null.
+    pub fn borrow(&self) -> Option<&GeckoType> {
         unsafe { transmute(self) }
     }
 
-    pub fn borrow_mut(&self) -> Option<&mut T> {
+    /// Gets a mutable reference to the underlying Gecko type, or `None` if
+    /// null.
+    pub fn borrow_mut(&self) -> Option<&mut GeckoType> {
         unsafe { transmute(self) }
     }
 }

components/style/gecko_bindings/sugar/refptr.rs

@@ -2,6 +2,8 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+//! A rust helper to ease the use of Gecko's refcounted types.
+
 use gecko_bindings::structs;
 use heapsize::HeapSizeOf;
 use std::{mem, ptr};
@@ -11,37 +13,39 @@ use std::ops::{Deref, DerefMut};
 /// Trait for all objects that have Addref() and Release
 /// methods and can be placed inside RefPtr<T>
 pub unsafe trait RefCounted {
+    /// Bump the reference count.
     fn addref(&self);
+    /// Decrease the reference count.
     unsafe fn release(&self);
 }
 
-/// Trait for types which can be shared across threads in RefPtr
+/// Trait for types which can be shared across threads in RefPtr.
 pub unsafe trait ThreadSafeRefCounted: RefCounted {}
 
+/// A custom RefPtr implementation to take into account Drop semantics and
+/// a bit less-painful memory management.
 #[derive(Debug)]
 pub struct RefPtr<T: RefCounted> {
     ptr: *mut T,
     _marker: PhantomData<T>,
 }
 
-/// A RefPtr that we know is uniquely owned
+/// A RefPtr that we know is uniquely owned.
 ///
-/// This is basically Box<T>, with the additional
+/// This is basically Box<T>, with the additional guarantee that the box can be
-/// guarantee that the box can be safely interpreted
+/// safely interpreted as a RefPtr<T> (with refcount 1)
-/// as a RefPtr<T> (with refcount 1)
 ///
-/// This is useful when you wish to create a refptr
+/// This is useful when you wish to create a refptr and mutate it temporarily,
-/// and mutate it temporarily, while it is still
+/// while it is still uniquely owned.
-/// uniquely owned.
 pub struct UniqueRefPtr<T: RefCounted>(RefPtr<T>);
 
 // There is no safe conversion from &T to RefPtr<T> (like Gecko has)
 // because this lets you break UniqueRefPtr's guarantee
 
 impl<T: RefCounted> RefPtr<T> {
-    /// Create a new RefPtr from an already addrefed
+    /// Create a new RefPtr from an already addrefed pointer obtained from FFI.
-    /// pointer obtained from FFI. Pointer
+    ///
-    /// must be valid, non-null and have been addrefed
+    /// The pointer must be valid, non-null and have been addrefed.
     pub unsafe fn from_addrefed(ptr: *mut T) -> Self {
         debug_assert!(!ptr.is_null());
         RefPtr {
@@ -50,8 +54,10 @@ impl<T: RefCounted> RefPtr<T> {
         }
     }
 
-    /// Create a new RefPtr from a pointer obtained
+    /// Create a new RefPtr from a pointer obtained from FFI.
-    /// from FFI. Pointer must be valid and non null.
+    ///
+    /// The pointer must be valid and non null.
+    ///
     /// This method calls addref() internally
     pub unsafe fn new(ptr: *mut T) -> Self {
         debug_assert!(!ptr.is_null());
@@ -63,8 +69,7 @@ impl<T: RefCounted> RefPtr<T> {
         ret
     }
 
-    /// Produces an FFI-compatible RefPtr that can be stored in
+    /// Produces an FFI-compatible RefPtr that can be stored in style structs.
-    /// style structs.
     ///
     /// structs::RefPtr does not have a destructor, so this may leak
     pub fn forget(self) -> structs::RefPtr<T> {
@@ -75,36 +80,36 @@ impl<T: RefCounted> RefPtr<T> {
         ret
     }
 
-    /// Returns the raw inner pointer
+    /// Returns the raw inner pointer to be fed back into FFI.
-    /// to be fed back into FFI
     pub fn get(&self) -> *mut T {
         self.ptr
     }
 
-    /// Addref the inner data
+    /// Addref the inner data, obviously leaky on its own.
-    ///
-    /// Leaky on its own
     pub fn addref(&self) {
         unsafe { (*self.ptr).addref(); }
     }
 
-    /// Release the inner data
+    /// Release the inner data.
     ///
-    /// Call only when the data actuall needs releasing
+    /// Call only when the data actually needs releasing.
     pub unsafe fn release(&self) {
         (*self.ptr).release();
     }
 }
 
 impl<T: RefCounted> UniqueRefPtr<T> {
-    /// Create a unique refptr from an already addrefed
+    /// Create a unique refptr from an already addrefed pointer obtained from
-    /// pointer obtained from FFI. The refcount must be one.
+    /// FFI.
+    ///
+    /// The refcount must be one.
+    ///
     /// The pointer must be valid and non null
     pub unsafe fn from_addrefed(ptr: *mut T) -> Self {
         UniqueRefPtr(RefPtr::from_addrefed(ptr))
     }
 
-    /// Convert to a RefPtr so that it can be used
+    /// Convert to a RefPtr so that it can be used.
     pub fn get(self) -> RefPtr<T> {
         self.0
     }
@@ -131,9 +136,9 @@ impl<T: RefCounted> DerefMut for UniqueRefPtr<T> {
 }
 
 impl<T: RefCounted> structs::RefPtr<T> {
-    /// Produces a Rust-side RefPtr from an FFI RefPtr, bumping the refcount
+    /// Produces a Rust-side RefPtr from an FFI RefPtr, bumping the refcount.
     ///
-    /// Must be called on a valid, non-null structs::RefPtr<T>
+    /// Must be called on a valid, non-null structs::RefPtr<T>.
     pub unsafe fn to_safe(&self) -> RefPtr<T> {
         debug_assert!(!self.mRawPtr.is_null());
         let r = RefPtr {
@@ -143,7 +148,8 @@ impl<T: RefCounted> structs::RefPtr<T> {
         r.addref();
         r
     }
-    /// Produces a Rust-side RefPtr, consuming the existing one (and not bumping the refcount)
+    /// Produces a Rust-side RefPtr, consuming the existing one (and not bumping
+    /// the refcount).
     pub unsafe fn into_safe(self) -> RefPtr<T> {
         debug_assert!(!self.mRawPtr.is_null());
         RefPtr {
@@ -152,9 +158,13 @@ impl<T: RefCounted> structs::RefPtr<T> {
         }
     }
 
-    /// Replace a structs::RefPtr<T> with a different one, appropriately addref/releasing
+    /// Replace a structs::RefPtr<T> with a different one, appropriately
+    /// addref/releasing.
     ///
-    /// Both `self` and `other` must be valid, but can be null
+    /// Both `self` and `other` must be valid, but can be null.
+    ///
+    /// Safe when called on an aliased pointer because the refcount in that case
+    /// needs to be at least two.
     pub unsafe fn set(&mut self, other: &Self) {
         self.clear();
         if !other.mRawPtr.is_null() {
@@ -163,9 +173,9 @@ impl<T: RefCounted> structs::RefPtr<T> {
     }
 
     /// Clear an instance of the structs::RefPtr<T>, by releasing
-    /// it and setting its contents to null
+    /// it and setting its contents to null.
     ///
-    /// `self` must be valid, but can be null
+    /// `self` must be valid, but can be null.
     pub unsafe fn clear(&mut self) {
         if !self.mRawPtr.is_null() {
             (*self.mRawPtr).release();
@@ -177,7 +187,7 @@ impl<T: RefCounted> structs::RefPtr<T> {
     /// consuming the `RefPtr<T>`, and releasing the old
     /// value in `self` if necessary.
     ///
-    /// `self` must be valid, possibly null
+    /// `self` must be valid, possibly null.
     pub fn set_move(&mut self, other: RefPtr<T>) {
         if !self.mRawPtr.is_null() {
             unsafe { (*self.mRawPtr).release(); }
@@ -215,9 +225,9 @@ impl<T: RefCounted> PartialEq for RefPtr<T> {
 unsafe impl<T: ThreadSafeRefCounted> Send for RefPtr<T> {}
 unsafe impl<T: ThreadSafeRefCounted> Sync for RefPtr<T> {}
 
-// Companion of NS_DECL_THREADSAFE_FFI_REFCOUNTING
+// Companion of NS_DECL_THREADSAFE_FFI_REFCOUNTING.
 //
-// Gets you a free RefCounted impl
+// Gets you a free RefCounted impl implemented via FFI.
 macro_rules! impl_threadsafe_refcount {
     ($t:ty, $addref:ident, $release:ident) => (
         unsafe impl RefCounted for $t {
@@ -244,5 +254,10 @@ impl_threadsafe_refcount!(::gecko_bindings::structs::nsStyleQuoteValues,
 impl_threadsafe_refcount!(::gecko_bindings::structs::nsCSSValueSharedList,
                           Gecko_AddRefCSSValueSharedListArbitraryThread,
                           Gecko_ReleaseCSSValueSharedListArbitraryThread);
+/// A Gecko `ThreadSafePrincipalHolder` wrapped in a safe refcounted pointer, to
+/// use during stylesheet parsing and style computation.
 pub type GeckoArcPrincipal = RefPtr<::gecko_bindings::structs::ThreadSafePrincipalHolder>;
+
+/// A Gecko `ThreadSafeURIHolder` wrapped in a safe refcounted pointer, to use
+/// during stylesheet parsing and style computation.
 pub type GeckoArcURI = RefPtr<::gecko_bindings::structs::ThreadSafeURIHolder>;

components/style/gecko_bindings/sugar/style_complex_color.rs

@@ -2,6 +2,8 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+//! Rust helpers to interact with Gecko's StyleComplexColor.
+
 use cssparser::Color;
 use gecko::values::{convert_nscolor_to_rgba, convert_rgba_to_nscolor};
 use gecko_bindings::structs::{nscolor, StyleComplexColor};
@@ -17,6 +19,7 @@ impl From<nscolor> for StyleComplexColor {
 }
 
 impl StyleComplexColor {
+    /// Create a `StyleComplexColor` value that represents `currentColor`.
     pub fn current_color() -> Self {
         StyleComplexColor {
             mColor: 0,
@@ -25,6 +28,7 @@ impl StyleComplexColor {
         }
     }
 
+    /// Create a `StyleComplexColor` value that represents `auto`.
     pub fn auto() -> Self {
         StyleComplexColor {
             mColor: 0,

components/style/gecko_string_cache/mod.rs

@@ -4,6 +4,8 @@
 
 #![allow(unsafe_code)]
 
+//! A drop-in replacement for string_cache, but backed by Gecko `nsIAtom`s.
+
 use gecko_bindings::bindings::Gecko_AddRefAtom;
 use gecko_bindings::bindings::Gecko_Atomize;
 use gecko_bindings::bindings::Gecko_ReleaseAtom;
@@ -19,7 +21,7 @@ use std::ops::Deref;
 use std::slice;
 
 #[macro_use]
-#[allow(improper_ctypes, non_camel_case_types)]
+#[allow(improper_ctypes, non_camel_case_types, missing_docs)]
 pub mod atom_macro;
 #[macro_use]
 pub mod namespace;
@@ -40,6 +42,8 @@ pub struct Atom(*mut WeakAtom);
 /// where `'a` is the lifetime of something that holds a strong reference to that atom.
 pub struct WeakAtom(nsIAtom);
 
+/// A BorrowedAtom for Gecko is just a weak reference to a `nsIAtom`, that
+/// hasn't been bumped.
 pub type BorrowedAtom<'a> = &'a WeakAtom;
 
 impl Deref for Atom {
@@ -75,21 +79,26 @@ unsafe impl Sync for Atom {}
 unsafe impl Sync for WeakAtom {}
 
 impl WeakAtom {
+    /// Construct a `WeakAtom` from a raw `nsIAtom`.
     #[inline]
     pub unsafe fn new<'a>(atom: *mut nsIAtom) -> &'a mut Self {
         &mut *(atom as *mut WeakAtom)
     }
 
+    /// Clone this atom, bumping the refcount if the atom is not static.
     #[inline]
     pub fn clone(&self) -> Atom {
         Atom::from(self.as_ptr())
     }
 
+    /// Get the atom hash.
     #[inline]
     pub fn get_hash(&self) -> u32 {
         self.0.mHash
     }
 
+    /// Get the atom as a slice of utf-16 chars.
+    #[inline]
     pub fn as_slice(&self) -> &[u16] {
         unsafe {
             slice::from_raw_parts((*self.as_ptr()).mString, self.len() as usize)
@@ -101,20 +110,29 @@ impl WeakAtom {
         char::decode_utf16(self.as_slice().iter().cloned())
     }
 
+    /// Execute `cb` with the string that this atom represents.
+    ///
+    /// Find alternatives to this function when possible, please, since it's
+    /// pretty slow.
     pub fn with_str<F, Output>(&self, cb: F) -> Output
         where F: FnOnce(&str) -> Output
     {
         // FIXME(bholley): We should measure whether it makes more sense to
         // cache the UTF-8 version in the Gecko atom table somehow.
-        let owned = String::from_utf16(self.as_slice()).unwrap();
+        let owned = self.to_string();
         cb(&owned)
     }
 
+    /// Convert this Atom into a string, decoding the UTF-16 bytes.
+    ///
+    /// Find alternatives to this function when possible, please, since it's
+    /// pretty slow.
     #[inline]
     pub fn to_string(&self) -> String {
         String::from_utf16(self.as_slice()).unwrap()
     }
 
+    /// Returns whether this atom is static.
     #[inline]
     pub fn is_static(&self) -> bool {
         unsafe {
@@ -122,6 +140,7 @@ impl WeakAtom {
         }
     }
 
+    /// Returns the length of the atom string.
     #[inline]
     pub fn len(&self) -> u32 {
         unsafe {
@@ -129,6 +148,7 @@ impl WeakAtom {
         }
     }
 
+    /// Returns the atom as a mutable pointer.
     #[inline]
     pub fn as_ptr(&self) -> *mut nsIAtom {
         let const_ptr: *const nsIAtom = &self.0;
@@ -152,6 +172,7 @@ impl fmt::Display for WeakAtom {
 }
 
 impl Atom {
+    /// Execute a callback with the atom represented by `ptr`.
     pub unsafe fn with<F>(ptr: *mut nsIAtom, callback: &mut F) where F: FnMut(&Atom) {
         let atom = Atom(WeakAtom::new(ptr));
         callback(&atom);

components/style/gecko_string_cache/namespace.rs

@@ -2,6 +2,8 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+//! A type to represent a namespace.
+
 use gecko_bindings::structs::nsIAtom;
 use std::borrow::{Borrow, Cow};
 use std::fmt;
@@ -13,9 +15,11 @@ macro_rules! ns {
     () => { $crate::string_cache::Namespace(atom!("")) }
 }
 
+/// A Gecko namespace is just a wrapped atom.
 #[derive(Debug, PartialEq, Eq, Clone, Default, Hash)]
 pub struct Namespace(pub Atom);
 
+/// A Gecko WeakNamespace is a wrapped WeakAtom.
 #[derive(Hash)]
 pub struct WeakNamespace(WeakAtom);
 
@@ -51,11 +55,14 @@ impl Borrow<WeakNamespace> for Namespace {
 }
 
 impl WeakNamespace {
+    /// Trivially construct a WeakNamespace.
     #[inline]
     pub unsafe fn new<'a>(atom: *mut nsIAtom) -> &'a Self {
         &*(atom as *const WeakNamespace)
     }
 
+    /// Clone this WeakNamespace to obtain a strong reference to the same
+    /// underlying namespace.
     #[inline]
     pub fn clone(&self) -> Namespace {
         Namespace(self.0.clone())

components/style/keyframes.rs

@@ -2,6 +2,10 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+//! Keyframes: https://drafts.csswg.org/css-animations/#keyframes
+
+#![deny(missing_docs)]
+
 use cssparser::{AtRuleParser, Parser, QualifiedRuleParser, RuleListParser};
 use cssparser::{DeclarationListParser, DeclarationParser, parse_one_rule};
 use parking_lot::RwLock;
@@ -14,8 +18,8 @@ use std::sync::Arc;
 use style_traits::ToCss;
 use stylesheets::{MemoryHoleReporter, Stylesheet};
 
-/// A number from 0 to 1, indicating the percentage of the animation where
+/// A number from 0 to 1, indicating the percentage of the animation when this
-/// this keyframe should run.
+/// keyframe should run.
 #[derive(Debug, Copy, Clone, PartialEq, PartialOrd)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub struct KeyframePercentage(pub f32);
@@ -37,6 +41,7 @@ impl ToCss for KeyframePercentage {
 }
 
 impl KeyframePercentage {
+    /// Trivially constructs a new `KeyframePercentage`.
     #[inline]
     pub fn new(value: f32) -> KeyframePercentage {
         debug_assert!(value >= 0. && value <= 1.);
@@ -67,6 +72,7 @@ impl KeyframePercentage {
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub struct KeyframeSelector(Vec<KeyframePercentage>);
 impl KeyframeSelector {
+    /// Return the list of percentages this selector contains.
     #[inline]
     pub fn percentages(&self) -> &[KeyframePercentage] {
         &self.0
@@ -77,6 +83,7 @@ impl KeyframeSelector {
         KeyframeSelector(percentages)
     }
 
+    /// Parse a keyframe selector from CSS input.
     pub fn parse(input: &mut Parser) -> Result<Self, ()> {
         input.parse_comma_separated(KeyframePercentage::parse)
              .map(KeyframeSelector)
@@ -87,12 +94,13 @@ impl KeyframeSelector {
 #[derive(Debug, Clone)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub struct Keyframe {
+    /// The selector this keyframe was specified from.
     pub selector: KeyframeSelector,
 
-    /// `!important` is not allowed in keyframe declarations,
+    /// The declaration block that was declared inside this keyframe.
-    /// so the second value of these tuples is always `Importance::Normal`.
+    ///
-    /// But including them enables `compute_style_for_animation_step` to create a `ApplicableDeclarationBlock`
+    /// Note that `!important` rules in keyframes don't apply, but we keep this
-    /// by cloning an `Arc<_>` (incrementing a reference count) rather than re-creating a `Vec<_>`.
+    /// `Arc` just for convenience.
     #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")]
     pub block: Arc<RwLock<PropertyDeclarationBlock>>,
 }
@@ -114,7 +122,10 @@ impl ToCss for Keyframe {
 
 
 impl Keyframe {
-    pub fn parse(css: &str, parent_stylesheet: &Stylesheet, extra_data: ParserContextExtraData)
+    /// Parse a CSS keyframe.
+    pub fn parse(css: &str,
+                 parent_stylesheet: &Stylesheet,
+                 extra_data: ParserContextExtraData)
                  -> Result<Arc<RwLock<Self>>, ()> {
         let error_reporter = Box::new(MemoryHoleReporter);
         let context = ParserContext::new_with_extra_data(parent_stylesheet.origin,
@@ -133,15 +144,19 @@ impl Keyframe {
 /// A keyframes step value. This can be a synthetised keyframes animation, that
 /// is, one autogenerated from the current computed values, or a list of
 /// declarations to apply.
-// TODO: Find a better name for this?
+///
+/// TODO: Find a better name for this?
 #[derive(Debug, Clone)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub enum KeyframesStepValue {
-    /// See `Keyframe::declarations`’s docs about the presence of `Importance`.
+    /// A step formed by a declaration block specified by the CSS.
     Declarations {
+        /// The declaration block per se.
         #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")]
         block: Arc<RwLock<PropertyDeclarationBlock>>
     },
+    /// A synthetic step computed from the current computed values at the time
+    /// of the animation.
     ComputedValues,
 }
 
@@ -162,7 +177,6 @@ pub struct KeyframesStep {
 }
 
 impl KeyframesStep {
-    #[allow(unsafe_code)]
     #[inline]
     fn new(percentage: KeyframePercentage,
            value: KeyframesStepValue) -> Self {
@@ -193,6 +207,7 @@ impl KeyframesStep {
 #[derive(Debug, Clone)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub struct KeyframesAnimation {
+    /// The difference steps of the animation.
     pub steps: Vec<KeyframesStep>,
     /// The properties that change in this animation.
     pub properties_changed: Vec<TransitionProperty>,
@@ -204,7 +219,6 @@ pub struct KeyframesAnimation {
 ///
 /// In practice, browsers seem to try to do their best job at it, so we might
 /// want to go through all the actual keyframes and deduplicate properties.
-#[allow(unsafe_code)]
 fn get_animated_properties(keyframe: &Keyframe) -> Vec<TransitionProperty> {
     let mut ret = vec![];
     // NB: declarations are already deduplicated, so we don't have to check for
@@ -219,6 +233,13 @@ fn get_animated_properties(keyframe: &Keyframe) -> Vec<TransitionProperty> {
 }
 
 impl KeyframesAnimation {
+    /// Create a keyframes animation from a given list of keyframes.
+    ///
+    /// This will return `None` if the list of keyframes is empty, or there are
+    /// no animated properties obtained from the keyframes.
+    ///
+    /// Otherwise, this will compute and sort the steps used for the animation,
+    /// and return the animation object.
     pub fn from_keyframes(keyframes: &[Arc<RwLock<Keyframe>>]) -> Option<Self> {
         if keyframes.is_empty() {
             return None;
@@ -273,6 +294,7 @@ struct KeyframeListParser<'a> {
     context: &'a ParserContext<'a>,
 }
 
+/// Parses a keyframe list from CSS input.
 pub fn parse_keyframe_list(context: &ParserContext, input: &mut Parser) -> Vec<Arc<RwLock<Keyframe>>> {
     RuleListParser::new_for_nested_rule(input, KeyframeListParser { context: context })
         .filter_map(Result::ok)

components/style/lib.rs

@@ -26,6 +26,7 @@
 #![cfg_attr(feature = "servo", feature(proc_macro))]
 
 #![deny(warnings)]
+#![deny(missing_docs)]
 
 // FIXME(bholley): We need to blanket-allow unsafe code in order to make the
 // gecko atom!() macro work. When Rust 1.14 is released [1], we can uncomment
@@ -88,7 +89,9 @@ extern crate time;
 extern crate unicode_segmentation;
 
 pub mod animation;
+#[allow(missing_docs)] // TODO: Under a rewrite.
 pub mod atomic_refcell;
+#[allow(missing_docs)] // TODO.
 pub mod attr;
 pub mod bezier;
 pub mod bloom;
@@ -105,8 +108,10 @@ pub mod font_metrics;
 #[cfg(feature = "gecko")] #[allow(unsafe_code)] pub mod gecko;
 #[cfg(feature = "gecko")] #[allow(unsafe_code)] pub mod gecko_bindings;
 pub mod keyframes;
+#[allow(missing_docs)] // TODO.
 pub mod logical_geometry;
 pub mod matching;
+#[allow(missing_docs)]
 pub mod media_queries;
 pub mod owning_handle;
 pub mod parallel;
@@ -145,15 +150,16 @@ use style_traits::ToCss;
 #[cfg(feature = "servo")] pub use html5ever_atoms::Namespace;
 
 /// The CSS properties supported by the style system.
-// Generated from the properties.mako.rs template by build.rs
+/// Generated from the properties.mako.rs template by build.rs
 #[macro_use]
 #[allow(unsafe_code)]
+#[deny(missing_docs)]
 pub mod properties {
     include!(concat!(env!("OUT_DIR"), "/properties.rs"));
 }
 
 #[cfg(feature = "gecko")]
-#[allow(unsafe_code)]
+#[allow(unsafe_code, missing_docs)]
 pub mod gecko_properties {
     include!(concat!(env!("OUT_DIR"), "/gecko_properties.rs"));
 }

components/style/owning_handle.rs

@@ -3,6 +3,9 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #![allow(unsafe_code)]
+#![deny(missing_docs)]
+
+//! A handle that encapsulate a reference to a given data along with its owner.
 
 use owning_ref::StableAddress;
 use std::ops::{Deref, DerefMut};
@@ -24,21 +27,23 @@ use std::ops::{Deref, DerefMut};
 ///
 /// This does foist some unsafety onto the callback, which needs an `unsafe`
 /// block to dereference the pointer. It would be almost good enough for
-/// OwningHandle to pass a transmuted &'statc reference to the callback
+/// OwningHandle to pass a transmuted &'static reference to the callback
 /// since the lifetime is infinite as far as the minted handle is concerned.
 /// However, even an `Fn` callback can still allow the reference to escape
 /// via a `StaticMutex` or similar, which technically violates the safety
 /// contract. Some sort of language support in the lifetime system could
 /// make this API a bit nicer.
 pub struct OwningHandle<O, H>
-    where O: StableAddress, H: Deref,
+    where O: StableAddress,
+          H: Deref,
 {
     handle: H,
     _owner: O,
 }
 
 impl<O, H> Deref for OwningHandle<O, H>
-    where O: StableAddress, H: Deref,
+    where O: StableAddress,
+          H: Deref,
 {
     type Target = H::Target;
     fn deref(&self) -> &H::Target {
@@ -47,11 +52,13 @@ impl<O, H> Deref for OwningHandle<O, H>
 }
 
 unsafe impl<O, H> StableAddress for OwningHandle<O, H>
-    where O: StableAddress, H: StableAddress,
+    where O: StableAddress,
+          H: StableAddress,
 {}
 
 impl<O, H> DerefMut for OwningHandle<O, H>
-    where O: StableAddress, H: DerefMut,
+    where O: StableAddress,
+          H: DerefMut,
 {
     fn deref_mut(&mut self) -> &mut H::Target {
         self.handle.deref_mut()
@@ -59,14 +66,15 @@ impl<O, H> DerefMut for OwningHandle<O, H>
 }
 
 impl<O, H> OwningHandle<O, H>
-    where O: StableAddress, H: Deref,
+    where O: StableAddress,
+          H: Deref,
 {
     /// Create a new OwningHandle. The provided callback will be invoked with
     /// a pointer to the object owned by `o`, and the returned value is stored
     /// as the object to which this `OwningHandle` will forward `Deref` and
     /// `DerefMut`.
     pub fn new<F>(o: O, f: F) -> Self
-        where F: Fn(*const O::Target) -> H
+        where F: Fn(*const O::Target) -> H,
     {
         let h: H;
         {

components/style/parser.rs

@@ -4,6 +4,8 @@
 
 //! The context within which CSS code is parsed.
 
+#![deny(missing_docs)]
+
 use cssparser::{Parser, SourcePosition};
 use error_reporting::ParseErrorReporter;
 #[cfg(feature = "gecko")]
@@ -11,37 +13,52 @@ use gecko_bindings::sugar::refptr::{GeckoArcPrincipal, GeckoArcURI};
 use servo_url::ServoUrl;
 use stylesheets::Origin;
 
+/// Extra data that the style backend may need to parse stylesheets.
 #[cfg(not(feature = "gecko"))]
 pub struct ParserContextExtraData;
 
+/// Extra data that the style backend may need to parse stylesheets.
 #[cfg(feature = "gecko")]
 pub struct ParserContextExtraData {
+    /// The base URI.
     pub base: Option<GeckoArcURI>,
+    /// The referrer URI.
     pub referrer: Option<GeckoArcURI>,
+    /// The principal that loaded this stylesheet.
     pub principal: Option<GeckoArcPrincipal>,
 }
 
-impl ParserContextExtraData {
+#[cfg(not(feature = "gecko"))]
-    #[cfg(not(feature = "gecko"))]
+impl Default for ParserContextExtraData {
-    pub fn default() -> ParserContextExtraData {
+    fn default() -> Self {
         ParserContextExtraData
     }
+}
 
-    #[cfg(feature = "gecko")]
+#[cfg(feature = "gecko")]
-    pub fn default() -> ParserContextExtraData {
+impl Default for ParserContextExtraData {
+    fn default() -> Self {
         ParserContextExtraData { base: None, referrer: None, principal: None }
     }
 }
 
+/// The data that the parser needs from outside in order to parse a stylesheet.
 pub struct ParserContext<'a> {
+    /// The `Origin` of the stylesheet, whether it's a user, author or
+    /// user-agent stylesheet.
     pub stylesheet_origin: Origin,
+    /// The base url we're parsing this stylesheet as.
     pub base_url: &'a ServoUrl,
+    /// An error reporter to report syntax errors.
     pub error_reporter: Box<ParseErrorReporter + Send>,
+    /// Implementation-dependent extra data.
     pub extra_data: ParserContextExtraData,
 }
 
 impl<'a> ParserContext<'a> {
-    pub fn new_with_extra_data(stylesheet_origin: Origin, base_url: &'a ServoUrl,
+    /// Create a `ParserContext` with extra data.
+    pub fn new_with_extra_data(stylesheet_origin: Origin,
+                               base_url: &'a ServoUrl,
                                error_reporter: Box<ParseErrorReporter + Send>,
                                extra_data: ParserContextExtraData)
                                -> ParserContext<'a> {
@@ -53,10 +70,13 @@ impl<'a> ParserContext<'a> {
         }
     }
 
-    pub fn new(stylesheet_origin: Origin, base_url: &'a ServoUrl, error_reporter: Box<ParseErrorReporter + Send>)
+    /// Create a parser context with the default extra data.
+    pub fn new(stylesheet_origin: Origin,
+               base_url: &'a ServoUrl,
+               error_reporter: Box<ParseErrorReporter + Send>)
                -> ParserContext<'a> {
         let extra_data = ParserContextExtraData::default();
-        ParserContext::new_with_extra_data(stylesheet_origin, base_url, error_reporter, extra_data)
+        Self::new_with_extra_data(stylesheet_origin, base_url, error_reporter, extra_data)
     }
 }
 
@@ -69,6 +89,11 @@ pub fn log_css_error(input: &mut Parser, position: SourcePosition, message: &str
 
 // XXXManishearth Replace all specified value parse impls with impls of this
 // trait. This will make it easy to write more generic values in the future.
-pub trait Parse {
+/// A trait to abstract parsing of a specified value given a `ParserContext` and
-    fn parse(context: &ParserContext, input: &mut Parser) -> Result<Self, ()> where Self: Sized;
+/// CSS input.
+pub trait Parse : Sized {
+    /// Parse a value of this type.
+    ///
+    /// Returns an error on failure.
+    fn parse(context: &ParserContext, input: &mut Parser) -> Result<Self, ()>;
 }

components/style/properties/gecko.mako.rs

@@ -48,9 +48,7 @@ use gecko::values::convert_nscolor_to_rgba;
 use gecko::values::convert_rgba_to_nscolor;
 use gecko::values::GeckoStyleCoordConvertible;
 use gecko::values::round_border_to_device_pixels;
-use gecko::values::StyleCoordHelpers;
 use logical_geometry::WritingMode;
-use properties::CascadePropertyFn;
 use properties::longhands;
 use std::fmt::{self, Debug};
 use std::mem::{transmute, zeroed};
@@ -146,11 +144,6 @@ impl ComputedValues {
         set_raw_initial_values(ptr::null_mut());
     }
 
-    #[inline]
-    pub fn do_cascade_property<F: FnOnce(&[CascadePropertyFn])>(f: F) {
-        f(&CASCADE_PROPERTY)
-    }
-
     % for style_struct in data.style_structs:
     #[inline]
     pub fn clone_${style_struct.name_lower}(&self) -> Arc<style_structs::${style_struct.name}> {
@@ -2663,9 +2656,3 @@ unsafe fn raw_initial_values() -> *mut ComputedValues {
 unsafe fn set_raw_initial_values(v: *mut ComputedValues) {
     INITIAL_VALUES_STORAGE.store(v as usize, Ordering::Relaxed);
 }
-
-static CASCADE_PROPERTY: [CascadePropertyFn; ${len(data.longhands)}] = [
-    % for property in data.longhands:
-        longhands::${property.ident}::cascade_property,
-    % endfor
-];

components/style/properties/helpers.mako.rs

@@ -405,7 +405,8 @@
             % endfor
         }
 
-        /// Represents a serializable set of all of the longhand properties that correspond to a shorthand
+        /// Represents a serializable set of all of the longhand properties that
+        /// correspond to a shorthand.
         pub struct LonghandsToSerialize<'a> {
             % for sub_property in shorthand.sub_properties:
                 pub ${sub_property.ident}: &'a DeclaredValue<longhands::${sub_property.ident}::SpecifiedValue>,
@@ -617,16 +618,22 @@
 </%def>
 
 <%def name="logical_setter(name, need_clone=False)">
+    /// Set the appropriate physical property for ${name} given a writing mode.
     pub fn set_${to_rust_ident(name)}(&mut self,
-                                 v: longhands::${to_rust_ident(name)}::computed_value::T,
+                                      v: longhands::${to_rust_ident(name)}::computed_value::T,
-                                 wm: WritingMode) {
+                                      wm: WritingMode) {
         <%self:logical_setter_helper name="${name}">
             <%def name="inner(physical_ident)">
                 self.set_${physical_ident}(v)
             </%def>
         </%self:logical_setter_helper>
     }
-    pub fn copy_${to_rust_ident(name)}_from(&mut self, other: &Self, wm: WritingMode) {
+
+    /// Copy the appropriate physical property from another struct for ${name}
+    /// given a writing mode.
+    pub fn copy_${to_rust_ident(name)}_from(&mut self,
+                                            other: &Self,
+                                            wm: WritingMode) {
         <%self:logical_setter_helper name="${name}">
             <%def name="inner(physical_ident)">
                 self.copy_${physical_ident}_from(other)
@@ -634,6 +641,8 @@
         </%self:logical_setter_helper>
     }
     % if need_clone:
+        /// Get the computed value for the appropriate physical property for
+        /// ${name} given a writing mode.
         pub fn clone_${to_rust_ident(name)}(&self, wm: WritingMode)
             -> longhands::${to_rust_ident(name)}::computed_value::T {
         <%self:logical_setter_helper name="${name}">

components/style/properties/properties.mako.rs

@@ -55,6 +55,8 @@ macro_rules! property_name {
 #[path="${repr(os.path.join(os.path.dirname(__file__), 'declaration_block.rs'))[1:-1]}"]
 pub mod declaration_block;
 
+/// A module with all the code for longhand properties.
+#[allow(missing_docs)]
 pub mod longhands {
     use cssparser::Parser;
     use parser::{Parse, ParserContext};
@@ -85,6 +87,9 @@ pub mod longhands {
     <%include file="/longhand/xul.mako.rs" />
 }
 
+/// A module with code for all the shorthand css properties, and a few
+/// serialization helpers.
+#[allow(missing_docs)]
 pub mod shorthands {
     use cssparser::Parser;
     use parser::{Parse, ParserContext};
@@ -98,14 +103,19 @@ pub mod shorthands {
         }
     }
 
+    /// Parses a property for four different sides per CSS syntax.
+    ///
+    ///  * Zero or more than four values is invalid.
+    ///  * One value sets them all
+    ///  * Two values set (top, bottom) and (left, right)
+    ///  * Three values set top, (left, right) and bottom
+    ///  * Four values set them in order
+    ///
+    /// returns the values in (top, right, bottom, left) order.
     pub fn parse_four_sides<F, T>(input: &mut Parser, parse_one: F) -> Result<(T, T, T, T), ()>
-        where F: Fn(&mut Parser) -> Result<T, ()>, T: Clone
+        where F: Fn(&mut Parser) -> Result<T, ()>,
+              T: Clone,
     {
-        // zero or more than four values is invalid.
-        // one value sets them all
-        // two values set (top, bottom) and (left, right)
-        // three values set top, (left, right) and bottom
-        // four values set them in order
         let top = try!(parse_one(input));
         let right;
         let bottom;
@@ -163,8 +173,6 @@ pub mod shorthands {
 /// This needs to be "included" by mako at least after all longhand modules,
 /// given they populate the global data.
 pub mod animated_properties {
-    #![deny(missing_docs)]
-
     <%include file="/helpers/animated_properties.mako.rs" />
 }
 
@@ -174,11 +182,14 @@ pub mod animated_properties {
 mod property_bit_field {
     use logical_geometry::WritingMode;
 
+    /// A bitfield for all longhand properties, in order to quickly test whether
+    /// we've seen one of them.
     pub struct PropertyBitField {
         storage: [u32; (${len(data.longhands)} - 1 + 32) / 32]
     }
 
     impl PropertyBitField {
+        /// Create a new `PropertyBitField`, with all the bits set to zero.
         #[inline]
         pub fn new() -> PropertyBitField {
             PropertyBitField { storage: [0; (${len(data.longhands)} - 1 + 32) / 32] }
@@ -188,13 +199,14 @@ mod property_bit_field {
         fn get(&self, bit: usize) -> bool {
             (self.storage[bit / 32] & (1 << (bit % 32))) != 0
         }
+
         #[inline]
         fn set(&mut self, bit: usize) {
             self.storage[bit / 32] |= 1 << (bit % 32)
         }
         % for i, property in enumerate(data.longhands):
             % if not property.derived_from:
-                #[allow(non_snake_case)]
+                #[allow(non_snake_case, missing_docs)]
                 #[inline]
                 pub fn get_${property.ident}(&self) -> bool {
                     self.get(${i})
@@ -206,7 +218,7 @@ mod property_bit_field {
                 }
             % endif
             % if property.logical:
-                #[allow(non_snake_case)]
+                #[allow(non_snake_case, missing_docs)]
                 pub fn get_physical_${property.ident}(&self, wm: WritingMode) -> bool {
                     <%helpers:logical_setter_helper name="${property.name}">
                         <%def name="inner(physical_ident)">
@@ -214,7 +226,7 @@ mod property_bit_field {
                         </%def>
                     </%helpers:logical_setter_helper>
                 }
-                #[allow(non_snake_case)]
+                #[allow(non_snake_case, missing_docs)]
                 pub fn set_physical_${property.ident}(&mut self, wm: WritingMode) {
                     <%helpers:logical_setter_helper name="${property.name}">
                         <%def name="inner(physical_ident)">
@@ -229,6 +241,8 @@ mod property_bit_field {
 
 % for property in data.longhands:
     % if not property.derived_from:
+        /// Perform CSS variable substitution if needed, and execute `f` with
+        /// the resulting declared value.
         #[allow(non_snake_case)]
         fn substitute_variables_${property.ident}<F>(
             value: &DeclaredValue<longhands::${property.ident}::SpecifiedValue>,
@@ -268,7 +282,8 @@ mod property_bit_field {
                 f: F,
                 error_reporter: &mut StdBox<ParseErrorReporter + Send>,
                 extra_data: ParserContextExtraData)
-                where F: FnOnce(&DeclaredValue<longhands::${property.ident}::SpecifiedValue>) {
+                where F: FnOnce(&DeclaredValue<longhands::${property.ident}::SpecifiedValue>)
+        {
             f(&
                 ::custom_properties::substitute(css, first_token_type, custom_properties)
                 .and_then(|css| {
@@ -308,7 +323,9 @@ mod property_bit_field {
     % endif
 % endfor
 
-/// Only keep the "winning" declaration for any given property, by importance then source order.
+/// Given a property declaration block, only keep the "winning" declaration for
+/// any given property, by importance then source order.
+///
 /// The input and output are in source order
 fn deduplicate_property_declarations(block: &mut PropertyDeclarationBlock) {
     let mut deduplicated = Vec::new();
@@ -379,10 +396,14 @@ fn remove_one<T, F: FnMut(&T) -> bool>(v: &mut Vec<T>, mut remove_this: F) {
     debug_assert_eq!(v.len(), previous_len - 1);
 }
 
+/// An enum to represent a CSS Wide keyword.
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
 pub enum CSSWideKeyword {
+    /// The `initial` keyword.
     InitialKeyword,
+    /// The `inherit` keyword.
     InheritKeyword,
+    /// The `unset` keyword.
     UnsetKeyword,
 }
 
@@ -397,15 +418,18 @@ impl Parse for CSSWideKeyword {
     }
 }
 
+/// An identifier for a given longhand property.
 #[derive(Clone, Copy, Eq, PartialEq, Debug)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub enum LonghandId {
     % for i, property in enumerate(data.longhands):
+        /// ${property.name}
         ${property.camel_case} = ${i},
     % endfor
 }
 
 impl LonghandId {
+    /// Get the name of this longhand property.
     pub fn name(&self) -> &'static str {
         match *self {
             % for property in data.longhands:
@@ -415,21 +439,26 @@ impl LonghandId {
     }
 }
 
+/// An identifier for a given shorthand property.
 #[derive(Clone, Copy, Eq, PartialEq, Debug)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub enum ShorthandId {
     % for property in data.shorthands:
+        /// ${property.name}
         ${property.camel_case},
     % endfor
 }
 
 impl ToCss for ShorthandId {
-    fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
+    fn to_css<W>(&self, dest: &mut W) -> fmt::Result
+        where W: fmt::Write,
+    {
         dest.write_str(self.name())
     }
 }
 
 impl ShorthandId {
+    /// Get the name for this shorthand property.
     pub fn name(&self) -> &'static str {
         match *self {
             % for property in data.shorthands:
@@ -438,6 +467,7 @@ impl ShorthandId {
         }
     }
 
+    /// Get the longhand ids that form this shorthand.
     pub fn longhands(&self) -> &'static [LonghandId] {
         % for property in data.shorthands:
             static ${property.ident.upper()}: &'static [LonghandId] = &[
@@ -453,8 +483,14 @@ impl ShorthandId {
         }
     }
 
+    /// Try to serialize the given declarations as this shorthand.
+    ///
+    /// Returns an error if writing to the stream fails, or if the declarations
+    /// do not map to a shorthand.
     pub fn longhands_to_css<'a, W, I>(&self, declarations: I, dest: &mut W) -> fmt::Result
-        where W: fmt::Write, I: Iterator<Item=&'a PropertyDeclaration> {
+        where W: fmt::Write,
+              I: Iterator<Item=&'a PropertyDeclaration>,
+    {
         match *self {
             % for property in data.shorthands:
                 ShorthandId::${property.camel_case} => {
@@ -501,9 +537,9 @@ impl ShorthandId {
                                                    is_first_serialization: &mut bool,
                                                    importance: Importance)
                                                    -> Result<bool, fmt::Error>
-    where W: Write,
+        where W: Write,
-          I: IntoIterator<Item=&'a PropertyDeclaration>,
+              I: IntoIterator<Item=&'a PropertyDeclaration>,
-          I::IntoIter: Clone,
+              I::IntoIter: Clone,
     {
         match self.get_shorthand_appendable_value(declarations) {
             None => Ok(false),
@@ -594,7 +630,9 @@ impl<T: HasViewportPercentage> HasViewportPercentage for DeclaredValue<T> {
 }
 
 impl<T: ToCss> ToCss for DeclaredValue<T> {
-    fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
+    fn to_css<W>(&self, dest: &mut W) -> fmt::Result
+        where W: fmt::Write,
+    {
         match *self {
             DeclaredValue::Value(ref inner) => inner.to_css(dest),
             DeclaredValue::WithVariables { ref css, from_shorthand: None, .. } => {
@@ -609,15 +647,21 @@ impl<T: ToCss> ToCss for DeclaredValue<T> {
     }
 }
 
+/// An identifier for a given property declaration, which can be either a
+/// longhand or a custom property.
 #[derive(PartialEq, Clone)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub enum PropertyDeclarationId<'a> {
+    /// A longhand.
     Longhand(LonghandId),
+    /// A custom property declaration.
     Custom(&'a ::custom_properties::Name),
 }
 
 impl<'a> ToCss for PropertyDeclarationId<'a> {
-    fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
+    fn to_css<W>(&self, dest: &mut W) -> fmt::Result
+        where W: fmt::Write,
+    {
         match *self {
             PropertyDeclarationId::Longhand(id) => dest.write_str(id.name()),
             PropertyDeclarationId::Custom(name) => write!(dest, "--{}", name),
@@ -626,6 +670,8 @@ impl<'a> ToCss for PropertyDeclarationId<'a> {
 }
 
 impl<'a> PropertyDeclarationId<'a> {
+    /// Whether a given declaration id is either the same as `other`, or a
+    /// longhand of it.
     pub fn is_or_is_longhand_of(&self, other: &PropertyId) -> bool {
         match *self {
             PropertyDeclarationId::Longhand(id) => {
@@ -641,6 +687,8 @@ impl<'a> PropertyDeclarationId<'a> {
         }
     }
 
+    /// Whether a given declaration id is a longhand belonging to this
+    /// shorthand.
     pub fn is_longhand_of(&self, shorthand: ShorthandId) -> bool {
         match *self {
             PropertyDeclarationId::Longhand(ref id) => shorthand.longhands().contains(id),
@@ -649,10 +697,15 @@ impl<'a> PropertyDeclarationId<'a> {
     }
 }
 
+/// Servo's representation of a CSS property, that is, either a longhand, a
+/// shorthand, or a custom property.
 #[derive(Eq, PartialEq, Clone)]
 pub enum PropertyId {
+    /// A longhand property.
     Longhand(LonghandId),
+    /// A shorthand property.
     Shorthand(ShorthandId),
+    /// A custom property.
     Custom(::custom_properties::Name),
 }
 
@@ -683,6 +736,8 @@ enum StaticId {
 include!(concat!(env!("OUT_DIR"), "/static_ids.rs"));
 
 impl PropertyId {
+    /// Returns a given property from the string `s`.
+    ///
     /// Returns Err(()) for unknown non-custom properties
     pub fn parse(s: Cow<str>) -> Result<Self, ()> {
         if let Ok(name) = ::custom_properties::parse_name(&s) {
@@ -697,6 +752,7 @@ impl PropertyId {
         }
     }
 
+    /// Returns a property id from Gecko's nsCSSPropertyID.
     #[cfg(feature = "gecko")]
     #[allow(non_upper_case_globals)]
     pub fn from_nscsspropertyid(id: nsCSSPropertyID) -> Result<Self, ()> {
@@ -725,6 +781,8 @@ impl PropertyId {
         }
     }
 
+    /// Given this property id, get it either as a shorthand or as a
+    /// `PropertyDeclarationId`.
     pub fn as_shorthand(&self) -> Result<ShorthandId, PropertyDeclarationId> {
         match *self {
             PropertyId::Shorthand(id) => Ok(id),
@@ -734,12 +792,16 @@ impl PropertyId {
     }
 }
 
+/// Servo's representation for a property declaration.
 #[derive(PartialEq, Clone)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub enum PropertyDeclaration {
     % for property in data.longhands:
+        /// ${property.name}
         ${property.camel_case}(DeclaredValue<longhands::${property.ident}::SpecifiedValue>),
     % endfor
+    /// A custom property declaration, with the property name and the declared
+    /// value.
     Custom(::custom_properties::Name, DeclaredValue<::custom_properties::SpecifiedValue>),
 }
 
@@ -758,12 +820,21 @@ impl HasViewportPercentage for PropertyDeclaration {
     }
 }
 
+/// The result of parsing a property declaration.
 #[derive(Eq, PartialEq, Copy, Clone)]
 pub enum PropertyDeclarationParseResult {
+    /// The property declaration was for an unknown property.
     UnknownProperty,
+    /// The property declaration was for a disabled experimental property.
     ExperimentalProperty,
+    /// The property declaration contained an invalid value.
     InvalidValue,
+    /// The declaration contained an animation property, and we were parsing
+    /// this as a keyframe block (so that property should be ignored).
+    ///
+    /// See: https://drafts.csswg.org/css-animations/#keyframes
     AnimationPropertyInKeyframeBlock,
+    /// The declaration was either valid or ignored.
     ValidOrIgnoredDeclaration,
 }
 
@@ -786,7 +857,9 @@ impl fmt::Debug for PropertyDeclaration {
 }
 
 impl ToCss for PropertyDeclaration {
-    fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
+    fn to_css<W>(&self, dest: &mut W) -> fmt::Result
+        where W: fmt::Write,
+    {
         match *self {
             % for property in data.longhands:
                 % if not property.derived_from:
@@ -803,6 +876,7 @@ impl ToCss for PropertyDeclaration {
 }
 
 impl PropertyDeclaration {
+    /// Given a property declaration, return the property declaration id.
     pub fn id(&self) -> PropertyDeclarationId {
         match *self {
             % for property in data.longhands:
@@ -816,7 +890,7 @@ impl PropertyDeclaration {
         }
     }
 
-    pub fn with_variables_from_shorthand(&self, shorthand: ShorthandId) -> Option< &str> {
+    fn with_variables_from_shorthand(&self, shorthand: ShorthandId) -> Option< &str> {
         match *self {
             % for property in data.longhands:
                 PropertyDeclaration::${property.camel_case}(ref value) => match *value {
@@ -848,9 +922,12 @@ impl PropertyDeclaration {
         }
     }
 
-    /// Return whether the value is stored as it was in the CSS source, preserving whitespace
+    /// Return whether the value is stored as it was in the CSS source,
-    /// (as opposed to being parsed into a more abstract data structure).
+    /// preserving whitespace (as opposed to being parsed into a more abstract
-    /// This is the case of custom properties and values that contain unsubstituted variables.
+    /// data structure).
+    ///
+    /// This is the case of custom properties and values that contain
+    /// unsubstituted variables.
     pub fn value_is_unparsed(&self) -> bool {
       match *self {
           % for property in data.longhands:
@@ -973,6 +1050,7 @@ impl PropertyDeclaration {
         }
     }
 
+    /// The shorthands that this longhand is part of.
     pub fn shorthands(&self) -> &'static [ShorthandId] {
         // first generate longhand to shorthands lookup map
         <%
@@ -1026,6 +1104,7 @@ impl PropertyDeclaration {
 #[cfg(feature = "gecko")]
 pub use gecko_properties::style_structs;
 
+/// The module where all the style structs are defined.
 #[cfg(feature = "servo")]
 pub mod style_structs {
     use fnv::FnvHasher;
@@ -1040,11 +1119,14 @@ pub mod style_structs {
         #[derive(PartialEq, Clone, Debug)]
         % endif
         #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+        /// The ${style_struct.name} style struct.
         pub struct ${style_struct.name} {
             % for longhand in style_struct.longhands:
+                /// The ${longhand.name} computed value.
                 pub ${longhand.ident}: longhands::${longhand.ident}::computed_value::T,
             % endfor
             % if style_struct.name == "Font":
+                /// The font hash, used for font caching.
                 pub hash: u64,
             % endif
         }
@@ -1065,17 +1147,20 @@ pub mod style_structs {
                 % if longhand.logical:
                     ${helpers.logical_setter(name=longhand.name)}
                 % else:
+                    /// Set ${longhand.name}.
                     #[allow(non_snake_case)]
                     #[inline]
                     pub fn set_${longhand.ident}(&mut self, v: longhands::${longhand.ident}::computed_value::T) {
                         self.${longhand.ident} = v;
                     }
+                    /// Set ${longhand.name} from other struct.
                     #[allow(non_snake_case)]
                     #[inline]
                     pub fn copy_${longhand.ident}_from(&mut self, other: &Self) {
                         self.${longhand.ident} = other.${longhand.ident}.clone();
                     }
                     % if longhand.need_clone:
+                        /// Get the computed value for ${longhand.name}.
                         #[allow(non_snake_case)]
                         #[inline]
                         pub fn clone_${longhand.ident}(&self) -> longhands::${longhand.ident}::computed_value::T {
@@ -1084,11 +1169,14 @@ pub mod style_structs {
                     % endif
                 % endif
                 % if longhand.need_index:
+                    /// If this longhand is indexed, get the number of elements.
                     #[allow(non_snake_case)]
                     pub fn ${longhand.ident}_count(&self) -> usize {
                         self.${longhand.ident}.0.len()
                     }
 
+                    /// If this longhand is indexed, get the element at given
+                    /// index.
                     #[allow(non_snake_case)]
                     pub fn ${longhand.ident}_at(&self, index: usize)
                         -> longhands::${longhand.ident}::computed_value::SingleComputedValue {
@@ -1098,14 +1186,18 @@ pub mod style_structs {
             % endfor
             % if style_struct.name == "Border":
                 % for side in ["top", "right", "bottom", "left"]:
+                    /// Whether the border-${side} property has nonzero width.
                     #[allow(non_snake_case)]
                     pub fn border_${side}_has_nonzero_width(&self) -> bool {
                         self.border_${side}_width != ::app_units::Au(0)
                     }
                 % endfor
             % elif style_struct.name == "Font":
+                /// Computes a font hash in order to be able to cache fonts
+                /// effectively in GFX and layout.
                 pub fn compute_font_hash(&mut self) {
-                    // Corresponds to the fields in `gfx::font_template::FontTemplateDescriptor`.
+                    // Corresponds to the fields in
+                    // `gfx::font_template::FontTemplateDescriptor`.
                     let mut hasher: FnvHasher = Default::default();
                     hasher.write_u16(self.font_weight as u16);
                     self.font_stretch.hash(&mut hasher);
@@ -1113,20 +1205,26 @@ pub mod style_structs {
                     self.hash = hasher.finish()
                 }
             % elif style_struct.name == "Outline":
+                /// Whether the outline-width property is non-zero.
                 #[inline]
                 pub fn outline_has_nonzero_width(&self) -> bool {
                     self.outline_width != ::app_units::Au(0)
                 }
             % elif style_struct.name == "Text":
                 <% text_decoration_field = 'text_decoration' if product == 'servo' else 'text_decoration_line' %>
+                /// Whether the text decoration has an underline.
                 #[inline]
                 pub fn has_underline(&self) -> bool {
                     self.${text_decoration_field}.underline
                 }
+
+                /// Whether the text decoration has an overline.
                 #[inline]
                 pub fn has_overline(&self) -> bool {
                     self.${text_decoration_field}.overline
                 }
+
+                /// Whether the text decoration has a line through.
                 #[inline]
                 pub fn has_line_through(&self) -> bool {
                     self.${text_decoration_field}.line_through
@@ -1141,6 +1239,7 @@ pub mod style_structs {
     impl style_structs::${style_struct.name} {
         % for longhand in style_struct.longhands:
             % if longhand.need_index:
+                /// Iterate over the values of ${longhand.name}.
                 #[allow(non_snake_case)]
                 #[inline]
                 pub fn ${longhand.ident}_iter(&self) -> ${longhand.camel_case}Iter {
@@ -1151,6 +1250,7 @@ pub mod style_structs {
                     }
                 }
 
+                /// Get a value mod `index` for the property ${longhand.name}.
                 #[allow(non_snake_case)]
                 #[inline]
                 pub fn ${longhand.ident}_mod(&self, index: usize)
@@ -1163,6 +1263,7 @@ pub mod style_structs {
 
     % for longhand in style_struct.longhands:
         % if longhand.need_index:
+            /// An iterator over the values of the ${longhand.name} properties.
             pub struct ${longhand.camel_case}Iter<'a> {
                 style_struct: &'a style_structs::${style_struct.name},
                 current: usize,
@@ -1189,9 +1290,16 @@ pub mod style_structs {
 #[cfg(feature = "gecko")]
 pub use gecko_properties::ComputedValues;
 
+/// A legacy alias for a servo-version of ComputedValues. Should go away soon.
 #[cfg(feature = "servo")]
 pub type ServoComputedValues = ComputedValues;
 
+/// The struct that Servo uses to represent computed values.
+///
+/// This struct contains an immutable atomically-reference-counted pointer to
+/// every kind of style struct.
+///
+/// When needed, the structs may be copied in order to get mutated.
 #[cfg(feature = "servo")]
 #[cfg_attr(feature = "servo", derive(Clone, Debug))]
 pub struct ComputedValues {
@@ -1228,40 +1336,50 @@ impl ComputedValues {
         }
     }
 
+    /// Get the initial computed values.
     pub fn initial_values() -> &'static Self { &*INITIAL_SERVO_VALUES }
 
-    #[inline]
-    pub fn do_cascade_property<F: FnOnce(&[CascadePropertyFn])>(f: F) {
-        f(&CASCADE_PROPERTY)
-    }
-
     % for style_struct in data.active_style_structs():
+        /// Clone the ${style_struct.name} struct.
         #[inline]
         pub fn clone_${style_struct.name_lower}(&self) -> Arc<style_structs::${style_struct.name}> {
-                self.${style_struct.ident}.clone()
+            self.${style_struct.ident}.clone()
-            }
+        }
+
+        /// Get a immutable reference to the ${style_struct.name} struct.
         #[inline]
         pub fn get_${style_struct.name_lower}(&self) -> &style_structs::${style_struct.name} {
             &self.${style_struct.ident}
         }
+
+        /// Get a mutable reference to the ${style_struct.name} struct.
         #[inline]
         pub fn mutate_${style_struct.name_lower}(&mut self) -> &mut style_structs::${style_struct.name} {
             Arc::make_mut(&mut self.${style_struct.ident})
         }
     % endfor
 
-    // Cloning the Arc here is fine because it only happens in the case where we have custom
+    /// Get the custom properties map if necessary.
-    // properties, and those are both rare and expensive.
+    ///
-    pub fn custom_properties(&self) -> Option<Arc<::custom_properties::ComputedValuesMap>> {
+    /// Cloning the Arc here is fine because it only happens in the case where
+    /// we have custom properties, and those are both rare and expensive.
+    fn custom_properties(&self) -> Option<Arc<::custom_properties::ComputedValuesMap>> {
         self.custom_properties.as_ref().map(|x| x.clone())
     }
 
+    /// Whether this style has a -moz-binding value. This is always false for
+    /// Servo for obvious reasons.
     pub fn has_moz_binding(&self) -> bool { false }
 
-    pub fn root_font_size(&self) -> Au { self.root_font_size }
+    /// Get the root font size.
-    pub fn set_root_font_size(&mut self, size: Au) { self.root_font_size = size }
+    fn root_font_size(&self) -> Au { self.root_font_size }
+
+    /// Set the root font size.
+    fn set_root_font_size(&mut self, size: Au) { self.root_font_size = size }
+    /// Set the writing mode for this style.
     pub fn set_writing_mode(&mut self, mode: WritingMode) { self.writing_mode = mode; }
 
+    /// Whether the current style is multicolumn.
     #[inline]
     pub fn is_multicol(&self) -> bool {
         let style = self.get_column();
@@ -1272,8 +1390,9 @@ impl ComputedValues {
     }
 
     /// Resolves the currentColor keyword.
-    /// Any color value form computed values (except for the 'color' property itself)
+    ///
-    /// should go through this method.
+    /// Any color value from computed values (except for the 'color' property
+    /// itself) should go through this method.
     ///
     /// Usage example:
     /// let top_color = style.resolve_color(style.Border.border_top_color);
@@ -1285,6 +1404,7 @@ impl ComputedValues {
         }
     }
 
+    /// Get the logical computed inline size.
     #[inline]
     pub fn content_inline_size(&self) -> computed::LengthOrPercentageOrAuto {
         let position_style = self.get_position();
@@ -1295,36 +1415,42 @@ impl ComputedValues {
         }
     }
 
+    /// Get the logical computed block size.
     #[inline]
     pub fn content_block_size(&self) -> computed::LengthOrPercentageOrAuto {
         let position_style = self.get_position();
         if self.writing_mode.is_vertical() { position_style.width } else { position_style.height }
     }
 
+    /// Get the logical computed min inline size.
     #[inline]
     pub fn min_inline_size(&self) -> computed::LengthOrPercentage {
         let position_style = self.get_position();
         if self.writing_mode.is_vertical() { position_style.min_height } else { position_style.min_width }
     }
 
+    /// Get the logical computed min block size.
     #[inline]
     pub fn min_block_size(&self) -> computed::LengthOrPercentage {
         let position_style = self.get_position();
         if self.writing_mode.is_vertical() { position_style.min_width } else { position_style.min_height }
     }
 
+    /// Get the logical computed max inline size.
     #[inline]
     pub fn max_inline_size(&self) -> computed::LengthOrPercentageOrNone {
         let position_style = self.get_position();
         if self.writing_mode.is_vertical() { position_style.max_height } else { position_style.max_width }
     }
 
+    /// Get the logical computed max block size.
     #[inline]
     pub fn max_block_size(&self) -> computed::LengthOrPercentageOrNone {
         let position_style = self.get_position();
         if self.writing_mode.is_vertical() { position_style.max_width } else { position_style.max_height }
     }
 
+    /// Get the logical computed padding for this writing mode.
     #[inline]
     pub fn logical_padding(&self) -> LogicalMargin<computed::LengthOrPercentage> {
         let padding_style = self.get_padding();
@@ -1336,6 +1462,7 @@ impl ComputedValues {
         ))
     }
 
+    /// Get the logical border width
     #[inline]
     pub fn border_width_for_writing_mode(&self, writing_mode: WritingMode) -> LogicalMargin<Au> {
         let border_style = self.get_border();
@@ -1347,11 +1474,13 @@ impl ComputedValues {
         ))
     }
 
+    /// Gets the logical computed border widths for this style.
     #[inline]
     pub fn logical_border_width(&self) -> LogicalMargin<Au> {
         self.border_width_for_writing_mode(self.writing_mode)
     }
 
+    /// Gets the logical computed margin from this style.
     #[inline]
     pub fn logical_margin(&self) -> LogicalMargin<computed::LengthOrPercentageOrAuto> {
         let margin_style = self.get_margin();
@@ -1363,6 +1492,7 @@ impl ComputedValues {
         ))
     }
 
+    /// Gets the logical position from this style.
     #[inline]
     pub fn logical_position(&self) -> LogicalMargin<computed::LengthOrPercentageOrAuto> {
         // FIXME(SimonSapin): should be the writing mode of the containing block, maybe?
@@ -1375,7 +1505,7 @@ impl ComputedValues {
         ))
     }
 
-    // http://dev.w3.org/csswg/css-transforms/#grouping-property-values
+    /// https://drafts.csswg.org/css-transforms/#grouping-property-values
     pub fn get_used_transform_style(&self) -> computed_values::transform_style::T {
         use computed_values::mix_blend_mode;
         use computed_values::transform_style;
@@ -1404,6 +1534,8 @@ impl ComputedValues {
         box_.transform_style
     }
 
+    /// Whether given this transform value, the compositor would require a
+    /// layer.
     pub fn transform_requires_layer(&self) -> bool {
         // Check if the transform matrix is 2D or 3D
         if let Some(ref transform_list) = self.get_box().transform.0 {
@@ -1436,6 +1568,7 @@ impl ComputedValues {
         false
     }
 
+    /// Serializes the computed value of this property as a string.
     pub fn computed_value_to_string(&self, property: PropertyDeclarationId) -> String {
         match property {
             % for style_struct in data.active_style_structs():
@@ -1504,6 +1637,7 @@ pub use self::lazy_static_module::INITIAL_SERVO_VALUES;
 
 // Use a module to work around #[cfg] on lazy_static! not being applied to every generated item.
 #[cfg(feature = "servo")]
+#[allow(missing_docs)]
 mod lazy_static_module {
     use logical_geometry::WritingMode;
     use std::sync::Arc;
@@ -1530,6 +1664,7 @@ mod lazy_static_module {
     }
 }
 
+/// A per-longhand function that performs the CSS cascade for that longhand.
 pub type CascadePropertyFn =
     extern "Rust" fn(declaration: &PropertyDeclaration,
                      inherited_style: &ComputedValues,
@@ -1539,7 +1674,8 @@ pub type CascadePropertyFn =
                      cascade_info: &mut Option<<&mut CascadeInfo>,
                      error_reporter: &mut StdBox<ParseErrorReporter + Send>);
 
-#[cfg(feature = "servo")]
+/// A per-longhand array of functions to perform the CSS cascade on each of
+/// them, effectively doing virtual dispatch.
 static CASCADE_PROPERTY: [CascadePropertyFn; ${len(data.longhands)}] = [
     % for property in data.longhands:
         longhands::${property.ident}::cascade_property,
@@ -1547,14 +1683,16 @@ static CASCADE_PROPERTY: [CascadePropertyFn; ${len(data.longhands)}] = [
 ];
 
 bitflags! {
+    /// A set of flags to tweak the behavior of the `cascade` function.
     pub flags CascadeFlags: u8 {
-        /// Whether the `ComputedValues` structure to be constructed should be considered
+        /// Whether the `ComputedValues` structure to be constructed should be
-        /// shareable.
+        /// considered shareable.
         const SHAREABLE = 0x01,
-        /// Whether to inherit all styles from the parent. If this flag is not present,
+        /// Whether to inherit all styles from the parent. If this flag is not
-        /// non-inherited styles are reset to their initial values.
+        /// present, non-inherited styles are reset to their initial values.
         const INHERIT_ALL = 0x02,
-        /// Whether to skip any root element and flex/grid item display style fixup.
+        /// Whether to skip any root element and flex/grid item display style
+        /// fixup.
         const SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP = 0x04,
     }
 }
@@ -1624,7 +1762,8 @@ pub fn apply_declarations<'a, F, I>(viewport_size: Size2D<Au>,
                                     font_metrics_provider: Option<<&FontMetricsProvider>,
                                     flags: CascadeFlags)
                                     -> ComputedValues
-    where F: Fn() -> I, I: Iterator<Item = &'a PropertyDeclaration>
+    where F: Fn() -> I,
+          I: Iterator<Item = &'a PropertyDeclaration>,
 {
     let inherited_custom_properties = inherited_style.custom_properties();
     let mut custom_properties = None;
@@ -1680,6 +1819,9 @@ pub fn apply_declarations<'a, F, I>(viewport_size: Size2D<Au>,
 
     // Set computed values, overwriting earlier declarations for the same
     // property.
+    //
+    // NB: The cacheable boolean is not used right now, but will be once we
+    // start caching computed values in the rule nodes.
     let mut cacheable = true;
     let mut seen = PropertyBitField::new();
 
@@ -1691,60 +1833,58 @@ pub fn apply_declarations<'a, F, I>(viewport_size: Size2D<Au>,
     //
     // To improve i-cache behavior, we outline the individual functions and use
     // virtual dispatch instead.
-    ComputedValues::do_cascade_property(|cascade_property| {
+    % for category_to_cascade_now in ["early", "other"]:
-        % for category_to_cascade_now in ["early", "other"]:
+        for declaration in iter_declarations() {
-            for declaration in iter_declarations() {
+            let longhand_id = match declaration.id() {
-                let longhand_id = match declaration.id() {
+                PropertyDeclarationId::Longhand(id) => id,
-                    PropertyDeclarationId::Longhand(id) => id,
+                PropertyDeclarationId::Custom(..) => continue,
-                    PropertyDeclarationId::Custom(..) => continue,
+            };
-                };
 
-                // The computed value of some properties depends on the
+            // The computed value of some properties depends on the
-                // (sometimes computed) value of *other* properties.
+            // (sometimes computed) value of *other* properties.
-                //
+            //
-                // So we classify properties into "early" and "other", such that
+            // So we classify properties into "early" and "other", such that
-                // the only dependencies can be from "other" to "early".
+            // the only dependencies can be from "other" to "early".
-                //
+            //
-                // We iterate applicable_declarations twice, first cascading
+            // We iterate applicable_declarations twice, first cascading
-                // "early" properties then "other".
+            // "early" properties then "other".
-                //
+            //
-                // Unfortunately, it’s not easy to check that this
+            // Unfortunately, it’s not easy to check that this
-                // classification is correct.
+            // classification is correct.
-                let is_early_property = matches!(*declaration,
+            let is_early_property = matches!(*declaration,
-                    PropertyDeclaration::FontSize(_) |
+                PropertyDeclaration::FontSize(_) |
-                    PropertyDeclaration::FontFamily(_) |
+                PropertyDeclaration::FontFamily(_) |
-                    PropertyDeclaration::Color(_) |
+                PropertyDeclaration::Color(_) |
-                    PropertyDeclaration::Position(_) |
+                PropertyDeclaration::Position(_) |
-                    PropertyDeclaration::Float(_) |
+                PropertyDeclaration::Float(_) |
-                    PropertyDeclaration::TextDecoration${'' if product == 'servo' else 'Line'}(_) |
+                PropertyDeclaration::TextDecoration${'' if product == 'servo' else 'Line'}(_) |
-                    PropertyDeclaration::WritingMode(_) |
+                PropertyDeclaration::WritingMode(_) |
-                    PropertyDeclaration::Direction(_) |
+                PropertyDeclaration::Direction(_) |
-                    PropertyDeclaration::TextOrientation(_)
+                PropertyDeclaration::TextOrientation(_)
-                );
+            );
-                if
+            if
-                    % if category_to_cascade_now == "early":
+                % if category_to_cascade_now == "early":
-                        !
+                    !
-                    % endif
+                % endif
-                    is_early_property
+                is_early_property
-                {
+            {
-                    continue
+                continue
-                }
-
-                let discriminant = longhand_id as usize;
-                (cascade_property[discriminant])(declaration,
-                                                 inherited_style,
-                                                 &mut context,
-                                                 &mut seen,
-                                                 &mut cacheable,
-                                                 &mut cascade_info,
-                                                 &mut error_reporter);
             }
-            % if category_to_cascade_now == "early":
+
-                let mode = get_writing_mode(context.style.get_inheritedbox());
+            let discriminant = longhand_id as usize;
-                context.style.set_writing_mode(mode);
+            (CASCADE_PROPERTY[discriminant])(declaration,
-            % endif
+                                             inherited_style,
-        % endfor
+                                             &mut context,
-    });
+                                             &mut seen,
+                                             &mut cacheable,
+                                             &mut cascade_info,
+                                             &mut error_reporter);
+        }
+        % if category_to_cascade_now == "early":
+            let mode = get_writing_mode(context.style.get_inheritedbox());
+            context.style.set_writing_mode(mode);
+        % endif
+    % endfor
 
     let mut style = context.style;
 
@@ -1852,6 +1992,9 @@ pub fn apply_declarations<'a, F, I>(viewport_size: Size2D<Au>,
 
 
     % if product == "gecko":
+        // FIXME(emilio): This is effectively creating a new nsStyleBackground
+        // and nsStyleSVG per element. We should only do this when necessary
+        // using the `seen` bitfield!
         style.mutate_background().fill_arrays();
         style.mutate_svg().fill_arrays();
     % endif

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/scoped_tls.rs

@@ -2,22 +2,31 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+//! Stack-scoped thread-local storage for rayon thread pools.
+
 #![allow(unsafe_code)]
+#![deny(missing_docs)]
 
 use rayon;
 use std::cell::{Ref, RefCell, RefMut};
 
-/// Stack-scoped thread-local storage for rayon thread pools.
+/// A scoped TLS set, that is alive during the `'scope` lifetime.
-
+///
-pub struct ScopedTLS<'a, T: Send> {
+/// We use this on Servo to construct thread-local contexts, but clear them once
-    pool: &'a rayon::ThreadPool,
+/// we're done with restyling.
+pub struct ScopedTLS<'scope, T: Send> {
+    pool: &'scope rayon::ThreadPool,
     slots: Box<[RefCell<Option<T>>]>,
 }
 
-unsafe impl<'a, T: Send> Sync for ScopedTLS<'a, T> {}
+/// The scoped TLS is `Sync` because no more than one worker thread can access a
+/// given slot.
+unsafe impl<'scope, T: Send> Sync for ScopedTLS<'scope, T> {}
 
-impl<'a, T: Send> ScopedTLS<'a, T> {
+impl<'scope, T: Send> ScopedTLS<'scope, T> {
-    pub fn new(p: &'a rayon::ThreadPool) -> Self {
+    /// Create a new scoped TLS that will last as long as this rayon threadpool
+    /// reference.
+    pub fn new(p: &'scope rayon::ThreadPool) -> Self {
         let count = p.num_threads();
         let mut v = Vec::with_capacity(count);
         for _ in 0..count {
@@ -30,16 +39,20 @@ impl<'a, T: Send> ScopedTLS<'a, T> {
         }
     }
 
+    /// Return an immutable reference to the `Option<T>` that this thread owns.
     pub fn borrow(&self) -> Ref<Option<T>> {
         let idx = self.pool.current_thread_index().unwrap();
         self.slots[idx].borrow()
     }
 
+    /// Return a mutable reference to the `Option<T>` that this thread owns.
     pub fn borrow_mut(&self) -> RefMut<Option<T>> {
         let idx = self.pool.current_thread_index().unwrap();
         self.slots[idx].borrow_mut()
     }
 
+    /// Ensure that the current data this thread owns is initialized, or
+    /// initialize it using `f`.
     pub fn ensure<F: FnOnce() -> T>(&self, f: F) -> RefMut<T> {
         let mut opt = self.borrow_mut();
         if opt.is_none() {

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/servo/mod.rs

@@ -2,5 +2,9 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+//! Servo-specific bits of the style system.
+//!
+//! These get compiled out on a Gecko build.
+
 pub mod restyle_damage;
 pub mod selector_parser;

components/style/servo/restyle_damage.rs

@@ -2,6 +2,11 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+//! The restyle damage is a hint that tells layout which kind of operations may
+//! be needed in presence of incremental style changes.
+
+#![deny(missing_docs)]
+
 use computed_values::display;
 use heapsize::HeapSizeOf;
 use properties::ServoComputedValues;
@@ -53,6 +58,8 @@ impl HeapSizeOf for ServoRestyleDamage {
 }
 
 impl ServoRestyleDamage {
+    /// Compute the appropriate restyle damage for a given style change between
+    /// `old` and `new`.
     pub fn compute(old: &Arc<ServoComputedValues>,
                    new: &Arc<ServoComputedValues>) -> ServoRestyleDamage {
         compute_damage(old, new)
@@ -60,10 +67,6 @@ impl ServoRestyleDamage {
 
     /// Returns a bitmask that represents a flow that needs to be rebuilt and
     /// reflowed.
-    ///
-    /// Use this instead of `ServoRestyleDamage::all()` because
-    /// `ServoRestyleDamage::all()` will result in unnecessary sequential resolution
-    /// of generated content.
     pub fn rebuild_and_reflow() -> ServoRestyleDamage {
         REPAINT | REPOSITION | STORE_OVERFLOW | BUBBLE_ISIZES | REFLOW_OUT_OF_FLOW | REFLOW |
             RECONSTRUCT_FLOW
@@ -81,8 +84,8 @@ impl ServoRestyleDamage {
         }
     }
 
-    /// Supposing the *parent* of a flow with the given `position` property has this damage,
+    /// Supposing the *parent* of a flow with the given `position` property has
-    /// returns the damage that we should add to this flow.
+    /// this damage, returns the damage that we should add to this flow.
     pub fn damage_for_child(self,
                             parent_is_absolutely_positioned: bool,
                             child_is_absolutely_positioned: bool)

components/style/sink.rs

@@ -2,10 +2,20 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+//! Small helpers to abstract over different containers.
+#![deny(missing_docs)]
+
 use smallvec::{Array, SmallVec};
 use std::marker::PhantomData;
 
+/// A trait to abstract over a `push` method that may be implemented for
+/// different kind of types.
+///
+/// Used to abstract over `Array`, `SmallVec` and `Vec`, and also to implement a
+/// type which `push` method does only tweak a byte when we only need to check
+/// for the presence of something.
 pub trait Push<T> {
+    /// Push a value into self.
     fn push(&mut self, value: T);
 }
 
@@ -21,13 +31,16 @@ impl<A: Array> Push<A::Item> for SmallVec<A> {
     }
 }
 
+/// A struct that implements `Push`, but only stores whether it's empty.
 pub struct ForgetfulSink<T>(bool, PhantomData<T>);
 
 impl<T> ForgetfulSink<T> {
+    /// Trivially construct a new `ForgetfulSink`.
     pub fn new() -> Self {
         ForgetfulSink(true, PhantomData)
     }
 
+    /// Whether this sink is empty or not.
     pub fn is_empty(&self) -> bool {
         self.0
     }

components/style/stylist.rs

@@ -4,10 +4,11 @@
 
 //! Selector matching.
 
+#![deny(missing_docs)]
+
 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};
@@ -28,7 +29,6 @@ use smallvec::VecLike;
 use std::borrow::Borrow;
 use std::collections::HashMap;
 use std::fmt;
-use std::hash::BuildHasherDefault;
 use std::hash::Hash;
 use std::slice;
 use std::sync::Arc;
@@ -36,7 +36,7 @@ use style_traits::viewport::ViewportConstraints;
 use stylesheets::{CssRule, Origin, StyleRule, Stylesheet, UserAgentStylesheets};
 use viewport::{self, MaybeNew, ViewportRule};
 
-pub type FnvHashMap<K, V> = HashMap<K, V, BuildHasherDefault<::fnv::FnvHasher>>;
+pub use ::fnv::FnvHashMap;
 
 /// This structure holds all the selectors and device characteristics
 /// for a given document. The selectors are converted into `Rule`s
@@ -83,6 +83,8 @@ pub struct Stylist {
     /// FIXME(emilio): Use the rule tree!
     precomputed_pseudo_element_decls: FnvHashMap<PseudoElement, Vec<ApplicableDeclarationBlock>>,
 
+    /// A monotonically increasing counter to represent the order on which a
+    /// style rule appears in a stylesheet, needed to sort them by source order.
     rules_source_order: usize,
 
     /// Selector dependencies used to compute restyle hints.
@@ -99,6 +101,7 @@ pub struct Stylist {
 }
 
 impl Stylist {
+    /// Construct a new `Stylist`, using a given `Device`.
     #[inline]
     pub fn new(device: Device) -> Self {
         let mut stylist = Stylist {
@@ -129,6 +132,12 @@ impl Stylist {
         stylist
     }
 
+    /// Update the stylist for the given document stylesheets, and optionally
+    /// with a set of user agent stylesheets.
+    ///
+    /// This method resets all the style data each time the stylesheets change
+    /// (which is indicated by the `stylesheets_changed` parameter), or the
+    /// device is dirty, which means we need to re-evaluate media queries.
     pub fn update(&mut self,
                   doc_stylesheets: &[Arc<Stylesheet>],
                   ua_stylesheets: Option<&UserAgentStylesheets>,
@@ -252,34 +261,16 @@ impl Stylist {
                 _ => {}
             }
         });
-
-        debug!("Stylist stats:");
-        debug!(" - Got {} sibling-affecting selectors",
-               self.sibling_affecting_selectors.len());
-        debug!(" - Got {} non-common-style-attribute-affecting selectors",
-               self.non_common_style_affecting_attributes_selectors.len());
-        debug!(" - Got {} deps for style-hint calculation",
-               self.state_deps.len());
-
-        SelectorImpl::each_precomputed_pseudo_element(|pseudo| {
-            // TODO: Consider not doing this and just getting the rules on the
-            // fly. It should be a bit slower, but we'd take rid of the
-            // extra field, and avoid this precomputation entirely.
-            if let Some(map) = self.pseudos_map.remove(&pseudo) {
-                let mut declarations = vec![];
-                map.user_agent.get_universal_rules(&mut declarations);
-                self.precomputed_pseudo_element_decls.insert(pseudo, declarations);
-            }
-        });
     }
 
 
     /// Computes the style for a given "precomputed" pseudo-element, taking the
     /// universal rules and applying them.
     ///
-    /// If `inherit_all` is true, then all properties are inherited from the parent; otherwise,
+    /// If `inherit_all` is true, then all properties are inherited from the
-    /// non-inherited properties are reset to their initial values. The flow constructor uses this
+    /// parent; otherwise, non-inherited properties are reset to their initial
-    /// flag when constructing anonymous flows.
+    /// values. The flow constructor uses this flag when constructing anonymous
+    /// flows.
     pub fn precomputed_values_for_pseudo(&self,
                                          pseudo: &PseudoElement,
                                          parent: Option<&Arc<ComputedValues>>,
@@ -291,7 +282,7 @@ impl Stylist {
             // use into_iter.
             let rule_node =
                 self.rule_tree.insert_ordered_rules(
-                    declarations.iter().map(|a| (a.source.clone(), a.importance)));
+                    declarations.into_iter().map(|a| (a.source.clone(), a.importance)));
 
             let mut flags = CascadeFlags::empty();
             if inherit_all {
@@ -339,6 +330,13 @@ impl Stylist {
             .values
     }
 
+    /// Computes a pseudo-element style lazily during layout.
+    ///
+    /// This can only be done for a certain set of pseudo-elements, like
+    /// :selection.
+    ///
+    /// Check the documentation on lazy pseudo-elements in
+    /// docs/components/style.md
     pub fn lazily_compute_pseudo_element_style<E>(&self,
                                                   element: &E,
                                                   pseudo: &PseudoElement,
@@ -363,7 +361,8 @@ impl Stylist {
                                           MatchingReason::ForStyling);
 
         let rule_node =
-            self.rule_tree.insert_ordered_rules(declarations.into_iter().map(|a| (a.source.clone(), a.importance)));
+            self.rule_tree.insert_ordered_rules(
+                declarations.into_iter().map(|a| (a.source, a.importance)));
 
         let computed =
             properties::cascade(self.device.au_viewport_size(),
@@ -376,6 +375,11 @@ impl Stylist {
         Some(ComputedStyle::new(rule_node, Arc::new(computed)))
     }
 
+    /// Set a given device, which may change the styles that apply to the
+    /// document.
+    ///
+    /// This means that we may need to rebuild style data even if the
+    /// stylesheets haven't changed.
     pub fn set_device(&mut self, mut device: Device, stylesheets: &[Arc<Stylesheet>]) {
         let cascaded_rule = ViewportRule {
             declarations: viewport::Cascade::from_stylesheets(stylesheets, &device).finish(),
@@ -383,6 +387,9 @@ impl Stylist {
 
         self.viewport_constraints = ViewportConstraints::maybe_new(device.viewport_size, &cascaded_rule);
         if let Some(ref constraints) = self.viewport_constraints {
+            // FIXME(emilio): creating a device here works, but is not really
+            // appropriate. I should get rid of this while doing the stylo media
+            // query work.
             device = Device::new(MediaType::Screen, constraints.size);
         }
 
@@ -408,21 +415,29 @@ impl Stylist {
         self.device = Arc::new(device);
     }
 
-    pub fn viewport_constraints(&self) -> &Option<ViewportConstraints> {
+    /// Returns the viewport constraints that apply to this document because of
-        &self.viewport_constraints
+    /// a @viewport rule.
+    pub fn viewport_constraints(&self) -> Option<&ViewportConstraints> {
+        self.viewport_constraints.as_ref()
     }
 
+    /// Sets the quirks mode of the document.
     pub fn set_quirks_mode(&mut self, enabled: bool) {
+        // FIXME(emilio): We don't seem to change the quirks mode dynamically
+        // during multiple layout passes, but this is totally bogus, in the
+        // sense that it's updated asynchronously.
+        //
+        // This should probably be an argument to `update`, and use the quirks
+        // mode info in the `SharedLayoutContext`.
         self.quirks_mode = enabled;
     }
 
     /// Returns the applicable CSS declarations for the given element.
+    ///
     /// This corresponds to `ElementRuleCollector` in WebKit.
     ///
-    /// The returned boolean indicates whether the style is *shareable*;
+    /// The returned `StyleRelations` indicate hints about which kind of rules
-    /// that is, whether the matched selectors are simple enough to allow the
+    /// have matched.
-    /// matching logic to be reduced to the logic in
-    /// `css::matching::PrivateMatchMethods::candidate_element_allows_for_style_sharing`.
     #[allow(unsafe_code)]
     pub fn push_applicable_declarations<E, V>(
                                         &self,
@@ -549,20 +564,28 @@ impl Stylist {
         relations
     }
 
+    /// Return whether the device is dirty, that is, whether the screen size or
+    /// media type have changed (for now).
     #[inline]
     pub fn is_device_dirty(&self) -> bool {
         self.is_device_dirty
     }
 
+    /// Returns the map of registered `@keyframes` animations.
     #[inline]
     pub fn animations(&self) -> &FnvHashMap<Atom, KeyframesAnimation> {
         &self.animations
     }
 
+    /// Whether two elements match the same not-common style-affecting attribute
+    /// rules.
+    ///
+    /// This is used to test elements and candidates in the style-sharing
+    /// candidate cache.
     pub fn match_same_not_common_style_affecting_attributes_rules<E>(&self,
                                                                      element: &E,
                                                                      candidate: &E) -> bool
-    where E: ElementExt
+        where E: ElementExt,
     {
         use selectors::matching::StyleRelations;
         use selectors::matching::matches_complex_selector;
@@ -591,15 +614,19 @@ impl Stylist {
         true
     }
 
+    /// Returns the rule root node.
     #[inline]
     pub fn rule_tree_root(&self) -> StrongRuleNode {
         self.rule_tree.root()
     }
 
+    /// Returns whether two elements match the same sibling-affecting rules.
+    ///
+    /// This is also for the style sharing candidate cache.
     pub fn match_same_sibling_affecting_rules<E>(&self,
                                                  element: &E,
                                                  candidate: &E) -> bool
-    where E: ElementExt
+        where E: ElementExt,
     {
         use selectors::matching::StyleRelations;
         use selectors::matching::matches_complex_selector;
@@ -629,17 +656,16 @@ impl Stylist {
         true
     }
 
-    pub fn compute_restyle_hint<E>(&self, element: &E,
+    /// Given an element, and a snapshot that represents a previous state of the
-                                   snapshot: &Snapshot,
+    /// element, compute the appropriate restyle hint, that is, the kind of
-                                   // NB: We need to pass current_state as an argument because
+    /// restyle we need to do.
-                                   // selectors::Element doesn't provide access to ElementState
+    pub fn compute_restyle_hint<E>(&self,
-                                   // directly, and computing it from the ElementState would be
+                                   element: &E,
-                                   // more expensive than getting it directly from the caller.
+                                   snapshot: &Snapshot)
-                                   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)
     }
 }
 
@@ -651,6 +677,9 @@ impl Drop for Stylist {
         // dropped all strong rule node references before now, then we will
         // leak them, since there will be no way to call gc() on the rule tree
         // after this point.
+        //
+        // TODO(emilio): We can at least assert all the elements in the free
+        // list are indeed free.
         unsafe { self.rule_tree.gc(); }
     }
 }
@@ -707,11 +736,15 @@ impl PerPseudoElementSelectorMap {
 /// Hence, the union of the rules keyed on each of element's classes, ID,
 /// element name, etc. will contain the Rules that actually match that
 /// element.
+///
+/// TODO: Tune the initial capacity of the HashMap
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub struct SelectorMap {
-    // TODO: Tune the initial capacity of the HashMap
+    /// A hash from an ID to rules which contain that ID selector.
     pub id_hash: FnvHashMap<Atom, Vec<Rule>>,
+    /// A hash from a class name to rules which contain that class selector.
     pub class_hash: FnvHashMap<Atom, Vec<Rule>>,
+    /// A hash from local name to rules which contain that local name selector.
     pub local_name_hash: FnvHashMap<LocalName, Vec<Rule>>,
     /// Same as local_name_hash, but keys are lower-cased.
     /// For HTML elements in HTML documents.
@@ -728,6 +761,7 @@ fn sort_by_key<T, F: Fn(&T) -> K, K: Ord>(v: &mut [T], f: F) {
 }
 
 impl SelectorMap {
+    /// Trivially constructs an empty `SelectorMap`.
     pub fn new() -> Self {
         SelectorMap {
             id_hash: HashMap::default(),
@@ -960,17 +994,28 @@ impl SelectorMap {
     }
 }
 
+/// A rule, that wraps a style rule, but represents a single selector of the
+/// rule.
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 #[derive(Clone, Debug)]
 pub struct Rule {
-    // This is an Arc because Rule will essentially be cloned for every element
+    /// The selector this struct represents.
-    // that it matches. Selector contains an owned vector (through
+    /// This is an Arc because Rule will essentially be cloned for every element
-    // ComplexSelector) and we want to avoid the allocation.
+    /// that it matches. Selector contains an owned vector (through
+    /// ComplexSelector) and we want to avoid the allocation.
+    ///
+    /// FIXME(emilio): We should be able to get rid of it and just use the style
+    /// rule? This predates the time where the rule was in `selectors`, and the
+    /// style rule was a generic parameter to it. It's not trivial though, due
+    /// to the specificity.
     #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")]
     pub selector: Arc<ComplexSelector<SelectorImpl>>,
+    /// The actual style rule.
     #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")]
     pub style_rule: Arc<RwLock<StyleRule>>,
+    /// The source order this style rule appears in.
     pub source_order: usize,
+    /// The specificity of the rule this selector represents.
     pub specificity: u32,
 }
 
@@ -985,19 +1030,28 @@ impl Rule {
     }
 }
 
-/// A property declaration together with its precedence among rules of equal specificity so that
+/// A property declaration together with its precedence among rules of equal
-/// we can sort them.
+/// specificity so that we can sort them.
+///
+/// This represents the declarations in a given declaration block for a given
+/// importance.
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 #[derive(Debug, Clone)]
 pub struct ApplicableDeclarationBlock {
+    /// The style source, either a style rule, or a property declaration block.
     #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")]
     pub source: StyleSource,
+    /// The importance of this declaration block.
     pub importance: Importance,
+    /// The source order of this block.
     pub source_order: usize,
+    /// The specificity of the selector this block is represented by.
     pub specificity: u32,
 }
 
 impl ApplicableDeclarationBlock {
+    /// Constructs an applicable declaration block from a given property
+    /// declaration block and importance.
     #[inline]
     pub fn from_declarations(declarations: Arc<RwLock<PropertyDeclarationBlock>>,
                              importance: Importance)
@@ -1011,6 +1065,8 @@ impl ApplicableDeclarationBlock {
     }
 }
 
+/// An iterator over the declarations that a given block represent, which is
+/// effectively a filter by importance.
 pub struct ApplicableDeclarationBlockIter<'a> {
     iter: slice::Iter<'a, (PropertyDeclaration, Importance)>,
     importance: Importance,