Add RwLock in more Arc’d things in stylesheets.

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<AtomicUsize>) {
     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());
-        }
+        })
     }
 }
 

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

components/style/keyframes.rs

@@ -171,12 +171,12 @@ fn get_animated_properties(keyframe: &Keyframe) -> Vec<TransitionProperty> {
 }
 
 impl KeyframesAnimation {
-    pub fn from_keyframes(keyframes: &[Arc<Keyframe>]) -> Option<Self> {
+    pub fn from_keyframes(keyframes: &[Arc<RwLock<Keyframe>>]) -> Option<Self> {
         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<Arc<Keyframe>> {
+pub fn parse_keyframe_list(context: &ParserContext, input: &mut Parser) -> Vec<Arc<RwLock<Keyframe>>> {
     RuleListParser::new_for_nested_rule(input, KeyframeListParser { context: context })
         .filter_map(Result::ok)
         .collect()
@@ -233,12 +234,12 @@ pub fn parse_keyframe_list(context: &ParserContext, input: &mut Parser) -> Vec<A
 enum Void {}
 impl<'a> AtRuleParser for KeyframeListParser<'a> {
     type Prelude = Void;
-    type AtRule = Arc<Keyframe>;
+    type AtRule = Arc<RwLock<Keyframe>>;
 }
 
 impl<'a> QualifiedRuleParser for KeyframeListParser<'a> {
     type Prelude = KeyframeSelector;
-    type QualifiedRule = Arc<Keyframe>;
+    type QualifiedRule = Arc<RwLock<Keyframe>>;
 
     fn parse_prelude(&mut self, input: &mut Parser) -> Result<Self::Prelude, ()> {
         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,
             })),
-        }))
+        })))
     }
 }
 

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 stylesheets::{CSSRule, Origin, Stylesheet, UserAgentStylesheets};
-use viewport::{MaybeNew, ViewportRuleCascade};
+use viewport::{self, MaybeNew, ViewportRule};
 
 pub type FnvHashMap<K, V> = HashMap<K, V, BuildHasherDefault<::fnv::FnvHasher>>;
 
@@ -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<Stylesheet>]) {
-        let cascaded_rule = stylesheets.iter()
+        let cascaded_rule = ViewportRule {
-            .flat_map(|s| s.effective_rules(&self.device).viewport())
+            declarations: viewport::Cascade::from_stylesheets(stylesheets, &device).finish(),
-            .cascade();
+        };
 
         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|
+            mq_eval_changed(&stylesheet.rules, &self.device, &device)
-                    media_rule.evaluate(&self.device) != media_rule.evaluate(&device))
         });
 
         self.device = device;

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<NamespaceRule>),
+    Namespace(Arc<RwLock<NamespaceRule>>),
-    Style(Arc<StyleRule>),
+    Style(Arc<RwLock<StyleRule>>),
-    Media(Arc<MediaRule>),
+    Media(Arc<RwLock<MediaRule>>),
-    FontFace(Arc<FontFaceRule>),
+    FontFace(Arc<RwLock<FontFaceRule>>),
-    Viewport(Arc<ViewportRule>),
+    Viewport(Arc<RwLock<ViewportRule>>),
-    Keyframes(Arc<KeyframesRule>),
+    Keyframes(Arc<RwLock<KeyframesRule>>),
 }
 
+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<F, R>(&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<Arc<Keyframe>>,
+    pub keyframes: Vec<Arc<RwLock<Keyframe>>>,
 }
 
 #[derive(Debug)]
 pub struct MediaRule {
-    pub media_queries: Arc<MediaQueryList>,
+    pub media_queries: Arc<RwLock<MediaQueryList>>,
     pub rules: Vec<CSSRule>,
 }
 
