Auto merge of #19490 - heycam:canvas-color, r=Aethanyc

allow Gecko FFI functions for color parsing to report errors

This is the Servo-side part of https://bugzilla.mozilla.org/show_bug.cgi?id=1420026.

<!-- 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/19490)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-12-04 23:32:15 -06:00 committed by GitHub
commit e563777008
4 changed files with 87 additions and 19 deletions

View file

@ -46,6 +46,8 @@ pub enum ContextualParseError<'a> {
InvalidCounterStyleExtendsWithAdditiveSymbols, InvalidCounterStyleExtendsWithAdditiveSymbols,
/// A media rule was invalid for some reason. /// A media rule was invalid for some reason.
InvalidMediaRule(&'a str, ParseError<'a>), InvalidMediaRule(&'a str, ParseError<'a>),
/// A value was not recognized.
UnsupportedValue(&'a str, ParseError<'a>),
} }
impl<'a> fmt::Display for ContextualParseError<'a> { impl<'a> fmt::Display for ContextualParseError<'a> {
@ -173,6 +175,9 @@ impl<'a> fmt::Display for ContextualParseError<'a> {
write!(f, "Invalid media rule: {}, ", media_rule)?; write!(f, "Invalid media rule: {}, ", media_rule)?;
parse_error_to_str(err, f) parse_error_to_str(err, f)
} }
ContextualParseError::UnsupportedValue(_value, ref err) => {
parse_error_to_str(err, f)
}
} }
} }
} }

View file

