servo: Merge #18142 - style: Less messy namespace handling (from emil…

…io:less-mess-ns); r=SimonSapin This PR accounts for the fact that the namespace table is only needed in `NestedRuleParser`, and only for style rules, in order to simplify the setup and be able to fix a few bugs wrt parsing of invalid rules. Source-Repo: https://github.com/servo/servo Source-Revision: 941e0dbb5a125861e20484271a1636e8035e5507
iaxat · Aug 18, 2017 · 10b883e · 10b883e
1 parent 5ccdf23
commit 10b883e
Show file tree

Hide file tree

Showing 9 changed files with 170 additions and 86 deletions.
diff --git a/servo/components/script/dom/css.rs b/servo/components/script/dom/css.rs
@@ -36,9 +36,13 @@ impl CSS {
         decl.push_str(&value);
         let decl = Declaration(decl);
         let url = win.Document().url();
-        let context = ParserContext::new_for_cssom(&url, win.css_error_reporter(), Some(CssRuleType::Supports),
-                                                   PARSING_MODE_DEFAULT,
-                                                   QuirksMode::NoQuirks);
+        let context = ParserContext::new_for_cssom(
+            &url,
+            win.css_error_reporter(),
+            Some(CssRuleType::Style),
+            PARSING_MODE_DEFAULT,
+            QuirksMode::NoQuirks
+        );
         decl.eval(&context)
     }
 
