Auto merge of #18209 - jdm:devirtualize, r=mbrubeck

Devirtualize CSS error reporting.

This removes a trait object from the path of reporting a CSS error.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] There are tests for these changes

<!-- 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/18209)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-08-24 13:19:25 -05:00 committed by GitHub
commit d4ddec8d33
28 changed files with 255 additions and 211 deletions

View file

@ -10,8 +10,8 @@ use Atom;
use computed_values::font_family::FamilyName;
use cssparser::{AtRuleParser, AtRuleType, BasicParseError, DeclarationListParser, DeclarationParser, Parser};
use cssparser::{CowRcStr, RuleListParser, SourceLocation, QualifiedRuleParser, Token, serialize_identifier};
use error_reporting::ContextualParseError;
use parser::{ParserContext, Parse};
use error_reporting::{ContextualParseError, ParseErrorReporter};
use parser::{ParserContext, ParserErrorContext, Parse};
use selectors::parser::SelectorParseError;
use shared_lock::{SharedRwLockReadGuard, ToCssWithGuard};
use std::fmt;
@ -210,20 +210,26 @@ macro_rules! font_feature_values_blocks {
}
/// Parses a `FontFeatureValuesRule`.
pub fn parse(context: &ParserContext, input: &mut Parser,
family_names: Vec<FamilyName>, location: SourceLocation)
-> FontFeatureValuesRule {
pub fn parse<R>(context: &ParserContext,
error_context: &ParserErrorContext<R>,
input: &mut Parser,
family_names: Vec<FamilyName>,
location: SourceLocation)
-> FontFeatureValuesRule
where R: ParseErrorReporter
{
let mut rule = FontFeatureValuesRule::new(family_names, location);
{
let mut iter = RuleListParser::new_for_nested_rule(input, FontFeatureValuesRuleParser {
context: context,
error_context: error_context,
rule: &mut rule,
});
while let Some(result) = iter.next() {
if let Err(err) = result {
let error = ContextualParseError::UnsupportedRule(err.slice, err.error);
context.log_css_error(err.location, error);
context.log_css_error(error_context, err.location, error);
}
}
}
@ -293,19 +299,20 @@ macro_rules! font_feature_values_blocks {
/// }
/// <feature-type> = @stylistic | @historical-forms | @styleset |
/// @character-variant | @swash | @ornaments | @annotation
struct FontFeatureValuesRuleParser<'a> {
struct FontFeatureValuesRuleParser<'a, R: 'a> {
context: &'a ParserContext<'a>,
error_context: &'a ParserErrorContext<'a, R>,
rule: &'a mut FontFeatureValuesRule,
}
/// Default methods reject all qualified rules.
impl<'a, 'i> QualifiedRuleParser<'i> for FontFeatureValuesRuleParser<'a> {
impl<'a, 'i, R: ParseErrorReporter> QualifiedRuleParser<'i> for FontFeatureValuesRuleParser<'a, R> {
type Prelude = ();
type QualifiedRule = ();
type Error = SelectorParseError<'i, StyleParseError<'i>>;
}
impl<'a, 'i> AtRuleParser<'i> for FontFeatureValuesRuleParser<'a> {
impl<'a, 'i, R: ParseErrorReporter> AtRuleParser<'i> for FontFeatureValuesRuleParser<'a, R> {
type Prelude = BlockType;
type AtRule = ();
type Error = SelectorParseError<'i, StyleParseError<'i>>;
@ -341,7 +348,7 @@ macro_rules! font_feature_values_blocks {
if let Err(err) = declaration {
let error = ContextualParseError::UnsupportedKeyframePropertyDeclaration(
err.slice, err.error);
self.context.log_css_error(err.location, error);
self.context.log_css_error(self.error_context, err.location, error);
}
}
},

View file

@ -6,8 +6,8 @@
use cssparser::{AtRuleParser, Parser, QualifiedRuleParser, RuleListParser, ParserInput, CowRcStr};
use cssparser::{DeclarationListParser, DeclarationParser, parse_one_rule, SourceLocation};
use error_reporting::{NullReporter, ContextualParseError};
use parser::ParserContext;
use error_reporting::{NullReporter, ContextualParseError, ParseErrorReporter};
use parser::{ParserContext, ParserErrorContext};
use properties::{Importance, PropertyDeclaration, PropertyDeclarationBlock, PropertyId, PropertyParserContext};
use properties::{PropertyDeclarationId, LonghandId, SourcePropertyDeclaration};
use properties::LonghandIdSet;
@ -218,11 +218,11 @@ impl Keyframe {
let mut context = ParserContext::new(
parent_stylesheet_contents.origin,
&url_data,
&error_reporter,
Some(CssRuleType::Keyframe),
PARSING_MODE_DEFAULT,
parent_stylesheet_contents.quirks_mode
);
let error_context = ParserErrorContext { error_reporter: &error_reporter };
context.namespaces = Some(&*namespaces);
let mut input = ParserInput::new(css);
let mut input = Parser::new(&mut input);
@ -230,6 +230,7 @@ impl Keyframe {
let mut declarations = SourcePropertyDeclaration::new();
let mut rule_parser = KeyframeListParser {
context: &context,
error_context: &error_context,
shared_lock: &lock,
declarations: &mut declarations,
};
@ -447,37 +448,42 @@ impl KeyframesAnimation {
/// 40%, 60%, 100% {
/// width: 100%;
/// }
struct KeyframeListParser<'a> {
struct KeyframeListParser<'a, R: 'a> {
context: &'a ParserContext<'a>,
error_context: &'a ParserErrorContext<'a, R>,
shared_lock: &'a SharedRwLock,
declarations: &'a mut SourcePropertyDeclaration,
}
/// Parses a keyframe list from CSS input.
pub fn parse_keyframe_list(
pub fn parse_keyframe_list<R>(
context: &ParserContext,
error_context: &ParserErrorContext<R>,
input: &mut Parser,
shared_lock: &SharedRwLock
) -> Vec<Arc<Locked<Keyframe>>> {
) -> Vec<Arc<Locked<Keyframe>>>
where R: ParseErrorReporter
{
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,
error_context: error_context,
shared_lock: shared_lock,
declarations: &mut declarations,
}).filter_map(Result::ok).collect()
}
enum Void {}
impl<'a, 'i> AtRuleParser<'i> for KeyframeListParser<'a> {
impl<'a, 'i, R> AtRuleParser<'i> for KeyframeListParser<'a, R> {
type Prelude = Void;
type AtRule = Arc<Locked<Keyframe>>;
type Error = SelectorParseError<'i, StyleParseError<'i>>;
}
impl<'a, 'i> QualifiedRuleParser<'i> for KeyframeListParser<'a> {
impl<'a, 'i, R: ParseErrorReporter> QualifiedRuleParser<'i> for KeyframeListParser<'a, R> {
type Prelude = KeyframeSelector;
type QualifiedRule = Arc<Locked<Keyframe>>;
type Error = SelectorParseError<'i, StyleParseError<'i>>;
@ -489,7 +495,7 @@ impl<'a, 'i> QualifiedRuleParser<'i> for KeyframeListParser<'a> {
Ok(sel) => Ok(sel),
Err(e) => {
let error = ContextualParseError::InvalidKeyframeRule(input.slice_from(start_position), e.clone());
self.context.log_css_error(start_location, error);
self.context.log_css_error(self.error_context, start_location, error);
Err(e)
}
}
@ -518,7 +524,7 @@ impl<'a, 'i> QualifiedRuleParser<'i> for KeyframeListParser<'a> {
Err(err) => {
iter.parser.declarations.clear();
let error = ContextualParseError::UnsupportedKeyframePropertyDeclaration(err.slice, err.error);
context.log_css_error(err.location, error);
context.log_css_error(self.error_context, err.location, error);
}
}
// `parse_important` is not called here, `!important` is not allowed in keyframe blocks.

View file

@ -26,7 +26,7 @@ pub mod viewport_rule;
use cssparser::{parse_one_rule, Parser, ParserInput};
use error_reporting::NullReporter;
use parser::ParserContext;
use parser::{ParserContext, ParserErrorContext};
use servo_arc::Arc;
use shared_lock::{DeepCloneParams, DeepCloneWithLock, Locked, SharedRwLock, SharedRwLockReadGuard, ToCssWithGuard};
use std::fmt;
@ -230,7 +230,6 @@ impl CssRule {
let context = ParserContext::new(
parent_stylesheet_contents.origin,
&url_data,
&error_reporter,
None,
PARSING_MODE_DEFAULT,
parent_stylesheet_contents.quirks_mode,
@ -246,6 +245,7 @@ impl CssRule {
let mut rule_parser = TopLevelRuleParser {
stylesheet_origin: parent_stylesheet_contents.origin,
context: context,
error_context: ParserErrorContext { error_reporter: &error_reporter },
shared_lock: &shared_lock,
loader: loader,
state: state,

View file

@ -9,10 +9,10 @@ use computed_values::font_family::FamilyName;
use counter_style::{parse_counter_style_body, parse_counter_style_name};
use cssparser::{AtRuleParser, AtRuleType, Parser, QualifiedRuleParser, RuleListParser};
use cssparser::{CowRcStr, SourceLocation, BasicParseError};
use error_reporting::ContextualParseError;
use error_reporting::{ContextualParseError, ParseErrorReporter};
use font_face::parse_font_face_block;
use media_queries::{parse_media_query_list, MediaList};
use parser::{Parse, ParserContext};
use parser::{Parse, ParserContext, ParserErrorContext};
use properties::parse_property_declaration_list;
use selector_parser::{SelectorImpl, SelectorParser};
use selectors::SelectorList;
@ -35,7 +35,7 @@ use values::KeyframesName;
use values::specified::url::SpecifiedUrl;
/// The parser for the top-level rules in a stylesheet.
pub struct TopLevelRuleParser<'a> {
pub struct TopLevelRuleParser<'a, R: 'a> {
/// The origin of the stylesheet we're parsing.
pub stylesheet_origin: Origin,
/// A reference to the lock we need to use to create rules.
@ -47,6 +47,8 @@ pub struct TopLevelRuleParser<'a> {
/// This won't contain any namespaces, and only nested parsers created with
/// `ParserContext::new_with_rule_type` will.
pub context: ParserContext<'a>,
/// The context required for reporting parse errors.
pub error_context: ParserErrorContext<'a, R>,
/// The current state of the parser.
pub state: State,
/// Whether we have tried to parse was invalid due to being in the wrong
@ -59,12 +61,13 @@ pub struct TopLevelRuleParser<'a> {
pub namespaces: &'a mut Namespaces,
}
impl<'b> TopLevelRuleParser<'b> {
fn nested<'a: 'b>(&'a self) -> NestedRuleParser<'a, 'b> {
impl<'b, R> TopLevelRuleParser<'b, R> {
fn nested<'a: 'b>(&'a self) -> NestedRuleParser<'a, 'b, R> {
NestedRuleParser {
stylesheet_origin: self.stylesheet_origin,
shared_lock: self.shared_lock,
context: &self.context,
error_context: &self.error_context,
namespaces: &self.namespaces,
}
}
@ -147,7 +150,7 @@ fn register_namespace(_: &Namespace) -> Result<(), ()> {
Ok(()) // servo doesn't use namespace ids
}
impl<'a, 'i> AtRuleParser<'i> for TopLevelRuleParser<'a> {
impl<'a, 'i, R: ParseErrorReporter> AtRuleParser<'i> for TopLevelRuleParser<'a, R> {
type Prelude = AtRulePrelude;
type AtRule = CssRule;
type Error = SelectorParseError<'i, StyleParseError<'i>>;
@ -256,7 +259,7 @@ pub struct QualifiedRuleParserPrelude {
source_location: SourceLocation,
}
impl<'a, 'i> QualifiedRuleParser<'i> for TopLevelRuleParser<'a> {
impl<'a, 'i, R: ParseErrorReporter> QualifiedRuleParser<'i> for TopLevelRuleParser<'a, R> {
type Prelude = QualifiedRuleParserPrelude;
type QualifiedRule = CssRule;
type Error = SelectorParseError<'i, StyleParseError<'i>>;
@ -281,14 +284,15 @@ impl<'a, 'i> QualifiedRuleParser<'i> for TopLevelRuleParser<'a> {
}
#[derive(Clone)] // shallow, relatively cheap .clone
struct NestedRuleParser<'a, 'b: 'a> {
struct NestedRuleParser<'a, 'b: 'a, R: 'b> {
stylesheet_origin: Origin,
shared_lock: &'a SharedRwLock,
context: &'a ParserContext<'b>,
error_context: &'a ParserErrorContext<'b, R>,
namespaces: &'a Namespaces,
}
impl<'a, 'b> NestedRuleParser<'a, 'b> {
impl<'a, 'b, R: ParseErrorReporter> NestedRuleParser<'a, 'b, R> {
fn parse_nested_rules(
&mut self,
input: &mut Parser,
@ -301,6 +305,7 @@ impl<'a, 'b> NestedRuleParser<'a, 'b> {
stylesheet_origin: self.stylesheet_origin,
shared_lock: self.shared_lock,
context: &context,
error_context: &self.error_context,
namespaces: self.namespaces,
};
@ -311,7 +316,7 @@ impl<'a, 'b> NestedRuleParser<'a, 'b> {
Ok(rule) => rules.push(rule),
Err(err) => {
let error = ContextualParseError::UnsupportedRule(err.slice, err.error);
self.context.log_css_error(err.location, error);
self.context.log_css_error(self.error_context, err.location, error);
}
}
}
@ -319,7 +324,7 @@ impl<'a, 'b> NestedRuleParser<'a, 'b> {
}
}
impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> {
impl<'a, 'b, 'i, R: ParseErrorReporter> AtRuleParser<'i> for NestedRuleParser<'a, 'b, R> {
type Prelude = AtRulePrelude;
type AtRule = CssRule;
type Error = SelectorParseError<'i, StyleParseError<'i>>;
@ -428,7 +433,7 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> {
);
Ok(CssRule::FontFace(Arc::new(self.shared_lock.wrap(
parse_font_face_block(&context, input, location).into()))))
parse_font_face_block(&context, self.error_context, input, location).into()))))
}
AtRulePrelude::FontFeatureValues(family_names, location) => {
let context =
@ -438,7 +443,7 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> {
self.namespaces,
);
Ok(CssRule::FontFeatureValues(Arc::new(self.shared_lock.wrap(
FontFeatureValuesRule::parse(&context, input, family_names, location)))))
FontFeatureValuesRule::parse(&context, self.error_context, input, family_names, location)))))
}
AtRulePrelude::CounterStyle(name) => {
let context =
@ -448,7 +453,7 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> {
self.namespaces,
);
Ok(CssRule::CounterStyle(Arc::new(self.shared_lock.wrap(
parse_counter_style_body(name, &context, input)?.into()))))
parse_counter_style_body(name, &context, self.error_context, input)?.into()))))
}
AtRulePrelude::Media(media_queries, location) => {
Ok(CssRule::Media(Arc::new(self.shared_lock.wrap(MediaRule {
@ -480,7 +485,7 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> {
self.namespaces,
);
Ok(CssRule::Viewport(Arc::new(self.shared_lock.wrap(
ViewportRule::parse(&context, input)?))))
ViewportRule::parse(&context, self.error_context, input)?))))
}
AtRulePrelude::Keyframes(name, prefix, location) => {
let context =
@ -492,7 +497,7 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> {
Ok(CssRule::Keyframes(Arc::new(self.shared_lock.wrap(KeyframesRule {
name: name,
keyframes: parse_keyframe_list(&context, input, self.shared_lock),
keyframes: parse_keyframe_list(&context, self.error_context, input, self.shared_lock),
vendor_prefix: prefix,
source_location: location,
}))))
@ -504,7 +509,7 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> {
CssRuleType::Page,
self.namespaces,
);
let declarations = parse_property_declaration_list(&context, input);
let declarations = parse_property_declaration_list(&context, self.error_context, input);
Ok(CssRule::Page(Arc::new(self.shared_lock.wrap(PageRule {
block: Arc::new(self.shared_lock.wrap(declarations)),
source_location: location,
@ -525,7 +530,7 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for NestedRuleParser<'a, 'b> {
}
}
impl<'a, 'b, 'i> QualifiedRuleParser<'i> for NestedRuleParser<'a, 'b> {
impl<'a, 'b, 'i, R: ParseErrorReporter> QualifiedRuleParser<'i> for NestedRuleParser<'a, 'b, R> {
type Prelude = QualifiedRuleParserPrelude;
type QualifiedRule = CssRule;
type Error = SelectorParseError<'i, StyleParseError<'i>>;
@ -561,7 +566,7 @@ impl<'a, 'b, 'i> QualifiedRuleParser<'i> for NestedRuleParser<'a, 'b> {
CssRuleType::Style,
self.namespaces,
);
let declarations = parse_property_declaration_list(&context, input);
let declarations = parse_property_declaration_list(&context, self.error_context, input);
Ok(CssRule::Style(Arc::new(self.shared_lock.wrap(StyleRule {
selectors: prelude.selectors,
block: Arc::new(self.shared_lock.wrap(declarations)),

View file

@ -10,7 +10,7 @@ use fnv::FnvHashMap;
use invalidation::media_queries::{MediaListKey, ToMediaListKey};
use media_queries::{MediaList, Device};
use parking_lot::RwLock;
use parser::ParserContext;
use parser::{ParserContext, ParserErrorContext};
use servo_arc::Arc;
use shared_lock::{DeepCloneParams, DeepCloneWithLock, Locked, SharedRwLock, SharedRwLockReadGuard};
use std::mem;
@ -67,13 +67,13 @@ pub struct StylesheetContents {
impl StylesheetContents {
/// Parse a given CSS string, with a given url-data, origin, and
/// quirks mode.
pub fn from_str(
pub fn from_str<R: ParseErrorReporter>(
css: &str,
url_data: UrlExtraData,
origin: Origin,
shared_lock: &SharedRwLock,
stylesheet_loader: Option<&StylesheetLoader>,
error_reporter: &ParseErrorReporter,
error_reporter: &R,
quirks_mode: QuirksMode,
line_number_offset: u64
) -> Self {
@ -313,12 +313,14 @@ impl StylesheetInDocument for DocumentStyleSheet {
impl Stylesheet {
/// Updates an empty stylesheet from a given string of text.
pub fn update_from_str(existing: &Stylesheet,
css: &str,
url_data: UrlExtraData,
stylesheet_loader: Option<&StylesheetLoader>,
error_reporter: &ParseErrorReporter,
line_number_offset: u64) {
pub fn update_from_str<R>(existing: &Stylesheet,
css: &str,
url_data: UrlExtraData,
stylesheet_loader: Option<&StylesheetLoader>,
error_reporter: &R,
line_number_offset: u64)
where R: ParseErrorReporter
{
let namespaces = RwLock::new(Namespaces::default());
let (rules, dirty_on_viewport_size_change, source_map_url) =
Stylesheet::parse_rules(
@ -347,14 +349,14 @@ impl Stylesheet {
*existing.contents.source_map_url.write() = source_map_url;
}
fn parse_rules(
fn parse_rules<R: ParseErrorReporter>(
css: &str,
url_data: &UrlExtraData,
origin: Origin,
namespaces: &mut Namespaces,
shared_lock: &SharedRwLock,
stylesheet_loader: Option<&StylesheetLoader>,
error_reporter: &ParseErrorReporter,
error_reporter: &R,
quirks_mode: QuirksMode,
line_number_offset: u64
) -> (Vec<CssRule>, bool, Option<String>) {
@ -366,17 +368,18 @@ impl Stylesheet {
ParserContext::new_with_line_number_offset(
origin,
url_data,
error_reporter,
line_number_offset,
PARSING_MODE_DEFAULT,
quirks_mode
);
let error_context = ParserErrorContext { error_reporter };
let rule_parser = TopLevelRuleParser {
stylesheet_origin: origin,
shared_lock: shared_lock,
loader: stylesheet_loader,
context: context,
error_context: error_context,
state: State::Start,
had_hierarchy_error: false,
namespaces: namespaces,
@ -393,7 +396,8 @@ impl Stylesheet {
Ok(rule) => rules.push(rule),
Err(err) => {
let error = ContextualParseError::InvalidRule(err.slice, err.error);
iter.parser.context.log_css_error(err.location, error);
iter.parser.context.log_css_error(&iter.parser.error_context,
err.location, error);
}
}
}
@ -408,16 +412,18 @@ impl Stylesheet {
///
/// Effectively creates a new stylesheet and forwards the hard work to
/// `Stylesheet::update_from_str`.
pub fn from_str(css: &str,
url_data: UrlExtraData,
origin: Origin,
media: Arc<Locked<MediaList>>,
shared_lock: SharedRwLock,
stylesheet_loader: Option<&StylesheetLoader>,
error_reporter: &ParseErrorReporter,
quirks_mode: QuirksMode,
line_number_offset: u64)
-> Stylesheet {
pub fn from_str<R: ParseErrorReporter>(
css: &str,
url_data: UrlExtraData,
origin: Origin,
media: Arc<Locked<MediaList>>,
shared_lock: SharedRwLock,
stylesheet_loader: Option<&StylesheetLoader>,
error_reporter: &R,
quirks_mode: QuirksMode,
line_number_offset: u64)
-> Stylesheet
{
let contents = StylesheetContents::from_str(
css,
url_data,

View file

@ -11,11 +11,11 @@ use app_units::Au;
use context::QuirksMode;
use cssparser::{AtRuleParser, DeclarationListParser, DeclarationParser, Parser, parse_important};
use cssparser::{CowRcStr, ToCss as ParserToCss};
use error_reporting::ContextualParseError;
use error_reporting::{ContextualParseError, ParseErrorReporter};
use euclid::TypedSize2D;
use font_metrics::get_metrics_provider_for_product;
use media_queries::Device;
use parser::{Parse, ParserContext};
use parser::{ParserContext, ParserErrorContext};
use properties::StyleBuilder;
use selectors::parser::SelectorParseError;
use shared_lock::{SharedRwLockReadGuard, ToCssWithGuard};
@ -346,8 +346,14 @@ fn is_whitespace_separator_or_equals(c: &char) -> bool {
WHITESPACE.contains(c) || SEPARATOR.contains(c) || *c == '='
}
impl Parse for ViewportRule {
fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
impl ViewportRule {
/// Parse a single @viewport rule.
pub fn parse<'i, 't, R>(context: &ParserContext,
error_context: &ParserErrorContext<R>,
input: &mut Parser<'i, 't>)
-> Result<Self, ParseError<'i>>
where R: ParseErrorReporter
{
let parser = ViewportRuleParser { context: context };
let mut cascade = Cascade::new();
@ -361,7 +367,7 @@ impl Parse for ViewportRule {
}
Err(err) => {
let error = ContextualParseError::UnsupportedViewportDescriptorDeclaration(err.slice, err.error);
context.log_css_error(err.location, error);
context.log_css_error(error_context, err.location, error);
}
}
}