@ -1577,7 +1577,7 @@ extern "C" {
} extern "C" { } extern "C" {
pub fn Servo_IsValidCSSColor ( value : * const nsAString , ) -> bool ; pub fn Servo_IsValidCSSColor ( value : * const nsAString , ) -> bool ;
} extern "C" { } extern "C" {
pub fn Servo_ComputeColor ( set : RawServoStyleSetBorrowedOrNull , current_color : nscolor , value : * const nsAString , result_color : * mut nscolor , ) -> bool ; pub fn Servo_ComputeColor ( set : RawServoStyleSetBorrowedOrNull , current_color : nscolor , value : * const nsAString , result_color : * mut nscolor , was_current_color : * mut bool , loader : * mut Loader , ) -> bool ;
} extern "C" { } extern "C" {
pub fn Servo_ParseIntersectionObserverRootMargin ( value : * const nsAString , result : * mut nsCSSRect , ) -> bool ; pub fn Servo_ParseIntersectionObserverRootMargin ( value : * const nsAString , result : * mut nsCSSRect , ) -> bool ;
} extern "C" { } extern "C" {

View file

@ -16,9 +16,8 @@ use style::gecko_bindings::bindings::Gecko_ReportUnexpectedCSSError;
use style::gecko_bindings::structs::{Loader, ServoStyleSheet, nsIURI}; use style::gecko_bindings::structs::{Loader, ServoStyleSheet, nsIURI};
use style::gecko_bindings::structs::ErrorReporter as GeckoErrorReporter; use style::gecko_bindings::structs::ErrorReporter as GeckoErrorReporter;
use style::gecko_bindings::structs::URLExtraData as RawUrlExtraData; use style::gecko_bindings::structs::URLExtraData as RawUrlExtraData;
use style::gecko_bindings::sugar::refptr::RefPtr;
use style::stylesheets::UrlExtraData; use style::stylesheets::UrlExtraData;
use style_traits::StyleParseErrorKind; use style_traits::{StyleParseErrorKind, ValueParseErrorKind};
pub type ErrorKind<'i> = ParseErrorKind<'i, StyleParseErrorKind<'i>>; pub type ErrorKind<'i> = ParseErrorKind<'i, StyleParseErrorKind<'i>>;
@ -29,10 +28,12 @@ impl ErrorReporter {
/// Create a new instance of the Gecko error reporter. /// Create a new instance of the Gecko error reporter.
pub fn new(sheet: *mut ServoStyleSheet, pub fn new(sheet: *mut ServoStyleSheet,
loader: *mut Loader, loader: *mut Loader,
url: *mut RawUrlExtraData) -> ErrorReporter { extra_data: *mut RawUrlExtraData) -> ErrorReporter {
unsafe { unsafe {
let url = RefPtr::from_ptr_ref(&url); let url = extra_data.as_ref()
ErrorReporter(Gecko_CreateCSSErrorReporter(sheet, loader, url.mBaseURI.raw::<nsIURI>())) .map(|d| d.mBaseURI.raw::<nsIURI>())
.unwrap_or(ptr::null_mut());
ErrorReporter(Gecko_CreateCSSErrorReporter(sheet, loader, url))
} }
} }
} }
@ -138,6 +139,9 @@ fn extract_error_params<'a>(err: ErrorKind<'a>) -> Option<ErrorParams<'a>> {
ParseErrorKind::Custom( ParseErrorKind::Custom(
StyleParseErrorKind::ExpectedIdentifier(token) StyleParseErrorKind::ExpectedIdentifier(token)
) |
ParseErrorKind::Custom(
StyleParseErrorKind::ValueError(ValueParseErrorKind::InvalidColor(token))
) => { ) => {
(Some(ErrorString::UnexpectedToken(token)), None) (Some(ErrorString::UnexpectedToken(token)), None)
} }
@ -197,7 +201,8 @@ impl<'a> ErrorHelpers<'a> for ContextualParseError<'a> {
ContextualParseError::UnsupportedRule(s, err) | ContextualParseError::UnsupportedRule(s, err) |
ContextualParseError::UnsupportedViewportDescriptorDeclaration(s, err) | ContextualParseError::UnsupportedViewportDescriptorDeclaration(s, err) |
ContextualParseError::UnsupportedCounterStyleDescriptorDeclaration(s, err) | ContextualParseError::UnsupportedCounterStyleDescriptorDeclaration(s, err) |
ContextualParseError::InvalidMediaRule(s, err) => { ContextualParseError::InvalidMediaRule(s, err) |
ContextualParseError::UnsupportedValue(s, err) => {
(s.into(), err.kind) (s.into(), err.kind)
} }
ContextualParseError::InvalidCounterStyleWithoutSymbols(s) | ContextualParseError::InvalidCounterStyleWithoutSymbols(s) |
@ -359,16 +364,30 @@ impl<'a> ErrorHelpers<'a> for ContextualParseError<'a> {
ContextualParseError::UnsupportedFontFeatureValuesDescriptor(..) | ContextualParseError::UnsupportedFontFeatureValuesDescriptor(..) |
ContextualParseError::InvalidFontFeatureValuesRule(..) => ContextualParseError::InvalidFontFeatureValuesRule(..) =>
(b"PEUnknownAtRule\0", Action::Skip), (b"PEUnknownAtRule\0", Action::Skip),
ContextualParseError::UnsupportedValue(_, ParseError { ref kind, .. }) => {
match *kind {
ParseErrorKind::Custom(
StyleParseErrorKind::ValueError(
ValueParseErrorKind::InvalidColor(..)
)
) => (b"PEColorNotColor", Action::Nothing),
_ => {
// Not the best error message, since we weren't parsing
// a declaration, just a value. But we don't produce
// UnsupportedValue errors other than InvalidColors
// currently.
debug_assert!(false, "should use a more specific error message");
(b"PEDeclDropped", Action::Nothing)
}
}
}
}; };
(None, msg, action) (None, msg, action)
} }
} }
impl ParseErrorReporter for ErrorReporter { impl ErrorReporter {
fn report_error(&self, pub fn report(&self, location: SourceLocation, error: ContextualParseError) {
_url: &UrlExtraData,
location: SourceLocation,
error: ContextualParseError) {
let (pre, name, action) = error.to_gecko_message(); let (pre, name, action) = error.to_gecko_message();
let suffix = match action { let suffix = match action {
Action::Nothing => ptr::null(), Action::Nothing => ptr::null(),
@ -400,3 +419,14 @@ impl ParseErrorReporter for ErrorReporter {
} }
} }
} }
impl ParseErrorReporter for ErrorReporter {
fn report_error(
&self,
_url: &UrlExtraData,
location: SourceLocation,
error: ContextualParseError
) {
self.report(location, error)
}
}

View file

@ -2,7 +2,7 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this * License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
use cssparser::{Parser, ParserInput}; use cssparser::{ParseErrorKind, Parser, ParserInput};
use cssparser::ToCss as ParserToCss; use cssparser::ToCss as ParserToCss;
use env_logger::LogBuilder; use env_logger::LogBuilder;
use malloc_size_of::MallocSizeOfOps; use malloc_size_of::MallocSizeOfOps;
@ -23,7 +23,7 @@ use style::data::{ElementStyles, self};
use style::dom::{ShowSubtreeData, TDocument, TElement, TNode}; use style::dom::{ShowSubtreeData, TDocument, TElement, TNode};
use style::driver; use style::driver;
use style::element_state::{DocumentState, ElementState}; use style::element_state::{DocumentState, ElementState};
use style::error_reporting::{NullReporter, ParseErrorReporter}; use style::error_reporting::{ContextualParseError, NullReporter, ParseErrorReporter};
use style::font_metrics::{FontMetricsProvider, get_metrics_provider_for_product}; use style::font_metrics::{FontMetricsProvider, get_metrics_provider_for_product};
use style::gecko::data::{GeckoStyleSheet, PerDocumentStyleData, PerDocumentStyleDataImpl}; use style::gecko::data::{GeckoStyleSheet, PerDocumentStyleData, PerDocumentStyleDataImpl};
use style::gecko::global_style_data::{GLOBAL_STYLE_DATA, GlobalStyleData, STYLE_THREAD_POOL}; use style::gecko::global_style_data::{GLOBAL_STYLE_DATA, GlobalStyleData, STYLE_THREAD_POOL};
@ -153,7 +153,7 @@ use style::values::distance::ComputeSquaredDistance;
use style::values::specified; use style::values::specified;
use style::values::specified::gecko::IntersectionObserverRootMargin; use style::values::specified::gecko::IntersectionObserverRootMargin;
use style::values::specified::source_size_list::SourceSizeList; use style::values::specified::source_size_list::SourceSizeList;
use style_traits::{ParsingMode, ToCss}; use style_traits::{ParsingMode, StyleParseErrorKind, ToCss};
use super::error_reporter::ErrorReporter; use super::error_reporter::ErrorReporter;
use super::stylesheet_loader::StylesheetLoader; use super::stylesheet_loader::StylesheetLoader;
@ -4557,10 +4557,31 @@ pub unsafe extern "C" fn Servo_SelectorList_Drop(list: RawServoSelectorListOwned
let _ = list.into_box::<::selectors::SelectorList<SelectorImpl>>(); let _ = list.into_box::<::selectors::SelectorList<SelectorImpl>>();
} }
fn parse_color(value: &str) -> Result<specified::Color, ()> { fn parse_color(
value: &str,
error_reporter: Option<&ErrorReporter>,
) -> Result<specified::Color, ()> {
let mut input = ParserInput::new(value); let mut input = ParserInput::new(value);
let mut parser = Parser::new(&mut input); let mut parser = Parser::new(&mut input);
parser.parse_entirely(specified::Color::parse_color).map_err(|_| ()) let start_position = parser.position();
parser.parse_entirely(specified::Color::parse_color).map_err(|err| {
if let Some(error_reporter) = error_reporter {
match err.kind {
ParseErrorKind::Custom(StyleParseErrorKind::ValueError(..)) => {
let location = err.location.clone();
let error = ContextualParseError::UnsupportedValue(
parser.slice_from(start_position),
err,
);
error_reporter.report(location, error);
}
// Ignore other kinds of errors that might be reported, such as
// ParseErrorKind::Basic(BasicParseErrorKind::UnexpectedToken),
// since Gecko doesn't report those to the error console.
_ => {}
}
}
})
} }
#[no_mangle] #[no_mangle]
@ -4568,7 +4589,7 @@ pub extern "C" fn Servo_IsValidCSSColor(
value: *const nsAString, value: *const nsAString,
) -> bool { ) -> bool {
let value = unsafe { (*value).to_string() }; let value = unsafe { (*value).to_string() };
parse_color(&value).is_ok() parse_color(&value, None).is_ok()
} }
#[no_mangle] #[no_mangle]
@ -4577,6 +4598,8 @@ pub extern "C" fn Servo_ComputeColor(
current_color: structs::nscolor, current_color: structs::nscolor,
value: *const nsAString, value: *const nsAString,
result_color: *mut structs::nscolor, result_color: *mut structs::nscolor,
was_current_color: *mut bool,
loader: *mut Loader,
) -> bool { ) -> bool {
use style::gecko; use style::gecko;
@ -4584,7 +4607,12 @@ pub extern "C" fn Servo_ComputeColor(
let value = unsafe { (*value).to_string() }; let value = unsafe { (*value).to_string() };
let result_color = unsafe { result_color.as_mut().unwrap() }; let result_color = unsafe { result_color.as_mut().unwrap() };
match parse_color(&value) { let reporter = unsafe { loader.as_mut() }.map(|loader| {
// Make an ErrorReporter that will report errors as being "from DOM".
ErrorReporter::new(ptr::null_mut(), loader, ptr::null_mut())
});
match parse_color(&value, reporter.as_ref()) {
Ok(specified_color) => { Ok(specified_color) => {
let computed_color = match raw_data { let computed_color = match raw_data {
Some(raw_data) => { Some(raw_data) => {
@ -4611,6 +4639,11 @@ pub extern "C" fn Servo_ComputeColor(
Some(computed_color) => { Some(computed_color) => {
let rgba = computed_color.to_rgba(current_color); let rgba = computed_color.to_rgba(current_color);
*result_color = gecko::values::convert_rgba_to_nscolor(&rgba); *result_color = gecko::values::convert_rgba_to_nscolor(&rgba);
if !was_current_color.is_null() {
unsafe {
*was_current_color = computed_color.is_currentcolor();
}
}
true true
} }
None => false, None => false,