diff options
author | bors-servo <lbergstrom+bors@mozilla.com> | 2017-06-02 05:38:48 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-06-02 05:38:48 -0700 |
commit | fa158a78b6f976156a93bcccae3fdd27046faf50 (patch) | |
tree | 4f9a7973bbde00cb1314863e039da907a6403360 | |
parent | fab1a6354f103bb264941fff2619bca931e48a40 (diff) | |
parent | bbd85ccbda2ced5994205a4362cca92b7e4837ab (diff) | |
download | servo-fa158a78b6f976156a93bcccae3fdd27046faf50.tar.gz servo-fa158a78b6f976156a93bcccae3fdd27046faf50.zip |
Auto merge of #17139 - emilio:parsing-simplify, r=nox
style: Simplify a bit the parsing code.
I plan to simplify it further, but this is worth landing on its own.
<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17139)
<!-- Reviewable:end -->
-rw-r--r-- | components/style/parser.rs | 40 | ||||
-rw-r--r-- | components/style/stylesheets.rs | 313 | ||||
-rw-r--r-- | components/style/values/specified/mod.rs | 63 |
3 files changed, 247 insertions, 169 deletions
diff --git a/components/style/parser.rs b/components/style/parser.rs index cad388651f9..162c690c87c 100644 --- a/components/style/parser.rs +++ b/components/style/parser.rs @@ -7,7 +7,6 @@ use context::QuirksMode; use cssparser::{Parser, SourcePosition, UnicodeRange}; use error_reporting::ParseErrorReporter; -use parking_lot::RwLock; use style_traits::OneOrMoreCommaSeparated; use stylesheets::{CssRuleType, Origin, UrlExtraData, Namespaces}; @@ -82,8 +81,8 @@ pub struct ParserContext<'a> { pub parsing_mode: ParsingMode, /// The quirks mode of this stylesheet. pub quirks_mode: QuirksMode, - /// The list of all namespaces active in the current stylesheet - pub namespaces: Option<&'a RwLock<Namespaces>>, + /// The currently active namespaces. + pub namespaces: Option<&'a Namespaces>, } impl<'a> ParserContext<'a> { @@ -108,19 +107,21 @@ impl<'a> ParserContext<'a> { } /// Create a parser context for on-the-fly parsing in CSSOM - pub fn new_for_cssom(url_data: &'a UrlExtraData, - error_reporter: &'a ParseErrorReporter, - rule_type: Option<CssRuleType>, - parsing_mode: ParsingMode, - quirks_mode: QuirksMode) - -> ParserContext<'a> { + pub fn new_for_cssom( + url_data: &'a UrlExtraData, + error_reporter: &'a ParseErrorReporter, + rule_type: Option<CssRuleType>, + parsing_mode: ParsingMode, + quirks_mode: QuirksMode + ) -> ParserContext<'a> { 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>) - -> ParserContext<'a> { + pub fn new_with_rule_type( + context: &'a ParserContext, + rule_type: Option<CssRuleType> + ) -> ParserContext<'a> { ParserContext { stylesheet_origin: context.stylesheet_origin, url_data: context.url_data, @@ -134,13 +135,14 @@ impl<'a> ParserContext<'a> { } /// Create a parser context for inline CSS which accepts additional line offset argument. - pub fn new_with_line_number_offset(stylesheet_origin: Origin, - url_data: &'a UrlExtraData, - error_reporter: &'a ParseErrorReporter, - line_number_offset: u64, - parsing_mode: ParsingMode, - quirks_mode: QuirksMode) - -> ParserContext<'a> { + pub fn new_with_line_number_offset( + stylesheet_origin: Origin, + url_data: &'a UrlExtraData, + error_reporter: &'a ParseErrorReporter, + line_number_offset: u64, + parsing_mode: ParsingMode, + quirks_mode: QuirksMode + ) -> ParserContext<'a> { ParserContext { stylesheet_origin: stylesheet_origin, url_data: url_data, diff --git a/components/style/stylesheets.rs b/components/style/stylesheets.rs index 3a0da395ec8..6347e39f0e9 100644 --- a/components/style/stylesheets.rs +++ b/components/style/stylesheets.rs @@ -40,7 +40,6 @@ use shared_lock::{SharedRwLock, Locked, ToCssWithGuard, SharedRwLockReadGuard}; use smallvec::SmallVec; use std::{fmt, mem}; use std::borrow::Borrow; -use std::cell::Cell; use std::mem::align_of; use std::os::raw::c_void; use std::slice; @@ -479,15 +478,19 @@ impl CssRule { loader: Option<&StylesheetLoader>) -> Result<(Self, State), SingleRuleParseError> { let error_reporter = NullReporter; - let mut context = ParserContext::new(parent_stylesheet.origin, - &parent_stylesheet.url_data, - &error_reporter, - None, - PARSING_MODE_DEFAULT, - parent_stylesheet.quirks_mode); - context.namespaces = Some(&parent_stylesheet.namespaces); + let context = ParserContext::new( + parent_stylesheet.origin, + &parent_stylesheet.url_data, + &error_reporter, + None, + PARSING_MODE_DEFAULT, + parent_stylesheet.quirks_mode + ); + let mut input = Parser::new(css); + let mut guard = parent_stylesheet.namespaces.write(); + // nested rules are in the body state let state = state.unwrap_or(State::Body); let mut rule_parser = TopLevelRuleParser { @@ -495,12 +498,13 @@ impl CssRule { context: context, shared_lock: &parent_stylesheet.shared_lock, loader: loader, - state: Cell::new(state), + state: state, + namespaces: Some(&mut *guard), }; match parse_one_rule(&mut input, &mut rule_parser) { - Ok(result) => Ok((result, rule_parser.state.get())), + Ok(result) => Ok((result, rule_parser.state)), Err(_) => { - if let State::Invalid = rule_parser.state.get() { + if let State::Invalid = rule_parser.state { Err(SingleRuleParseError::Hierarchy) } else { Err(SingleRuleParseError::Syntax) @@ -510,8 +514,10 @@ impl CssRule { } /// Deep clones this CssRule. - fn deep_clone_with_lock(&self, - lock: &SharedRwLock) -> CssRule { + fn deep_clone_with_lock( + &self, + lock: &SharedRwLock + ) -> CssRule { let guard = lock.read(); match *self { CssRule::Namespace(ref arc) => { @@ -1215,10 +1221,19 @@ impl Stylesheet { let namespaces = RwLock::new(Namespaces::default()); // FIXME: we really should update existing.url_data with the given url_data, // otherwise newly inserted rule may not have the right base url. - let (rules, dirty_on_viewport_size_change) = Stylesheet::parse_rules( - css, url_data, existing.origin, &namespaces, - &existing.shared_lock, stylesheet_loader, error_reporter, - existing.quirks_mode, line_number_offset); + let (rules, dirty_on_viewport_size_change) = + Stylesheet::parse_rules( + css, + url_data, + existing.origin, + &mut *namespaces.write(), + &existing.shared_lock, + stylesheet_loader, + error_reporter, + existing.quirks_mode, + line_number_offset + ); + mem::swap(&mut *existing.namespaces.write(), &mut *namespaces.write()); existing.dirty_on_viewport_size_change .store(dirty_on_viewport_size_change, Ordering::Release); @@ -1228,35 +1243,45 @@ impl Stylesheet { *existing.rules.write_with(&mut guard) = CssRules(rules); } - fn parse_rules(css: &str, - url_data: &UrlExtraData, - origin: Origin, - namespaces: &RwLock<Namespaces>, - shared_lock: &SharedRwLock, - stylesheet_loader: Option<&StylesheetLoader>, - error_reporter: &ParseErrorReporter, - quirks_mode: QuirksMode, - line_number_offset: u64) - -> (Vec<CssRule>, bool) { + fn parse_rules( + css: &str, + url_data: &UrlExtraData, + origin: Origin, + namespaces: &mut Namespaces, + shared_lock: &SharedRwLock, + stylesheet_loader: Option<&StylesheetLoader>, + error_reporter: &ParseErrorReporter, + quirks_mode: QuirksMode, + line_number_offset: u64 + ) -> (Vec<CssRule>, bool) { let mut rules = Vec::new(); let mut input = Parser::new(css); - let mut context = ParserContext::new_with_line_number_offset(origin, url_data, error_reporter, - line_number_offset, - PARSING_MODE_DEFAULT, - quirks_mode); - context.namespaces = Some(namespaces); + + let context = + ParserContext::new_with_line_number_offset( + origin, + url_data, + error_reporter, + line_number_offset, + PARSING_MODE_DEFAULT, + quirks_mode + ); + let rule_parser = TopLevelRuleParser { stylesheet_origin: origin, shared_lock: shared_lock, loader: stylesheet_loader, context: context, - state: Cell::new(State::Start), + state: State::Start, + namespaces: Some(namespaces), }; input.look_for_viewport_percentages(); { - let mut iter = RuleListParser::new_for_stylesheet(&mut input, rule_parser); + let mut iter = + RuleListParser::new_for_stylesheet(&mut input, rule_parser); + while let Some(result) = iter.next() { match result { Ok(rule) => rules.push(rule), @@ -1289,9 +1314,17 @@ impl Stylesheet { -> Stylesheet { let namespaces = RwLock::new(Namespaces::default()); let (rules, dirty_on_viewport_size_change) = Stylesheet::parse_rules( - css, &url_data, origin, &namespaces, - &shared_lock, stylesheet_loader, error_reporter, quirks_mode, line_number_offset, + css, + &url_data, + origin, + &mut *namespaces.write(), + &shared_lock, + stylesheet_loader, + error_reporter, + quirks_mode, + line_number_offset, ); + Stylesheet { origin: origin, url_data: url_data, @@ -1477,7 +1510,8 @@ struct TopLevelRuleParser<'a> { shared_lock: &'a SharedRwLock, loader: Option<&'a StylesheetLoader>, context: ParserContext<'a>, - state: Cell<State>, + state: State, + namespaces: Option<&'a mut Namespaces>, } impl<'b> TopLevelRuleParser<'b> { @@ -1549,84 +1583,89 @@ impl<'a> AtRuleParser for TopLevelRuleParser<'a> { type Prelude = AtRulePrelude; type AtRule = CssRule; - fn parse_prelude(&mut self, name: &str, input: &mut Parser) - -> Result<AtRuleType<AtRulePrelude, CssRule>, ()> { + fn parse_prelude( + &mut self, + name: &str, + input: &mut Parser + ) -> Result<AtRuleType<AtRulePrelude, CssRule>, ()> { let location = get_location_with_offset(input.current_source_location(), self.context.line_number_offset); match_ignore_ascii_case! { name, "import" => { - if self.state.get() <= State::Imports { - self.state.set(State::Imports); - let url_string = input.expect_url_or_string()?; - let specified_url = SpecifiedUrl::parse_from_string(url_string, &self.context)?; - - let media = parse_media_query_list(&self.context, input); - let media = Arc::new(self.shared_lock.wrap(media)); - - let noop_loader = NoOpLoader; - let loader = if !specified_url.is_invalid() { - self.loader.expect("Expected a stylesheet loader for @import") - } else { - &noop_loader - }; - - let mut specified_url = Some(specified_url); - let arc = loader.request_stylesheet(media, &mut |media| { - ImportRule { - url: specified_url.take().unwrap(), - stylesheet: Arc::new(Stylesheet { - rules: CssRules::new(Vec::new(), self.shared_lock), - media: media, - shared_lock: self.shared_lock.clone(), - origin: self.context.stylesheet_origin, - url_data: self.context.url_data.clone(), - namespaces: RwLock::new(Namespaces::default()), - dirty_on_viewport_size_change: AtomicBool::new(false), - disabled: AtomicBool::new(false), - quirks_mode: self.context.quirks_mode, - }), - source_location: location, - } - }, &mut |import_rule| { - Arc::new(self.shared_lock.wrap(import_rule)) - }); - return Ok(AtRuleType::WithoutBlock(CssRule::Import(arc))) - } else { - self.state.set(State::Invalid); + if self.state > State::Imports { + self.state = State::Invalid; return Err(()) // "@import must be before any rule but @charset" } + + self.state = State::Imports; + let url_string = input.expect_url_or_string()?; + let specified_url = SpecifiedUrl::parse_from_string(url_string, &self.context)?; + + let media = parse_media_query_list(&self.context, input); + let media = Arc::new(self.shared_lock.wrap(media)); + + let noop_loader = NoOpLoader; + let loader = if !specified_url.is_invalid() { + self.loader.expect("Expected a stylesheet loader for @import") + } else { + &noop_loader + }; + + let mut specified_url = Some(specified_url); + let arc = loader.request_stylesheet(media, &mut |media| { + ImportRule { + url: specified_url.take().unwrap(), + stylesheet: Arc::new(Stylesheet { + rules: CssRules::new(Vec::new(), self.shared_lock), + media: media, + shared_lock: self.shared_lock.clone(), + origin: self.context.stylesheet_origin, + url_data: self.context.url_data.clone(), + namespaces: RwLock::new(Namespaces::default()), + dirty_on_viewport_size_change: AtomicBool::new(false), + disabled: AtomicBool::new(false), + quirks_mode: self.context.quirks_mode, + }), + source_location: location, + } + }, &mut |import_rule| { + Arc::new(self.shared_lock.wrap(import_rule)) + }); + + return Ok(AtRuleType::WithoutBlock(CssRule::Import(arc))) }, "namespace" => { - if self.state.get() <= State::Namespaces { - self.state.set(State::Namespaces); - - let prefix_result = input.try(|input| input.expect_ident()); - let url = Namespace::from(try!(input.expect_url_or_string())); - - let id = register_namespace(&url)?; - - let opt_prefix = if let Ok(prefix) = prefix_result { - let prefix = Prefix::from(prefix); - self.context.namespaces.expect("namespaces must be set whilst parsing rules") - .write().prefixes.insert(prefix.clone(), (url.clone(), id)); - Some(prefix) - } else { - self.context.namespaces.expect("namespaces must be set whilst parsing rules") - .write().default = Some((url.clone(), id)); - None - }; - - return Ok(AtRuleType::WithoutBlock(CssRule::Namespace(Arc::new( - self.shared_lock.wrap(NamespaceRule { - prefix: opt_prefix, - url: url, - source_location: location, - }) - )))) - } else { - self.state.set(State::Invalid); + if self.state > State::Namespaces { + self.state = State::Invalid; return Err(()) // "@namespace must be before any rule but @charset and @import" } + self.state = State::Namespaces; + + let prefix_result = input.try(|input| input.expect_ident()); + let url = Namespace::from(try!(input.expect_url_or_string())); + + let id = register_namespace(&url)?; + + let mut namespaces = self.namespaces.as_mut().unwrap(); + + let opt_prefix = if let Ok(prefix) = prefix_result { + let prefix = Prefix::from(prefix); + namespaces + .prefixes + .insert(prefix.clone(), (url.clone(), id)); + Some(prefix) + } else { + namespaces.default = Some((url.clone(), id)); + None + }; + + return Ok(AtRuleType::WithoutBlock(CssRule::Namespace(Arc::new( + self.shared_lock.wrap(NamespaceRule { + prefix: opt_prefix, + url: url, + source_location: location, + }) + )))) }, // @charset is removed by rust-cssparser if it’s the first rule in the stylesheet // anything left is invalid. @@ -1634,11 +1673,18 @@ impl<'a> AtRuleParser for TopLevelRuleParser<'a> { _ => {} } // Don't allow starting with an invalid state - if self.state.get() > State::Body { - self.state.set(State::Invalid); + if self.state > State::Body { + self.state = State::Invalid; return Err(()); } - self.state.set(State::Body); + self.state = State::Body; + + // "Freeze" the namespace map (no more namespace rules can be parsed + // after this point), and stick it in the context. + if self.namespaces.is_some() { + let namespaces = &*self.namespaces.take().unwrap(); + self.context.namespaces = Some(namespaces); + } AtRuleParser::parse_prelude(&mut self.nested(), name, input) } @@ -1655,13 +1701,24 @@ impl<'a> QualifiedRuleParser for TopLevelRuleParser<'a> { #[inline] fn parse_prelude(&mut self, input: &mut Parser) -> Result<SelectorList<SelectorImpl>, ()> { - self.state.set(State::Body); + self.state = State::Body; + + // "Freeze" the namespace map (no more namespace rules can be parsed + // after this point), and stick it in the context. + if self.namespaces.is_some() { + let namespaces = &*self.namespaces.take().unwrap(); + self.context.namespaces = Some(namespaces); + } + QualifiedRuleParser::parse_prelude(&mut self.nested(), input) } #[inline] - fn parse_block(&mut self, prelude: SelectorList<SelectorImpl>, input: &mut Parser) - -> Result<CssRule, ()> { + fn parse_block( + &mut self, + prelude: SelectorList<SelectorImpl>, + input: &mut Parser + ) -> Result<CssRule, ()> { QualifiedRuleParser::parse_block(&mut self.nested(), prelude, input) } } @@ -1674,13 +1731,19 @@ struct NestedRuleParser<'a, 'b: 'a> { } impl<'a, 'b> NestedRuleParser<'a, 'b> { - fn parse_nested_rules(&self, input: &mut Parser, rule_type: CssRuleType) -> Arc<Locked<CssRules>> { + fn parse_nested_rules( + &mut self, + input: &mut Parser, + rule_type: CssRuleType + ) -> Arc<Locked<CssRules>> { let context = ParserContext::new_with_rule_type(self.context, Some(rule_type)); + let nested_parser = NestedRuleParser { stylesheet_origin: self.stylesheet_origin, shared_lock: self.shared_lock, context: &context, }; + let mut iter = RuleListParser::new_for_nested_rule(input, nested_parser); let mut rules = Vec::new(); while let Some(result) = iter.next() { @@ -1711,10 +1774,17 @@ impl<'a, 'b> AtRuleParser for NestedRuleParser<'a, 'b> { type Prelude = AtRulePrelude; type AtRule = CssRule; - fn parse_prelude(&mut self, name: &str, input: &mut Parser) - -> Result<AtRuleType<AtRulePrelude, CssRule>, ()> { - let location = get_location_with_offset(input.current_source_location(), - self.context.line_number_offset); + fn parse_prelude( + &mut self, + name: &str, + input: &mut Parser + ) -> Result<AtRuleType<AtRulePrelude, CssRule>, ()> { + let location = + get_location_with_offset( + input.current_source_location(), + self.context.line_number_offset + ); + match_ignore_ascii_case! { name, "media" => { let media_queries = parse_media_query_list(self.context, input); @@ -1855,16 +1925,19 @@ impl<'a, 'b> QualifiedRuleParser for NestedRuleParser<'a, 'b> { type QualifiedRule = CssRule; fn parse_prelude(&mut self, input: &mut Parser) -> Result<SelectorList<SelectorImpl>, ()> { - let ns = self.context.namespaces.expect("namespaces must be set when parsing rules").read(); let selector_parser = SelectorParser { stylesheet_origin: self.stylesheet_origin, - namespaces: &*ns, + namespaces: self.context.namespaces.unwrap(), }; + SelectorList::parse(&selector_parser, input) } - fn parse_block(&mut self, prelude: SelectorList<SelectorImpl>, input: &mut Parser) - -> Result<CssRule, ()> { + fn parse_block( + &mut self, + prelude: SelectorList<SelectorImpl>, + input: &mut Parser + ) -> Result<CssRule, ()> { let location = get_location_with_offset(input.current_source_location(), self.context.line_number_offset); let context = ParserContext::new_with_rule_type(self.context, Some(CssRuleType::Style)); diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index cc8855b15de..7bd45a3e753 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -1311,23 +1311,25 @@ impl Parse for Attr { #[cfg(feature = "gecko")] /// Get the namespace id from the namespace map -pub fn get_id_for_namespace(namespace: &Namespace, context: &ParserContext) -> Result<NamespaceId, ()> { - if let Some(map) = context.namespaces { - if let Some(ref entry) = map.read().prefixes.get(&namespace.0) { - Ok(entry.1) - } else { - Err(()) +fn get_id_for_namespace(namespace: &Namespace, context: &ParserContext) -> Result<NamespaceId, ()> { + let namespaces_map = match context.namespaces { + Some(map) => map, + None => { + // If we don't have a namespace map (e.g. in inline styles) + // we can't parse namespaces + return Err(()); } - } else { - // if we don't have a namespace map (e.g. in inline styles) - // we can't parse namespaces - Err(()) + }; + + match namespaces_map.prefixes.get(&namespace.0) { + Some(entry) => Ok(entry.1), + None => Err(()), } } #[cfg(feature = "servo")] /// Get the namespace id from the namespace map -pub fn get_id_for_namespace(_: &Namespace, _: &ParserContext) -> Result<NamespaceId, ()> { +fn get_id_for_namespace(_: &Namespace, _: &ParserContext) -> Result<NamespaceId, ()> { Ok(()) } @@ -1340,27 +1342,28 @@ impl Attr { let first = input.try(|i| i.expect_ident()).ok(); if let Ok(token) = input.try(|i| i.next_including_whitespace()) { match token { - Token::Delim('|') => { - // must be followed by an ident - let second_token = match input.next_including_whitespace()? { - Token::Ident(second) => second, - _ => return Err(()), - }; - let ns_with_id = if let Some(ns) = first { - let ns: Namespace = ns.into(); - let id = get_id_for_namespace(&ns, context)?; - Some((ns, id)) - } else { - None - }; - return Ok(Attr { - namespace: ns_with_id, - attribute: second_token.into_owned(), - }) - } - _ => return Err(()) + Token::Delim('|') => {} + _ => return Err(()), } + // must be followed by an ident + let second_token = match input.next_including_whitespace()? { + Token::Ident(second) => second, + _ => return Err(()), + }; + + let ns_with_id = if let Some(ns) = first { + let ns: Namespace = ns.into(); + let id = get_id_for_namespace(&ns, context)?; + Some((ns, id)) + } else { + None + }; + return Ok(Attr { + namespace: ns_with_id, + attribute: second_token.into_owned(), + }) } + if let Some(first) = first { Ok(Attr { namespace: None, |