From 577bd152772d61088cd31209e29cdf3e32cbdbb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 1 Jan 2017 18:44:11 +0100 Subject: [PATCH 01/44] style: remove ComputedStyle::do_cascade_property. It's defined to be the same in both Servo and Gecko, so it's pointless right now. --- components/style/properties/gecko.mako.rs | 12 --- .../style/properties/properties.mako.rs | 102 +++++++++--------- 2 files changed, 50 insertions(+), 64 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 5e1b906a4f7..746ae31f2e5 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -50,7 +50,6 @@ 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 +145,6 @@ impl ComputedValues { set_raw_initial_values(ptr::null_mut()); } - #[inline] - pub fn do_cascade_property(f: F) { - f(&CASCADE_PROPERTY) - } - % for style_struct in data.style_structs: #[inline] pub fn clone_${style_struct.name_lower}(&self) -> Arc { @@ -2663,9 +2657,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 -]; diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 9d7247130f2..e27731c433a 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -1691,60 +1691,58 @@ pub fn apply_declarations<'a, F, I>(viewport_size: Size2D, // // 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 declaration in iter_declarations() { - let longhand_id = match declaration.id() { - PropertyDeclarationId::Longhand(id) => id, - PropertyDeclarationId::Custom(..) => continue, - }; + % for category_to_cascade_now in ["early", "other"]: + for declaration in iter_declarations() { + let longhand_id = match declaration.id() { + PropertyDeclarationId::Longhand(id) => id, + PropertyDeclarationId::Custom(..) => continue, + }; - // The computed value of some properties depends on the - // (sometimes computed) value of *other* properties. - // - // So we classify properties into "early" and "other", such that - // the only dependencies can be from "other" to "early". - // - // We iterate applicable_declarations twice, first cascading - // "early" properties then "other". - // - // Unfortunately, it’s not easy to check that this - // classification is correct. - let is_early_property = matches!(*declaration, - PropertyDeclaration::FontSize(_) | - PropertyDeclaration::FontFamily(_) | - PropertyDeclaration::Color(_) | - PropertyDeclaration::Position(_) | - PropertyDeclaration::Float(_) | - PropertyDeclaration::TextDecoration${'' if product == 'servo' else 'Line'}(_) | - PropertyDeclaration::WritingMode(_) | - PropertyDeclaration::Direction(_) | - PropertyDeclaration::TextOrientation(_) - ); - if - % if category_to_cascade_now == "early": - ! - % endif - is_early_property - { - 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); + // The computed value of some properties depends on the + // (sometimes computed) value of *other* properties. + // + // So we classify properties into "early" and "other", such that + // the only dependencies can be from "other" to "early". + // + // We iterate applicable_declarations twice, first cascading + // "early" properties then "other". + // + // Unfortunately, it’s not easy to check that this + // classification is correct. + let is_early_property = matches!(*declaration, + PropertyDeclaration::FontSize(_) | + PropertyDeclaration::FontFamily(_) | + PropertyDeclaration::Color(_) | + PropertyDeclaration::Position(_) | + PropertyDeclaration::Float(_) | + PropertyDeclaration::TextDecoration${'' if product == 'servo' else 'Line'}(_) | + PropertyDeclaration::WritingMode(_) | + PropertyDeclaration::Direction(_) | + PropertyDeclaration::TextOrientation(_) + ); + if + % if category_to_cascade_now == "early": + ! + % endif + is_early_property + { + continue } - % if category_to_cascade_now == "early": - let mode = get_writing_mode(context.style.get_inheritedbox()); - context.style.set_writing_mode(mode); - % endif - % endfor - }); + + 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()); + context.style.set_writing_mode(mode); + % endif + % endfor let mut style = context.style; From b556449a968c316537b80bcd704c2a0a1f36a091 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 1 Jan 2017 18:47:02 +0100 Subject: [PATCH 02/44] style: Require docs in the less-straightforward part of the properties module. --- components/style/lib.rs | 1 + components/style/properties/helpers.mako.rs | 17 +- .../style/properties/properties.mako.rs | 241 ++++++++++++++---- 3 files changed, 207 insertions(+), 52 deletions(-) diff --git a/components/style/lib.rs b/components/style/lib.rs index 53d4e97ee3f..30d585e3261 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -148,6 +148,7 @@ use style_traits::ToCss; // 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")); } diff --git a/components/style/properties/helpers.mako.rs b/components/style/properties/helpers.mako.rs index 83feda735c2..e1fac491910 100644 --- a/components/style/properties/helpers.mako.rs +++ b/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, @@ -617,16 +618,22 @@ <%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, - wm: WritingMode) { + v: longhands::${to_rust_ident(name)}::computed_value::T, + wm: WritingMode) { <%self:logical_setter_helper name="${name}"> <%def name="inner(physical_ident)"> self.set_${physical_ident}(v) } - 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 @@ } % 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}"> diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index e27731c433a..75ea5dff3d5 100644 --- a/components/style/properties/properties.mako.rs +++ b/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(input: &mut Parser, parse_one: F) -> Result<(T, T, T, T), ()> - where F: Fn(&mut Parser) -> Result, T: Clone + where F: Fn(&mut Parser) -> Result, + 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 { } - #[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}( value: &DeclaredValue, @@ -268,7 +282,8 @@ mod property_bit_field { f: F, error_reporter: &mut StdBox, extra_data: ParserContextExtraData) - where F: FnOnce(&DeclaredValue) { + where F: FnOnce(&DeclaredValue) + { 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 bool>(v: &mut Vec, 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(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + fn to_css(&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 { + where W: fmt::Write, + I: Iterator, + { 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 - where W: Write, - I: IntoIterator, - I::IntoIter: Clone, + where W: Write, + I: IntoIterator, + I::IntoIter: Clone, { match self.get_shorthand_appendable_value(declarations) { None => Ok(false), @@ -594,7 +630,9 @@ impl HasViewportPercentage for DeclaredValue { } impl ToCss for DeclaredValue { - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + fn to_css(&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 ToCss for DeclaredValue { } } +/// 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(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + fn to_css(&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) -> Result { 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 { @@ -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 { 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), % 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(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + fn to_css(&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 - /// (as opposed to being parsed into a more abstract data structure). - /// This is the case of custom properties and values that contain unsubstituted variables. + /// Return whether the value is stored as it was in the CSS source, + /// preserving whitespace (as opposed to being parsed into a more abstract + /// 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: 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 { - 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 - // properties, and those are both rare and expensive. - pub fn custom_properties(&self) -> Option> { + /// Get the custom properties map if necessary. + /// + /// 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> { 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 } - pub fn set_root_font_size(&mut self, size: Au) { self.root_font_size = size } + /// Get the root font 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 { 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 { 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 { self.border_width_for_writing_mode(self.writing_mode) } + /// Gets the logical computed margin from this style. #[inline] pub fn logical_margin(&self) -> LogicalMargin { 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 { // 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); -#[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 - /// shareable. + /// Whether the `ComputedValues` structure to be constructed should be + /// considered shareable. const SHAREABLE = 0x01, - /// Whether to inherit all styles from the parent. If this flag is not present, - /// non-inherited styles are reset to their initial values. + /// Whether to inherit all styles from the parent. If this flag is not + /// 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, font_metrics_provider: Option<<&FontMetricsProvider>, flags: CascadeFlags) -> ComputedValues - where F: Fn() -> I, I: Iterator + where F: Fn() -> I, + I: Iterator, { 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, // 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(); @@ -1850,6 +1992,9 @@ pub fn apply_declarations<'a, F, I>(viewport_size: Size2D, % 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 From a5005a9967991ceae7c1cca2be08e0a1f9f70493 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 1 Jan 2017 22:07:20 +0100 Subject: [PATCH 03/44] style: document the sink module. --- components/style/sink.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/components/style/sink.rs b/components/style/sink.rs index 33aa26ac1af..343bf07d869 100644 --- a/components/style/sink.rs +++ b/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 { + /// Push a value into self. fn push(&mut self, value: T); } @@ -21,13 +31,16 @@ impl Push for SmallVec { } } +/// A struct that implements `Push`, but only stores whether it's empty. pub struct ForgetfulSink(bool, PhantomData); impl ForgetfulSink { + /// 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 } From 2cef9c948ec6e5a0f135ccffdf06d967294b2d1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 1 Jan 2017 22:11:05 +0100 Subject: [PATCH 04/44] style: Document the restyle_damage module. --- components/style/servo/restyle_damage.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/components/style/servo/restyle_damage.rs b/components/style/servo/restyle_damage.rs index e56bcd5a27c..5e3416aa63c 100644 --- a/components/style/servo/restyle_damage.rs +++ b/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, new: &Arc) -> 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, - /// returns the damage that we should add to this flow. + /// Supposing the *parent* of a flow with the given `position` property has + /// 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) From 7ad3ac054ae399412aba18663f39b4c76ef30f20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 1 Jan 2017 22:12:00 +0100 Subject: [PATCH 05/44] style: Document the servo-specific module. --- components/style/servo/mod.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/components/style/servo/mod.rs b/components/style/servo/mod.rs index fd7f1b7c566..29025b432a7 100644 --- a/components/style/servo/mod.rs +++ b/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; From 121b718e9961ab499ab128b73b48663f7ffe6c1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 01:08:59 +0100 Subject: [PATCH 06/44] style: Remove a bunch of duplicated logic in add_stylesheet. This is remminiscent of my recent @import refactor, and stayed there incorrectly. Note that it's the same code that lives in `update` (where it should live). --- components/style/stylist.rs | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 4cb4a65b081..9faf56395ca 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -252,25 +252,6 @@ 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); - } - }); } From 082866aba68c27b1e77a7b0fa710c6d49d8cd3f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 01:09:52 +0100 Subject: [PATCH 07/44] style: Export fnv's FnvHashMap, instead of rewriting it. --- components/style/stylist.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 9faf56395ca..5bfcba258fe 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -36,7 +36,7 @@ use style_traits::viewport::ViewportConstraints; use stylesheets::{CssRule, Origin, StyleRule, Stylesheet, UserAgentStylesheets}; use viewport::{self, MaybeNew, ViewportRule}; -pub type FnvHashMap = HashMap>; +pub use ::fnv::FnvHashMap; /// This structure holds all the selectors and device characteristics /// for a given document. The selectors are converted into `Rule`s From c5f2142d8fd619ec979e7a963637b1f439499d6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 01:10:16 +0100 Subject: [PATCH 08/44] style: Document the stylist module. --- components/layout_thread/lib.rs | 13 ++-- components/style/stylist.rs | 125 ++++++++++++++++++++++++++------ 2 files changed, 108 insertions(+), 30 deletions(-) diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index f9f4c371a94..78a1295c1b5 100644 --- a/components/layout_thread/lib.rs +++ b/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 = match constraints { - Some(ref constraints) => { + self.viewport_size = + rw_data.stylist.viewport_constraints().map_or(current_screen_size, |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()) { diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 5bfcba258fe..587855115b2 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -4,6 +4,8 @@ //! Selector matching. +#![deny(missing_docs)] + use {Atom, LocalName}; use data::ComputedStyle; use dom::PresentationalHintsSynthetizer; @@ -28,7 +30,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; @@ -83,6 +84,8 @@ pub struct Stylist { /// FIXME(emilio): Use the rule tree! precomputed_pseudo_element_decls: FnvHashMap>, + /// 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 +102,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 +133,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], ua_stylesheets: Option<&UserAgentStylesheets>, @@ -258,9 +268,10 @@ impl Stylist { /// 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, - /// non-inherited properties are reset to their initial values. The flow constructor uses this - /// flag when constructing anonymous flows. + /// If `inherit_all` is true, then all properties are inherited from the + /// parent; otherwise, non-inherited properties are reset to their initial + /// values. The flow constructor uses this flag when constructing anonymous + /// flows. pub fn precomputed_values_for_pseudo(&self, pseudo: &PseudoElement, parent: Option<&Arc>, @@ -272,7 +283,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 { @@ -320,6 +331,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(&self, element: &E, pseudo: &PseudoElement, @@ -344,7 +362,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(), @@ -357,6 +376,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]) { let cascaded_rule = ViewportRule { declarations: viewport::Cascade::from_stylesheets(stylesheets, &device).finish(), @@ -364,6 +388,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); } @@ -389,21 +416,29 @@ impl Stylist { self.device = Arc::new(device); } - pub fn viewport_constraints(&self) -> &Option { - &self.viewport_constraints + /// Returns the viewport constraints that apply to this document because of + /// 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*; - /// that is, whether the matched selectors are simple enough to allow the - /// matching logic to be reduced to the logic in - /// `css::matching::PrivateMatchMethods::candidate_element_allows_for_style_sharing`. + /// The returned `StyleRelations` indicate hints about which kind of rules + /// have matched. #[allow(unsafe_code)] pub fn push_applicable_declarations( &self, @@ -530,20 +565,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 { &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(&self, element: &E, candidate: &E) -> bool - where E: ElementExt + where E: ElementExt, { use selectors::matching::StyleRelations; use selectors::matching::matches_complex_selector; @@ -572,15 +615,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(&self, element: &E, candidate: &E) -> bool - where E: ElementExt + where E: ElementExt, { use selectors::matching::StyleRelations; use selectors::matching::matches_complex_selector; @@ -610,7 +657,11 @@ impl Stylist { true } - pub fn compute_restyle_hint(&self, element: &E, + /// Given an element, and a snapshot that represents a previous state of the + /// element, compute the appropriate restyle hint, that is, the kind of + /// restyle we need to do. + pub fn compute_restyle_hint(&self, + element: &E, snapshot: &Snapshot, // NB: We need to pass current_state as an argument because // selectors::Element doesn't provide access to ElementState @@ -618,7 +669,7 @@ impl Stylist { // more expensive than getting it directly from the caller. current_state: ElementState) -> RestyleHint - where E: ElementExt + Clone + where E: ElementExt + Clone, { self.state_deps.compute_hint(element, snapshot, current_state) } @@ -632,6 +683,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(); } } } @@ -688,11 +742,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>, + /// A hash from a class name to rules which contain that class selector. pub class_hash: FnvHashMap>, + /// A hash from local name to rules which contain that local name selector. pub local_name_hash: FnvHashMap>, /// Same as local_name_hash, but keys are lower-cased. /// For HTML elements in HTML documents. @@ -709,6 +767,7 @@ fn sort_by_key K, K: Ord>(v: &mut [T], f: F) { } impl SelectorMap { + /// Trivially constructs an empty `SelectorMap`. pub fn new() -> Self { SelectorMap { id_hash: HashMap::default(), @@ -941,17 +1000,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 - // that it matches. Selector contains an owned vector (through - // ComplexSelector) and we want to avoid the allocation. + /// The selector this struct represents. + /// This is an Arc because Rule will essentially be cloned for every element + /// 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>, + /// The actual style rule. #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")] pub style_rule: Arc>, + /// The source order this style rule appears in. pub source_order: usize, + /// The specificity of the rule this selector represents. pub specificity: u32, } @@ -966,19 +1036,28 @@ impl Rule { } } -/// A property declaration together with its precedence among rules of equal specificity so that -/// we can sort them. +/// A property declaration together with its precedence among rules of equal +/// 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>, importance: Importance) @@ -992,6 +1071,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, From fa8874fb14f2fb84c74d74ae26a6147b2ec307ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 1 Jan 2017 23:53:34 +0100 Subject: [PATCH 09/44] style: Document the restyle hints code, and make it operate on `TElement`. This removes the annoying constraint of having to provide the current state from outside of the restyle hints code. --- components/style/data.rs | 4 +- components/style/restyle_hints.rs | 46 ++++++++++----- components/style/selector_parser.rs | 90 +++++++++++++++++++++-------- components/style/stylist.rs | 14 ++--- 4 files changed, 101 insertions(+), 53 deletions(-) diff --git a/components/style/data.rs b/components/style/data.rs index e499c5262aa..ddd85ef9ad3 100644 --- a/components/style/data.rs +++ b/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(), - state); + self.snapshot.as_ref().unwrap()); // If the hint includes a directive for later siblings, strip it out and // notify the caller to modify the base hint for future siblings. diff --git a/components/style/restyle_hints.rs b/components/style/restyle_hints.rs index 3f29d437609..89a8d273595 100644 --- a/components/style/restyle_hints.rs +++ b/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 { } 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 + 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>) { let mut cur = selector; let mut combinator: Option = 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(&self, el: &E, - snapshot: &Snapshot, - current_state: ElementState) + /// Compute a restyle hint given an element and a snapshot, per the rules + /// explained in the rest of the documentation. + pub fn compute_hint(&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; diff --git a/components/style/selector_parser.rs b/components/style/selector_parser.rs index 9b444f17474..e8ed76bd01f 100644 --- a/components/style/selector_parser.rs +++ b/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 = ::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, ()> { 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. -/// `::before` and `::after`). They inherit styles normally as another -/// 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. +/// If you're implementing a public selector for `Servo` 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 + 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(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(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) -> 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, value: &AttrValue) -> bool { // FIXME(pcwalton): Remove once we start actually supporting RTL text. This is in diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 587855115b2..1a1dfabaf3d 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -8,8 +8,7 @@ use {Atom, LocalName}; use data::ComputedStyle; -use dom::PresentationalHintsSynthetizer; -use element_state::*; +use dom::{PresentationalHintsSynthetizer, TElement}; use error_reporting::StdoutErrorReporter; use keyframes::KeyframesAnimation; use media_queries::{Device, MediaType}; @@ -662,16 +661,11 @@ impl Stylist { /// restyle we need to do. pub fn compute_restyle_hint(&self, element: &E, - snapshot: &Snapshot, - // NB: We need to pass current_state as an argument because - // selectors::Element doesn't provide access to ElementState - // directly, and computing it from the ElementState would be - // more expensive than getting it directly from the caller. - current_state: ElementState) + snapshot: &Snapshot) -> RestyleHint - where E: ElementExt + Clone, + where E: TElement, { - self.state_deps.compute_hint(element, snapshot, current_state) + self.state_deps.compute_hint(element, snapshot) } } From e8d947430a06ad16748e0ed16f605151e0d46d17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 00:00:22 +0100 Subject: [PATCH 10/44] style: Document and clean up the parser module. --- components/style/parser.rs | 45 +++++++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/components/style/parser.rs b/components/style/parser.rs index 0295607805a..d1e37ce4527 100644 --- a/components/style/parser.rs +++ b/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, + /// The referrer URI. pub referrer: Option, + /// The principal that loaded this stylesheet. pub principal: Option, } -impl ParserContextExtraData { - #[cfg(not(feature = "gecko"))] - pub fn default() -> ParserContextExtraData { +#[cfg(not(feature = "gecko"))] +impl Default for ParserContextExtraData { + fn default() -> Self { ParserContextExtraData } +} - #[cfg(feature = "gecko")] - pub fn default() -> ParserContextExtraData { +#[cfg(feature = "gecko")] +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, + /// 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, 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) + /// Create a parser context with the default extra data. + pub fn new(stylesheet_origin: Origin, + base_url: &'a ServoUrl, + error_reporter: Box) -> 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 { - fn parse(context: &ParserContext, input: &mut Parser) -> Result where Self: Sized; +/// A trait to abstract parsing of a specified value given a `ParserContext` and +/// 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; } From c7bcbad3b9eb174c460fa0b33eb978740d124dee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 00:08:59 +0100 Subject: [PATCH 11/44] style: Document the `StyleContext` module. --- components/style/context.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/components/style/context.rs b/components/style/context.rs index f649f64afbf..57a256d4911 100644 --- a/components/style/context.rs +++ b/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) -> 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, @@ -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 { + /// A cache to share style among siblings. pub style_sharing_candidate_cache: StyleSharingCandidateCache, + /// The bloom filter used to fast-reject selector-matching. pub bloom_filter: StyleBloom, /// A channel on which new animations that have been triggered by style /// recalculation can be sent. @@ -81,6 +100,7 @@ pub struct ThreadLocalStyleContext { } impl ThreadLocalStyleContext { + /// 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 ThreadLocalStyleContext { } } +/// 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, } From 081b8399e6f8bb2a5a75ff2730d4f43adcc3ff50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 00:16:26 +0100 Subject: [PATCH 12/44] style: Document the scoped_tls module. --- components/style/scoped_tls.rs | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/components/style/scoped_tls.rs b/components/style/scoped_tls.rs index 1f695aaa4c9..92cfd2a4659 100644 --- a/components/style/scoped_tls.rs +++ b/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. - -pub struct ScopedTLS<'a, T: Send> { - pool: &'a rayon::ThreadPool, +/// A scoped TLS set, that is alive during the `'scope` lifetime. +/// +/// We use this on Servo to construct thread-local contexts, but clear them once +/// we're done with restyling. +pub struct ScopedTLS<'scope, T: Send> { + pool: &'scope rayon::ThreadPool, slots: Box<[RefCell>]>, } -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> { - pub fn new(p: &'a rayon::ThreadPool) -> Self { +impl<'scope, T: Send> ScopedTLS<'scope, T> { + /// 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` that this thread owns. pub fn borrow(&self) -> Ref> { let idx = self.pool.current_thread_index().unwrap(); self.slots[idx].borrow() } + /// Return a mutable reference to the `Option` that this thread owns. pub fn borrow_mut(&self) -> RefMut> { 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 T>(&self, f: F) -> RefMut { let mut opt = self.borrow_mut(); if opt.is_none() { From b31470dddbc64f7a109b220e4e4be20bbb0fee69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 00:20:03 +0100 Subject: [PATCH 13/44] style: Document the owning_handle module. --- components/style/owning_handle.rs | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/components/style/owning_handle.rs b/components/style/owning_handle.rs index f4daf750cb0..1294bf7e59f 100644 --- a/components/style/owning_handle.rs +++ b/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 - where O: StableAddress, H: Deref, + where O: StableAddress, + H: Deref, { handle: H, _owner: O, } impl Deref for OwningHandle - where O: StableAddress, H: Deref, + where O: StableAddress, + H: Deref, { type Target = H::Target; fn deref(&self) -> &H::Target { @@ -47,11 +52,13 @@ impl Deref for OwningHandle } unsafe impl StableAddress for OwningHandle - where O: StableAddress, H: StableAddress, + where O: StableAddress, + H: StableAddress, {} impl DerefMut for OwningHandle - 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 DerefMut for OwningHandle } impl OwningHandle - 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(o: O, f: F) -> Self - where F: Fn(*const O::Target) -> H + where F: Fn(*const O::Target) -> H, { let h: H; { From 3f7f914e165da93eaad4ac2d276c63f3f3a3e895 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 00:21:58 +0100 Subject: [PATCH 14/44] style: Allow the lack of documentation for the media_queries module. I'm rewriting all that stuff soon, so it's kind of pointless to document the existing stuff right now. --- components/style/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/components/style/lib.rs b/components/style/lib.rs index 30d585e3261..1d3091ca60a 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -107,6 +107,7 @@ pub mod font_metrics; pub mod keyframes; pub mod logical_geometry; pub mod matching; +#[allow(missing_docs)] pub mod media_queries; pub mod owning_handle; pub mod parallel; From 0b3c1a89246cb9ed54001636d3e3d19c7510ee37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 00:31:17 +0100 Subject: [PATCH 15/44] style: Document the keyframes module. --- components/style/keyframes.rs | 44 ++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/components/style/keyframes.rs b/components/style/keyframes.rs index c90e68a4630..dff585a1fee 100644 --- a/components/style/keyframes.rs +++ b/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 -/// this keyframe should run. +/// A number from 0 to 1, indicating the percentage of the animation when this +/// 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); 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 { 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, - /// so the second value of these tuples is always `Importance::Normal`. - /// But including them enables `compute_style_for_animation_step` to create a `ApplicableDeclarationBlock` - /// by cloning an `Arc<_>` (incrementing a reference count) rather than re-creating a `Vec<_>`. + /// The declaration block that was declared inside this keyframe. + /// + /// Note that `!important` rules in keyframes don't apply, but we keep this + /// `Arc` just for convenience. #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")] pub block: Arc>, } @@ -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>, ()> { 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> }, + /// 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, /// The properties that change in this animation. pub properties_changed: Vec, @@ -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 { 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 { } 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>]) -> Option { 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>> { RuleListParser::new_for_nested_rule(input, KeyframeListParser { context: context }) .filter_map(Result::ok) From c9ffd9b429ba05f80b4a3be92ff1740826b7d096 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 00:33:56 +0100 Subject: [PATCH 16/44] style: Allow the `logical_geometry` module to be undocumented, for now. --- components/style/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/components/style/lib.rs b/components/style/lib.rs index 1d3091ca60a..55e6124ad35 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -105,6 +105,7 @@ 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)] From 3cd6cba1ee417f4845c749f433dd91d2d5a84df5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 00:35:29 +0100 Subject: [PATCH 17/44] style: Allow the atomic_refcell module to be undocumented, for now. Given Bobby is rewriting it as I write this. --- components/style/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/components/style/lib.rs b/components/style/lib.rs index 55e6124ad35..148f559b35b 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -88,6 +88,7 @@ extern crate time; extern crate unicode_segmentation; pub mod animation; +#[allow(missing_docs)] // TODO: Under a rewrite. pub mod atomic_refcell; pub mod attr; pub mod bezier; From c44826f1ccc9d287291dcf84fd20dbdf846da2a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 00:55:47 +0100 Subject: [PATCH 18/44] style: Document the custom_properties module. --- components/style/custom_properties.rs | 59 +++++++++++++++++++-------- 1 file changed, 41 insertions(+), 18 deletions(-) diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index d8e8913c41e..6346ae20098 100644 --- a/components/style/custom_properties.rs +++ b/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>, } +/// 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(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + fn to_css(&self, dest: &mut W) -> fmt::Result + where W: fmt::Write, + { dest.write_str(&self.css) } } impl ToCss for ComputedValue { - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + fn to_css(&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; impl ComputedValue { @@ -172,8 +189,8 @@ fn parse_declaration_value<'i, 't> }) } -/// Like parse_declaration_value, -/// but accept `!` and `;` since they are only invalid at the top level +/// Like parse_declaration_value, but accept `!` and `;` since they are only +/// invalid at the top level fn parse_declaration_value_block(input: &mut Parser, references: &mut Option>, 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 - } else { - return Ok((first_token_type, last_token_type)) - } + token = match input.next_including_whitespace_and_comments() { + Ok(token) => token, + Err(..) => return Ok((first_token_type, last_token_type)), + }; } } @@ -306,8 +322,8 @@ fn parse_var_function<'i, 't>(input: &mut Parser<'i, 't>, Ok(()) } -/// Add one custom property declaration to a map, -/// unless another with the same name was already there. +/// Add one custom property declaration to a map, unless another with the same +/// name was already there. pub fn cascade<'a>(custom_properties: &mut Option>>, inherited: &'a Option>>, seen: &mut HashSet<&'a Name>, @@ -351,6 +367,12 @@ pub fn cascade<'a>(custom_properties: &mut Option>, inherited: &Option>>) -> Option>> { @@ -363,7 +385,9 @@ pub fn finish_cascade(specified_values_map: Option) { let mut to_remove = HashSet::new(); { @@ -514,10 +538,9 @@ fn substitute_block(input: &mut Parser, }); set_position_at_next_iteration = false; } - let token = if let Ok(token) = next { - token - } else { - break + let token = match next { + Ok(token) => token, + Err(..) => break, }; match token { Token::Function(ref name) if name.eq_ignore_ascii_case("var") => { From 369c2f299ee6b472fa8a71c752b11078c7a81c8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 01:00:29 +0100 Subject: [PATCH 19/44] style: Simplify the cascade function in the custom_properties module. --- components/style/custom_properties.rs | 70 ++++++++++++++------------- 1 file changed, 36 insertions(+), 34 deletions(-) diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index 6346ae20098..8c6c3675c02 100644 --- a/components/style/custom_properties.rs +++ b/components/style/custom_properties.rs @@ -329,41 +329,43 @@ pub fn cascade<'a>(custom_properties: &mut Option, name: &'a Name, specified_value: &'a DeclaredValue) { - let was_not_already_present = seen.insert(name); - if was_not_already_present { - let map = match *custom_properties { - Some(ref mut map) => map, - None => { - *custom_properties = Some(match *inherited { - Some(ref inherited) => inherited.iter().map(|(key, inherited_value)| { - (key, BorrowedSpecifiedValue { - css: &inherited_value.css, - first_token_type: inherited_value.first_token_type, - last_token_type: inherited_value.last_token_type, - references: None - }) - }).collect(), - None => HashMap::new(), - }); - custom_properties.as_mut().unwrap() - } - }; - 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. + let was_already_present = !seen.insert(name); + if was_already_present { + return; + } + + let map = match *custom_properties { + Some(ref mut map) => map, + None => { + *custom_properties = Some(match *inherited { + Some(ref inherited) => inherited.iter().map(|(key, inherited_value)| { + (key, BorrowedSpecifiedValue { + css: &inherited_value.css, + first_token_type: inherited_value.first_token_type, + last_token_type: inherited_value.last_token_type, + references: None + }) + }).collect(), + None => HashMap::new(), + }); + custom_properties.as_mut().unwrap() } + }; + 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. } } From c6144df470a47888b98b1c56bbf3d203b80ca893 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 01:00:41 +0100 Subject: [PATCH 20/44] style: Document the font_face module. --- components/style/font_face.rs | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/components/style/font_face.rs b/components/style/font_face.rs index 70f904c17a4..d7eaaf79e6c 100644 --- a/components/style/font_face.rs +++ b/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(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + fn to_css(&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, } impl ToCss for UrlSource { - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + fn to_css(&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, } impl ToCss for FontFaceRule { // Serialization of FontFaceRule is not specced. - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { + fn to_css(&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 { 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); From 939557260cce6bddcc7d23d00af0b9526d7d05c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 00:57:49 +0100 Subject: [PATCH 21/44] style: Allow the attr module to be without documentation, for now. --- components/style/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/components/style/lib.rs b/components/style/lib.rs index 148f559b35b..f6c310a18c8 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -90,6 +90,7 @@ 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; From 51134b9dc819330a983a87ad11a894a3f781766c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 01:01:47 +0100 Subject: [PATCH 22/44] style: Disallow new undocumented code by default for Servo. Still a bunch of Gecko-only code to document. --- components/style/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/components/style/lib.rs b/components/style/lib.rs index f6c310a18c8..70737c81644 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -26,6 +26,7 @@ #![cfg_attr(feature = "servo", feature(proc_macro))] #![deny(warnings)] +#![cfg_attr(feature = "servo", 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 From d10cc314fe0c46ce855bb03fba469faaa7a542a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 01:56:08 +0100 Subject: [PATCH 23/44] style: Document the StyleComplexColor sugar. --- components/style/gecko_bindings/sugar/style_complex_color.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/components/style/gecko_bindings/sugar/style_complex_color.rs b/components/style/gecko_bindings/sugar/style_complex_color.rs index d428e6bf741..cd861b1c0bc 100644 --- a/components/style/gecko_bindings/sugar/style_complex_color.rs +++ b/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 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, From f97e5ff9aa70f895caba4b77f0f60af4b437288f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 01:56:28 +0100 Subject: [PATCH 24/44] style: Document the RefPtr sugar module. --- .../style/gecko_bindings/sugar/refptr.rs | 85 +++++++++++-------- 1 file changed, 50 insertions(+), 35 deletions(-) diff --git a/components/style/gecko_bindings/sugar/refptr.rs b/components/style/gecko_bindings/sugar/refptr.rs index 220bca116e1..91c83259925 100644 --- a/components/style/gecko_bindings/sugar/refptr.rs +++ b/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 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 { ptr: *mut T, _marker: PhantomData, } -/// A RefPtr that we know is uniquely owned +/// A RefPtr that we know is uniquely owned. /// -/// This is basically Box, with the additional -/// guarantee that the box can be safely interpreted -/// as a RefPtr (with refcount 1) +/// This is basically Box, with the additional guarantee that the box can be +/// safely interpreted as a RefPtr (with refcount 1) /// -/// This is useful when you wish to create a refptr -/// and mutate it temporarily, while it is still -/// uniquely owned. +/// This is useful when you wish to create a refptr and mutate it temporarily, +/// while it is still uniquely owned. pub struct UniqueRefPtr(RefPtr); // There is no safe conversion from &T to RefPtr (like Gecko has) // because this lets you break UniqueRefPtr's guarantee impl RefPtr { - /// Create a new RefPtr from an already addrefed - /// pointer obtained from FFI. Pointer - /// must be valid, non-null and have been addrefed + /// Create a new RefPtr from an already addrefed pointer obtained from FFI. + /// + /// 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 RefPtr { } } - /// Create a new RefPtr from a pointer obtained - /// from FFI. Pointer must be valid and non null. + /// Create a new RefPtr from a pointer obtained from FFI. + /// + /// 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 RefPtr { ret } - /// Produces an FFI-compatible RefPtr that can be stored in - /// style structs. + /// Produces an FFI-compatible RefPtr that can be stored in style structs. /// /// structs::RefPtr does not have a destructor, so this may leak pub fn forget(self) -> structs::RefPtr { @@ -75,36 +80,36 @@ impl RefPtr { ret } - /// Returns the raw inner pointer - /// to be fed back into FFI + /// Returns the raw inner pointer to be fed back into FFI. pub fn get(&self) -> *mut T { self.ptr } - /// Addref the inner data - /// - /// Leaky on its own + /// Addref the inner data, obviously 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 UniqueRefPtr { - /// Create a unique refptr from an already addrefed - /// pointer obtained from FFI. The refcount must be one. + /// Create a unique refptr from an already addrefed pointer obtained from + /// 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 { self.0 } @@ -131,9 +136,9 @@ impl DerefMut for UniqueRefPtr { } impl structs::RefPtr { - /// 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 + /// Must be called on a valid, non-null structs::RefPtr. pub unsafe fn to_safe(&self) -> RefPtr { debug_assert!(!self.mRawPtr.is_null()); let r = RefPtr { @@ -143,7 +148,8 @@ impl structs::RefPtr { 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 { debug_assert!(!self.mRawPtr.is_null()); RefPtr { @@ -152,9 +158,13 @@ impl structs::RefPtr { } } - /// Replace a structs::RefPtr with a different one, appropriately addref/releasing + /// Replace a structs::RefPtr 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 structs::RefPtr { } /// Clear an instance of the structs::RefPtr, 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 structs::RefPtr { /// consuming the `RefPtr`, 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) { if !self.mRawPtr.is_null() { unsafe { (*self.mRawPtr).release(); } @@ -215,9 +225,9 @@ impl PartialEq for RefPtr { unsafe impl Send for RefPtr {} unsafe impl Sync for RefPtr {} -// 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>; From 1d19e81c8bacf109a924b0021d4b4e187f19daad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 02:59:20 +0100 Subject: [PATCH 25/44] style: Document and make clearer the ownership helpers. --- .../style/gecko_bindings/sugar/ownership.rs | 172 +++++++++++------- 1 file changed, 111 insertions(+), 61 deletions(-) diff --git a/components/style/gecko_bindings/sugar/ownership.rs b/components/style/gecko_bindings/sugar/ownership.rs index edd8394f31c..abcf5c65a8a 100644 --- a/components/style/gecko_bindings/sugar/ownership.rs +++ b/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 -/// Gecko FFI type -/// The correspondence is not defined at this stage, -/// use HasArcFFI or similar traits to define it +/// Indicates that a given Servo type has a corresponding Gecko FFI type. 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 -/// as the corresponding HasFFI::FFIType type +/// Indicates that a given Servo type has the same layout as the corresponding +/// `HasFFI::FFIType` type. pub unsafe trait HasSimpleFFI : HasFFI { #[inline] - /// Given a Servo-side reference, converts it to an - /// FFI-safe reference which can be passed to Gecko + /// Given a Servo-side reference, converts it to an FFI-safe reference which + /// 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 - /// FFI-safe mutable reference which can be passed to Gecko + /// Given a Servo-side mutable reference, converts it to an FFI-safe mutable + /// 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 - /// converts it to a Servo-side reference + /// Given an FFI-safe reference obtained from Gecko converts it to a + /// 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 - /// converts it to a Servo-side mutable reference + /// Given an FFI-safe mutable reference obtained from Gecko converts it to a + /// 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 -> &GeckoType fn into_ffi(self: Box) -> Owned { 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 -/// via Strong and Borrowed +/// Should be implemented by types which are passed over FFI as Arcs via Strong +/// and Borrowed. /// -/// In this case, the FFIType is the rough equivalent of ArcInner +/// In this case, the FFIType is the rough equivalent of ArcInner. 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 -> &GeckoType fn arc_as_borrowed<'a>(arc: &'a Arc) -> &'a &Self::FFIType { unsafe { transmute::<&Arc, &&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 fn arc_from_borrowed<'a>(ptr: &'a Option<&Self::FFIType>) -> Option<&'a Arc> { 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 { - ptr: *const T, - _marker: PhantomData, +pub struct Strong { + ptr: *const GeckoType, + _marker: PhantomData, } -impl Strong { +impl Strong { #[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, - /// converts it into a servo-side Arc + /// Given a non-null strong FFI reference, converts it into a servo-side + /// Arc. + /// /// Panics on null. /// /// Strong -> Arc - pub fn into_arc(self) -> Arc where U: HasArcFFI { + pub fn into_arc(self) -> Arc + where ServoType: HasArcFFI, + { self.into_arc_opt().unwrap() } @@ -159,7 +177,9 @@ impl Strong { /// Returns None on null. /// /// Strong -> Arc - pub fn into_arc_opt(self) -> Option> where U: HasArcFFI { + pub fn into_arc_opt(self) -> Option> + where ServoType: HasArcFFI, + { if self.is_null() { None } else { @@ -168,12 +188,15 @@ impl Strong { } #[inline] - /// Given a reference to a strong FFI reference, - /// converts it to a reference to a servo-side Arc + /// Given a reference to a strong FFI reference, converts it to a reference + /// to a servo-side Arc. + /// /// Returns None on null. /// /// Strong -> Arc - pub fn as_arc_opt(&self) -> Option<&Arc> where U: HasArcFFI { + pub fn as_arc_opt(&self) -> Option<&Arc> + where ServoType: HasArcFFI, + { if self.is_null() { None } else { @@ -182,30 +205,42 @@ impl Strong { } #[inline] - /// Produces a null strong FFI reference + /// Produces a null strong FFI reference. pub fn null() -> Self { - unsafe { transmute(ptr::null::()) } + unsafe { transmute(ptr::null::()) } } } +/// A few helpers implemented on top of Arc 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 -> Strong fn into_strong(self) -> Strong<::FFIType>; + /// Produces a (nullable) borrowed FFI reference by borrowing an Arc. /// /// &Arc -> 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<&::FFIType>; } unsafe impl FFIArcHelpers for Arc { type Inner = T; + #[inline] fn into_strong(self) -> Strong { unsafe { transmute(self) } } + #[inline] fn as_borrowed_opt(&self) -> Option<&T::FFIType> { let borrowedptr = self as *const Arc as *const Option<&T::FFIType>; @@ -215,51 +250,62 @@ unsafe impl FFIArcHelpers for Arc { #[repr(C)] #[derive(Debug)] -/// Gecko-FFI-safe owned pointer -/// Cannot be null -/// Leaks on drop. Please don't drop this. -pub struct Owned { - ptr: *mut T, - _marker: PhantomData, +/// Gecko-FFI-safe owned pointer. +/// +/// Cannot be null, and leaks on drop, so needs to be converted into a rust-side +/// `Box` before. +pub struct Owned { + ptr: *mut GeckoType, + _marker: PhantomData, } -impl Owned { - /// Owned -> Box - pub fn into_box(self) -> Box where U: HasBoxFFI { +impl Owned { + /// Gets this `Owned` type as a `Box`. + pub fn into_box(self) -> Box + where ServoType: HasBoxFFI, + { unsafe { transmute(self) } } - pub fn maybe(self) -> OwnedOrNull { + + /// Converts this instance to a (non-null) instance of `OwnedOrNull`. + pub fn maybe(self) -> OwnedOrNull { unsafe { transmute(self) } } } -impl Deref for Owned { - type Target = T; - fn deref(&self) -> &T { +impl Deref for Owned { + type Target = GeckoType; + fn deref(&self) -> &GeckoType { unsafe { &*self.ptr } } } -impl DerefMut for Owned { - fn deref_mut(&mut self) -> &mut T { +impl DerefMut for Owned { + fn deref_mut(&mut self) -> &mut GeckoType { unsafe { &mut *self.ptr } } } #[repr(C)] -/// Gecko-FFI-safe owned pointer -/// Can be null -pub struct OwnedOrNull { - ptr: *mut T, - _marker: PhantomData, +/// Gecko-FFI-safe owned pointer. +/// +/// Can be null, and just as `Owned` leaks on `Drop`. +pub struct OwnedOrNull { + ptr: *mut GeckoType, + _marker: PhantomData, } -impl OwnedOrNull { +impl OwnedOrNull { + /// Returns whether this pointer is null. + #[inline] pub fn is_null(&self) -> bool { - self.ptr == ptr::null_mut() + self.ptr.is_null() } - /// OwnedOrNull -> Option> - pub fn into_box_opt(self) -> Option> where U: HasBoxFFI { + + /// Returns an owned pointer if this is non-null, and `None` otherwise. + pub fn into_box_opt(self) -> Option> + where ServoType: HasBoxFFI, + { if self.is_null() { None } else { @@ -267,8 +313,8 @@ impl OwnedOrNull { } } - /// OwnedOrNull -> Option> - pub fn into_owned_opt(self) -> Option> { + /// Returns an `Owned` if non-null, `None` otherwise. + pub fn into_owned_opt(self) -> Option> { if self.is_null() { None } else { @@ -276,11 +322,15 @@ impl OwnedOrNull { } } - 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) } } } From 1fc48dc7305efee49c891b94b0456df81bc288a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 03:02:52 +0100 Subject: [PATCH 26/44] style: Document nsTArray sugar module. --- components/style/gecko_bindings/sugar/ns_t_array.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/components/style/gecko_bindings/sugar/ns_t_array.rs b/components/style/gecko_bindings/sugar/ns_t_array.rs index c7b824a11dc..5d9c005f0e3 100644 --- a/components/style/gecko_bindings/sugar/ns_t_array.rs +++ b/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 nsTArray { unsafe { self.clear() } } - // unsafe because the array may contain uninits - // This will not call constructors, either manually - // add bindings or run the typed ensurecapacity call - // on the gecko side + /// Resize and set the length of the array to `len`. + /// + /// unsafe because the array may contain uninitialized members. + /// + /// 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); From 3a75545629a2f29754581841ec8c4fe3999802cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 03:04:44 +0100 Subject: [PATCH 27/44] style: Document the bindings module, allow missing docs in autogenerated files. --- components/style/gecko_bindings/mod.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/components/style/gecko_bindings/mod.rs b/components/style/gecko_bindings/mod.rs index d86bfb60484..fa874c7ca42 100644 --- a/components/style/gecko_bindings/mod.rs +++ b/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)] { From 89f3e34f48b40481ef15189f1ea8951ad68db4cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 03:05:42 +0100 Subject: [PATCH 28/44] style: Describe the sugar module. --- components/style/gecko_bindings/sugar/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/components/style/gecko_bindings/sugar/mod.rs b/components/style/gecko_bindings/sugar/mod.rs index 5c7eb198184..b4c94dcbe70 100644 --- a/components/style/gecko_bindings/sugar/mod.rs +++ b/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; From 1bd246905c174aa23bfa3595b404df7f058855d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 03:27:27 +0100 Subject: [PATCH 29/44] style: Document sugar for nsStyleAutoArray. --- .../gecko_bindings/sugar/ns_style_auto_array.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/components/style/gecko_bindings/sugar/ns_style_auto_array.rs b/components/style/gecko_bindings/sugar/ns_style_auto_array.rs index 4b01116f4ea..5730382d7d1 100644 --- a/components/style/gecko_bindings/sugar/ns_style_auto_array.rs +++ b/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 nsStyleAutoArray { + /// Mutably iterate over the array elements. pub fn iter_mut(&mut self) -> Chain, IterMut> { once(&mut self.mFirstElement).chain(self.mOtherElements.iter_mut()) } + + /// Iterate over the array elements. pub fn iter(&self) -> Chain, Iter> { once(&self.mFirstElement).chain(self.mOtherElements.iter()) } - // Note that often structs containing autoarrays will have - // additional member fields that contain the length, which must be kept - // in sync + /// Returns the length of the array. + /// + /// 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() } From 96fb1378d7307674b24ed4bcafdaa24dce3e1bf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 03:28:08 +0100 Subject: [PATCH 30/44] style: Document the sugar for `nsStyleCoord`. --- .../gecko_bindings/sugar/ns_style_coord.rs | 87 +++++++++++++++---- 1 file changed, 69 insertions(+), 18 deletions(-) diff --git a/components/style/gecko_bindings/sugar/ns_style_coord.rs b/components/style/gecko_bindings/sugar/ns_style_coord.rs index 9aac36e9915..84ea7d68456 100644 --- a/components/style/gecko_bindings/sugar/ns_style_coord.rs +++ b/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 - /// This is unsafe since it's possible to modify - /// the unit without changing the union + /// Get mutably the unit and 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 - /// Currently, this only happens if the nsStyleUnit - /// is a Calc + /// Clean up any resources used by the union. + /// + /// 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(&mut self, other: &T) { unsafe { self.reset(); - { - let (unit, union) = self.values_mut(); - *unit = other.unit(); - *union = other.union(); - } + self.copy_from_unchecked(other); 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(&mut self, other: &T) { - let (unit, union) = self.values_mut(); - *unit = other.unit(); - *union = other.union(); + let (unit, union) = self.values_mut(); + *unit = other.unit(); + *union = other.union(); } - /// Useful for initializing uninits - /// (set_value may segfault on uninits) + /// Useful for initializing uninits, given that `set_value` may segfault on + /// 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) From 277835445538c21d5e7d34e28ff22fc23c7ee1dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 03:32:55 +0100 Subject: [PATCH 31/44] style: Document nsCSSShadowArray sugar. --- .../style/gecko_bindings/sugar/ns_css_shadow_array.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/components/style/gecko_bindings/sugar/ns_css_shadow_array.rs b/components/style/gecko_bindings/sugar/ns_css_shadow_array.rs index bfc9c2c6432..5de5147b54b 100644 --- a/components/style/gecko_bindings/sugar/ns_css_shadow_array.rs +++ b/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 { + /// 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 { } } } + + /// 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() { From bcac8265c9a15513ad39bb71807edd54bec71da9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 03:45:21 +0100 Subject: [PATCH 32/44] style: Document Gecko's selector parser. --- components/style/gecko/selector_parser.rs | 59 +++++++++++++++++++++-- 1 file changed, 54 insertions(+), 5 deletions(-) diff --git a/components/style/gecko/selector_parser.rs b/components/style/gecko/selector_parser.rs index d9fdf6a4431..44ea037ef62 100644 --- a/components/style/gecko/selector_parser.rs +++ b/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 -/// 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. +/// In Gecko, we represent pseudo-elements as plain `Atom`s. +/// +/// 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 { 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(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() } From 9b6cf8dc1bd7e940afc2e4e7b5c74f82b928b498 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 03:47:43 +0100 Subject: [PATCH 33/44] style: Document the nsCOMPtr sugar. --- components/style/gecko_bindings/sugar/ns_com_ptr.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/components/style/gecko_bindings/sugar/ns_com_ptr.rs b/components/style/gecko_bindings/sugar/ns_com_ptr.rs index a7056320a5a..c7ad3af5e35 100644 --- a/components/style/gecko_bindings/sugar/ns_com_ptr.rs +++ b/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 nsCOMPtr { + /// 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 { From 7b4d3deae789d7f375376bfe39e09259cb1bc27a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 04:53:00 +0100 Subject: [PATCH 34/44] style: Ditch GeckoStyleCoordHelpers, and implement directly on nsStyleCoord. --- components/style/gecko/conversions.rs | 2 +- components/style/gecko/values.rs | 18 +++++++++--------- components/style/properties/gecko.mako.rs | 1 - 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs index 73cbc31c634..8b9aaeec065 100644 --- a/components/style/gecko/conversions.rs +++ b/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}; diff --git a/components/style/gecko/values.rs b/components/style/gecko/values.rs index e21b2f8ae71..23a4920d060 100644 --- a/components/style/gecko/values.rs +++ b/components/style/gecko/values.rs @@ -14,22 +14,22 @@ use values::computed::{Angle, LengthOrPercentageOrNone, Number}; use values::computed::{LengthOrPercentage, LengthOrPercentageOrAuto}; use values::computed::basic_shape::ShapeRadius; -pub trait StyleCoordHelpers { - fn set(&mut self, val: T); +/// A trait that defines an interface to convert from and to `nsStyleCoord`s. +pub trait GeckoStyleCoordConvertible : Sized { + /// Convert this to a `nsStyleCoord`. + fn to_gecko_style_coord(&self, coord: &mut T); + /// Given a `nsStyleCoord`, try to get a value of this type.. + fn from_gecko_style_coord(coord: &T) -> Option; } -impl StyleCoordHelpers for nsStyleCoord { +impl nsStyleCoord { #[inline] - fn set(&mut self, val: T) { + /// Set this `nsStyleCoord` value to `val`. + pub fn set(&mut self, val: T) { val.to_gecko_style_coord(self); } } -pub trait GeckoStyleCoordConvertible : Sized { - fn to_gecko_style_coord(&self, coord: &mut T); - fn from_gecko_style_coord(coord: &T) -> Option; -} - impl GeckoStyleCoordConvertible for Either { fn to_gecko_style_coord(&self, coord: &mut T) { match *self { diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 746ae31f2e5..ab0f6b4d46c 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -48,7 +48,6 @@ 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::longhands; use std::fmt::{self, Debug}; From ee48599d1bf6c2d5b4c5455add93c60b87eacbc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 04:53:38 +0100 Subject: [PATCH 35/44] style: Document Gecko conversions module. --- components/style/gecko/conversions.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs index 8b9aaeec065..4c2d3c16329 100644 --- a/components/style/gecko/conversions.rs +++ b/components/style/gecko/conversions.rs @@ -99,6 +99,7 @@ impl From for nsStyleCoord_CalcValue { } impl LengthOrPercentageOrAuto { + /// Convert this value in an appropriate `nsStyleCoord::CalcValue`. pub fn to_calc_value(&self) -> Option { match *self { LengthOrPercentageOrAuto::Length(au) => { @@ -132,6 +133,7 @@ impl From 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)); From dba73dc618cb14a6116480a9fccfd1418977541f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 04:54:29 +0100 Subject: [PATCH 36/44] style: Document Gecko's data module. --- components/style/gecko/data.rs | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/components/style/gecko/data.rs b/components/style/gecko/data.rs index da334bfa4bf..f67e3871bbc 100644 --- a/components/style/gecko/data.rs +++ b/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, @@ -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, + /// Unused. Will go away when we actually implement transitions and + /// animations properly. pub new_animations_receiver: Receiver, + /// Unused. Will go away when we actually implement transitions and + /// animations properly. pub running_animations: Arc>>>, + /// Unused. Will go away when we actually implement transitions and + /// animations properly. pub expired_animations: Arc>>>, - // FIXME(bholley): This shouldn't be per-document. + /// The worker thread pool. + /// FIXME(bholley): This shouldn't be per-document. pub work_queue: Option, + /// 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); lazy_static! { + /// The number of layout threads, computed statically. pub static ref NUM_THREADS: usize = { match env::var("STYLO_THREADS").map(|s| s.parse::().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 = 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 { self.0.borrow() } + /// Get an mutable reference to this style data. pub fn borrow_mut(&self) -> AtomicRefMut { 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 From 99b0f53064b68459e923b16afeb8d2a472831bf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 04:54:51 +0100 Subject: [PATCH 37/44] style: Describe the gecko module. --- components/style/gecko/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/components/style/gecko/mod.rs b/components/style/gecko/mod.rs index 7f227f154dc..0e49405d159 100644 --- a/components/style/gecko/mod.rs +++ b/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; From 40e5ec90186c8defaeabd696a94a1ba8200ff00f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 04:55:23 +0100 Subject: [PATCH 38/44] style: Document Gecko's restyle-damage code. --- components/style/gecko/restyle_damage.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/components/style/gecko/restyle_damage.rs b/components/style/gecko/restyle_damage.rs index a1c7fc2c4e9..074259920cd 100644 --- a/components/style/gecko/restyle_damage.rs +++ b/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) -> 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) } From 58173367bee0f741de65c05749bbf15628f3519a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 04:55:54 +0100 Subject: [PATCH 39/44] style: Document Gecko's snapshot code. --- components/style/gecko/snapshot.rs | 11 ++++++++++- components/style/gecko/snapshot_helpers.rs | 6 ++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/components/style/gecko/snapshot.rs b/components/style/gecko/snapshot.rs index 08d5288a8d0..4398fbf8e29 100644 --- a/components/style/gecko/snapshot.rs +++ b/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, diff --git a/components/style/gecko/snapshot_helpers.rs b/components/style/gecko/snapshot_helpers.rs index ee7fdd14421..9216c92cd2f 100644 --- a/components/style/gecko/snapshot_helpers.rs +++ b/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 = 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(item: T, name: &Atom, getter: ClassOrClassList) -> bool @@ -30,6 +34,8 @@ pub fn has_class(item: T, } +/// Given an item, a callback, and a getter, execute `callback` for each class +/// this `item` has. pub fn each_class(item: T, mut callback: F, getter: ClassOrClassList) From 632c99676b15a810298cb8409b4c3878760e7b67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 04:56:18 +0100 Subject: [PATCH 40/44] style: Document Gecko's traversal code. Actually, I think most of the functions in DOMTraversalContext deserve a nice comment, but that's probably fine as a followup, I want to land the require-docs thing now. --- components/style/gecko/traversal.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/components/style/gecko/traversal.rs b/components/style/gecko/traversal.rs index 7f83fbe5426..13c9ddabddf 100644 --- a/components/style/gecko/traversal.rs +++ b/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, From 94f8c0fc639c1e4eaf4cd33b86a18f178722188f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 04:57:22 +0100 Subject: [PATCH 41/44] style: Document the remaining public parts of Gecko's values module. --- components/style/gecko/values.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/components/style/gecko/values.rs b/components/style/gecko/values.rs index 23a4920d060..a187f2f78c6 100644 --- a/components/style/gecko/values.rs +++ b/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}; @@ -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 { From e9d3ac03a913717222865d279f60486d6f7844ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 04:58:01 +0100 Subject: [PATCH 42/44] style: Document Gecko's wrapper module. --- components/style/gecko/wrapper.rs | 58 +++++++++++++++++++++++++------ 1 file changed, 48 insertions(+), 10 deletions(-) diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index a3fe48ccc89..6764269c282 100644 --- a/components/style/gecko/wrapper.rs +++ b/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 -// 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 -// could implement refcounting if need be (at a potentially non-trivial -// performance cost) by implementing Drop and making LayoutFoo non-Copy. +/// A simple wrapper over a non-null Gecko node (`nsINode`) pointer. +/// +/// Important: We don't currently refcount the DOM, because the wrapper lifetime +/// 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 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 -// like Servo does. However, for nodes with anonymous children, we use a custom -// (heavier-weight) Gecko-implemented iterator. +/// A wrapper on top of two kind of iterators, depending on the parent being +/// iterated. +/// +/// 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>), + /// 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 { 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(&self, _hints: &mut V) - where V: Push + where V: Push, { // 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; } From 2cebd3bc961d3592845a928b129220bc43176d3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 04:58:35 +0100 Subject: [PATCH 43/44] style: Document gecko_string_cache. --- components/style/gecko_string_cache/mod.rs | 25 +++++++++++++++++-- .../style/gecko_string_cache/namespace.rs | 7 ++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/components/style/gecko_string_cache/mod.rs b/components/style/gecko_string_cache/mod.rs index 5165b21f5d4..784ae732b52 100644 --- a/components/style/gecko_string_cache/mod.rs +++ b/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(&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(ptr: *mut nsIAtom, callback: &mut F) where F: FnMut(&Atom) { let atom = Atom(WeakAtom::new(ptr)); callback(&atom); diff --git a/components/style/gecko_string_cache/namespace.rs b/components/style/gecko_string_cache/namespace.rs index a89c1873423..117f1923151 100644 --- a/components/style/gecko_string_cache/namespace.rs +++ b/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 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()) From a8fdf5b6b9cf615c7ad18d01871491f3e0ca2562 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Jan 2017 04:59:15 +0100 Subject: [PATCH 44/44] style: Require documentation also for a Gecko build. --- components/style/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/components/style/lib.rs b/components/style/lib.rs index 70737c81644..e740d6ad385 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -26,7 +26,7 @@ #![cfg_attr(feature = "servo", feature(proc_macro))] #![deny(warnings)] -#![cfg_attr(feature = "servo", deny(missing_docs))] +#![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 @@ -150,7 +150,7 @@ 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)] @@ -159,7 +159,7 @@ pub mod properties { } #[cfg(feature = "gecko")] -#[allow(unsafe_code)] +#[allow(unsafe_code, missing_docs)] pub mod gecko_properties { include!(concat!(env!("OUT_DIR"), "/gecko_properties.rs")); }