@@ -49,9 +53,13 @@ impl CSS {
         let cond = parse_condition_or_declaration(&mut input);
         if let Ok(cond) = cond {
             let url = win.Document().url();
-            let context = ParserContext::new_for_cssom(&url, win.css_error_reporter(), Some(CssRuleType::Supports),
-                                                       PARSING_MODE_DEFAULT,
-                                                       QuirksMode::NoQuirks);
+            let context = ParserContext::new_for_cssom(
+                &url,
+                win.css_error_reporter(),
+                Some(CssRuleType::Style),
+                PARSING_MODE_DEFAULT,
+                QuirksMode::NoQuirks
+            );
             cond.eval(&context)
         } else {
             false

diff --git a/servo/components/style/parser.rs b/servo/components/style/parser.rs
@@ -61,13 +61,14 @@ pub struct ParserContext<'a> {
 
 impl<'a> ParserContext<'a> {
     /// Create a parser context.
-    pub fn new(stylesheet_origin: Origin,
-               url_data: &'a UrlExtraData,
-               error_reporter: &'a ParseErrorReporter,
-               rule_type: Option<CssRuleType>,
-               parsing_mode: ParsingMode,
-               quirks_mode: QuirksMode)
-               -> ParserContext<'a> {
+    pub fn new(
+        stylesheet_origin: Origin,
+        url_data: &'a UrlExtraData,
+        error_reporter: &'a ParseErrorReporter,
+        rule_type: Option<CssRuleType>,
+        parsing_mode: ParsingMode,
+        quirks_mode: QuirksMode,
+    ) -> ParserContext<'a> {
         ParserContext {
             stylesheet_origin: stylesheet_origin,
             url_data: url_data,
@@ -88,23 +89,31 @@ impl<'a> ParserContext<'a> {
         parsing_mode: ParsingMode,
         quirks_mode: QuirksMode
     ) -> ParserContext<'a> {
-        Self::new(Origin::Author, url_data, error_reporter, rule_type, parsing_mode, quirks_mode)
+        Self::new(
+            Origin::Author,
+            url_data,
+            error_reporter,
+            rule_type,
+            parsing_mode,
+            quirks_mode,
+        )
     }
 
     /// Create a parser context based on a previous context, but with a modified rule type.
     pub fn new_with_rule_type(
         context: &'a ParserContext,
-        rule_type: Option<CssRuleType>
+        rule_type: CssRuleType,
+        namespaces: &'a Namespaces,
     ) -> ParserContext<'a> {
         ParserContext {
             stylesheet_origin: context.stylesheet_origin,
             url_data: context.url_data,
             error_reporter: context.error_reporter,
-            rule_type: rule_type,
+            rule_type: Some(rule_type),
             line_number_offset: context.line_number_offset,
             parsing_mode: context.parsing_mode,
             quirks_mode: context.quirks_mode,
-            namespaces: context.namespaces,
+            namespaces: Some(namespaces),
         }
     }
 
@@ -115,7 +124,7 @@ impl<'a> ParserContext<'a> {
         error_reporter: &'a ParseErrorReporter,
         line_number_offset: u64,
         parsing_mode: ParsingMode,
-        quirks_mode: QuirksMode
+        quirks_mode: QuirksMode,
     ) -> ParserContext<'a> {
         ParserContext {
             stylesheet_origin: stylesheet_origin,

diff --git a/servo/components/style/stylesheets/font_feature_values_rule.rs b/servo/components/style/stylesheets/font_feature_values_rule.rs
@@ -322,14 +322,17 @@ macro_rules! font_feature_values_blocks {
                 }
             }
 
-            fn parse_block<'t>(&mut self, prelude: Self::Prelude, input: &mut Parser<'i, 't>)
-                               -> Result<Self::AtRule, ParseError<'i>> {
-                let context = ParserContext::new_with_rule_type(self.context, Some(CssRuleType::FontFeatureValues));
+            fn parse_block<'t>(
+                &mut self,
+                prelude: Self::Prelude,
+                input: &mut Parser<'i, 't>
+            ) -> Result<Self::AtRule, ParseError<'i>> {
+                debug_assert_eq!(self.context.rule_type(), CssRuleType::FontFeatureValues);
                 match prelude {
                     $(
                         BlockType::$ident_camel => {
                             let parser = FFVDeclarationsParser {
-                                context: &context,
+                                context: &self.context,
                                 declarations: &mut self.rule.$ident,
                             };
 
@@ -338,7 +341,7 @@ macro_rules! font_feature_values_blocks {
                                 if let Err(err) = declaration {
                                     let error = ContextualParseError::UnsupportedKeyframePropertyDeclaration(
                                         err.slice, err.error);
-                                    context.log_css_error(err.location, error);
+                                    self.context.log_css_error(err.location, error);
                                 }
                             }
                         },

diff --git a/servo/components/style/stylesheets/keyframes_rule.rs b/servo/components/style/stylesheets/keyframes_rule.rs
@@ -214,12 +214,16 @@ impl Keyframe {
     ) -> Result<Arc<Locked<Self>>, ParseError<'i>> {
         let url_data = parent_stylesheet_contents.url_data.read();
         let error_reporter = NullReporter;
-        let context = ParserContext::new(parent_stylesheet_contents.origin,
-                                         &url_data,
-                                         &error_reporter,
-                                         Some(CssRuleType::Keyframe),
-                                         PARSING_MODE_DEFAULT,
-                                         parent_stylesheet_contents.quirks_mode);
+        let namespaces = parent_stylesheet_contents.namespaces.read();
+        let mut context = ParserContext::new(
+            parent_stylesheet_contents.origin,
+            &url_data,
+            &error_reporter,
+            Some(CssRuleType::Keyframe),
+            PARSING_MODE_DEFAULT,
+            parent_stylesheet_contents.quirks_mode
+        );
+        context.namespaces = Some(&*namespaces);
         let mut input = ParserInput::new(css);
         let mut input = Parser::new(&mut input);
 
@@ -450,8 +454,14 @@ struct KeyframeListParser<'a> {
 }
 
 /// Parses a keyframe list from CSS input.
-pub fn parse_keyframe_list(context: &ParserContext, input: &mut Parser, shared_lock: &SharedRwLock)
-                           -> Vec<Arc<Locked<Keyframe>>> {
+pub fn parse_keyframe_list(
+    context: &ParserContext,
+    input: &mut Parser,
+    shared_lock: &SharedRwLock
+) -> Vec<Arc<Locked<Keyframe>>> {
+    debug_assert!(context.namespaces.is_some(),
+                  "Parsing a keyframe list from a context without namespaces?");
+
     let mut declarations = SourcePropertyDeclaration::new();
     RuleListParser::new_for_nested_rule(input, KeyframeListParser {
         context: context,
@@ -487,7 +497,13 @@ impl<'a, 'i> QualifiedRuleParser<'i> for KeyframeListParser<'a> {
 
     fn parse_block<'t>(&mut self, prelude: Self::Prelude, input: &mut Parser<'i, 't>)
                        -> Result<Self::QualifiedRule, ParseError<'i>> {
-        let context = ParserContext::new_with_rule_type(self.context, Some(CssRuleType::Keyframe));
+        let context =
+            ParserContext::new_with_rule_type(
+                self.context,
+                CssRuleType::Keyframe,
+                self.context.namespaces.unwrap(),
+            );
+
         let parser = KeyframeDeclarationParser {
             context: &context,
             declarations: self.declarations,

diff --git a/servo/components/style/stylesheets/mod.rs b/servo/components/style/stylesheets/mod.rs
@@ -134,7 +134,7 @@ impl MallocSizeOfWithGuard for CssRule {
 }
 
 #[allow(missing_docs)]
-#[derive(PartialEq, Eq, Copy, Clone)]
+#[derive(PartialEq, Eq, Copy, Clone, Debug)]
 pub enum CssRuleType {
     // https://drafts.csswg.org/cssom/#the-cssrule-interface
     Style               = 1,
@@ -249,7 +249,7 @@ impl CssRule {
             loader: loader,
             state: state,
             had_hierarchy_error: false,
-            namespaces: Some(&mut *guard),
+            namespaces: &mut *guard,
         };
 
         parse_one_rule(&mut input, &mut rule_parser)