Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Devirtualize CSS error reporting. #18209

Merged
merged 1 commit into from
Aug 24, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 0 additions & 2 deletions components/script/dom/css.rs
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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