allow Gecko FFI functions for color parsing to report errors

components/style/error_reporting.rs

@@ -46,6 +46,8 @@ pub enum ContextualParseError<'a> {
     InvalidCounterStyleExtendsWithAdditiveSymbols,
     /// A media rule was invalid for some reason.
     InvalidMediaRule(&'a str, ParseError<'a>),
+    /// A value was not recognized.
+    UnsupportedValue(&'a str, ParseError<'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)?;
                 parse_error_to_str(err, f)
             }
+            ContextualParseError::UnsupportedValue(_value, ref err) => {
+                parse_error_to_str(err, f)
+            }
         }
     }
 }

components/style/gecko/generated/bindings.rs

@@ -1577,7 +1577,7 @@ extern "C" {
 } extern "C" {
  pub fn Servo_IsValidCSSColor ( value : * const nsAString , ) -> bool ; 
 } 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" {
  pub fn Servo_ParseIntersectionObserverRootMargin ( value : * const nsAString , result : * mut nsCSSRect , ) -> bool ; 
 } extern "C" {

ports/geckolib/error_reporter.rs

@@ -16,9 +16,8 @@ use style::gecko_bindings::bindings::Gecko_ReportUnexpectedCSSError;
 use style::gecko_bindings::structs::{Loader, ServoStyleSheet, nsIURI};
 use style::gecko_bindings::structs::ErrorReporter as GeckoErrorReporter;
 use style::gecko_bindings::structs::URLExtraData as RawUrlExtraData;
-use style::gecko_bindings::sugar::refptr::RefPtr;
 use style::stylesheets::UrlExtraData;
-use style_traits::StyleParseErrorKind;
+use style_traits::{StyleParseErrorKind, ValueParseErrorKind};
 
 pub type ErrorKind<'i> = ParseErrorKind<'i, StyleParseErrorKind<'i>>;
 
@@ -29,10 +28,12 @@ impl ErrorReporter {
     /// Create a new instance of the Gecko error reporter.
     pub fn new(sheet: *mut ServoStyleSheet,
                loader: *mut Loader,
-               url: *mut RawUrlExtraData) -> ErrorReporter {
+               extra_data: *mut RawUrlExtraData) -> ErrorReporter {
         unsafe {
-            let url = RefPtr::from_ptr_ref(&url);
-            ErrorReporter(Gecko_CreateCSSErrorReporter(sheet, loader, url.mBaseURI.raw::<nsIURI>()))
+            let url = extra_data.as_ref()
+                .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(
             StyleParseErrorKind::ExpectedIdentifier(token)
+        ) |
+        ParseErrorKind::Custom(
+            StyleParseErrorKind::ValueError(ValueParseErrorKind::InvalidColor(token))
         ) => {
             (Some(ErrorString::UnexpectedToken(token)), None)
         }
@@ -197,7 +201,8 @@ impl<'a> ErrorHelpers<'a> for ContextualParseError<'a> {
             ContextualParseError::UnsupportedRule(s, err) |
             ContextualParseError::UnsupportedViewportDescriptorDeclaration(s, err) |
             ContextualParseError::UnsupportedCounterStyleDescriptorDeclaration(s, err) |
-            ContextualParseError::InvalidMediaRule(s, err) => {
+            ContextualParseError::InvalidMediaRule(s, err) |
+            ContextualParseError::UnsupportedValue(s, err) => {
                 (s.into(), err.kind)
             }
             ContextualParseError::InvalidCounterStyleWithoutSymbols(s) |
@@ -359,16 +364,30 @@ impl<'a> ErrorHelpers<'a> for ContextualParseError<'a> {
             ContextualParseError::UnsupportedFontFeatureValuesDescriptor(..) |
             ContextualParseError::InvalidFontFeatureValuesRule(..) =>
                 (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)
     }
 }
 
-impl ParseErrorReporter for ErrorReporter {
-    fn report_error(&self,
-                    _url: &UrlExtraData,
-                    location: SourceLocation,
-                    error: ContextualParseError) {
+impl ErrorReporter {
+    pub fn report(&self, location: SourceLocation, error: ContextualParseError) {
         let (pre, name, action) = error.to_gecko_message();
         let suffix = match action {
             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)
+    }
+}

ports/geckolib/glue.rs

@@ -2,7 +2,7 @@
  * 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/. */
 
-use cssparser::{Parser, ParserInput};
+use cssparser::{ParseErrorKind, Parser, ParserInput};
 use cssparser::ToCss as ParserToCss;
 use env_logger::LogBuilder;
 use malloc_size_of::MallocSizeOfOps;
@@ -23,7 +23,7 @@ use style::data::{ElementStyles, self};
 use style::dom::{ShowSubtreeData, TDocument, TElement, TNode};
 use style::driver;
 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::gecko::data::{GeckoStyleSheet, PerDocumentStyleData, PerDocumentStyleDataImpl};
 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::gecko::IntersectionObserverRootMargin;
 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::stylesheet_loader::StylesheetLoader;
 
@@ -4557,18 +4557,39 @@ pub unsafe extern "C" fn Servo_SelectorList_Drop(list: RawServoSelectorListOwned
     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 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]
 pub extern "C" fn Servo_IsValidCSSColor(
     value: *const nsAString,
 ) -> bool {
     let value = unsafe { (*value).to_string() };
-    parse_color(&value).is_ok()
+    parse_color(&value, None).is_ok()
 }
 
 #[no_mangle]
@@ -4577,14 +4598,21 @@ pub extern "C" fn Servo_ComputeColor(
     current_color: structs::nscolor,
     value: *const nsAString,
     result_color: *mut structs::nscolor,
+    was_current_color: *mut bool,
+    loader: *mut Loader,
 ) -> bool {
     use style::gecko;
 
     let current_color = gecko::values::convert_nscolor_to_rgba(current_color);
     let value = unsafe { (*value).to_string() };
     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) => {
             let computed_color = match raw_data {
                 Some(raw_data) => {
@@ -4611,6 +4639,11 @@ pub extern "C" fn Servo_ComputeColor(
                 Some(computed_color) => {
                     let rgba = computed_color.to_rgba(current_color);
                     *result_color = gecko::values::convert_rgba_to_nscolor(&rgba);
+                    if !was_current_color.is_null() {
+                        unsafe {
+                            *was_current_color = computed_color.is_currentcolor();
+                        }
+                    }
                     true
                 }
                 None => false,