style: Don't let @namespace rules that aren't going to be inserted affect the namespace map.

Bug: 1464865
Reviewed-by: xidorn
MozReview-Commit-ID: 9bjlEBExqsr

components/style/stylesheets/mod.rs

@@ -46,7 +46,7 @@ pub use self::media_rule::MediaRule;
 pub use self::namespace_rule::NamespaceRule;
 pub use self::origin::{Origin, OriginSet, OriginSetIterator, PerOrigin, PerOriginIter};
 pub use self::page_rule::PageRule;
-pub use self::rule_parser::{State, TopLevelRuleParser};
+pub use self::rule_parser::{State, TopLevelRuleParser, InsertRuleContext};
 pub use self::rule_list::{CssRules, CssRulesHelpers};
 pub use self::rules_iterator::{AllRules, EffectiveRules};
 pub use self::rules_iterator::{NestedRuleIterationCondition, RulesIterator};
@@ -178,12 +178,6 @@ pub enum CssRuleType {
     Viewport = 15,
 }
 
-#[allow(missing_docs)]
-pub enum SingleRuleParseError {
-    Syntax,
-    Hierarchy,
-}
-
 #[allow(missing_docs)]
 pub enum RulesMutateError {
     Syntax,
@@ -192,15 +186,6 @@ pub enum RulesMutateError {
     InvalidState,
 }
 
-impl From<SingleRuleParseError> for RulesMutateError {
-    fn from(other: SingleRuleParseError) -> Self {
-        match other {
-            SingleRuleParseError::Syntax => RulesMutateError::Syntax,
-            SingleRuleParseError::Hierarchy => RulesMutateError::HierarchyRequest,
-        }
-    }
-}
-
 impl CssRule {
     /// Returns the CSSOM rule type of this rule.
     pub fn rule_type(&self) -> CssRuleType {
@@ -236,11 +221,12 @@ impl CssRule {
     /// Input state is None for a nested rule
     pub fn parse(
         css: &str,
+        insert_rule_context: InsertRuleContext,
         parent_stylesheet_contents: &StylesheetContents,
         shared_lock: &SharedRwLock,
-        state: Option<State>,
+        state: State,
         loader: Option<&StylesheetLoader>,
-    ) -> Result<(Self, State), SingleRuleParseError> {
+    ) -> Result<Self, RulesMutateError> {
         let url_data = parent_stylesheet_contents.url_data.read();
         let error_reporter = NullReporter;
         let context = ParserContext::new(
@@ -257,28 +243,23 @@ impl CssRule {
         let mut guard = parent_stylesheet_contents.namespaces.write();
 
         // nested rules are in the body state
-        let state = state.unwrap_or(State::Body);
         let mut rule_parser = TopLevelRuleParser {
             stylesheet_origin: parent_stylesheet_contents.origin,
-            context: context,
+            context,
             error_context: ParserErrorContext {
                 error_reporter: &error_reporter,
             },
             shared_lock: &shared_lock,
-            loader: loader,
+            loader,
-            state: state,
+            state,
-            had_hierarchy_error: false,
+            dom_error: None,
             namespaces: &mut *guard,
+            insert_rule_context: Some(insert_rule_context),
         };
 
         parse_one_rule(&mut input, &mut rule_parser)
-            .map(|result| (result, rule_parser.state))
             .map_err(|_| {
-                if rule_parser.take_had_hierarchy_error() {
+                rule_parser.dom_error.unwrap_or(RulesMutateError::Syntax)
-                    SingleRuleParseError::Hierarchy
-                } else {
-                    SingleRuleParseError::Syntax
-                }
             })
     }
 }

components/style/stylesheets/rule_list.rs

@@ -13,7 +13,7 @@ use std::fmt::{self, Write};
 use str::CssStringWriter;
 use stylesheets::{CssRule, RulesMutateError};
 use stylesheets::loader::StylesheetLoader;
-use stylesheets::rule_parser::State;
+use stylesheets::rule_parser::{InsertRuleContext, State};
 use stylesheets::stylesheet::StylesheetContents;
 
 /// A list of CSS rules.
@@ -141,7 +141,7 @@ impl CssRulesHelpers for RawOffsetArc<Locked<CssRules>> {
         nested: bool,
         loader: Option<&StylesheetLoader>,
     ) -> Result<CssRule, RulesMutateError> {
-        let state = {
+        let new_rule = {
             let read_guard = lock.read();
             let rules = self.read_with(&read_guard);
 
@@ -151,39 +151,33 @@ impl CssRulesHelpers for RawOffsetArc<Locked<CssRules>> {
             }
 
             // Computes the parser state at the given index
-            if nested {
+            let state = if nested {
-                None
+                State::Body
             } else if index == 0 {
-                Some(State::Start)
+                State::Start
             } else {
-                rules.0.get(index - 1).map(CssRule::rule_state)
+                rules.0.get(index - 1).map(CssRule::rule_state).unwrap_or(State::Body)
-            }
+            };
-        };
 
-        // Step 3, 4
+            let insert_rule_context = InsertRuleContext {
-        // XXXManishearth should we also store the namespace map?
+                rule_list: &rules.0,
-        let (new_rule, new_state) =
+                index,
-            CssRule::parse(&rule, parent_stylesheet_contents, lock, state, loader)?;
+            };
+
+            // Steps 3, 4, 5, 6
+            CssRule::parse(
+                &rule,
+                insert_rule_context,
+                parent_stylesheet_contents,
+                lock,
+                state,
+                loader,
+            )?
+        };
 
         {
             let mut write_guard = lock.write();
             let rules = self.write_with(&mut write_guard);
-            // Step 5
-            // Computes the maximum allowed parser state at a given index.
-            let rev_state = rules.0.get(index).map_or(State::Body, CssRule::rule_state);
-            if new_state > rev_state {
-                // We inserted a rule too early, e.g. inserting
-                // a regular style rule before @namespace rules
-                return Err(RulesMutateError::HierarchyRequest);
-            }
-
-            // Step 6
-            if let CssRule::Namespace(..) = new_rule {
-                if !rules.only_ns_or_import() {
-                    return Err(RulesMutateError::InvalidState);
-                }
-            }
-
             rules.0.insert(index, new_rule.clone());
         }
 

components/style/stylesheets/rule_parser.rs

@@ -19,7 +19,7 @@ use servo_arc::Arc;
 use shared_lock::{Locked, SharedRwLock};
 use str::starts_with_ignore_ascii_case;
 use style_traits::{ParseError, StyleParseErrorKind};
-use stylesheets::{CssRule, CssRuleType, CssRules, Origin, StylesheetLoader};
+use stylesheets::{CssRule, CssRuleType, CssRules, Origin, RulesMutateError, StylesheetLoader};
 use stylesheets::{DocumentRule, FontFeatureValuesRule, KeyframesRule, MediaRule};
 use stylesheets::{NamespaceRule, PageRule, StyleRule, SupportsRule, ViewportRule};
 use stylesheets::document_rule::DocumentCondition;
@@ -31,6 +31,14 @@ use stylesheets::viewport_rule;
 use values::{CssUrl, CustomIdent, KeyframesName};
 use values::computed::font::FamilyName;
 
+/// The information we need particularly to do CSSOM insertRule stuff.
+pub struct InsertRuleContext<'a> {
+    /// The rule list we're about to insert into.
+    pub rule_list: &'a [CssRule],
+    /// The index we're about to get inserted at.
+    pub index: usize,
+}
+
 /// The parser for the top-level rules in a stylesheet.
 pub struct TopLevelRuleParser<'a, R: 'a> {
     /// The origin of the stylesheet we're parsing.
@@ -51,11 +59,13 @@ pub struct TopLevelRuleParser<'a, R: 'a> {
     /// Whether we have tried to parse was invalid due to being in the wrong
     /// place (e.g. an @import rule was found while in the `Body` state). Reset
     /// to `false` when `take_had_hierarchy_error` is called.
-    pub had_hierarchy_error: bool,
+    pub dom_error: Option<RulesMutateError>,
     /// The namespace map we use for parsing. Needs to start as `Some()`, and
     /// will be taken out after parsing namespace rules, and that reference will
     /// be moved to `ParserContext`.
     pub namespaces: &'a mut Namespaces,
+    /// The info we need insert a rule in a list.
+    pub insert_rule_context: Option<InsertRuleContext<'a>>,
 }
 
 impl<'b, R> TopLevelRuleParser<'b, R> {
@@ -74,14 +84,43 @@ impl<'b, R> TopLevelRuleParser<'b, R> {
         self.state
     }
 
-    /// Returns whether we previously tried to parse a rule that was invalid
+    /// Checks whether we can parse a rule that would transition us to
-    /// due to being in the wrong place (e.g. an @import rule was found after
+    /// `new_state`.
-    /// a regular style rule).  The state of this flag is reset when this
+    ///
-    /// function is called.
+    /// This is usually a simple branch, but we may need more bookkeeping if
-    pub fn take_had_hierarchy_error(&mut self) -> bool {
+    /// doing `insertRule` from CSSOM.
-        let had_hierarchy_error = self.had_hierarchy_error;
+    fn check_state(&mut self, new_state: State) -> bool {
-        self.had_hierarchy_error = false;
+        if self.state > new_state {
-        had_hierarchy_error
+            self.dom_error = Some(RulesMutateError::HierarchyRequest);
+            return false;
+        }
+
+        let ctx = match self.insert_rule_context {
+            Some(ref ctx) => ctx,
+            None => return true,
+        };
+
+        let next_rule_state = match ctx.rule_list.get(ctx.index) {
+            None => return true,
+            Some(rule) => rule.rule_state(),
+        };
+
+        if new_state > next_rule_state {
+            self.dom_error = Some(RulesMutateError::HierarchyRequest);
+            return false;
+        }
+
+        // If there's anything that isn't a namespace rule (or import rule, but
+        // we checked that already at the beginning), reject with a
+        // StateError.
+        if new_state == State::Namespaces &&
+            ctx.rule_list[ctx.index..].iter().any(|r| !matches!(*r, CssRule::Namespace(..)))
+        {
+            self.dom_error = Some(RulesMutateError::InvalidState);
+            return false;
+        }
+
+        true
     }
 }
 
@@ -151,9 +190,7 @@ impl<'a, 'i, R: ParseErrorReporter> AtRuleParser<'i> for TopLevelRuleParser<'a,
         let location = input.current_source_location();
         match_ignore_ascii_case! { &*name,
             "import" => {
-                if self.state > State::Imports {
+                if !self.check_state(State::Imports) {
-                    // "@import must be before any rule but @charset"
-                    self.had_hierarchy_error = true;
                     return Err(input.new_custom_error(StyleParseErrorKind::UnexpectedImportRule))
                 }
 
@@ -168,9 +205,7 @@ impl<'a, 'i, R: ParseErrorReporter> AtRuleParser<'i> for TopLevelRuleParser<'a,
                 return Ok(AtRuleType::WithoutBlock(prelude));
             },
             "namespace" => {
-                if self.state > State::Namespaces {
+                if !self.check_state(State::Namespaces) {
-                    // "@namespace must be before any rule but @charset and @import"
-                    self.had_hierarchy_error = true;
                     return Err(input.new_custom_error(StyleParseErrorKind::UnexpectedNamespaceRule))
                 }
 
@@ -190,12 +225,16 @@ impl<'a, 'i, R: ParseErrorReporter> AtRuleParser<'i> for TopLevelRuleParser<'a,
             // @charset is removed by rust-cssparser if it’s the first rule in the stylesheet
             // anything left is invalid.
             "charset" => {
-                self.had_hierarchy_error = true;
+                self.dom_error = Some(RulesMutateError::HierarchyRequest);
                 return Err(input.new_custom_error(StyleParseErrorKind::UnexpectedCharsetRule))
             }
             _ => {}
         }
 
+        if !self.check_state(State::Body) {
+            return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
+        }
+
         AtRuleParser::parse_prelude(&mut self.nested(), name, input)
     }
 
@@ -264,6 +303,10 @@ impl<'a, 'i, R: ParseErrorReporter> QualifiedRuleParser<'i> for TopLevelRulePars
         &mut self,
         input: &mut Parser<'i, 't>,
     ) -> Result<QualifiedRuleParserPrelude, ParseError<'i>> {
+        if !self.check_state(State::Body) {
+            return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
+        }
+
         QualifiedRuleParser::parse_prelude(&mut self.nested(), input)
     }
 

components/style/stylesheets/stylesheet.rs

@@ -368,7 +368,8 @@ impl Stylesheet {
             context,
             error_context,
             state: State::Start,
-            had_hierarchy_error: false,
+            dom_error: None,
+            insert_rule_context: None,
             namespaces,
         };