From de1fe51dc64935633e2e0c19edc3d50b9aa29858 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 24 Apr 2017 07:56:50 +0200 Subject: [PATCH 1/3] Rename MemoryHoleReporter to NullReporter --- components/style/error_reporting.rs | 14 ++++++++++++++ components/style/keyframes.rs | 5 +++-- components/style/stylesheets.rs | 18 ++---------------- 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/components/style/error_reporting.rs b/components/style/error_reporting.rs index fb41a777e31..9ad89a54cc5 100644 --- a/components/style/error_reporting.rs +++ b/components/style/error_reporting.rs @@ -42,3 +42,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 + } +} diff --git a/components/style/keyframes.rs b/components/style/keyframes.rs index 542481b287c..d7e3485fc7d 100644 --- a/components/style/keyframes.rs +++ b/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>, ()> { - let error_reporter = MemoryHoleReporter; + let error_reporter = NullReporter; let context = ParserContext::new(parent_stylesheet.origin, &parent_stylesheet.url_data, &error_reporter, diff --git a/components/style/stylesheets.rs b/components/style/stylesheets.rs index 8d2c59744b5..bb08dd247d1 100644 --- a/components/style/stylesheets.rs +++ b/components/style/stylesheets.rs @@ -10,7 +10,7 @@ use {Atom, Prefix, Namespace}; use cssparser::{AtRuleParser, Parser, QualifiedRuleParser}; use cssparser::{AtRuleType, RuleListParser, SourcePosition, 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, 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, From 7fe57ecaea3fc039391dae57364e6808afacb500 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 24 Apr 2017 08:00:20 +0200 Subject: [PATCH 2/3] Rename StdoutErrorReporter to RustLogReporter. --- components/layout_thread/lib.rs | 6 ++-- components/style/error_reporting.rs | 12 +++++--- components/style/gecko/wrapper.rs | 4 +-- .../helpers/animated_properties.mako.rs | 4 +-- components/style/stylesheets.rs | 2 +- components/style/stylist.rs | 8 +++--- ports/geckolib/glue.rs | 28 +++++++++---------- 7 files changed, 34 insertions(+), 30 deletions(-) diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index c7f3471753a..a4c67f606eb 100644 --- a/components/layout_thread/lib.rs +++ b/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::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 { MediaList::empty(), shared_lock.clone(), None, - &StdoutErrorReporter)) + &RustLogReporter)) } let shared_lock = SharedRwLock::new(); @@ -1600,7 +1600,7 @@ fn get_ua_stylesheets() -> Result { 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")); diff --git a/components/style/error_reporting.rs b/components/style/error_reporting.rs index 9ad89a54cc5..a7458ecd794 100644 --- a/components/style/error_reporting.rs +++ b/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! -pub struct StdoutErrorReporter; -impl ParseErrorReporter for StdoutErrorReporter { +/// This logging is silent by default, and can be enabled with a `RUST_LOG=style=info` +/// environment variable. +/// (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, diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index bde69979ba8..6c72aa0b3d5 100644 --- a/components/style/gecko/wrapper.rs +++ b/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 { diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index 2816db15a36..07279afe909 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/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 { - 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: diff --git a/components/style/stylesheets.rs b/components/style/stylesheets.rs index bb08dd247d1..b5ac86188d6 100644 --- a/components/style/stylesheets.rs +++ b/components/style/stylesheets.rs @@ -8,7 +8,7 @@ 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, NullReporter}; #[cfg(feature = "servo")] diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 8b698ab692d..38dbe24b4c1 100644 --- a/components/style/stylist.rs +++ b/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())) } diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index fc28f1cedf5..01dd54823d5 100644 --- a/ports/geckolib/glue.rs +++ b/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) From 512944c32cb07581161c485c87f48618565bc843 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 24 Apr 2017 08:27:26 +0200 Subject: [PATCH 3/3] =?UTF-8?q?Don=E2=80=99t=20report=20CSS=20parsing=20er?= =?UTF-8?q?rors=20from=20user-agent=20stylesheets.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This used to make `RUST_LOG=style` basically useless. --- components/layout_thread/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index a4c67f606eb..c17d3bd9b48 100644 --- a/components/layout_thread/lib.rs +++ b/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::RustLogReporter; +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 { MediaList::empty(), shared_lock.clone(), None, - &RustLogReporter)) + &NullReporter)) } let shared_lock = SharedRwLock::new();