Auto merge of #16585 - servo:log, r=emilio

Don’t log CSS parsing errors in user-agent stylesheets

This used to make `RUST_LOG=style` basically useless.

<!-- 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/16585)
<!-- Reviewable:end -->

components/layout_thread/lib.rs

@@ -110,7 +110,7 @@ use style::context::{QuirksMode, ReflowGoal, SharedStyleContext};
 use style::context::{StyleSystemOptions, ThreadLocalStyleContextCreationInfo};
 use style::data::StoredRestyleHint;
 use style::dom::{ShowSubtree, ShowSubtreeDataAndPrimaryValues, TElement, TNode};
-use style::error_reporting::StdoutErrorReporter;
+use style::error_reporting::{NullReporter, RustLogReporter};
 use style::logical_geometry::LogicalPoint;
 use style::media_queries::{Device, MediaList, MediaType};
 use style::servo::restyle_damage::{REFLOW, REFLOW_OUT_OF_FLOW, REPAINT, REPOSITION, STORE_OVERFLOW};
@@ -1587,7 +1587,7 @@ fn get_ua_stylesheets() -> Result<UserAgentStylesheets, &'static str> {
             MediaList::empty(),
             shared_lock.clone(),
             None,
-            &StdoutErrorReporter))
+            &NullReporter))
     }
 
     let shared_lock = SharedRwLock::new();
@@ -1600,7 +1600,7 @@ fn get_ua_stylesheets() -> Result<UserAgentStylesheets, &'static str> {
     for &(ref contents, ref url) in &opts::get().user_stylesheets {
         user_or_user_agent_stylesheets.push(Stylesheet::from_bytes(
             &contents, url.clone(), None, None, Origin::User, MediaList::empty(),
-            shared_lock.clone(), None, &StdoutErrorReporter));
+            shared_lock.clone(), None, &RustLogReporter));
     }
 
     let quirks_mode_stylesheet = try!(parse_ua_stylesheet(&shared_lock, "quirks-mode.css"));

components/style/error_reporting.rs

@@ -24,11 +24,15 @@ pub trait ParseErrorReporter : Sync + Send {
                     line_number_offset: u64);
 }
 
-/// An error reporter that reports the errors to the `info` log channel.
+/// An error reporter that uses [the `log` crate](https://github.com/rust-lang-nursery/log)
+/// at `info` level.
 ///
