Handle parser state in CSSOM insert_rule

components/script/dom/cssrulelist.rs

@@ -47,7 +47,8 @@ impl CSSRuleList {
     }
 
     // https://drafts.csswg.org/cssom/#insert-a-css-rule
-    pub fn insert_rule(&self, rule: &str, idx: u32) -> Fallible<u32> {
+    pub fn insert_rule(&self, rule: &str, idx: u32, nested: bool) -> Fallible<u32> {
+        use style::stylesheets::SingleRuleParseError;
         /// Insert an item into a vector, appending if it is out of bounds
         fn insert<T>(vec: &mut Vec<T>, index: usize, item: T) {
             if index >= vec.len() {
@@ -61,22 +62,41 @@ impl CSSRuleList {
         let doc = window.Document();
         let index = idx as usize;
 
-        // Step 1, 2
-        // XXXManishearth get url from correct location
-        // XXXManishearth should we also store the namespace map?
-        let new_rule = try!(StyleCssRule::from_str(&rule, Origin::Author,
-                                                   doc.url().clone(),
-                                                   ParserContextExtraData::default())
-                          .map_err(|_| Error::Syntax));
 
-        {
+        let new_rule = {
             let rules = self.rules.0.read();
+            let state = if nested {
+                None
+            } else {
+                Some(CssRules::state_at_index(&rules, index))
+            };
+
+            let rev_state = CssRules::state_at_index_rev(&rules, index);
+
+            // Step 1, 2
+            // XXXManishearth get url from correct location
+            // XXXManishearth should we also store the namespace map?
+            let parse_result = StyleCssRule::parse(&rule, Origin::Author,
+                                               doc.url().clone(),
+                                               ParserContextExtraData::default(),
+                                               state);
+
+            if let Err(SingleRuleParseError::Syntax) = parse_result {
+                return Err(Error::Syntax)
+            }
+
             // Step 3, 4
             if index > rules.len() {
                 return Err(Error::IndexSize);
             }
 
-            // XXXManishearth Step 5 (throw HierarchyRequestError in invalid situations)
+            let (new_rule, new_state) = try!(parse_result.map_err(|_| Error::HierarchyRequest));
+
+            if new_state > rev_state {
+                // We inserted a rule too early, e.g. inserting
+                // a regular style rule before @namespace rules
+                return Err((Error::HierarchyRequest));
+            }
 
             // Step 6
             if let StyleCssRule::Namespace(..) = new_rule {
@@ -84,7 +104,9 @@ impl CSSRuleList {
                     return Err(Error::InvalidState);
                 }
             }
-        }
+
+            new_rule
+        };
 
         insert(&mut self.rules.0.write(), index, new_rule.clone());
         let dom_rule = CSSRule::new_specific(&window, &self.sheet, new_rule);

components/script/dom/cssstylesheet.rs

@@ -59,7 +59,7 @@ impl CSSStyleSheetMethods for CSSStyleSheet {
     // https://drafts.csswg.org/cssom/#dom-cssstylesheet-insertrule
     fn InsertRule(&self, rule: DOMString, index: u32) -> Fallible<u32> {
         // XXXManishearth check origin clean flag
-        self.rulelist().insert_rule(&rule, index)
+        self.rulelist().insert_rule(&rule, index, /* nested */ false)
     }
 
     // https://drafts.csswg.org/cssom/#dom-cssstylesheet-deleterule

components/style/stylesheets.rs

@@ -6,7 +6,7 @@
 
 use {Atom, Prefix, Namespace};
 use cssparser::{AtRuleParser, Parser, QualifiedRuleParser, decode_stylesheet_bytes};
-use cssparser::{AtRuleType, RuleListParser, SourcePosition, Token};
+use cssparser::{AtRuleType, RuleListParser, SourcePosition, Token, parse_one_rule};
 use cssparser::ToCss as ParserToCss;
 use encoding::EncodingRef;
 use error_reporting::ParseErrorReporter;
@@ -70,6 +70,36 @@ impl CssRules {
             }
         })
     }
+
+    // Provides the parser state at a given insertion index
+    pub fn state_at_index(rules: &[CssRule], at: usize) -> State {
+        let mut state = State::Start;
+        for rule in rules.iter().take(at) {
+            state = match *rule {
+                // CssRule::Charset(..) => State::Start,
+                // CssRule::Import(..) => State::Imports,
+                CssRule::Namespace(..) => State::Namespaces,
+                _ => State::Body,
+            };
+        }
+        state
+    }
+
+    // Provides the maximum allowed parser state at a given index,
+    // searching in reverse. If inserting at this index, the parser
+    // must not be in a state greater than this post-insertion.
+    pub fn state_at_index_rev(rules: &[CssRule], at: usize) -> State {
+        let mut state = State::Body;
+        for rule in rules.iter().skip(at).rev() {
+            state = match *rule {
+                // CssRule::Charset(..) => State::Start,
+                // CssRule::Import(..) => State::Imports,
+                CssRule::Namespace(..) => State::Namespaces,
+                _ => State::Body,
+            };
+        }
+        state
+    }
 }
 
 #[derive(Debug)]
@@ -119,6 +149,11 @@ impl ParseErrorReporter for MemoryHoleReporter {
     }
 }
 
+pub enum SingleRuleParseError {
+    Syntax,
+    Hierarchy,
+}
+
 impl CssRule {
     /// Call `f` with the slice of rules directly contained inside this rule.
     ///
@@ -142,25 +177,35 @@ impl CssRule {
         }
     }
 
-    pub fn from_str(css: &str, origin: Origin,
+    // input state is None for a nested rule
-                    base_url: Url, extra_data: ParserContextExtraData) -> Result<Self, ()> {
+    // Returns a parsed CSS rule and the final state of the parser
+    pub fn parse(css: &str, origin: Origin,
+                    base_url: Url,
+                    extra_data: ParserContextExtraData,
+                    state: Option<State>) -> Result<(Self, State), SingleRuleParseError> {
         let error_reporter = Box::new(MemoryHoleReporter);
         let mut namespaces = Namespaces::default();
-        let rule_parser = TopLevelRuleParser {
+        let context = ParserContext::new_with_extra_data(origin, &base_url,
-            context: ParserContext::new_with_extra_data(origin, &base_url, error_reporter.clone(),
+                                                         error_reporter.clone(),
-                                                        extra_data),
+                                                         extra_data);
-            state: Cell::new(State::Start),
+        let mut input = Parser::new(css);
+
+        // nested rules are in the body state
+        let state = state.unwrap_or(State::Body);
+        let mut rule_parser = TopLevelRuleParser {
+            context: context,
+            state: Cell::new(state),
             namespaces: &mut namespaces,
         };
-        let mut input = Parser::new(css);
+        match parse_one_rule(&mut input, &mut rule_parser) {
-        let mut iter = RuleListParser::new_for_stylesheet(&mut input, rule_parser);
+            Ok(result) => Ok((result, rule_parser.state.get())),
-        if let Some(Ok(rule)) = iter.next() {
+            Err(_) => {
-            if iter.next().is_some() {
+                if let State::Invalid = rule_parser.state.get() {
-                return Err(());
+                    Err(SingleRuleParseError::Hierarchy)
+                } else {
+                    Err(SingleRuleParseError::Syntax)
+                }
             }
-            return Ok(rule);
-        } else {
-            return Err(());
         }
     }
 }
@@ -418,11 +463,12 @@ impl<'b> TopLevelRuleParser<'b> {
 }
 
 #[derive(Eq, PartialEq, Ord, PartialOrd, Copy, Clone)]
-enum State {
+pub enum State {
     Start = 1,
     Imports = 2,
     Namespaces = 3,
     Body = 4,
+    Invalid = 5,
 }
 
 
@@ -451,6 +497,7 @@ impl<'a> AtRuleParser for TopLevelRuleParser<'a> {
                     // TODO: support @import
                     return Err(())  // "@import is not supported yet"
                 } else {
+                    self.state.set(State::Invalid);
                     return Err(())  // "@import must be before any rule but @charset"
                 }
             },
@@ -477,6 +524,7 @@ impl<'a> AtRuleParser for TopLevelRuleParser<'a> {
                         }
                     )))))
                 } else {
+                    self.state.set(State::Invalid);
                     return Err(())  // "@namespace must be before any rule but @charset and @import"
                 }
             },
@@ -485,7 +533,11 @@ impl<'a> AtRuleParser for TopLevelRuleParser<'a> {
             "charset" => return Err(()), // (insert appropriate error message)
             _ => {}
         }
-
+        // Don't allow starting with an invalid state
+        if self.state.get() > State::Body {
+            self.state.set(State::Invalid);
+            return Err(());
+        }
         self.state.set(State::Body);
         AtRuleParser::parse_prelude(&mut self.nested(), name, input)
     }