Skip to content

Commit

Permalink
Auto merge of #18209 - jdm:devirtualize, r=mbrubeck
Browse files Browse the repository at this point in the history
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 -->
  • Loading branch information
bors-servo committed Aug 24, 2017
2 parents 6eb46b1 + 1297c0f commit d4ddec8
Show file tree
Hide file tree
Showing 28 changed files with 255 additions and 211 deletions.
2 changes: 0 additions & 2 deletions components/script/dom/css.rs
Expand Up @@ -38,7 +38,6 @@ impl CSS {
let url = win.Document().url();
let context = ParserContext::new_for_cssom(
&url,
win.css_error_reporter(),
Some(CssRuleType::Style),
PARSING_MODE_DEFAULT,
QuirksMode::NoQuirks
Expand All @@ -55,7 +54,6 @@ impl CSS {
let url = win.Document().url();
let context = ParserContext::new_for_cssom(
&url,
win.css_error_reporter(),
Some(CssRuleType::Style),
PARSING_MODE_DEFAULT,
QuirksMode::NoQuirks
Expand Down
2 changes: 1 addition & 1 deletion components/script/dom/cssmediarule.rs
Expand Up @@ -75,7 +75,7 @@ impl CSSMediaRule {
let win = global.as_window();
let url = win.get_url();
let quirks_mode = win.Document().quirks_mode();
let context = ParserContext::new_for_cssom(&url, win.css_error_reporter(), Some(CssRuleType::Media),
let context = ParserContext::new_for_cssom(&url, Some(CssRuleType::Media),
PARSING_MODE_DEFAULT,
quirks_mode);
let new_medialist = parse_media_query_list(&context, &mut input);
Expand Down
2 changes: 1 addition & 1 deletion components/script/dom/csssupportsrule.rs
Expand Up @@ -63,7 +63,7 @@ impl CSSSupportsRule {
let win = global.as_window();
let url = win.Document().url();
let quirks_mode = win.Document().quirks_mode();
let context = ParserContext::new_for_cssom(&url, win.css_error_reporter(), Some(CssRuleType::Supports),
let context = ParserContext::new_for_cssom(&url, Some(CssRuleType::Supports),
PARSING_MODE_DEFAULT,
quirks_mode);
let enabled = cond.eval(&context);
Expand Down
3 changes: 1 addition & 2 deletions components/script/dom/htmllinkelement.rs
Expand Up @@ -285,9 +285,8 @@ impl HTMLLinkElement {

let mut input = ParserInput::new(&mq_str);
let mut css_parser = CssParser::new(&mut input);
let win = document.window();
let doc_url = document.url();
let context = CssParserContext::new_for_cssom(&doc_url, win.css_error_reporter(), Some(CssRuleType::Media),
let context = CssParserContext::new_for_cssom(&doc_url, Some(CssRuleType::Media),
PARSING_MODE_DEFAULT,
document.quirks_mode());
let media = parse_media_query_list(&context, &mut css_parser);
Expand Down
1 change: 0 additions & 1 deletion components/script/dom/htmlstyleelement.rs
Expand Up @@ -86,7 +86,6 @@ impl HTMLStyleElement {
let data = node.GetTextContent().expect("Element.textContent must be a string");
let url = win.get_url();
let context = CssParserContext::new_for_cssom(&url,
win.css_error_reporter(),
Some(CssRuleType::Media),
PARSING_MODE_DEFAULT,
doc.quirks_mode());
Expand Down
6 changes: 3 additions & 3 deletions components/script/dom/medialist.rs
Expand Up @@ -77,7 +77,7 @@ impl MediaListMethods for MediaList {
let win = global.as_window();
let url = win.get_url();
let quirks_mode = win.Document().quirks_mode();
let context = ParserContext::new_for_cssom(&url, win.css_error_reporter(), Some(CssRuleType::Media),
let context = ParserContext::new_for_cssom(&url, Some(CssRuleType::Media),
PARSING_MODE_DEFAULT,
quirks_mode);
*media_queries = parse_media_query_list(&context, &mut parser);
Expand Down Expand Up @@ -114,7 +114,7 @@ impl MediaListMethods for MediaList {
let win = global.as_window();
let url = win.get_url();
let quirks_mode = win.Document().quirks_mode();
let context = ParserContext::new_for_cssom(&url, win.css_error_reporter(), Some(CssRuleType::Media),
let context = ParserContext::new_for_cssom(&url, Some(CssRuleType::Media),
PARSING_MODE_DEFAULT,
quirks_mode);
let m = MediaQuery::parse(&context, &mut parser);
Expand Down Expand Up @@ -143,7 +143,7 @@ impl MediaListMethods for MediaList {
let win = global.as_window();
let url = win.get_url();
let quirks_mode = win.Document().quirks_mode();
let context = ParserContext::new_for_cssom(&url, win.css_error_reporter(), Some(CssRuleType::Media),
let context = ParserContext::new_for_cssom(&url, Some(CssRuleType::Media),
PARSING_MODE_DEFAULT,
quirks_mode);
let m = MediaQuery::parse(&context, &mut parser);
Expand Down
5 changes: 2 additions & 3 deletions components/script/dom/window.rs
Expand Up @@ -105,7 +105,6 @@ use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::mpsc::{Sender, channel};
use std::sync::mpsc::TryRecvError::{Disconnected, Empty};
use style::context::ReflowGoal;
use style::error_reporting::ParseErrorReporter;
use style::media_queries;
use style::parser::ParserContext as CssParserContext;
use style::properties::PropertyId;
Expand Down Expand Up @@ -377,7 +376,7 @@ impl Window {
&self.bluetooth_extra_permission_data
}

pub fn css_error_reporter(&self) -> &ParseErrorReporter {
pub fn css_error_reporter(&self) -> &CSSErrorReporter {
&self.error_reporter
}

Expand Down Expand Up @@ -1012,7 +1011,7 @@ impl WindowMethods for Window {
let mut parser = Parser::new(&mut input);
let url = self.get_url();
let quirks_mode = self.Document().quirks_mode();
let context = CssParserContext::new_for_cssom(&url, self.css_error_reporter(), Some(CssRuleType::Media),
let context = CssParserContext::new_for_cssom(&url, Some(CssRuleType::Media),
PARSING_MODE_DEFAULT,
quirks_mode);
let media_query_list = media_queries::parse_media_query_list(&context, &mut parser);
Expand Down
17 changes: 11 additions & 6 deletions components/style/counter_style/mod.rs
Expand Up @@ -9,10 +9,10 @@
use Atom;
use cssparser::{AtRuleParser, DeclarationListParser, DeclarationParser};
use cssparser::{Parser, Token, serialize_identifier, BasicParseError, CowRcStr};
use error_reporting::ContextualParseError;
use error_reporting::{ContextualParseError, ParseErrorReporter};
#[cfg(feature = "gecko")] use gecko::rules::CounterStyleDescriptors;
#[cfg(feature = "gecko")] use gecko_bindings::structs::nsCSSCounterDesc;
use parser::{ParserContext, Parse};
use parser::{ParserContext, ParserErrorContext, Parse};
use selectors::parser::SelectorParseError;
use shared_lock::{SharedRwLockReadGuard, ToCssWithGuard};
use std::ascii::AsciiExt;
Expand Down Expand Up @@ -50,8 +50,13 @@ pub fn parse_counter_style_name<'i, 't>(input: &mut Parser<'i, 't>) -> Result<Cu
}

/// Parse the body (inside `{}`) of an @counter-style rule
pub fn parse_counter_style_body<'i, 't>(name: CustomIdent, context: &ParserContext, input: &mut Parser<'i, 't>)
-> Result<CounterStyleRuleData, ParseError<'i>> {
pub fn parse_counter_style_body<'i, 't, R>(name: CustomIdent,
context: &ParserContext,
error_context: &ParserErrorContext<R>,
input: &mut Parser<'i, 't>)
-> Result<CounterStyleRuleData, ParseError<'i>>
where R: ParseErrorReporter
{
let start = input.current_source_location();
let mut rule = CounterStyleRuleData::empty(name);
{
Expand All @@ -63,7 +68,7 @@ pub fn parse_counter_style_body<'i, 't>(name: CustomIdent, context: &ParserConte
while let Some(declaration) = iter.next() {
if let Err(err) = declaration {
let error = ContextualParseError::UnsupportedCounterStyleDescriptorDeclaration(err.slice, err.error);
context.log_css_error(err.location, error)
context.log_css_error(error_context, err.location, error)
}
}
}
Expand Down Expand Up @@ -95,7 +100,7 @@ pub fn parse_counter_style_body<'i, 't>(name: CustomIdent, context: &ParserConte
_ => None
};
if let Some(error) = error {
context.log_css_error(start, error);
context.log_css_error(error_context, start, error);
Err(StyleParseError::UnspecifiedError.into())
} else {
Ok(rule)
Expand Down
40 changes: 22 additions & 18 deletions components/style/encoding_support.rs
Expand Up @@ -49,17 +49,19 @@ impl Stylesheet {
///
/// Takes care of decoding the network bytes and forwards the resulting
/// string to `Stylesheet::from_str`.
pub fn from_bytes(bytes: &[u8],
url_data: UrlExtraData,
protocol_encoding_label: Option<&str>,
environment_encoding: Option<EncodingRef>,
origin: Origin,
media: MediaList,
shared_lock: SharedRwLock,
stylesheet_loader: Option<&StylesheetLoader>,
error_reporter: &ParseErrorReporter,
quirks_mode: QuirksMode)
-> Stylesheet {
pub fn from_bytes<R>(bytes: &[u8],
url_data: UrlExtraData,
protocol_encoding_label: Option<&str>,
environment_encoding: Option<EncodingRef>,
origin: Origin,
media: MediaList,
shared_lock: SharedRwLock,
stylesheet_loader: Option<&StylesheetLoader>,
error_reporter: &R,
quirks_mode: QuirksMode)
-> Stylesheet
where R: ParseErrorReporter
{
let (string, _) = decode_stylesheet_bytes(
bytes, protocol_encoding_label, environment_encoding);
Stylesheet::from_str(&string,
Expand All @@ -75,13 +77,15 @@ impl Stylesheet {

/// Updates an empty stylesheet with a set of bytes that reached over the
/// network.
pub fn update_from_bytes(existing: &Stylesheet,
bytes: &[u8],
protocol_encoding_label: Option<&str>,
environment_encoding: Option<EncodingRef>,
url_data: UrlExtraData,
stylesheet_loader: Option<&StylesheetLoader>,
error_reporter: &ParseErrorReporter) {
pub fn update_from_bytes<R>(existing: &Stylesheet,
bytes: &[u8],
protocol_encoding_label: Option<&str>,
environment_encoding: Option<EncodingRef>,
url_data: UrlExtraData,
stylesheet_loader: Option<&StylesheetLoader>,
error_reporter: &R)
where R: ParseErrorReporter
{
let (string, _) = decode_stylesheet_bytes(
bytes, protocol_encoding_label, environment_encoding);
Self::update_from_str(existing,
Expand Down
15 changes: 10 additions & 5 deletions components/style/font_face.rs
Expand Up @@ -13,10 +13,10 @@ use computed_values::{font_feature_settings, font_stretch, font_style, font_weig
use computed_values::font_family::FamilyName;
use cssparser::{AtRuleParser, DeclarationListParser, DeclarationParser, Parser};
use cssparser::{SourceLocation, CowRcStr};
use error_reporting::ContextualParseError;
use error_reporting::{ContextualParseError, ParseErrorReporter};
#[cfg(feature = "gecko")] use gecko_bindings::structs::CSSFontFaceDescriptors;
#[cfg(feature = "gecko")] use cssparser::UnicodeRange;
use parser::{ParserContext, Parse};
use parser::{ParserContext, ParserErrorContext, Parse};
#[cfg(feature = "gecko")]
use properties::longhands::font_language_override;
use selectors::parser::SelectorParseError;
Expand Down Expand Up @@ -108,8 +108,13 @@ impl Parse for FontWeight {
/// Parse the block inside a `@font-face` rule.
///
/// Note that the prelude parsing code lives in the `stylesheets` module.
pub fn parse_font_face_block(context: &ParserContext, input: &mut Parser, location: SourceLocation)
-> FontFaceRuleData {
pub fn parse_font_face_block<R>(context: &ParserContext,
error_context: &ParserErrorContext<R>,
input: &mut Parser,
location: SourceLocation)
-> FontFaceRuleData
where R: ParseErrorReporter
{
let mut rule = FontFaceRuleData::empty(location);
{
let parser = FontFaceRuleParser {
Expand All @@ -120,7 +125,7 @@ pub fn parse_font_face_block(context: &ParserContext, input: &mut Parser, locati
while let Some(declaration) = iter.next() {
if let Err(err) = declaration {
let error = ContextualParseError::UnsupportedFontFaceDescriptor(err.slice, err.error);
context.log_css_error(err.location, error)
context.log_css_error(error_context, err.location, error)
}
}
}
Expand Down
11 changes: 7 additions & 4 deletions components/style/gecko/wrapper.rs
Expand Up @@ -467,10 +467,13 @@ impl<'le> fmt::Debug for GeckoElement<'le> {

impl<'le> GeckoElement<'le> {
/// Parse the style attribute of an element.
pub fn parse_style_attribute(value: &str,
url_data: &UrlExtraData,
quirks_mode: QuirksMode,
reporter: &ParseErrorReporter) -> PropertyDeclarationBlock {
pub fn parse_style_attribute<R>(value: &str,
url_data: &UrlExtraData,
quirks_mode: QuirksMode,
reporter: &R)
-> PropertyDeclarationBlock
where R: ParseErrorReporter
{
parse_style_attribute(value, url_data, reporter, quirks_mode)
}

Expand Down
24 changes: 13 additions & 11 deletions components/style/parser.rs
Expand Up @@ -38,15 +38,19 @@ pub fn assert_parsing_mode_match() {
}
}

/// The context required to report a parse error.
pub struct ParserErrorContext<'a, R: 'a> {
/// An error reporter to report syntax errors.
pub error_reporter: &'a R,
}

/// The data that the parser needs from outside in order to parse a stylesheet.
pub struct ParserContext<'a> {
/// The `Origin` of the stylesheet, whether it's a user, author or
/// user-agent stylesheet.
pub stylesheet_origin: Origin,
/// The extra data we need for resolving url values.
pub url_data: &'a UrlExtraData,
/// An error reporter to report syntax errors.
pub error_reporter: &'a ParseErrorReporter,
/// The current rule type, if any.
pub rule_type: Option<CssRuleType>,
/// Line number offsets for inline stylesheets
Expand All @@ -64,15 +68,13 @@ impl<'a> 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,
error_reporter: error_reporter,
rule_type: rule_type,
line_number_offset: 0u64,
parsing_mode: parsing_mode,
Expand All @@ -84,15 +86,13 @@ 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> {
Self::new(
Origin::Author,
url_data,
error_reporter,
rule_type,
parsing_mode,
quirks_mode,
Expand All @@ -108,7 +108,6 @@ impl<'a> ParserContext<'a> {
ParserContext {
stylesheet_origin: context.stylesheet_origin,
url_data: context.url_data,
error_reporter: context.error_reporter,
rule_type: Some(rule_type),
line_number_offset: context.line_number_offset,
parsing_mode: context.parsing_mode,
Expand All @@ -121,15 +120,13 @@ impl<'a> 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,
error_reporter: error_reporter,
rule_type: None,
line_number_offset: line_number_offset,
parsing_mode: parsing_mode,
Expand All @@ -144,12 +141,17 @@ impl<'a> ParserContext<'a> {
}

/// Record a CSS parse error with this context’s error reporting.
pub fn log_css_error(&self, location: SourceLocation, error: ContextualParseError) {
pub fn log_css_error<R>(&self,
context: &ParserErrorContext<R>,
location: SourceLocation,
error: ContextualParseError)
where R: ParseErrorReporter
{
let location = SourceLocation {
line: location.line + self.line_number_offset as u32,
column: location.column,
};
self.error_reporter.report_error(self.url_data, location, error)
context.error_reporter.report_error(self.url_data, location, error)
}
}

Expand Down

0 comments on commit d4ddec8

Please sign in to comment.