-
-impl MediaRule {
-    #[inline]
-    pub fn evaluate(&self, device: &Device) -> bool {
-        self.media_queries.evaluate(device)
-    }
-}
-
 #[derive(Debug)]
 pub struct StyleRule {
     pub selectors: Vec<Selector<TheSelectorImpl>>,
@@ -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> {
+    pub fn effective_rules<F>(&self, device: &Device, mut f: F) where F: FnMut(&CSSRule) {
-        Rules::new(self.rules.iter(), Some(device))
+        effective_rules(&self.rules, device, &mut f);
     }
 }
 
-/// `CSSRule` iterator.
+fn effective_rules<F>(rules: &[CSSRule], device: &Device, f: &mut F) where F: FnMut(&CSSRule) {
-///
+    for rule in rules {
-/// The iteration order is pre-order. Specifically, this implies that a
+        f(rule);
-/// conditional group rule will come before its nested rules.
+        rule.with_nested_rules_and_mq(|rules, mq| {
-pub struct Rules<'a> {
+            if let Some(media_queries) = mq {
-    // 2 because normal case is likely to be just one level of nesting (@media)
+                if !media_queries.evaluate(device) {
-    stack: SmallVec<[slice::Iter<'a, CSSRule>; 2]>,
+                    return
-    device: Option<&'a Device>
+                }
-}
+            }
-
+            effective_rules(rules, device, f)
-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 }
     }
 }
 
-impl<'a> Iterator for Rules<'a> {
+macro_rules! rule_filter {
-    type Item = &'a CSSRule;
+    ($( $method: ident($variant:ident => $rule_type: ident), )+) => {
-
+        impl Stylesheet {
-    fn next(&mut self) -> Option<&'a CSSRule> {
+            $(
-        while !self.stack.is_empty() {
+                pub fn $method<F>(&self, device: &Device, mut f: F) where F: FnMut(&$rule_type) {
-            let top = self.stack.len() - 1;
+                    self.effective_rules(device, |rule| {
-            while let Some(rule) = self.stack[top].next() {
+                        if let CSSRule::$variant(ref lock) = *rule {
-                // handle conditional group rules
+                            let rule = lock.read();
-                if let &CSSRule::Media(ref rule) = rule {
+                            f(&rule)
-                    if let Some(device) = self.device {
-                        if rule.evaluate(device) {
-                            self.stack.push(rule.rules.iter());
-                        } else {
-                            continue
                         }
-                    } else {
+                    })
-                        self.stack.push(rule.rules.iter());
-                    }
                 }
-
+            )+
-                return Some(rule)
-            }
-
-            self.stack.pop();
-        }
-        None
-    }
-
-    #[inline]
-    fn size_hint(&self) -> (usize, Option<usize>) {
-        // 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<Item=&'a CSSRule> {
-                #[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<Item=&'a CSSRule> {
-                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<usize>) {
-                    (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.
+rule_filter! {
-pub trait CSSRuleIteratorExt<'a>: Iterator<Item=&'a CSSRule> + Sized {
+    effective_style_rules(Style => StyleRule),
-    /// Yield only @font-face rules.
+    effective_media_rules(Media => MediaRule),
-    fn font_face(self) -> rule_filter::FontFace<'a, Self>;
+    effective_font_face_rules(FontFace => FontFaceRule),
-
+    effective_viewport_rules(Viewport => ViewportRule),
-    /// Yield only @media rules.
+    effective_keyframes_rules(Keyframes => KeyframesRule),
-    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<Item=&'a CSSRule> {
-    #[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)
-    }
 }
 
 fn parse_nested_rules(context: &ParserContext, input: &mut Parser) -> Vec<CSSRule> {
@@ -399,7 +287,7 @@ enum AtRulePrelude {
     /// A @font-face rule prelude.
     FontFace,
     /// A @media rule prelude, with its media queries.
-    Media(Arc<MediaQueryList>),
+    Media(Arc<RwLock<MediaQueryList>>),
     /// 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 {
+                    return Ok(AtRuleType::WithoutBlock(CSSRule::Namespace(Arc::new(RwLock::new(
-                        prefix: opt_prefix,
+                        NamespaceRule {
-                        url: url,
+                            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<CSSRule, ()> {
         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<Selector<TheSelectorImpl>>, input: &mut Parser)
                    -> Result<CSSRule, ()> {
-        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)))
-        })))
+        }))))
     }
 }

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 mut cascade = Cascade::new();
-        let valid_declarations = DeclarationListParser::new(input, parser)
+        let mut parser = DeclarationListParser::new(input, parser);
-            .filter_map(|result| {
+        while let Some(result) = parser.next() {
-                match result {
+            match result {
-                    Ok(declarations) => Some(declarations),
+                Ok(declarations) => {
-                    Err(range) => {
+                    for declarations in declarations {
-                        errors.push(range);
+                        cascade.add(Cow::Owned(declarations))
-                        None
                     }
                 }
-            })
+                Err(range) => {
-            .flat_map(|declarations| declarations.into_iter())
+                    let pos = range.start;
-            .collect::<Vec<_>>();
+                    let message = format!("Unsupported @viewport descriptor declaration: '{}'",
-
+                                          parser.input.slice(range));
-        for range in errors {
+                    log_css_error(parser.input, pos, &*message, &context);
-            let pos = range.start;
+                }
-            let message = format!("Unsupported @viewport descriptor declaration: '{}'",
+            }
-                                  input.slice(range));
-            log_css_error(input, pos, &*message, &context);
         }
-
+        Ok(ViewportRule { declarations: cascade.finish() })
-        Ok(ViewportRule { declarations: valid_declarations.iter().cascade() })
     }
 
-    #[allow(unsafe_code)]
     pub fn from_meta(content: &str) -> Option<ViewportRule> {
         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<Item=&'a ViewportRule>
-{
-    #[inline]
-    fn cascade(self) -> ViewportRule {
-        ViewportRule {
-            declarations: self.flat_map(|r| r.declarations.iter()).cascade()
-        }
-    }
-}
-
-trait ViewportDescriptorDeclarationCascade: Iterator + Sized {
-    fn cascade(self) -> Vec<ViewportDescriptorDeclaration>;
-}
-
 /// 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)]
+pub struct Cascade {
-fn cascade<'a, I>(iter: I) -> Vec<ViewportDescriptorDeclaration>
+    declarations: Vec<Option<(usize, ViewportDescriptorDeclaration)>>,
-    where I: Iterator<Item=&'a ViewportDescriptorDeclaration>
+    count_so_far: usize,
-{
+}
-    let mut declarations: Vec<Option<(usize, &'a ViewportDescriptorDeclaration)>> =
-        vec![None; VIEWPORT_DESCRIPTOR_VARIANTS];
 
-    // index is used to reconstruct order of appearance after all declarations
+impl Cascade {
-    // have been added to the map
+    pub fn new() -> Self {
-    let mut index = 0;
+        Cascade {
-    for declaration in iter {
+            declarations: vec![None; VIEWPORT_DESCRIPTOR_VARIANTS],
-        let descriptor = declaration.descriptor.discriminant_value();
+            count_so_far: 0,
-
-        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;
-            }
         }
     }
 
-    // sort the descriptors by order of appearance
+    pub fn from_stylesheets<'a, I>(stylesheets: I, device: &Device) -> Self
-    declarations.sort_by_key(|entry| entry.map(|(index, _)| index));
+    where I: IntoIterator, I::Item: AsRef<Stylesheet> {
-    declarations.into_iter().filter_map(|entry| entry.map(|(_, decl)| *decl)).collect::<Vec<_>>()
+        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
+    pub fn add(&mut self, declaration: Cow<ViewportDescriptorDeclaration>)  {
-    where I: Iterator<Item=&'a ViewportDescriptorDeclaration>
+        let descriptor = declaration.descriptor.discriminant_value();
-{
+
-    #[inline]
+        match self.declarations[descriptor] {
-    fn cascade(self) -> Vec<ViewportDescriptorDeclaration> {
+            Some((ref mut order_of_appearance, ref mut entry_declaration)) => {
-        cascade(self)
+                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<ViewportDescriptorDeclaration> {
+        // 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()
     }
 }
 

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<F>(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<F>(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());
 }
 

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,
                         })),
-                    }),
+                    })),
                 ]
-            }))
+            })))
 
         ],
     };

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,
+        Box::new(Stylesheet::from_str(
-                              ParserContextExtraData::default());
+            $css,
+            Url::parse("http://localhost").unwrap(),
+            Origin::$origin,
+            $error_reporter,
+            ParserContextExtraData::default()
+        ))
     }
 }
 
@@ -33,10 +38,10 @@ fn test_viewport_rule<F>(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()
+    let declarations = Cascade::from_stylesheets(&stylesheets, &device).finish();
-        .flat_map(|s| s.effective_rules(&device).viewport())
-        .cascade()
-        .declarations;
     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()
+    let declarations = Cascade::from_stylesheets(&stylesheets, &device).finish();
-        .flat_map(|s| s.effective_rules(&device).viewport())
-        .cascade()
-        .declarations;
     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);