-/// TODO(emilio): The name of this reporter is a lie, and should be renamed!
+/// This logging is silent by default, and can be enabled with a `RUST_LOG=style=info`
-pub struct StdoutErrorReporter;
+/// environment variable.
-impl ParseErrorReporter for StdoutErrorReporter {
+/// (See [`env_logger`](https://rust-lang-nursery.github.io/log/env_logger/).)
+pub struct RustLogReporter;
+
+impl ParseErrorReporter for RustLogReporter {
     fn report_error(&self,
                     input: &mut Parser,
                     position: SourcePosition,
@@ -42,3 +46,17 @@ impl ParseErrorReporter for StdoutErrorReporter {
         }
     }
 }
+
+/// Error reporter which silently forgets errors
+pub struct NullReporter;
+
+impl ParseErrorReporter for NullReporter {
+    fn report_error(&self,
+            _: &mut Parser,
+            _: SourcePosition,
+            _: &str,
+            _: &UrlExtraData,
+            _: u64) {
+        // do nothing
+    }
+}

components/style/gecko/wrapper.rs

@@ -21,7 +21,7 @@ use data::ElementData;
 use dom::{self, AnimationRules, DescendantsBit, LayoutIterator, NodeInfo, TElement, TNode, UnsafeNode};
 use dom::{OpaqueNode, PresentationalHintsSynthetizer};
 use element_state::ElementState;
-use error_reporting::StdoutErrorReporter;
+use error_reporting::RustLogReporter;
 use font_metrics::{FontMetricsProvider, FontMetricsQueryResult};
 use gecko::global_style_data::GLOBAL_STYLE_DATA;
 use gecko::selector_parser::{SelectorImpl, NonTSPseudoClass, PseudoElement};
@@ -324,7 +324,7 @@ impl<'le> GeckoElement<'le> {
     /// Parse the style attribute of an element.
     pub fn parse_style_attribute(value: &str,
                                  url_data: &UrlExtraData) -> PropertyDeclarationBlock {
-        parse_style_attribute(value, url_data, &StdoutErrorReporter)
+        parse_style_attribute(value, url_data, &RustLogReporter)
     }
 
     fn flags(&self) -> u32 {

components/style/keyframes.rs

@@ -8,6 +8,7 @@
 
 use cssparser::{AtRuleParser, Parser, QualifiedRuleParser, RuleListParser};
 use cssparser::{DeclarationListParser, DeclarationParser, parse_one_rule};
+use error_reporting::NullReporter;
 use parser::{LengthParsingMode, ParserContext, log_css_error};
 use properties::{Importance, PropertyDeclaration, PropertyDeclarationBlock, PropertyId};
 use properties::{PropertyDeclarationId, LonghandId, ParsedDeclaration};
@@ -18,7 +19,7 @@ use shared_lock::{SharedRwLock, SharedRwLockReadGuard, Locked, ToCssWithGuard};
 use std::fmt;
 use std::sync::Arc;
 use style_traits::ToCss;
-use stylesheets::{CssRuleType, MemoryHoleReporter, Stylesheet, VendorPrefix};
+use stylesheets::{CssRuleType, Stylesheet, VendorPrefix};
 
 /// A number from 0 to 1, indicating the percentage of the animation when this
 /// keyframe should run.
@@ -125,7 +126,7 @@ impl Keyframe {
     /// Parse a CSS keyframe.
     pub fn parse(css: &str, parent_stylesheet: &Stylesheet)
                  -> Result<Arc<Locked<Self>>, ()> {
-        let error_reporter = MemoryHoleReporter;
+        let error_reporter = NullReporter;
         let context = ParserContext::new(parent_stylesheet.origin,
                                          &parent_stylesheet.url_data,
                                          &error_reporter,

components/style/properties/helpers/animated_properties.mako.rs

@@ -413,7 +413,7 @@ impl AnimationValue {
     /// Construct an AnimationValue from a property declaration
     pub fn from_declaration(decl: &PropertyDeclaration, context: &mut Context,
                             initial: &ComputedValues) -> Option<Self> {
-        use error_reporting::StdoutErrorReporter;
+        use error_reporting::RustLogReporter;
         use properties::LonghandId;
         use properties::DeclaredValue;
 
@@ -467,7 +467,7 @@ impl AnimationValue {
             },
             PropertyDeclaration::WithVariables(id, ref variables) => {
                 let custom_props = context.style().custom_properties();
-                let reporter = StdoutErrorReporter;
+                let reporter = RustLogReporter;
                 match id {
                     % for prop in data.longhands:
                     % if prop.animatable:

components/style/stylesheets.rs

@@ -8,9 +8,9 @@
 
 use {Atom, Prefix, Namespace};
 use cssparser::{AtRuleParser, Parser, QualifiedRuleParser};
-use cssparser::{AtRuleType, RuleListParser, SourcePosition, Token, parse_one_rule};
+use cssparser::{AtRuleType, RuleListParser, Token, parse_one_rule};
 use cssparser::ToCss as ParserToCss;
-use error_reporting::ParseErrorReporter;
+use error_reporting::{ParseErrorReporter, NullReporter};
 #[cfg(feature = "servo")]
 use font_face::FontFaceRuleData;
 use font_face::parse_font_face_block;
@@ -321,20 +321,6 @@ pub enum CssRuleType {
     Viewport            = 15,
 }
 
-/// Error reporter which silently forgets errors
-pub struct MemoryHoleReporter;
-
-impl ParseErrorReporter for MemoryHoleReporter {
-    fn report_error(&self,
-            _: &mut Parser,
-            _: SourcePosition,
-            _: &str,
-            _: &UrlExtraData,
-            _: u64) {
-        // do nothing
-    }
-}
-
 #[allow(missing_docs)]
 pub enum SingleRuleParseError {
     Syntax,
@@ -418,7 +404,7 @@ impl CssRule {
                  state: Option<State>,
                  loader: Option<&StylesheetLoader>)
                  -> Result<(Self, State), SingleRuleParseError> {
-        let error_reporter = MemoryHoleReporter;
+        let error_reporter = NullReporter;
         let mut namespaces = parent_stylesheet.namespaces.write();
         let context = ParserContext::new(parent_stylesheet.origin,
                                          &parent_stylesheet.url_data,

components/style/stylist.rs

@@ -10,7 +10,7 @@ use {Atom, LocalName};
 use bit_vec::BitVec;
 use data::ComputedStyle;
 use dom::{AnimationRules, PresentationalHintsSynthetizer, TElement};
-use error_reporting::StdoutErrorReporter;
+use error_reporting::RustLogReporter;
 use font_metrics::FontMetricsProvider;
 use keyframes::KeyframesAnimation;
 use media_queries::Device;
@@ -415,7 +415,7 @@ impl Stylist {
                                 parent.map(|p| &**p),
                                 parent.map(|p| &**p),
                                 None,
-                                &StdoutErrorReporter,
+                                &RustLogReporter,
                                 font_metrics,
                                 cascade_flags);
         ComputedStyle::new(rule_node, Arc::new(computed))
@@ -533,7 +533,7 @@ impl Stylist {
                                 Some(&**parent),
                                 Some(&**parent),
                                 None,
-                                &StdoutErrorReporter,
+                                &RustLogReporter,
                                 font_metrics,
                                 CascadeFlags::empty());
 
@@ -863,7 +863,7 @@ impl Stylist {
                                      Some(parent_style),
                                      Some(parent_style),
                                      None,
-                                     &StdoutErrorReporter,
+                                     &RustLogReporter,
                                      &metrics,
                                      CascadeFlags::empty()))
     }

ports/geckolib/glue.rs

@@ -18,7 +18,7 @@ use style::context::{ThreadLocalStyleContext, ThreadLocalStyleContextCreationInf
 use style::data::{ElementData, ElementStyles, RestyleData};
 use style::dom::{AnimationOnlyDirtyDescendants, DirtyDescendants};
 use style::dom::{ShowSubtreeData, TElement, TNode};
-use style::error_reporting::StdoutErrorReporter;
+use style::error_reporting::RustLogReporter;
 use style::font_metrics::get_metrics_provider_for_product;
 use style::gecko::data::{PerDocumentStyleData, PerDocumentStyleDataImpl};
 use style::gecko::global_style_data::{GLOBAL_STYLE_DATA, GlobalStyleData};
@@ -157,7 +157,7 @@ fn create_shared_context<'a>(global_style_data: &GlobalStyleData,
         running_animations: per_doc_data.running_animations.clone(),
         expired_animations: per_doc_data.expired_animations.clone(),
         // FIXME(emilio): Stop boxing here.
-        error_reporter: Box::new(StdoutErrorReporter),
+        error_reporter: Box::new(RustLogReporter),
         local_context_creation_data: Mutex::new(local_context_data),
         timer: Timer::new(),
         // FIXME Find the real QuirksMode information for this document
@@ -511,7 +511,7 @@ pub extern "C" fn Servo_StyleSheet_Empty(mode: SheetParsingMode) -> RawServoStyl
     Arc::new(Stylesheet::from_str(
         "", unsafe { dummy_url_data() }.clone(), origin,
         Arc::new(shared_lock.wrap(MediaList::empty())),
-        shared_lock, None, &StdoutErrorReporter, 0u64)
+        shared_lock, None, &RustLogReporter, 0u64)
     ).into_strong()
 }
 
@@ -554,7 +554,7 @@ pub extern "C" fn Servo_StyleSheet_FromUTF8Bytes(loader: *mut Loader,
 
     Arc::new(Stylesheet::from_str(
         input, url_data.clone(), origin, media,
-        shared_lock, loader, &StdoutErrorReporter, 0u64)
+        shared_lock, loader, &RustLogReporter, 0u64)
     ).into_strong()
 }
 
@@ -582,7 +582,7 @@ pub extern "C" fn Servo_StyleSheet_ClearAndUpdate(stylesheet: RawServoStyleSheet
 
     let sheet = Stylesheet::as_arc(&stylesheet);
     Stylesheet::update_from_str(&sheet, input, url_data,
-                                loader, &StdoutErrorReporter);
+                                loader, &RustLogReporter);
 }
 
 #[no_mangle]
@@ -1005,7 +1005,7 @@ pub extern "C" fn Servo_ParseProperty(property: nsCSSPropertyID, value: *const n
     let value = unsafe { value.as_ref().unwrap().as_str_unchecked() };
 
     let url_data = unsafe { RefPtr::from_ptr_ref(&data) };
-    let reporter = StdoutErrorReporter;
+    let reporter = RustLogReporter;
     let context = ParserContext::new(Origin::Author, url_data, &reporter,
                                      Some(CssRuleType::Style), LengthParsingMode::Default);
 
@@ -1028,7 +1028,7 @@ pub extern "C" fn Servo_ParseEasing(easing: *const nsAString,
     use style::properties::longhands::transition_timing_function;
 
     let url_data = unsafe { RefPtr::from_ptr_ref(&data) };
-    let reporter = StdoutErrorReporter;
+    let reporter = RustLogReporter;
     let context = ParserContext::new(Origin::Author, url_data, &reporter,
                                      Some(CssRuleType::Style), LengthParsingMode::Default);
     let easing = unsafe { (*easing).to_string() };
@@ -1170,7 +1170,7 @@ fn set_property(declarations: RawServoDeclarationBlockBorrowed, property_id: Pro
         structs::LengthParsingMode::Default => LengthParsingMode::Default,
         structs::LengthParsingMode::SVG => LengthParsingMode::SVG,
     };
-    if let Ok(parsed) = parse_one_declaration(property_id, value, url_data, &StdoutErrorReporter,
+    if let Ok(parsed) = parse_one_declaration(property_id, value, url_data, &RustLogReporter,
                                               length_parsing_mode) {
         let importance = if is_important { Importance::Important } else { Importance::Normal };
         write_locked_arc(declarations, |decls: &mut PropertyDeclarationBlock| {
@@ -1263,7 +1263,7 @@ pub extern "C" fn Servo_MediaList_SetText(list: RawServoMediaListBorrowed, text:
     let text = unsafe { text.as_ref().unwrap().as_str_unchecked() };
     let mut parser = Parser::new(&text);
     let url_data = unsafe { dummy_url_data() };
-    let reporter = StdoutErrorReporter;
+    let reporter = RustLogReporter;
     let context = ParserContext::new_for_cssom(url_data, &reporter, Some(CssRuleType::Media),
                                                LengthParsingMode::Default);
      write_locked_arc(list, |list: &mut MediaList| {
@@ -1294,7 +1294,7 @@ pub extern "C" fn Servo_MediaList_AppendMedium(list: RawServoMediaListBorrowed,
                                                new_medium: *const nsACString) {
     let new_medium = unsafe { new_medium.as_ref().unwrap().as_str_unchecked() };
     let url_data = unsafe { dummy_url_data() };
-    let reporter = StdoutErrorReporter;
+    let reporter = RustLogReporter;
     let context = ParserContext::new_for_cssom(url_data, &reporter, Some(CssRuleType::Media),
                                                LengthParsingMode::Default);
     write_locked_arc(list, |list: &mut MediaList| {
@@ -1307,7 +1307,7 @@ pub extern "C" fn Servo_MediaList_DeleteMedium(list: RawServoMediaListBorrowed,
                                                old_medium: *const nsACString) -> bool {
     let old_medium = unsafe { old_medium.as_ref().unwrap().as_str_unchecked() };
     let url_data = unsafe { dummy_url_data() };
-    let reporter = StdoutErrorReporter;
+    let reporter = RustLogReporter;
     let context = ParserContext::new_for_cssom(url_data, &reporter, Some(CssRuleType::Media),
                                                LengthParsingMode::Default);
     write_locked_arc(list, |list: &mut MediaList| list.delete_medium(&context, old_medium))
@@ -1659,7 +1659,7 @@ pub extern "C" fn Servo_DeclarationBlock_SetBackgroundImage(declarations:
 
     let url_data = unsafe { RefPtr::from_ptr_ref(&raw_extra_data) };
     let string = unsafe { (*value).to_string() };
-    let error_reporter = StdoutErrorReporter;
+    let error_reporter = RustLogReporter;
     let context = ParserContext::new(Origin::Author, url_data, &error_reporter,
                                      Some(CssRuleType::Style), LengthParsingMode::Default);
     if let Ok(url) = SpecifiedUrl::parse_from_string(string.into(), &context) {
@@ -1699,7 +1699,7 @@ pub extern "C" fn Servo_CSSSupports2(property: *const nsACString, value: *const
     let value = unsafe { value.as_ref().unwrap().as_str_unchecked() };
 
     let url_data = unsafe { dummy_url_data() };
-    parse_one_declaration(id, &value, url_data, &StdoutErrorReporter, LengthParsingMode::Default).is_ok()
+    parse_one_declaration(id, &value, url_data, &RustLogReporter, LengthParsingMode::Default).is_ok()
 }
 
 #[no_mangle]
@@ -1709,7 +1709,7 @@ pub extern "C" fn Servo_CSSSupports(cond: *const nsACString) -> bool {
     let cond = parse_condition_or_declaration(&mut input);
     if let Ok(cond) = cond {
         let url_data = unsafe { dummy_url_data() };
-        let reporter = StdoutErrorReporter;
+        let reporter = RustLogReporter;
         let context = ParserContext::new_for_cssom(url_data, &reporter, Some(CssRuleType::Style),
                                                    LengthParsingMode::Default);
         cond.eval(&context)