diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index f7d4bd05fde..0a555617f0b 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -113,7 +113,7 @@ use style::media_queries::{Device, MediaType}; use style::parallel::WorkQueueData; use style::parser::ParserContextExtraData; use style::selector_matching::Stylist; -use style::stylesheets::{CSSRuleIteratorExt, Origin, Stylesheet, UserAgentStylesheets}; +use style::stylesheets::{Origin, Stylesheet, UserAgentStylesheets}; use style::thread_state; use style::timer::Timer; use style::workqueue::WorkQueue; @@ -354,21 +354,21 @@ fn add_font_face_rules(stylesheet: &Stylesheet, outstanding_web_fonts_counter: &Arc) { if opts::get().load_webfonts_synchronously { let (sender, receiver) = ipc::channel().unwrap(); - for font_face in stylesheet.effective_rules(&device).font_face() { + stylesheet.effective_font_face_rules(&device, |font_face| { let effective_sources = font_face.effective_sources(); font_cache_thread.add_web_font(font_face.family.clone(), effective_sources, sender.clone()); receiver.recv().unwrap(); - } + }) } else { - for font_face in stylesheet.effective_rules(&device).font_face() { + stylesheet.effective_font_face_rules(&device, |font_face| { let effective_sources = font_face.effective_sources(); outstanding_web_fonts_counter.fetch_add(1, Ordering::SeqCst); font_cache_thread.add_web_font(font_face.family.clone(), effective_sources, (*font_cache_sender).clone()); - } + }) } } diff --git a/components/script/dom/htmlmetaelement.rs b/components/script/dom/htmlmetaelement.rs index 1b057783908..219b0cbce90 100644 --- a/components/script/dom/htmlmetaelement.rs +++ b/components/script/dom/htmlmetaelement.rs @@ -16,6 +16,7 @@ use dom::htmlelement::HTMLElement; use dom::htmlheadelement::HTMLHeadElement; use dom::node::{Node, UnbindContext, document_from_node}; use dom::virtualmethods::VirtualMethods; +use parking_lot::RwLock; use std::ascii::AsciiExt; use std::sync::Arc; use string_cache::Atom; @@ -80,7 +81,7 @@ impl HTMLMetaElement { if !content.is_empty() { if let Some(translated_rule) = ViewportRule::from_meta(&**content) { *self.stylesheet.borrow_mut() = Some(Arc::new(Stylesheet { - rules: vec![CSSRule::Viewport(Arc::new(translated_rule))], + rules: vec![CSSRule::Viewport(Arc::new(RwLock::new(translated_rule)))], origin: Origin::Author, media: None, // Viewport constraints are always recomputed on resize; they don't need to diff --git a/components/style/keyframes.rs b/components/style/keyframes.rs index 3283e535fc8..f825fe11a52 100644 --- a/components/style/keyframes.rs +++ b/components/style/keyframes.rs @@ -171,12 +171,12 @@ fn get_animated_properties(keyframe: &Keyframe) -> Vec { } impl KeyframesAnimation { - pub fn from_keyframes(keyframes: &[Arc]) -> Option { + pub fn from_keyframes(keyframes: &[Arc>]) -> Option { if keyframes.is_empty() { return None; } - let animated_properties = get_animated_properties(&keyframes[0]); + let animated_properties = get_animated_properties(&keyframes[0].read()); if animated_properties.is_empty() { return None; } @@ -184,6 +184,7 @@ impl KeyframesAnimation { let mut steps = vec![]; for keyframe in keyframes { + let keyframe = keyframe.read(); for percentage in keyframe.selector.0.iter() { steps.push(KeyframesStep::new(*percentage, KeyframesStepValue::Declarations { block: keyframe.block.clone(), @@ -224,7 +225,7 @@ struct KeyframeListParser<'a> { context: &'a ParserContext<'a>, } -pub fn parse_keyframe_list(context: &ParserContext, input: &mut Parser) -> Vec> { +pub fn parse_keyframe_list(context: &ParserContext, input: &mut Parser) -> Vec>> { RuleListParser::new_for_nested_rule(input, KeyframeListParser { context: context }) .filter_map(Result::ok) .collect() @@ -233,12 +234,12 @@ pub fn parse_keyframe_list(context: &ParserContext, input: &mut Parser) -> Vec AtRuleParser for KeyframeListParser<'a> { type Prelude = Void; - type AtRule = Arc; + type AtRule = Arc>; } impl<'a> QualifiedRuleParser for KeyframeListParser<'a> { type Prelude = KeyframeSelector; - type QualifiedRule = Arc; + type QualifiedRule = Arc>; fn parse_prelude(&mut self, input: &mut Parser) -> Result { let start = input.position(); @@ -271,13 +272,13 @@ impl<'a> QualifiedRuleParser for KeyframeListParser<'a> { } // `parse_important` is not called here, `!important` is not allowed in keyframe blocks. } - Ok(Arc::new(Keyframe { + Ok(Arc::new(RwLock::new(Keyframe { selector: prelude, block: Arc::new(RwLock::new(PropertyDeclarationBlock { declarations: declarations, important_count: 0, })), - })) + }))) } } diff --git a/components/style/selector_matching.rs b/components/style/selector_matching.rs index e7a4c47f6a5..02f06d20d1d 100644 --- a/components/style/selector_matching.rs +++ b/components/style/selector_matching.rs @@ -30,8 +30,8 @@ use std::slice; use std::sync::Arc; use string_cache::Atom; use style_traits::viewport::ViewportConstraints; -use stylesheets::{CSSRule, CSSRuleIteratorExt, Origin, Stylesheet, UserAgentStylesheets}; -use viewport::{MaybeNew, ViewportRuleCascade}; +use stylesheets::{CSSRule, Origin, Stylesheet, UserAgentStylesheets}; +use viewport::{self, MaybeNew, ViewportRule}; pub type FnvHashMap = HashMap>; @@ -162,79 +162,90 @@ impl Stylist { if !stylesheet.is_effective_for_device(&self.device) { return; } - let mut rules_source_order = self.rules_source_order; - for rule in stylesheet.effective_rules(&self.device) { + // Work around borrowing all of `self` if `self.something` is used in it + // instead of just `self.something` + macro_rules! borrow_self_field { + ($($x: ident),+) => { + $( + let $x = &mut self.$x; + )+ + } + } + borrow_self_field!(pseudos_map, element_map, state_deps, sibling_affecting_selectors, + non_common_style_affecting_attributes_selectors, rules_source_order, + animations, precomputed_pseudo_element_decls); + stylesheet.effective_rules(&self.device, |rule| { match *rule { CSSRule::Style(ref style_rule) => { + let style_rule = style_rule.read(); for selector in &style_rule.selectors { let map = if let Some(ref pseudo) = selector.pseudo_element { - self.pseudos_map + pseudos_map .entry(pseudo.clone()) .or_insert_with(PerPseudoElementSelectorMap::new) .borrow_for_origin(&stylesheet.origin) } else { - self.element_map.borrow_for_origin(&stylesheet.origin) + element_map.borrow_for_origin(&stylesheet.origin) }; map.insert(Rule { selector: selector.complex_selector.clone(), declarations: style_rule.block.clone(), specificity: selector.specificity, - source_order: rules_source_order, + source_order: *rules_source_order, }); } - rules_source_order += 1; + *rules_source_order += 1; for selector in &style_rule.selectors { - self.state_deps.note_selector(&selector.complex_selector); + state_deps.note_selector(&selector.complex_selector); if selector.affects_siblings() { - self.sibling_affecting_selectors.push(selector.clone()); + sibling_affecting_selectors.push(selector.clone()); } if selector.matches_non_common_style_affecting_attribute() { - self.non_common_style_affecting_attributes_selectors.push(selector.clone()); + non_common_style_affecting_attributes_selectors.push(selector.clone()); } } - - self.rules_source_order = rules_source_order; } CSSRule::Keyframes(ref keyframes_rule) => { - debug!("Found valid keyframes rule: {:?}", keyframes_rule); + let keyframes_rule = keyframes_rule.read(); + debug!("Found valid keyframes rule: {:?}", *keyframes_rule); if let Some(animation) = KeyframesAnimation::from_keyframes(&keyframes_rule.keyframes) { debug!("Found valid keyframe animation: {:?}", animation); - self.animations.insert(keyframes_rule.name.clone(), + animations.insert(keyframes_rule.name.clone(), animation); } else { // If there's a valid keyframes rule, even if it doesn't // produce an animation, should shadow other animations // with the same name. - self.animations.remove(&keyframes_rule.name); + animations.remove(&keyframes_rule.name); } } // We don't care about any other rule. _ => {} } - } + }); debug!("Stylist stats:"); debug!(" - Got {} sibling-affecting selectors", - self.sibling_affecting_selectors.len()); + sibling_affecting_selectors.len()); debug!(" - Got {} non-common-style-attribute-affecting selectors", - self.non_common_style_affecting_attributes_selectors.len()); + non_common_style_affecting_attributes_selectors.len()); debug!(" - Got {} deps for style-hint calculation", - self.state_deps.len()); + state_deps.len()); TheSelectorImpl::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) { + if let Some(map) = pseudos_map.remove(&pseudo) { let mut declarations = vec![]; map.user_agent.get_universal_rules(&mut declarations); - self.precomputed_pseudo_element_decls.insert(pseudo, declarations); + precomputed_pseudo_element_decls.insert(pseudo, declarations); } }) } @@ -296,18 +307,32 @@ impl Stylist { } pub fn set_device(&mut self, mut device: Device, stylesheets: &[Arc]) { - let cascaded_rule = stylesheets.iter() - .flat_map(|s| s.effective_rules(&self.device).viewport()) - .cascade(); + let cascaded_rule = ViewportRule { + declarations: viewport::Cascade::from_stylesheets(stylesheets, &device).finish(), + }; self.viewport_constraints = ViewportConstraints::maybe_new(device.viewport_size, &cascaded_rule); if let Some(ref constraints) = self.viewport_constraints { device = Device::new(MediaType::Screen, constraints.size); } + fn mq_eval_changed(rules: &[CSSRule], before: &Device, after: &Device) -> bool { + for rule in rules { + if rule.with_nested_rules_and_mq(|rules, mq| { + if let Some(mq) = mq { + if mq.evaluate(before) != mq.evaluate(after) { + return true + } + } + mq_eval_changed(rules, before, after) + }) { + return true + } + } + false + } self.is_device_dirty |= stylesheets.iter().any(|stylesheet| { - stylesheet.rules().media().any(|media_rule| - media_rule.evaluate(&self.device) != media_rule.evaluate(&device)) + mq_eval_changed(&stylesheet.rules, &self.device, &device) }); self.device = device; diff --git a/components/style/stylesheets.rs b/components/style/stylesheets.rs index 9cd106a2110..715ee3fe3d3 100644 --- a/components/style/stylesheets.rs +++ b/components/style/stylesheets.rs @@ -16,10 +16,7 @@ use parser::{ParserContext, ParserContextExtraData, log_css_error}; use properties::{PropertyDeclarationBlock, parse_property_declaration_list}; use selector_impl::TheSelectorImpl; use selectors::parser::{Selector, parse_selector_list}; -use smallvec::SmallVec; use std::cell::Cell; -use std::iter::Iterator; -use std::slice; use std::sync::Arc; use string_cache::{Atom, Namespace}; use url::Url; @@ -67,14 +64,36 @@ pub enum CSSRule { // No Charset here, CSSCharsetRule has been removed from CSSOM // https://drafts.csswg.org/cssom/#changes-from-5-december-2013 - Namespace(Arc), - Style(Arc), - Media(Arc), - FontFace(Arc), - Viewport(Arc), - Keyframes(Arc), + Namespace(Arc>), + Style(Arc>), + Media(Arc>), + FontFace(Arc>), + Viewport(Arc>), + Keyframes(Arc>), } +impl CSSRule { + /// Call `f` with the slice of rules directly contained inside this rule. + /// + /// Note that only some types of rules can contain rules. An empty slice is used for others. + pub fn with_nested_rules_and_mq(&self, mut f: F) -> R + where F: FnMut(&[CSSRule], Option<&MediaQueryList>) -> R { + match *self { + CSSRule::Namespace(_) | + CSSRule::Style(_) | + CSSRule::FontFace(_) | + CSSRule::Viewport(_) | + CSSRule::Keyframes(_) => { + f(&[], None) + } + CSSRule::Media(ref lock) => { + let media_rule = lock.read(); + let mq = media_rule.media_queries.read(); + f(&media_rule.rules, Some(&mq)) + } + } + } +} #[derive(Debug, PartialEq)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] @@ -87,23 +106,15 @@ pub struct NamespaceRule { #[derive(Debug)] pub struct KeyframesRule { pub name: Atom, - pub keyframes: Vec>, + pub keyframes: Vec>>, } #[derive(Debug)] pub struct MediaRule { - pub media_queries: Arc, + pub media_queries: Arc>, pub rules: Vec, } - -impl MediaRule { - #[inline] - pub fn evaluate(&self, device: &Device) -> bool { - self.media_queries.evaluate(device) - } -} - #[derive(Debug)] pub struct StyleRule { pub selectors: Vec>, @@ -189,12 +200,6 @@ impl Stylesheet { self.media.as_ref().map_or(true, |ref media| media.evaluate(device)) } - /// Return an iterator over all the rules within the style-sheet. - #[inline] - pub fn rules(&self) -> Rules { - Rules::new(self.rules.iter(), None) - } - /// Return an iterator over the effective rules within the style-sheet, as /// according to the supplied `Device`. /// @@ -202,165 +207,48 @@ impl Stylesheet { /// nested rules will be skipped. Use `rules` if all rules need to be /// examined. #[inline] - pub fn effective_rules<'a>(&'a self, device: &'a Device) -> Rules<'a> { - Rules::new(self.rules.iter(), Some(device)) + pub fn effective_rules(&self, device: &Device, mut f: F) where F: FnMut(&CSSRule) { + effective_rules(&self.rules, device, &mut f); } } -/// `CSSRule` iterator. -/// -/// The iteration order is pre-order. Specifically, this implies that a -/// conditional group rule will come before its nested rules. -pub struct Rules<'a> { - // 2 because normal case is likely to be just one level of nesting (@media) - stack: SmallVec<[slice::Iter<'a, CSSRule>; 2]>, - device: Option<&'a Device> -} - -impl<'a> Rules<'a> { - fn new(iter: slice::Iter<'a, CSSRule>, device: Option<&'a Device>) -> Rules<'a> { - let mut stack: SmallVec<[slice::Iter<'a, CSSRule>; 2]> = SmallVec::new(); - stack.push(iter); - - Rules { stack: stack, device: device } +fn effective_rules(rules: &[CSSRule], device: &Device, f: &mut F) where F: FnMut(&CSSRule) { + for rule in rules { + f(rule); + rule.with_nested_rules_and_mq(|rules, mq| { + if let Some(media_queries) = mq { + if !media_queries.evaluate(device) { + return + } + } + effective_rules(rules, device, f) + }) } } -impl<'a> Iterator for Rules<'a> { - type Item = &'a CSSRule; - - fn next(&mut self) -> Option<&'a CSSRule> { - while !self.stack.is_empty() { - let top = self.stack.len() - 1; - while let Some(rule) = self.stack[top].next() { - // handle conditional group rules - if let &CSSRule::Media(ref rule) = rule { - if let Some(device) = self.device { - if rule.evaluate(device) { - self.stack.push(rule.rules.iter()); - } else { - continue +macro_rules! rule_filter { + ($( $method: ident($variant:ident => $rule_type: ident), )+) => { + impl Stylesheet { + $( + pub fn $method(&self, device: &Device, mut f: F) where F: FnMut(&$rule_type) { + self.effective_rules(device, |rule| { + if let CSSRule::$variant(ref lock) = *rule { + let rule = lock.read(); + f(&rule) } - } else { - self.stack.push(rule.rules.iter()); - } + }) } - - return Some(rule) - } - - self.stack.pop(); - } - None - } - - #[inline] - fn size_hint(&self) -> (usize, Option) { - // TODO: track total number of rules in style-sheet for upper bound? - (0, None) - } -} - -pub mod rule_filter { - //! Specific `CSSRule` variant iterators. - - use std::marker::PhantomData; - use super::{CSSRule, KeyframesRule, MediaRule, StyleRule}; - use super::super::font_face::FontFaceRule; - use super::super::viewport::ViewportRule; - - macro_rules! rule_filter { - ($variant:ident -> $value:ty) => { - /// An iterator that only yields rules that are of the synonymous `CSSRule` variant. - #[must_use = "iterator adaptors are lazy and do nothing unless consumed"] - pub struct $variant<'a, I> { - iter: I, - _lifetime: PhantomData<&'a ()> - } - - impl<'a, I> $variant<'a, I> - where I: Iterator { - #[inline] - pub fn new(iter: I) -> $variant<'a, I> { - $variant { - iter: iter, - _lifetime: PhantomData - } - } - } - - impl<'a, I> Iterator for $variant<'a, I> - where I: Iterator { - type Item = &'a $value; - - fn next(&mut self) -> Option<&'a $value> { - while let Some(rule) = self.iter.next() { - match *rule { - CSSRule::$variant(ref value) => return Some(value), - _ => continue - } - } - None - } - - #[inline] - fn size_hint(&self) -> (usize, Option) { - (0, self.iter.size_hint().1) - } - } + )+ } } - - rule_filter!(Media -> MediaRule); - rule_filter!(Style -> StyleRule); - rule_filter!(FontFace -> FontFaceRule); - rule_filter!(Viewport -> ViewportRule); - rule_filter!(Keyframes -> KeyframesRule); } -/// Extension methods for `CSSRule` iterators. -pub trait CSSRuleIteratorExt<'a>: Iterator + Sized { - /// Yield only @font-face rules. - fn font_face(self) -> rule_filter::FontFace<'a, Self>; - - /// Yield only @media rules. - fn media(self) -> rule_filter::Media<'a, Self>; - - /// Yield only style rules. - fn style(self) -> rule_filter::Style<'a, Self>; - - /// Yield only @viewport rules. - fn viewport(self) -> rule_filter::Viewport<'a, Self>; - - /// Yield only @keyframes rules. - fn keyframes(self) -> rule_filter::Keyframes<'a, Self>; -} - -impl<'a, I> CSSRuleIteratorExt<'a> for I where I: Iterator { - #[inline] - fn font_face(self) -> rule_filter::FontFace<'a, I> { - rule_filter::FontFace::new(self) - } - - #[inline] - fn media(self) -> rule_filter::Media<'a, I> { - rule_filter::Media::new(self) - } - - #[inline] - fn style(self) -> rule_filter::Style<'a, I> { - rule_filter::Style::new(self) - } - - #[inline] - fn viewport(self) -> rule_filter::Viewport<'a, I> { - rule_filter::Viewport::new(self) - } - - #[inline] - fn keyframes(self) -> rule_filter::Keyframes<'a, I> { - rule_filter::Keyframes::new(self) - } +rule_filter! { + effective_style_rules(Style => StyleRule), + effective_media_rules(Media => MediaRule), + effective_font_face_rules(FontFace => FontFaceRule), + effective_viewport_rules(Viewport => ViewportRule), + effective_keyframes_rules(Keyframes => KeyframesRule), } fn parse_nested_rules(context: &ParserContext, input: &mut Parser) -> Vec { @@ -399,7 +287,7 @@ enum AtRulePrelude { /// A @font-face rule prelude. FontFace, /// A @media rule prelude, with its media queries. - Media(Arc), + Media(Arc>), /// A @viewport rule prelude. Viewport, /// A @keyframes rule, with its animation name. @@ -440,10 +328,12 @@ impl<'a> AtRuleParser for TopLevelRuleParser<'a> { None }; - return Ok(AtRuleType::WithoutBlock(CSSRule::Namespace(Arc::new(NamespaceRule { - prefix: opt_prefix, - url: url, - })))) + return Ok(AtRuleType::WithoutBlock(CSSRule::Namespace(Arc::new(RwLock::new( + NamespaceRule { + prefix: opt_prefix, + url: url, + } + ))))) } else { return Err(()) // "@namespace must be before any rule but @charset and @import" } @@ -498,7 +388,7 @@ impl<'a, 'b> AtRuleParser for NestedRuleParser<'a, 'b> { match_ignore_ascii_case! { name, "media" => { let media_queries = parse_media_query_list(input); - Ok(AtRuleType::WithBlock(AtRulePrelude::Media(Arc::new(media_queries)))) + Ok(AtRuleType::WithBlock(AtRulePrelude::Media(Arc::new(RwLock::new(media_queries))))) }, "font-face" => { Ok(AtRuleType::WithBlock(AtRulePrelude::FontFace)) @@ -527,22 +417,24 @@ impl<'a, 'b> AtRuleParser for NestedRuleParser<'a, 'b> { fn parse_block(&mut self, prelude: AtRulePrelude, input: &mut Parser) -> Result { match prelude { AtRulePrelude::FontFace => { - Ok(CSSRule::FontFace(Arc::new(try!(parse_font_face_block(self.context, input))))) + Ok(CSSRule::FontFace(Arc::new(RwLock::new( + try!(parse_font_face_block(self.context, input)))))) } AtRulePrelude::Media(media_queries) => { - Ok(CSSRule::Media(Arc::new(MediaRule { + Ok(CSSRule::Media(Arc::new(RwLock::new(MediaRule { media_queries: media_queries, rules: parse_nested_rules(self.context, input), - }))) + })))) } AtRulePrelude::Viewport => { - Ok(CSSRule::Viewport(Arc::new(try!(ViewportRule::parse(input, self.context))))) + Ok(CSSRule::Viewport(Arc::new(RwLock::new( + try!(ViewportRule::parse(input, self.context)))))) } AtRulePrelude::Keyframes(name) => { - Ok(CSSRule::Keyframes(Arc::new(KeyframesRule { + Ok(CSSRule::Keyframes(Arc::new(RwLock::new(KeyframesRule { name: name, keyframes: parse_keyframe_list(&self.context, input), - }))) + })))) } } } @@ -558,9 +450,9 @@ impl<'a, 'b> QualifiedRuleParser for NestedRuleParser<'a, 'b> { fn parse_block(&mut self, prelude: Vec>, input: &mut Parser) -> Result { - Ok(CSSRule::Style(Arc::new(StyleRule { + Ok(CSSRule::Style(Arc::new(RwLock::new(StyleRule { selectors: prelude, block: Arc::new(RwLock::new(parse_property_declaration_list(self.context, input))) - }))) + })))) } } diff --git a/components/style/viewport.rs b/components/style/viewport.rs index ef89484b21a..f46659dd7c4 100644 --- a/components/style/viewport.rs +++ b/components/style/viewport.rs @@ -12,15 +12,17 @@ use cssparser::{AtRuleParser, DeclarationListParser, DeclarationParser, Parser, use cssparser::ToCss; use euclid::scale_factor::ScaleFactor; use euclid::size::{Size2D, TypedSize2D}; +use media_queries::Device; use parser::{ParserContext, log_css_error}; use properties::ComputedValues; use std::ascii::AsciiExt; +use std::borrow::Cow; use std::fmt; use std::iter::Enumerate; use std::str::Chars; use style_traits::ViewportPx; use style_traits::viewport::{Orientation, UserZoom, ViewportConstraints, Zoom}; -use stylesheets::Origin; +use stylesheets::{Stylesheet, Origin}; use values::computed::{Context, ToComputedValue}; use values::specified::{Length, LengthOrPercentageOrAuto, ViewportPercentageLength}; @@ -54,7 +56,7 @@ macro_rules! declare_viewport_descriptor_inner { [ ] $number_of_variants: expr ) => { - #[derive(Copy, Clone, Debug, PartialEq)] + #[derive(Clone, Debug, PartialEq)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub enum ViewportDescriptor { $( @@ -188,7 +190,7 @@ struct ViewportRuleParser<'a, 'b: 'a> { context: &'a ParserContext<'b> } -#[derive(Copy, Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub struct ViewportDescriptorDeclaration { pub origin: Origin, @@ -309,31 +311,26 @@ impl ViewportRule { { let parser = ViewportRuleParser { context: context }; - let mut errors = vec![]; - let valid_declarations = DeclarationListParser::new(input, parser) - .filter_map(|result| { - match result { - Ok(declarations) => Some(declarations), - Err(range) => { - errors.push(range); - None + let mut cascade = Cascade::new(); + let mut parser = DeclarationListParser::new(input, parser); + while let Some(result) = parser.next() { + match result { + Ok(declarations) => { + for declarations in declarations { + cascade.add(Cow::Owned(declarations)) } } - }) - .flat_map(|declarations| declarations.into_iter()) - .collect::>(); - - for range in errors { - let pos = range.start; - let message = format!("Unsupported @viewport descriptor declaration: '{}'", - input.slice(range)); - log_css_error(input, pos, &*message, &context); + Err(range) => { + let pos = range.start; + let message = format!("Unsupported @viewport descriptor declaration: '{}'", + parser.input.slice(range)); + log_css_error(parser.input, pos, &*message, &context); + } + } } - - Ok(ViewportRule { declarations: valid_declarations.iter().cascade() }) + Ok(ViewportRule { declarations: cascade.finish() }) } - #[allow(unsafe_code)] pub fn from_meta(content: &str) -> Option { let mut declarations = vec![None; VIEWPORT_DESCRIPTOR_VARIANTS]; macro_rules! push_descriptor { @@ -471,25 +468,6 @@ impl ViewportRule { } } -pub trait ViewportRuleCascade: Iterator + Sized { - fn cascade(self) -> ViewportRule; -} - -impl<'a, I> ViewportRuleCascade for I - where I: Iterator -{ - #[inline] - fn cascade(self) -> ViewportRule { - ViewportRule { - declarations: self.flat_map(|r| r.declarations.iter()).cascade() - } - } -} - -trait ViewportDescriptorDeclarationCascade: Iterator + Sized { - fn cascade(self) -> Vec; -} - /// Computes the cascade precedence as according to /// http://dev.w3.org/csswg/css-cascade/#cascade-origin fn cascade_precendence(origin: Origin, important: bool) -> u8 { @@ -512,45 +490,53 @@ impl ViewportDescriptorDeclaration { } } -#[allow(unsafe_code)] -fn cascade<'a, I>(iter: I) -> Vec - where I: Iterator -{ - let mut declarations: Vec> = - vec![None; VIEWPORT_DESCRIPTOR_VARIANTS]; +pub struct Cascade { + declarations: Vec>, + count_so_far: usize, +} - // index is used to reconstruct order of appearance after all declarations - // have been added to the map - let mut index = 0; - for declaration in iter { - let descriptor = declaration.descriptor.discriminant_value(); - - match declarations[descriptor] { - Some((ref mut entry_index, ref mut entry_declaration)) => { - if declaration.higher_or_equal_precendence(entry_declaration) { - *entry_declaration = declaration; - *entry_index = index; - index += 1; - } - } - ref mut entry @ None => { - *entry = Some((index, declaration)); - index += 1; - } +impl Cascade { + pub fn new() -> Self { + Cascade { + declarations: vec![None; VIEWPORT_DESCRIPTOR_VARIANTS], + count_so_far: 0, } } - // sort the descriptors by order of appearance - declarations.sort_by_key(|entry| entry.map(|(index, _)| index)); - declarations.into_iter().filter_map(|entry| entry.map(|(_, decl)| *decl)).collect::>() -} + pub fn from_stylesheets<'a, I>(stylesheets: I, device: &Device) -> Self + where I: IntoIterator, I::Item: AsRef { + let mut cascade = Self::new(); + for stylesheet in stylesheets { + stylesheet.as_ref().effective_viewport_rules(device, |rule| { + for declaration in &rule.declarations { + cascade.add(Cow::Borrowed(declaration)) + } + }) + } + cascade + } -impl<'a, I> ViewportDescriptorDeclarationCascade for I - where I: Iterator -{ - #[inline] - fn cascade(self) -> Vec { - cascade(self) + pub fn add(&mut self, declaration: Cow) { + let descriptor = declaration.descriptor.discriminant_value(); + + match self.declarations[descriptor] { + Some((ref mut order_of_appearance, ref mut entry_declaration)) => { + if declaration.higher_or_equal_precendence(entry_declaration) { + *entry_declaration = declaration.into_owned(); + *order_of_appearance = self.count_so_far; + } + } + ref mut entry @ None => { + *entry = Some((self.count_so_far, declaration.into_owned())); + } + } + self.count_so_far += 1; + } + + pub fn finish(mut self) -> Vec { + // sort the descriptors by order of appearance + self.declarations.sort_by_key(|entry| entry.as_ref().map(|&(index, _)| index)); + self.declarations.into_iter().filter_map(|entry| entry.map(|(_, decl)| decl)).collect() } } diff --git a/tests/unit/style/media_queries.rs b/tests/unit/style/media_queries.rs index 480503b8835..4565b57ae1c 100644 --- a/tests/unit/style/media_queries.rs +++ b/tests/unit/style/media_queries.rs @@ -9,7 +9,7 @@ use std::borrow::ToOwned; use style::error_reporting::ParseErrorReporter; use style::media_queries::*; use style::parser::ParserContextExtraData; -use style::stylesheets::{Stylesheet, Origin, CSSRuleIteratorExt}; +use style::stylesheets::{Stylesheet, Origin, CSSRule}; use style::values::specified; use url::Url; @@ -28,18 +28,30 @@ fn test_media_rule(css: &str, callback: F) where F: Fn(&MediaQueryList, &str) let stylesheet = Stylesheet::from_str(css, url, Origin::Author, Box::new(CSSErrorReporterTest), ParserContextExtraData::default()); let mut rule_count = 0; - for rule in stylesheet.rules().media() { + media_queries(&stylesheet.rules, &mut |mq| { rule_count += 1; - callback(&rule.media_queries, css); - } + callback(mq, css); + }); assert!(rule_count > 0); } +fn media_queries(rules: &[CSSRule], f: &mut F) where F: FnMut(&MediaQueryList) { + for rule in rules { + rule.with_nested_rules_and_mq(|rules, mq| { + if let Some(mq) = mq { + f(mq) + } + media_queries(rules, f) + }) + } +} + fn media_query_test(device: &Device, css: &str, expected_rule_count: usize) { let url = Url::parse("http://localhost").unwrap(); let ss = Stylesheet::from_str(css, url, Origin::Author, Box::new(CSSErrorReporterTest), ParserContextExtraData::default()); - let rule_count = ss.effective_rules(device).style().count(); + let mut rule_count = 0; + ss.effective_style_rules(device, |_| rule_count += 1); assert!(rule_count == expected_rule_count, css.to_owned()); } diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs index dd1b8eba094..c664da50731 100644 --- a/tests/unit/style/stylesheets.rs +++ b/tests/unit/style/stylesheets.rs @@ -56,11 +56,11 @@ fn test_parse_stylesheet() { media: None, dirty_on_viewport_size_change: false, rules: vec![ - CSSRule::Namespace(Arc::new(NamespaceRule { + CSSRule::Namespace(Arc::new(RwLock::new(NamespaceRule { prefix: None, url: NsAtom(Atom::from("http://www.w3.org/1999/xhtml")) - })), - CSSRule::Style(Arc::new(StyleRule { + }))), + CSSRule::Style(Arc::new(RwLock::new(StyleRule { selectors: vec![ Selector { complex_selector: Arc::new(ComplexSelector { @@ -98,8 +98,8 @@ fn test_parse_stylesheet() { ], important_count: 2, })), - })), - CSSRule::Style(Arc::new(StyleRule { + }))), + CSSRule::Style(Arc::new(RwLock::new(StyleRule { selectors: vec![ Selector { complex_selector: Arc::new(ComplexSelector { @@ -144,8 +144,8 @@ fn test_parse_stylesheet() { ], important_count: 0, })), - })), - CSSRule::Style(Arc::new(StyleRule { + }))), + CSSRule::Style(Arc::new(RwLock::new(StyleRule { selectors: vec![ Selector { complex_selector: Arc::new(ComplexSelector { @@ -220,11 +220,11 @@ fn test_parse_stylesheet() { ], important_count: 0, })), - })), - CSSRule::Keyframes(Arc::new(KeyframesRule { + }))), + CSSRule::Keyframes(Arc::new(RwLock::new(KeyframesRule { name: "foo".into(), keyframes: vec![ - Arc::new(Keyframe { + Arc::new(RwLock::new(Keyframe { selector: KeyframeSelector::new_for_unit_testing( vec![KeyframePercentage::new(0.)]), block: Arc::new(RwLock::new(PropertyDeclarationBlock { @@ -235,8 +235,8 @@ fn test_parse_stylesheet() { ], important_count: 0, })) - }), - Arc::new(Keyframe { + })), + Arc::new(RwLock::new(Keyframe { selector: KeyframeSelector::new_for_unit_testing( vec![KeyframePercentage::new(1.)]), block: Arc::new(RwLock::new(PropertyDeclarationBlock { @@ -251,9 +251,9 @@ fn test_parse_stylesheet() { ], important_count: 0, })), - }), + })), ] - })) + }))) ], }; diff --git a/tests/unit/style/viewport.rs b/tests/unit/style/viewport.rs index 2fcfc217b2f..bde97310e41 100644 --- a/tests/unit/style/viewport.rs +++ b/tests/unit/style/viewport.rs @@ -9,7 +9,7 @@ use media_queries::CSSErrorReporterTest; use style::error_reporting::ParseErrorReporter; use style::media_queries::{Device, MediaType}; use style::parser::{ParserContext, ParserContextExtraData}; -use style::stylesheets::{Stylesheet, Origin, CSSRuleIteratorExt}; +use style::stylesheets::{Stylesheet, Origin}; use style::values::specified::Length::{self, ViewportPercentage}; use style::values::specified::LengthOrPercentageOrAuto::{self, Auto}; use style::values::specified::ViewportPercentageLength::Vw; @@ -19,8 +19,13 @@ use url::Url; macro_rules! stylesheet { ($css:expr, $origin:ident, $error_reporter:expr) => { - Stylesheet::from_str($css, Url::parse("http://localhost").unwrap(), Origin::$origin, $error_reporter, - ParserContextExtraData::default()); + Box::new(Stylesheet::from_str( + $css, + Url::parse("http://localhost").unwrap(), + Origin::$origin, + $error_reporter, + ParserContextExtraData::default() + )) } } @@ -33,10 +38,10 @@ fn test_viewport_rule(css: &str, ::util::prefs::PrefValue::Boolean(true)); let stylesheet = stylesheet!(css, Author, Box::new(CSSErrorReporterTest)); let mut rule_count = 0; - for rule in stylesheet.effective_rules(&device).viewport() { + stylesheet.effective_viewport_rules(&device, |rule| { rule_count += 1; callback(&rule.declarations, css); - } + }); assert!(rule_count > 0); } @@ -253,10 +258,7 @@ fn multiple_stylesheets_cascading() { stylesheet!("@viewport { min-width: 200px; min-height: 200px; }", User, error_reporter.clone()), stylesheet!("@viewport { min-width: 300px; }", Author, error_reporter.clone())]; - let declarations = stylesheets.iter() - .flat_map(|s| s.effective_rules(&device).viewport()) - .cascade() - .declarations; + let declarations = Cascade::from_stylesheets(&stylesheets, &device).finish(); assert_decl_len!(declarations == 3); assert_decl_eq!(&declarations[0], UserAgent, Zoom: Zoom::Number(1.)); assert_decl_eq!(&declarations[1], User, MinHeight: viewport_length!(200., px)); @@ -268,10 +270,7 @@ fn multiple_stylesheets_cascading() { User, error_reporter.clone()), stylesheet!("@viewport { min-width: 300px !important; min-height: 300px !important; zoom: 3 !important; }", Author, error_reporter.clone())]; - let declarations = stylesheets.iter() - .flat_map(|s| s.effective_rules(&device).viewport()) - .cascade() - .declarations; + let declarations = Cascade::from_stylesheets(&stylesheets, &device).finish(); assert_decl_len!(declarations == 3); assert_decl_eq!(&declarations[0], UserAgent, MinWidth: viewport_length!(100., px), !important); assert_decl_eq!(&declarations[1], User, MinHeight: viewport_length!(200., px), !important);