From bff52ae4a1ea2b73ed4c57a51b3552f6ef30ea78 Mon Sep 17 00:00:00 2001 From: Mats Palmgren Date: Fri, 16 Aug 2019 06:30:24 +0000 Subject: [PATCH 01/24] style: Implement layout for 'display: flow-root list-item'. Differential Revision: https://phabricator.services.mozilla.com/D42087 --- components/style/values/specified/box.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/components/style/values/specified/box.rs b/components/style/values/specified/box.rs index 7030a669397..146704ce89e 100644 --- a/components/style/values/specified/box.rs +++ b/components/style/values/specified/box.rs @@ -688,6 +688,7 @@ impl SpecifiedValueInfo for Display { "contents", "flex", "flow-root", + "flow-root list-item", "grid", "inline", "inline-block", From 9404ac89b707d9f56028d66a5008324b8b8ac639 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 19 Aug 2019 19:59:48 +0000 Subject: [PATCH 02/24] style: Don't clamp font-size calc() factors too early. These two bugs (bug 1572738 and bug 1572451) are stylo regressions. When font-family changes, we try to recompute the font-size with a length / percentage combinations in case the generic family changes, so the user preferences are kept. When calc() is involved, we clamp to non-negative too early, via NonNegativeLength::scale_by. I think we should generally dump this "try to track font-size across calc()" thingie, as as various comments note it is not quite perfect, and it's not clear how it should work in presence of min()/max(). This patch fixes the issue and simplifies code a bit, I may consider removing this altogether in a follow-up. Differential Revision: https://phabricator.services.mozilla.com/D41776 --- components/style/properties/gecko.mako.rs | 6 +- .../style/properties/longhands/font.mako.rs | 2 +- components/style/values/computed/font.rs | 15 +- components/style/values/computed/length.rs | 14 +- components/style/values/computed/mod.rs | 4 +- components/style/values/generics/font.rs | 103 +---------- components/style/values/specified/font.rs | 171 ++++++++++++++---- components/style/values/specified/text.rs | 3 +- 8 files changed, 149 insertions(+), 169 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 1f3e862df49..bd02a7ac944 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -1146,7 +1146,7 @@ fn static_assert() { } pub fn set_font_size(&mut self, v: FontSize) { - use crate::values::generics::font::KeywordSize; + use crate::values::specified::font::KeywordSize; let size = v.size(); self.gecko.mScriptUnconstrainedSize = size.0; @@ -1167,7 +1167,7 @@ fn static_assert() { KeywordSize::XXXLarge => structs::NS_STYLE_FONT_SIZE_XXXLARGE, } as u8; self.gecko.mFontSizeFactor = info.factor; - self.gecko.mFontSizeOffset = info.offset.0.to_i32_au(); + self.gecko.mFontSizeOffset = info.offset.to_i32_au(); } else { self.gecko.mFontSizeKeyword = structs::NS_STYLE_FONT_SIZE_NO_KEYWORD as u8; self.gecko.mFontSizeFactor = 1.; @@ -1176,7 +1176,7 @@ fn static_assert() { } pub fn clone_font_size(&self) -> FontSize { - use crate::values::generics::font::{KeywordInfo, KeywordSize}; + use crate::values::specified::font::{KeywordInfo, KeywordSize}; let size = Au(self.gecko.mSize).into(); let kw = match self.gecko.mFontSizeKeyword as u32 { structs::NS_STYLE_FONT_SIZE_XXSMALL => KeywordSize::XXSmall, diff --git a/components/style/properties/longhands/font.mako.rs b/components/style/properties/longhands/font.mako.rs index b8ce027be4e..148b6fe33f6 100644 --- a/components/style/properties/longhands/font.mako.rs +++ b/components/style/properties/longhands/font.mako.rs @@ -401,7 +401,7 @@ ${helpers.predefined_type( is_system_font: true, }, font_size: FontSize { - size: cx.maybe_zoom_text(Au(system.size).into()), + size: NonNegative(cx.maybe_zoom_text(Au(system.size).into())), keyword_info: None }, font_weight, diff --git a/components/style/values/computed/font.rs b/components/style/values/computed/font.rs index 407a547013f..0063a6946ba 100644 --- a/components/style/values/computed/font.rs +++ b/components/style/values/computed/font.rs @@ -9,11 +9,11 @@ use crate::gecko_bindings::sugar::refptr::RefPtr; #[cfg(feature = "gecko")] use crate::gecko_bindings::{bindings, structs}; use crate::values::animated::{ToAnimatedValue, ToAnimatedZero}; -use crate::values::computed::{Angle, Context, Integer, NonNegativeLength, NonNegativePercentage}; +use crate::values::computed::{Angle, Context, Integer, Length, NonNegativeLength, NonNegativePercentage}; use crate::values::computed::{Number, Percentage, ToComputedValue}; -use crate::values::generics::font as generics; +use crate::values::generics::{font as generics, NonNegative}; use crate::values::generics::font::{FeatureTagValue, FontSettings, VariationValue}; -use crate::values::specified::font::{self as specified, MAX_FONT_WEIGHT, MIN_FONT_WEIGHT}; +use crate::values::specified::font::{self as specified, KeywordInfo, MAX_FONT_WEIGHT, MIN_FONT_WEIGHT}; use crate::values::specified::length::{FontBaseSize, NoCalcLength}; use crate::values::CSSFloat; use crate::Atom; @@ -88,9 +88,6 @@ pub struct FontSize { pub keyword_info: Option, } -/// Additional information for computed keyword-derived font sizes. -pub type KeywordInfo = generics::KeywordInfo; - impl FontWeight { /// Value for normal pub fn normal() -> Self { @@ -162,17 +159,17 @@ impl FontSize { } impl ToAnimatedValue for FontSize { - type AnimatedValue = NonNegativeLength; + type AnimatedValue = Length; #[inline] fn to_animated_value(self) -> Self::AnimatedValue { - self.size + self.size.0 } #[inline] fn from_animated_value(animated: Self::AnimatedValue) -> Self { FontSize { - size: animated.clamp(), + size: NonNegative(animated.clamp_to_non_negative()), keyword_info: None, } } diff --git a/components/style/values/computed/length.rs b/components/style/values/computed/length.rs index 80b16dcecc9..88eb8f3955a 100644 --- a/components/style/values/computed/length.rs +++ b/components/style/values/computed/length.rs @@ -297,7 +297,7 @@ impl specified::CalcLengthPercentage { ) -> LengthPercentage { self.to_computed_value_with_zoom( context, - |abs| context.maybe_zoom_text(abs.into()).0, + |abs| context.maybe_zoom_text(abs.into()), base_size, ) } @@ -360,8 +360,8 @@ impl LengthPercentage { self.unclamped_length().px() == 0.0 && self.percentage.0 == 0.0 } - // CSSFloat doesn't implement Hash, so does CSSPixelLength. Therefore, we still use Au as the - // hash key. + // CSSFloat doesn't implement Hash, so does CSSPixelLength. Therefore, we + // still use Au as the hash key. #[allow(missing_docs)] pub fn to_hash_key(&self) -> (Au, NotNan) { ( @@ -840,14 +840,6 @@ impl NonNegativeLength { self } } - - /// Scale this NonNegativeLength. - /// We scale NonNegativeLength by zero if the factor is negative because it doesn't - /// make sense to scale a negative factor on a non-negative length. - #[inline] - pub fn scale_by(&self, factor: f32) -> Self { - Self::new(self.0.px() * factor.max(0.)) - } } impl From for NonNegativeLength { diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index bd78d376b35..d5a61746675 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -230,7 +230,7 @@ impl<'a> Context<'a> { /// Apply text-zoom if enabled. #[cfg(feature = "gecko")] - pub fn maybe_zoom_text(&self, size: NonNegativeLength) -> NonNegativeLength { + pub fn maybe_zoom_text(&self, size: CSSPixelLength) -> CSSPixelLength { // We disable zoom for by unsetting the // -x-text-zoom property, which leads to a false value // in mAllowZoom @@ -243,7 +243,7 @@ impl<'a> Context<'a> { /// (Servo doesn't do text-zoom) #[cfg(feature = "servo")] - pub fn maybe_zoom_text(&self, size: NonNegativeLength) -> NonNegativeLength { + pub fn maybe_zoom_text(&self, size: CSSPixelLength) -> CSSPixelLength { size } } diff --git a/components/style/values/generics/font.rs b/components/style/values/generics/font.rs index eb8eac191f0..2972fa01c68 100644 --- a/components/style/values/generics/font.rs +++ b/components/style/values/generics/font.rs @@ -5,14 +5,13 @@ //! Generic types for font stuff. use crate::parser::{Parse, ParserContext}; -use app_units::Au; use byteorder::{BigEndian, ReadBytesExt}; use cssparser::Parser; use num_traits::One; use std::fmt::{self, Write}; use std::io::Cursor; -use style_traits::{CssWriter, KeywordsCollectFn, ParseError}; -use style_traits::{SpecifiedValueInfo, StyleParseErrorKind, ToCss}; +use style_traits::{CssWriter, ParseError}; +use style_traits::{StyleParseErrorKind, ToCss}; /// https://drafts.csswg.org/css-fonts-4/#feature-tag-value #[derive( @@ -172,104 +171,6 @@ impl Parse for FontTag { } } -#[derive( - Animate, - Clone, - ComputeSquaredDistance, - Copy, - Debug, - MallocSizeOf, - PartialEq, - ToAnimatedValue, - ToAnimatedZero, - ToCss, - ToShmem, -)] -/// Additional information for keyword-derived font sizes. -pub struct KeywordInfo { - /// The keyword used - pub kw: KeywordSize, - /// A factor to be multiplied by the computed size of the keyword - #[css(skip)] - pub factor: f32, - /// An additional Au offset to add to the kw*factor in the case of calcs - #[css(skip)] - pub offset: Length, -} - -impl KeywordInfo -where - Au: Into, -{ - /// KeywordInfo value for font-size: medium - pub fn medium() -> Self { - KeywordSize::Medium.into() - } -} - -impl From for KeywordInfo -where - Au: Into, -{ - fn from(x: KeywordSize) -> Self { - KeywordInfo { - kw: x, - factor: 1., - offset: Au(0).into(), - } - } -} - -impl SpecifiedValueInfo for KeywordInfo { - fn collect_completion_keywords(f: KeywordsCollectFn) { - ::collect_completion_keywords(f); - } -} - -/// CSS font keywords -#[derive( - Animate, - Clone, - ComputeSquaredDistance, - Copy, - Debug, - MallocSizeOf, - Parse, - PartialEq, - SpecifiedValueInfo, - ToAnimatedValue, - ToAnimatedZero, - ToCss, - ToShmem, -)] -#[allow(missing_docs)] -pub enum KeywordSize { - #[css(keyword = "xx-small")] - XXSmall, - XSmall, - Small, - Medium, - Large, - XLarge, - #[css(keyword = "xx-large")] - XXLarge, - #[css(keyword = "xxx-large")] - XXXLarge, -} - -impl KeywordSize { - /// Convert to an HTML value - #[inline] - pub fn html_size(self) -> u8 { - self as u8 - } -} - -impl Default for KeywordSize { - fn default() -> Self { - KeywordSize::Medium - } -} /// A generic value for the `font-style` property. /// diff --git a/components/style/values/specified/font.rs b/components/style/values/specified/font.rs index 127b1bfc2fb..d42272e396b 100644 --- a/components/style/values/specified/font.rs +++ b/components/style/values/specified/font.rs @@ -11,9 +11,9 @@ use crate::properties::longhands::system_font::SystemFont; use crate::values::computed::font::{FamilyName, FontFamilyList, FontStyleAngle, SingleFontFamily}; use crate::values::computed::{font as computed, Length, NonNegativeLength}; use crate::values::computed::{Angle as ComputedAngle, Percentage as ComputedPercentage}; -use crate::values::computed::{Context, ToComputedValue}; +use crate::values::computed::{CSSPixelLength, Context, ToComputedValue}; use crate::values::generics::font::{self as generics, FeatureTagValue, FontSettings, FontTag}; -use crate::values::generics::font::{KeywordSize, VariationValue}; +use crate::values::generics::font::VariationValue; use crate::values::generics::NonNegative; use crate::values::specified::length::{FontBaseSize, AU_PER_PT, AU_PER_PX}; use crate::values::specified::{AllowQuirks, Angle, Integer, LengthPercentage}; @@ -481,6 +481,115 @@ impl ToComputedValue for FontStretch { } } +/// CSS font keywords +#[derive( + Animate, + Clone, + ComputeSquaredDistance, + Copy, + Debug, + MallocSizeOf, + Parse, + PartialEq, + SpecifiedValueInfo, + ToAnimatedValue, + ToAnimatedZero, + ToCss, + ToShmem, +)] +#[allow(missing_docs)] +pub enum KeywordSize { + #[css(keyword = "xx-small")] + XXSmall, + XSmall, + Small, + Medium, + Large, + XLarge, + #[css(keyword = "xx-large")] + XXLarge, + #[css(keyword = "xxx-large")] + XXXLarge, +} + +impl KeywordSize { + /// Convert to an HTML value + #[inline] + pub fn html_size(self) -> u8 { + self as u8 + } +} + +impl Default for KeywordSize { + fn default() -> Self { + KeywordSize::Medium + } +} + +#[derive( + Animate, + Clone, + ComputeSquaredDistance, + Copy, + Debug, + MallocSizeOf, + PartialEq, + ToAnimatedValue, + ToAnimatedZero, + ToCss, + ToShmem, +)] +/// Additional information for keyword-derived font sizes. +pub struct KeywordInfo { + /// The keyword used + pub kw: KeywordSize, + /// A factor to be multiplied by the computed size of the keyword + #[css(skip)] + pub factor: f32, + /// An additional fixed offset to add to the kw * factor in the case of + /// `calc()`. + #[css(skip)] + pub offset: CSSPixelLength, +} + +impl KeywordInfo { + /// KeywordInfo value for font-size: medium + pub fn medium() -> Self { + Self::new(KeywordSize::Medium) + } + + fn new(kw: KeywordSize) -> Self { + KeywordInfo { + kw, + factor: 1., + offset: CSSPixelLength::new(0.), + } + } + + /// Computes the final size for this font-size keyword, accounting for + /// text-zoom. + fn to_computed_value(&self, context: &Context) -> CSSPixelLength { + let base = context.maybe_zoom_text(self.kw.to_computed_value(context).0); + base * self.factor + context.maybe_zoom_text(self.offset) + } + + /// Given a parent keyword info (self), apply an additional factor/offset to + /// it. + pub fn compose(self, factor: f32, offset: CSSPixelLength) -> Self { + KeywordInfo { + kw: self.kw, + factor: self.factor * factor, + offset: self.offset * factor + offset, + } + } +} + +impl SpecifiedValueInfo for KeywordInfo { + fn collect_completion_keywords(f: KeywordsCollectFn) { + ::collect_completion_keywords(f); + } +} + #[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss, ToShmem)] /// A specified font-size value pub enum FontSize { @@ -652,27 +761,6 @@ impl ToComputedValue for FontSizeAdjust { } } -/// Additional information for specified keyword-derived font sizes. -pub type KeywordInfo = generics::KeywordInfo; - -impl KeywordInfo { - /// Computes the final size for this font-size keyword, accounting for - /// text-zoom. - pub fn to_computed_value(&self, context: &Context) -> NonNegativeLength { - let base = context.maybe_zoom_text(self.kw.to_computed_value(context)); - base.scale_by(self.factor) + context.maybe_zoom_text(self.offset) - } - - /// Given a parent keyword info (self), apply an additional factor/offset to it - pub fn compose(self, factor: f32, offset: NonNegativeLength) -> Self { - KeywordInfo { - kw: self.kw, - factor: self.factor * factor, - offset: self.offset.scale_by(factor) + offset, - } - } -} - /// This is the ratio applied for font-size: larger /// and smaller by both Firefox and Chrome const LARGER_FONT_SIZE_RATIO: f32 = 1.2; @@ -790,7 +878,7 @@ impl FontSize { /// pub fn from_html_size(size: u8) -> Self { FontSize::Keyword( - match size { + KeywordInfo::new(match size { // If value is less than 1, let it be 1. 0 | 1 => KeywordSize::XSmall, 2 => KeywordSize::Small, @@ -800,8 +888,7 @@ impl FontSize { 6 => KeywordSize::XXLarge, // If value is greater than 7, let it be 7. _ => KeywordSize::XXXLarge, - } - .into(), + }) ) } @@ -819,7 +906,7 @@ impl FontSize { .get_parent_font() .clone_font_size() .keyword_info - .map(|i| i.compose(factor, Au(0).into())) + .map(|i| i.compose(factor, CSSPixelLength::new(0.))) }; let mut info = None; let size = match *self { @@ -829,16 +916,16 @@ impl FontSize { // Tack the em unit onto the factor info = compose_keyword(em); } - value.to_computed_value(context, base_size).into() + value.to_computed_value(context, base_size) }, FontSize::Length(LengthPercentage::Length(NoCalcLength::ServoCharacterWidth( value, - ))) => value.to_computed_value(base_size.resolve(context)).into(), + ))) => value.to_computed_value(base_size.resolve(context)), FontSize::Length(LengthPercentage::Length(NoCalcLength::Absolute(ref l))) => { - context.maybe_zoom_text(l.to_computed_value(context).into()) + context.maybe_zoom_text(l.to_computed_value(context)) }, FontSize::Length(LengthPercentage::Length(ref l)) => { - l.to_computed_value(context).into() + l.to_computed_value(context) }, FontSize::Length(LengthPercentage::Percentage(pc)) => { // If the parent font was keyword-derived, this is too. @@ -871,29 +958,33 @@ impl FontSize { context, FontBaseSize::InheritedStyleButStripEmUnits, ) - .length_component(); + .unclamped_length(); - info = parent.keyword_info.map(|i| i.compose(ratio, abs.into())); + info = parent.keyword_info.map(|i| i.compose(ratio, abs)); } let calc = calc.to_computed_value_zoomed(context, base_size); - calc.to_used_value(base_size.resolve(context)).into() + // FIXME(emilio): we _could_ use clamp_to_non_negative() + // everywhere, without affecting behavior in theory, since the + // others should reject negatives during parsing. But SMIL + // allows parsing negatives, and relies on us _not_ doing that + // clamping. That's so bonkers :( + CSSPixelLength::from(calc.to_used_value(base_size.resolve(context))) + .clamp_to_non_negative() }, FontSize::Keyword(i) => { // As a specified keyword, this is keyword derived info = Some(i); - i.to_computed_value(context) + i.to_computed_value(context).clamp_to_non_negative() }, FontSize::Smaller => { info = compose_keyword(1. / LARGER_FONT_SIZE_RATIO); FontRelativeLength::Em(1. / LARGER_FONT_SIZE_RATIO) .to_computed_value(context, base_size) - .into() }, FontSize::Larger => { info = compose_keyword(LARGER_FONT_SIZE_RATIO); FontRelativeLength::Em(LARGER_FONT_SIZE_RATIO) .to_computed_value(context, base_size) - .into() }, FontSize::System(_) => { @@ -903,12 +994,12 @@ impl FontSize { } #[cfg(feature = "gecko")] { - context.cached_system_font.as_ref().unwrap().font_size.size + context.cached_system_font.as_ref().unwrap().font_size.size.0 } }, }; computed::FontSize { - size: size, + size: NonNegative(size), keyword_info: info, } } @@ -952,7 +1043,7 @@ impl FontSize { } if let Ok(kw) = input.try(KeywordSize::parse) { - return Ok(FontSize::Keyword(kw.into())); + return Ok(FontSize::Keyword(KeywordInfo::new(kw))); } try_match_ident_ignore_ascii_case! { input, diff --git a/components/style/values/specified/text.rs b/components/style/values/specified/text.rs index 1686b9f424d..ebc8e8f86ee 100644 --- a/components/style/values/specified/text.rs +++ b/components/style/values/specified/text.rs @@ -90,8 +90,7 @@ impl ToComputedValue for LineHeight { let result = match non_negative_lp.0 { LengthPercentage::Length(NoCalcLength::Absolute(ref abs)) => { context - .maybe_zoom_text(abs.to_computed_value(context).into()) - .0 + .maybe_zoom_text(abs.to_computed_value(context)) }, LengthPercentage::Length(ref length) => length.to_computed_value(context), LengthPercentage::Percentage(ref p) => FontRelativeLength::Em(p.0) From d794c8527de161a91273dd2339f1763b4a073610 Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Tue, 20 Aug 2019 01:30:43 +0000 Subject: [PATCH 03/24] style: Clamp negative values for font-size-adjust animations. Differential Revision: https://phabricator.services.mozilla.com/D42586 --- components/style/properties/longhands/font.mako.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/style/properties/longhands/font.mako.rs b/components/style/properties/longhands/font.mako.rs index 148b6fe33f6..b4b2ab0598c 100644 --- a/components/style/properties/longhands/font.mako.rs +++ b/components/style/properties/longhands/font.mako.rs @@ -80,7 +80,7 @@ ${helpers.predefined_type( engines="gecko", initial_value="computed::FontSizeAdjust::none()", initial_specified_value="specified::FontSizeAdjust::none()", - animation_value_type="ComputedValue", + animation_value_type="FontSizeAdjust", spec="https://drafts.csswg.org/css-fonts/#propdef-font-size-adjust", )} From dd5715d4f1997f526310cae6cdf7b9b468aaf1fa Mon Sep 17 00:00:00 2001 From: Mats Palmgren Date: Mon, 19 Aug 2019 21:19:04 +0000 Subject: [PATCH 04/24] style: Remove display:-moz-inline-grid/-moz-inline-stack. Differential Revision: https://phabricator.services.mozilla.com/D42552 --- components/style/values/specified/box.rs | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/components/style/values/specified/box.rs b/components/style/values/specified/box.rs index 146704ce89e..3e18a23d964 100644 --- a/components/style/values/specified/box.rs +++ b/components/style/values/specified/box.rs @@ -105,16 +105,12 @@ pub enum DisplayInside { #[cfg(feature = "gecko")] MozGrid, #[cfg(feature = "gecko")] - MozInlineGrid, - #[cfg(feature = "gecko")] MozGridGroup, #[cfg(feature = "gecko")] MozGridLine, #[cfg(feature = "gecko")] MozStack, #[cfg(feature = "gecko")] - MozInlineStack, - #[cfg(feature = "gecko")] MozDeck, #[cfg(feature = "gecko")] MozGroupbox, @@ -243,16 +239,12 @@ impl Display { #[cfg(feature = "gecko")] pub const MozGrid: Self = Self::new(DisplayOutside::XUL, DisplayInside::MozGrid); #[cfg(feature = "gecko")] - pub const MozInlineGrid: Self = Self::new(DisplayOutside::XUL, DisplayInside::MozInlineGrid); - #[cfg(feature = "gecko")] pub const MozGridGroup: Self = Self::new(DisplayOutside::XUL, DisplayInside::MozGridGroup); #[cfg(feature = "gecko")] pub const MozGridLine: Self = Self::new(DisplayOutside::XUL, DisplayInside::MozGridLine); #[cfg(feature = "gecko")] pub const MozStack: Self = Self::new(DisplayOutside::XUL, DisplayInside::MozStack); #[cfg(feature = "gecko")] - pub const MozInlineStack: Self = Self::new(DisplayOutside::XUL, DisplayInside::MozInlineStack); - #[cfg(feature = "gecko")] pub const MozDeck: Self = Self::new(DisplayOutside::XUL, DisplayInside::MozDeck); #[cfg(feature = "gecko")] pub const MozGroupbox: Self = Self::new(DisplayOutside::XUL, DisplayInside::MozGroupbox); @@ -422,7 +414,6 @@ impl Display { #[cfg(feature = "gecko")] DisplayOutside::XUL => match self.inside() { DisplayInside::MozBox => Display::MozInlineBox, - DisplayInside::MozStack => Display::MozInlineStack, _ => *self, }, _ => *self, @@ -469,10 +460,6 @@ impl ToCss for Display { Display::WebkitInlineBox => dest.write_str("-webkit-inline-box"), #[cfg(feature = "gecko")] Display::MozInlineBox => dest.write_str("-moz-inline-box"), - #[cfg(feature = "gecko")] - Display::MozInlineGrid => dest.write_str("-moz-inline-grid"), - #[cfg(feature = "gecko")] - Display::MozInlineStack => dest.write_str("-moz-inline-stack"), #[cfg(any(feature = "servo-layout-2013", feature = "gecko"))] Display::TableCaption => dest.write_str("table-caption"), _ => match (outside, inside) { @@ -662,16 +649,12 @@ impl Parse for Display { #[cfg(feature = "gecko")] "-moz-grid" if moz_display_values_enabled(context) => Display::MozGrid, #[cfg(feature = "gecko")] - "-moz-inline-grid" if moz_display_values_enabled(context) => Display::MozInlineGrid, - #[cfg(feature = "gecko")] "-moz-grid-group" if moz_display_values_enabled(context) => Display::MozGridGroup, #[cfg(feature = "gecko")] "-moz-grid-line" if moz_display_values_enabled(context) => Display::MozGridLine, #[cfg(feature = "gecko")] "-moz-stack" if moz_display_values_enabled(context) => Display::MozStack, #[cfg(feature = "gecko")] - "-moz-inline-stack" if moz_display_values_enabled(context) => Display::MozInlineStack, - #[cfg(feature = "gecko")] "-moz-deck" if moz_display_values_enabled(context) => Display::MozDeck, #[cfg(feature = "gecko")] "-moz-groupbox" if moz_display_values_enabled(context) => Display::MozGroupbox, From b8108eaa80c00084898f521d806faa12b1f53312 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 21 Aug 2019 18:25:21 +0000 Subject: [PATCH 05/24] style: Remove unused impl_coord_copy. Differential Revision: https://phabricator.services.mozilla.com/D42857 --- components/style/properties/gecko.mako.rs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index bd02a7ac944..ae252f46a83 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -321,18 +321,6 @@ impl ${style_struct.gecko_struct_name} { } -<%def name="impl_coord_copy(ident, gecko_ffi_name)"> - #[allow(non_snake_case)] - pub fn copy_${ident}_from(&mut self, other: &Self) { - self.gecko.${gecko_ffi_name}.copy_from(&other.gecko.${gecko_ffi_name}); - } - - #[allow(non_snake_case)] - pub fn reset_${ident}(&mut self, other: &Self) { - self.copy_${ident}_from(other) - } - - <%! def get_gecko_property(ffi_name, self_param = "self"): return "%s.gecko.%s" % (self_param, ffi_name) From 7c4dc9e79728e5ae212d3d1effa8f7df4330d51f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 21 Aug 2019 22:45:26 +0000 Subject: [PATCH 06/24] style: Make font-variant-alternates use cbindgen. Differential Revision: https://phabricator.services.mozilla.com/D42859 --- components/style/properties/gecko.mako.rs | 118 +--------------------- components/style/values/computed/font.rs | 4 +- components/style/values/specified/font.rs | 26 ++--- 3 files changed, 18 insertions(+), 130 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index ae252f46a83..4fc4615020a 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -1247,6 +1247,8 @@ fn static_assert() { ${impl_simple_type_with_conversion("font_synthesis", "mFont.synthesis")} + ${impl_simple("font_variant_alternates", "mFont.variantAlternates")} + pub fn set_font_size_adjust(&mut self, v: longhands::font_size_adjust::computed_value::T) { use crate::properties::longhands::font_size_adjust::computed_value::T; match v { @@ -1319,122 +1321,6 @@ fn static_assert() { ${impl_simple("_moz_script_level", "mScriptLevel")} <% impl_simple_type_with_conversion("font_language_override", "mFont.languageOverride") %> - pub fn set_font_variant_alternates( - &mut self, - v: values::computed::font::FontVariantAlternates, - ) { - use crate::gecko_bindings::bindings::{Gecko_ClearAlternateValues, Gecko_AppendAlternateValues}; - % for value in "normal swash stylistic ornaments annotation styleset character_variant historical".split(): - use crate::gecko_bindings::structs::NS_FONT_VARIANT_ALTERNATES_${value.upper()}; - % endfor - use crate::values::specified::font::VariantAlternates; - - unsafe { - Gecko_ClearAlternateValues(&mut self.gecko.mFont, v.len()); - } - - if v.0.is_empty() { - self.gecko.mFont.variantAlternates = NS_FONT_VARIANT_ALTERNATES_NORMAL as u16; - return; - } - - for val in v.0.iter() { - match *val { - % for value in "Swash Stylistic Ornaments Annotation".split(): - VariantAlternates::${value}(ref ident) => { - self.gecko.mFont.variantAlternates |= NS_FONT_VARIANT_ALTERNATES_${value.upper()} as u16; - unsafe { - Gecko_AppendAlternateValues(&mut self.gecko.mFont, - NS_FONT_VARIANT_ALTERNATES_${value.upper()}, - ident.0.as_ptr()); - } - }, - % endfor - % for value in "styleset character_variant".split(): - VariantAlternates::${to_camel_case(value)}(ref slice) => { - self.gecko.mFont.variantAlternates |= NS_FONT_VARIANT_ALTERNATES_${value.upper()} as u16; - for ident in slice.iter() { - unsafe { - Gecko_AppendAlternateValues(&mut self.gecko.mFont, - NS_FONT_VARIANT_ALTERNATES_${value.upper()}, - ident.0.as_ptr()); - } - } - }, - % endfor - VariantAlternates::HistoricalForms => { - self.gecko.mFont.variantAlternates |= NS_FONT_VARIANT_ALTERNATES_HISTORICAL as u16; - } - } - } - } - - #[allow(non_snake_case)] - pub fn copy_font_variant_alternates_from(&mut self, other: &Self) { - use crate::gecko_bindings::bindings::Gecko_CopyAlternateValuesFrom; - - self.gecko.mFont.variantAlternates = other.gecko.mFont.variantAlternates; - unsafe { - Gecko_CopyAlternateValuesFrom(&mut self.gecko.mFont, &other.gecko.mFont); - } - } - - pub fn reset_font_variant_alternates(&mut self, other: &Self) { - self.copy_font_variant_alternates_from(other) - } - - pub fn clone_font_variant_alternates(&self) -> values::computed::font::FontVariantAlternates { - % for value in "normal swash stylistic ornaments annotation styleset character_variant historical".split(): - use crate::gecko_bindings::structs::NS_FONT_VARIANT_ALTERNATES_${value.upper()}; - % endfor - use crate::values::specified::font::VariantAlternates; - use crate::values::specified::font::VariantAlternatesList; - use crate::values::CustomIdent; - - if self.gecko.mFont.variantAlternates == NS_FONT_VARIANT_ALTERNATES_NORMAL as u16 { - return VariantAlternatesList(vec![].into_boxed_slice()); - } - - let mut alternates = Vec::with_capacity(self.gecko.mFont.alternateValues.len()); - if self.gecko.mFont.variantAlternates & (NS_FONT_VARIANT_ALTERNATES_HISTORICAL as u16) != 0 { - alternates.push(VariantAlternates::HistoricalForms); - } - - <% - property_need_ident_list = "styleset character_variant".split() - %> - % for value in property_need_ident_list: - let mut ${value}_list = Vec::new(); - % endfor - - for gecko_alternate_value in self.gecko.mFont.alternateValues.iter() { - let ident = Atom::from(gecko_alternate_value.value.to_string()); - match gecko_alternate_value.alternate { - % for value in "Swash Stylistic Ornaments Annotation".split(): - NS_FONT_VARIANT_ALTERNATES_${value.upper()} => { - alternates.push(VariantAlternates::${value}(CustomIdent(ident))); - }, - % endfor - % for value in property_need_ident_list: - NS_FONT_VARIANT_ALTERNATES_${value.upper()} => { - ${value}_list.push(CustomIdent(ident)); - }, - % endfor - _ => { - panic!("Found unexpected value for font-variant-alternates"); - } - } - } - - % for value in property_need_ident_list: - if !${value}_list.is_empty() { - alternates.push(VariantAlternates::${to_camel_case(value)}(${value}_list.into_boxed_slice())); - } - % endfor - - VariantAlternatesList(alternates.into_boxed_slice()) - } - ${impl_simple_type_with_conversion("font_variant_ligatures", "mFont.variantLigatures")} ${impl_simple_type_with_conversion("font_variant_east_asian", "mFont.variantEastAsian")} ${impl_simple_type_with_conversion("font_variant_numeric", "mFont.variantNumeric")} diff --git a/components/style/values/computed/font.rs b/components/style/values/computed/font.rs index 0063a6946ba..fee8c24ee3b 100644 --- a/components/style/values/computed/font.rs +++ b/components/style/values/computed/font.rs @@ -644,10 +644,10 @@ impl ToAnimatedValue for FontSizeAdjust { pub type FontVariantAlternates = specified::VariantAlternatesList; impl FontVariantAlternates { - #[inline] /// Get initial value with VariantAlternatesList + #[inline] pub fn get_initial_value() -> Self { - specified::VariantAlternatesList(vec![].into_boxed_slice()) + Self::default() } } diff --git a/components/style/values/specified/font.rs b/components/style/values/specified/font.rs index d42272e396b..9e680ec7758 100644 --- a/components/style/values/specified/font.rs +++ b/components/style/values/specified/font.rs @@ -1089,17 +1089,20 @@ bitflags! { #[derive( Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss, ToResolvedValue, ToShmem, )] +#[repr(C, u8)] /// Set of variant alternates +/// +/// cbindgen:derive-tagged-enum-copy-constructor=true pub enum VariantAlternates { /// Enables display of stylistic alternates #[css(function)] Stylistic(CustomIdent), /// Enables display with stylistic sets #[css(comma, function)] - Styleset(#[css(iterable)] Box<[CustomIdent]>), + Styleset(#[css(iterable)] crate::OwnedSlice), /// Enables display of specific character variants #[css(comma, function)] - CharacterVariant(#[css(iterable)] Box<[CustomIdent]>), + CharacterVariant(#[css(iterable)] crate::OwnedSlice), /// Enables display of swash glyphs #[css(function)] Swash(CustomIdent), @@ -1114,11 +1117,12 @@ pub enum VariantAlternates { } #[derive( - Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss, ToResolvedValue, ToShmem, + Clone, Debug, Default, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss, ToResolvedValue, ToShmem, )] +#[repr(transparent)] /// List of Variant Alternates pub struct VariantAlternatesList( - #[css(if_empty = "normal", iterable)] pub Box<[VariantAlternates]>, + #[css(if_empty = "normal", iterable)] crate::OwnedSlice, ); impl VariantAlternatesList { @@ -1150,7 +1154,7 @@ impl FontVariantAlternates { #[inline] /// Get initial specified value with VariantAlternatesList pub fn get_initial_specified_value() -> Self { - FontVariantAlternates::Value(VariantAlternatesList(vec![].into_boxed_slice())) + FontVariantAlternates::Value(Default::default()) } system_font_methods!(FontVariantAlternates, font_variant_alternates); @@ -1184,16 +1188,14 @@ impl Parse for FontVariantAlternates { _: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { - let mut alternates = Vec::new(); if input .try(|input| input.expect_ident_matching("normal")) .is_ok() { - return Ok(FontVariantAlternates::Value(VariantAlternatesList( - alternates.into_boxed_slice(), - ))); + return Ok(FontVariantAlternates::Value(Default::default())) } + let mut alternates = Vec::new(); let mut parsed_alternates = VariantAlternatesParsingFlags::empty(); macro_rules! check_if_parsed( ($input:expr, $flag:path) => ( @@ -1247,7 +1249,7 @@ impl Parse for FontVariantAlternates { let location = i.current_source_location(); CustomIdent::from_ident(location, i.expect_ident()?, &[]) })?; - alternates.push(VariantAlternates::Styleset(idents.into_boxed_slice())); + alternates.push(VariantAlternates::Styleset(idents.into())); Ok(()) }, "character-variant" => { @@ -1256,7 +1258,7 @@ impl Parse for FontVariantAlternates { let location = i.current_source_location(); CustomIdent::from_ident(location, i.expect_ident()?, &[]) })?; - alternates.push(VariantAlternates::CharacterVariant(idents.into_boxed_slice())); + alternates.push(VariantAlternates::CharacterVariant(idents.into())); Ok(()) }, _ => return Err(i.new_custom_error(StyleParseErrorKind::UnspecifiedError)), @@ -1270,7 +1272,7 @@ impl Parse for FontVariantAlternates { return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)); } Ok(FontVariantAlternates::Value(VariantAlternatesList( - alternates.into_boxed_slice(), + alternates.into(), ))) } } From 14c7e133ade82a463277f788c9839f33e6d8dcb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 21 Aug 2019 18:35:31 +0000 Subject: [PATCH 07/24] style: Avoid some signed -> unsigned conversions for box-ordinal-group. This is consistent with the `order` property anyhow, and allows to simplify some code. Negatives are still not parsed, but rust uses a similar representation for all CSS values and so should C++. Differential Revision: https://phabricator.services.mozilla.com/D42912 --- components/style/properties/gecko.mako.rs | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 4fc4615020a..e8f2c521a6f 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -3040,19 +3040,7 @@ clip-path ${impl_simple_type_with_conversion("_moz_force_broken_image_icon", "mForceBrokenImageIcon")} -<%self:impl_trait style_struct_name="XUL" - skip_longhands="-moz-box-ordinal-group"> - #[allow(non_snake_case)] - pub fn set__moz_box_ordinal_group(&mut self, v: i32) { - self.gecko.mBoxOrdinal = v as u32; - } - - ${impl_simple_copy("_moz_box_ordinal_group", "mBoxOrdinal")} - - #[allow(non_snake_case)] - pub fn clone__moz_box_ordinal_group(&self) -> i32 { - self.gecko.mBoxOrdinal as i32 - } +<%self:impl_trait style_struct_name="XUL"> % for style_struct in data.style_structs: From 432b417d989b1b53c4e27da5b2dc3a35f53a0b1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 21 Aug 2019 19:47:53 +0000 Subject: [PATCH 08/24] style: Avoid manual paint-order glue code. Differential Revision: https://phabricator.services.mozilla.com/D42913 --- components/style/properties/gecko.mako.rs | 13 +------------ components/style/values/specified/svg.rs | 9 +++++---- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index e8f2c521a6f..dc0ceceefa1 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -2599,18 +2599,7 @@ clip-path <%self:impl_trait style_struct_name="InheritedSVG" - skip_longhands="paint-order stroke-dasharray"> - pub fn set_paint_order(&mut self, v: longhands::paint_order::computed_value::T) { - self.gecko.mPaintOrder = v.0; - } - - ${impl_simple_copy('paint_order', 'mPaintOrder')} - - pub fn clone_paint_order(&self) -> longhands::paint_order::computed_value::T { - use crate::properties::longhands::paint_order::computed_value::T; - T(self.gecko.mPaintOrder) - } - + skip_longhands="stroke-dasharray"> pub fn set_stroke_dasharray(&mut self, v: longhands::stroke_dasharray::computed_value::T) { use crate::gecko_bindings::structs::nsStyleSVG_STROKE_DASHARRAY_CONTEXT as CONTEXT_VALUE; use crate::values::generics::svg::SVGStrokeDashArray; diff --git a/components/style/values/specified/svg.rs b/components/style/values/specified/svg.rs index 4858fbaec6b..6ae1eac2d4a 100644 --- a/components/style/values/specified/svg.rs +++ b/components/style/values/specified/svg.rs @@ -106,13 +106,13 @@ pub enum PaintOrder { } /// Number of non-normal components -const PAINT_ORDER_COUNT: u8 = 3; +pub const PAINT_ORDER_COUNT: u8 = 3; /// Number of bits for each component -const PAINT_ORDER_SHIFT: u8 = 2; +pub const PAINT_ORDER_SHIFT: u8 = 2; /// Mask with above bits set -const PAINT_ORDER_MASK: u8 = 0b11; +pub const PAINT_ORDER_MASK: u8 = 0b11; /// The specified value is tree `PaintOrder` values packed into the /// bitfields below, as a six-bit field, of 3 two-bit pairs @@ -135,6 +135,7 @@ const PAINT_ORDER_MASK: u8 = 0b11; ToResolvedValue, ToShmem, )] +#[repr(transparent)] pub struct SVGPaintOrder(pub u8); impl SVGPaintOrder { @@ -146,7 +147,7 @@ impl SVGPaintOrder { /// Get variant of `paint-order` pub fn order_at(&self, pos: u8) -> PaintOrder { // Safe because PaintOrder covers all possible patterns. - unsafe { ::std::mem::transmute((self.0 >> pos * PAINT_ORDER_SHIFT) & PAINT_ORDER_MASK) } + unsafe { std::mem::transmute((self.0 >> pos * PAINT_ORDER_SHIFT) & PAINT_ORDER_MASK) } } } From ee106992e1850d428ca66631b4b72bc3e2ab7225 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 21 Aug 2019 20:02:13 +0000 Subject: [PATCH 09/24] style: Simplify -x-span property. Differential Revision: https://phabricator.services.mozilla.com/D42914 --- components/style/properties/gecko.mako.rs | 15 +------- .../style/properties/longhands/table.mako.rs | 4 +-- components/style/values/computed/mod.rs | 2 -- components/style/values/computed/table.rs | 7 ---- components/style/values/specified/mod.rs | 2 -- components/style/values/specified/table.rs | 34 ------------------- 6 files changed, 3 insertions(+), 61 deletions(-) delete mode 100644 components/style/values/computed/table.rs delete mode 100644 components/style/values/specified/table.rs diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index dc0ceceefa1..7739014410e 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -2296,20 +2296,7 @@ fn static_assert() { -<%self:impl_trait style_struct_name="Table" skip_longhands="-x-span"> - #[allow(non_snake_case)] - pub fn set__x_span(&mut self, v: longhands::_x_span::computed_value::T) { - self.gecko.mSpan = v.0 - } - - #[allow(non_snake_case)] - pub fn clone__x_span(&self) -> longhands::_x_span::computed_value::T { - longhands::_x_span::computed_value::T( - self.gecko.mSpan - ) - } - - ${impl_simple_copy('_x_span', 'mSpan')} +<%self:impl_trait style_struct_name="Table"> <%self:impl_trait style_struct_name="Effects" skip_longhands="clip"> diff --git a/components/style/properties/longhands/table.mako.rs b/components/style/properties/longhands/table.mako.rs index 4475faaee37..0564942fc0a 100644 --- a/components/style/properties/longhands/table.mako.rs +++ b/components/style/properties/longhands/table.mako.rs @@ -18,8 +18,8 @@ ${helpers.single_keyword( ${helpers.predefined_type( "-x-span", - "XSpan", - "computed::XSpan(1)", + "Integer", + "1", engines="gecko", spec="Internal-only (for `` pres attr)", animation_value_type="none", diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index d5a61746675..a565fc5f93b 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -75,7 +75,6 @@ pub use self::resolution::Resolution; pub use self::svg::MozContextProperties; pub use self::svg::{SVGLength, SVGOpacity, SVGPaint, SVGPaintKind}; pub use self::svg::{SVGPaintOrder, SVGStrokeDashArray, SVGWidth}; -pub use self::table::XSpan; pub use self::text::{InitialLetter, LetterSpacing, LineBreak, LineHeight}; pub use self::text::{OverflowWrap, TextOverflow, WordBreak, WordSpacing}; pub use self::text::{TextAlign, TextEmphasisPosition, TextEmphasisStyle}; @@ -116,7 +115,6 @@ pub mod position; pub mod rect; pub mod resolution; pub mod svg; -pub mod table; pub mod text; pub mod time; pub mod transform; diff --git a/components/style/values/computed/table.rs b/components/style/values/computed/table.rs deleted file mode 100644 index 22d09447ba9..00000000000 --- a/components/style/values/computed/table.rs +++ /dev/null @@ -1,7 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ - -//! Computed types for table properties. - -pub use crate::values::specified::table::XSpan; diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index 16897043da9..ddb890ba395 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -78,7 +78,6 @@ pub use self::svg::MozContextProperties; pub use self::svg::{SVGLength, SVGOpacity, SVGPaint}; pub use self::svg::{SVGPaintOrder, SVGStrokeDashArray, SVGWidth}; pub use self::svg_path::SVGPathData; -pub use self::table::XSpan; pub use self::text::{InitialLetter, LetterSpacing, LineBreak, LineHeight, TextAlign}; pub use self::text::{OverflowWrap, TextEmphasisPosition, TextEmphasisStyle, WordBreak}; pub use self::text::{TextAlignKeyword, TextDecorationLine, TextOverflow, WordSpacing}; @@ -122,7 +121,6 @@ pub mod resolution; pub mod source_size_list; pub mod svg; pub mod svg_path; -pub mod table; pub mod text; pub mod time; pub mod transform; diff --git a/components/style/values/specified/table.rs b/components/style/values/specified/table.rs deleted file mode 100644 index 2caace07eb3..00000000000 --- a/components/style/values/specified/table.rs +++ /dev/null @@ -1,34 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ - -//! Specified types for table properties. - -use crate::parser::{Parse, ParserContext}; -use cssparser::Parser; -use style_traits::{ParseError, StyleParseErrorKind}; - -#[derive( - Clone, - Copy, - Debug, - MallocSizeOf, - PartialEq, - SpecifiedValueInfo, - ToComputedValue, - ToCss, - ToResolvedValue, - ToShmem, -)] -/// span. for `` pres attr -pub struct XSpan(#[css(skip)] pub i32); - -impl Parse for XSpan { - // never parse it, only set via presentation attribute - fn parse<'i, 't>( - _: &ParserContext, - input: &mut Parser<'i, 't>, - ) -> Result> { - Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) - } -} From b0f3a68a6cb38823dba8556be91d8486e08eec78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 26 Aug 2019 09:58:53 +0000 Subject: [PATCH 10/24] style: Rustfmt declaration_block.rs. Differential Revision: https://phabricator.services.mozilla.com/D43400 --- .../style/properties/declaration_block.rs | 485 ++++++++++-------- 1 file changed, 258 insertions(+), 227 deletions(-) diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index 6078f6712a7..930d4400d3d 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -6,34 +6,36 @@ #![deny(missing_docs)] +use super::*; use crate::context::QuirksMode; -use cssparser::{DeclarationListParser, parse_important, ParserInput, CowRcStr}; -use cssparser::{Parser, AtRuleParser, DeclarationParser, Delimiter, ParseErrorKind}; -use crate::custom_properties::{CustomPropertiesBuilder, CssEnvironment}; -use crate::error_reporting::{ParseErrorReporter, ContextualParseError}; -use itertools::Itertools; +use crate::custom_properties::{CssEnvironment, CustomPropertiesBuilder}; +use crate::error_reporting::{ContextualParseError, ParseErrorReporter}; use crate::parser::ParserContext; use crate::properties::animated_properties::{AnimationValue, AnimationValueMap}; +use crate::selector_parser::SelectorImpl; use crate::shared_lock::Locked; +use crate::str::{CssString, CssStringBorrow, CssStringWriter}; +use crate::stylesheets::{CssRuleType, Origin, UrlExtraData}; +use crate::values::computed::Context; +use cssparser::{parse_important, CowRcStr, DeclarationListParser, ParserInput}; +use cssparser::{AtRuleParser, DeclarationParser, Delimiter, ParseErrorKind, Parser}; +use itertools::Itertools; +use selectors::SelectorList; use smallbitvec::{self, SmallBitVec}; use smallvec::SmallVec; use std::fmt::{self, Write}; use std::iter::{DoubleEndedIterator, Zip}; use std::slice::Iter; -use crate::str::{CssString, CssStringBorrow, CssStringWriter}; use style_traits::{CssWriter, ParseError, ParsingMode, StyleParseErrorKind, ToCss}; -use crate::stylesheets::{CssRuleType, Origin, UrlExtraData}; -use super::*; -use crate::values::computed::Context; -use crate::selector_parser::SelectorImpl; -use selectors::SelectorList; /// The animation rules. /// /// The first one is for Animation cascade level, and the second one is for /// Transition cascade level. -pub struct AnimationRules(pub Option>>, - pub Option>>); +pub struct AnimationRules( + pub Option>>, + pub Option>>, +); impl AnimationRules { /// Returns whether these animation rules represents an actual rule or not. @@ -122,8 +124,16 @@ impl<'a> Iterator for DeclarationImportanceIterator<'a> { #[inline] fn next(&mut self) -> Option { - self.iter.next().map(|(decl, important)| - (decl, if important { Importance::Important } else { Importance::Normal })) + self.iter.next().map(|(decl, important)| { + ( + decl, + if important { + Importance::Important + } else { + Importance::Normal + }, + ) + }) } #[inline] @@ -135,13 +145,21 @@ impl<'a> Iterator for DeclarationImportanceIterator<'a> { impl<'a> DoubleEndedIterator for DeclarationImportanceIterator<'a> { #[inline(always)] fn next_back(&mut self) -> Option { - self.iter.next_back().map(|(decl, important)| - (decl, if important { Importance::Important } else { Importance::Normal })) + self.iter.next_back().map(|(decl, important)| { + ( + decl, + if important { + Importance::Important + } else { + Importance::Normal + }, + ) + }) } } /// Iterator for AnimationValue to be generated from PropertyDeclarationBlock. -pub struct AnimationValueIterator<'a, 'cx, 'cx_a:'cx> { +pub struct AnimationValueIterator<'a, 'cx, 'cx_a: 'cx> { iter: DeclarationImportanceIterator<'a>, context: &'cx mut Context<'cx_a>, default_values: &'a ComputedValues, @@ -149,7 +167,7 @@ pub struct AnimationValueIterator<'a, 'cx, 'cx_a:'cx> { extra_custom_properties: Option<&'a Arc>, } -impl<'a, 'cx, 'cx_a:'cx> AnimationValueIterator<'a, 'cx, 'cx_a> { +impl<'a, 'cx, 'cx_a: 'cx> AnimationValueIterator<'a, 'cx, 'cx_a> { fn new( declarations: &'a PropertyDeclarationBlock, context: &'cx mut Context<'cx_a>, @@ -165,7 +183,7 @@ impl<'a, 'cx, 'cx_a:'cx> AnimationValueIterator<'a, 'cx, 'cx_a> { } } -impl<'a, 'cx, 'cx_a:'cx> Iterator for AnimationValueIterator<'a, 'cx, 'cx_a> { +impl<'a, 'cx, 'cx_a: 'cx> Iterator for AnimationValueIterator<'a, 'cx, 'cx_a> { type Item = AnimationValue; #[inline] fn next(&mut self) -> Option { @@ -256,7 +274,7 @@ impl PropertyDeclarationBlock { /// Return an iterator of (AnimatableLonghand, AnimationValue). #[inline] - pub fn to_animation_value_iter<'a, 'cx, 'cx_a:'cx>( + pub fn to_animation_value_iter<'a, 'cx, 'cx_a: 'cx>( &'a self, context: &'cx mut Context<'cx_a>, default_values: &'a ComputedValues, @@ -300,7 +318,10 @@ impl PropertyDeclarationBlock { /// NOTE: This is linear time in the case of custom properties or in the /// case the longhand is actually in the declaration block. #[inline] - pub fn get(&self, property: PropertyDeclarationId) -> Option<(&PropertyDeclaration, Importance)> { + pub fn get( + &self, + property: PropertyDeclarationId, + ) -> Option<(&PropertyDeclaration, Importance)> { if let PropertyDeclarationId::Longhand(id) = property { if !self.contains(id) { return None; @@ -350,9 +371,7 @@ impl PropertyDeclarationBlock { // We don't print !important when serializing individual properties, // so we treat this as a normal-importance property match shorthand.get_shorthand_appendable_value(list.iter().cloned()) { - Some(appendable_value) => { - append_declaration_value(dest, appendable_value) - } + Some(appendable_value) => append_declaration_value(dest, appendable_value), None => return Ok(()), } } @@ -360,7 +379,11 @@ impl PropertyDeclarationBlock { /// Find the value of the given property in this block and serialize it /// /// - pub fn property_value_to_css(&self, property: &PropertyId, dest: &mut CssStringWriter) -> fmt::Result { + pub fn property_value_to_css( + &self, + property: &PropertyId, + dest: &mut CssStringWriter, + ) -> fmt::Result { // Step 1.1: done when parsing a string to PropertyId // Step 1.2 @@ -394,11 +417,12 @@ impl PropertyDeclarationBlock { } else { Importance::Normal } - } + }, Err(longhand_or_custom) => { // Step 3 - self.get(longhand_or_custom).map_or(Importance::Normal, |(_, importance)| importance) - } + self.get(longhand_or_custom) + .map_or(Importance::Normal, |(_, importance)| importance) + }, } } @@ -407,7 +431,9 @@ impl PropertyDeclarationBlock { /// and it doesn't exist in the block, and returns false otherwise. #[inline] fn is_definitely_new(&self, decl: &PropertyDeclaration) -> bool { - decl.id().as_longhand().map_or(false, |id| !self.longhands.contains(id)) + decl.id() + .as_longhand() + .map_or(false, |id| !self.longhands.contains(id)) } /// Adds or overrides the declaration for a given property in this block. @@ -421,11 +447,11 @@ impl PropertyDeclarationBlock { ) -> bool { let all_shorthand_len = match drain.all_shorthand { AllShorthand::NotSet => 0, - AllShorthand::CSSWideKeyword(_) | - AllShorthand::WithVariables(_) => shorthands::ALL_SHORTHAND_MAX_LEN, + AllShorthand::CSSWideKeyword(_) | AllShorthand::WithVariables(_) => { + shorthands::ALL_SHORTHAND_MAX_LEN + }, }; - let push_calls_count = - drain.declarations.len() + all_shorthand_len; + let push_calls_count = drain.declarations.len() + all_shorthand_len; // With deduplication the actual length increase may be less than this. self.declarations.reserve(push_calls_count); @@ -434,9 +460,12 @@ impl PropertyDeclarationBlock { for decl in &mut drain.declarations { changed |= self.push(decl, importance); } - drain.all_shorthand.declarations().fold(changed, |changed, decl| { - changed | self.push(decl, importance) - }) + drain + .all_shorthand + .declarations() + .fold(changed, |changed, decl| { + changed | self.push(decl, importance) + }) } /// Adds or overrides the declaration for a given property in this block. @@ -444,11 +473,7 @@ impl PropertyDeclarationBlock { /// Returns whether the declaration has changed. /// /// This is only used for parsing and internal use. - pub fn push( - &mut self, - declaration: PropertyDeclaration, - importance: Importance, - ) -> bool { + pub fn push(&mut self, declaration: PropertyDeclaration, importance: Importance) -> bool { if !self.is_definitely_new(&declaration) { let mut index_to_remove = None; for (i, slot) in self.declarations.iter_mut().enumerate() { @@ -498,78 +523,92 @@ impl PropertyDeclarationBlock { // Check whether we are updating for an all shorthand change. if !matches!(source_declarations.all_shorthand, AllShorthand::NotSet) { debug_assert!(source_declarations.declarations.is_empty()); - return source_declarations.all_shorthand.declarations().any(|decl| { - self.is_definitely_new(&decl) || - self.declarations.iter().enumerate() - .find(|&(_, ref d)| d.id() == decl.id()) - .map_or(true, |(i, d)| { - let important = self.declarations_importance[i]; - *d != decl || important != importance.important() - }) - }); + return source_declarations + .all_shorthand + .declarations() + .any(|decl| { + self.is_definitely_new(&decl) || + self.declarations + .iter() + .enumerate() + .find(|&(_, ref d)| d.id() == decl.id()) + .map_or(true, |(i, d)| { + let important = self.declarations_importance[i]; + *d != decl || important != importance.important() + }) + }); } // Fill `updates` with update information. let mut any_update = false; let new_count = &mut updates.new_count; let any_removal = &mut updates.any_removal; let updates = &mut updates.updates; - updates.extend(source_declarations.declarations.iter().map(|declaration| { - if self.is_definitely_new(declaration) { - return DeclarationUpdate::Append; - } - let longhand_id = declaration.id().as_longhand(); - if let Some(longhand_id) = longhand_id { - if let Some(logical_group) = longhand_id.logical_group() { - let mut needs_append = false; - for (pos, decl) in self.declarations.iter().enumerate().rev() { - let id = match decl.id().as_longhand() { - Some(id) => id, - None => continue, - }; - if id == longhand_id { - if needs_append { - return DeclarationUpdate::AppendAndRemove { pos }; + updates.extend( + source_declarations + .declarations + .iter() + .map(|declaration| { + if self.is_definitely_new(declaration) { + return DeclarationUpdate::Append; + } + let longhand_id = declaration.id().as_longhand(); + if let Some(longhand_id) = longhand_id { + if let Some(logical_group) = longhand_id.logical_group() { + let mut needs_append = false; + for (pos, decl) in self.declarations.iter().enumerate().rev() { + let id = match decl.id().as_longhand() { + Some(id) => id, + None => continue, + }; + if id == longhand_id { + if needs_append { + return DeclarationUpdate::AppendAndRemove { pos }; + } + let important = self.declarations_importance[pos]; + if decl == declaration && important == importance.important() { + return DeclarationUpdate::None; + } + return DeclarationUpdate::UpdateInPlace { pos }; + } + if !needs_append && + id.logical_group() == Some(logical_group) && + id.is_logical() != longhand_id.is_logical() + { + needs_append = true; + } } + unreachable!("Longhand should be found in loop above"); + } + } + self.declarations + .iter() + .enumerate() + .find(|&(_, ref decl)| decl.id() == declaration.id()) + .map_or(DeclarationUpdate::Append, |(pos, decl)| { let important = self.declarations_importance[pos]; if decl == declaration && important == importance.important() { - return DeclarationUpdate::None; + DeclarationUpdate::None + } else { + DeclarationUpdate::UpdateInPlace { pos } } - return DeclarationUpdate::UpdateInPlace { pos }; - } - if !needs_append && - id.logical_group() == Some(logical_group) && - id.is_logical() != longhand_id.is_logical() { - needs_append = true; - } - } - unreachable!("Longhand should be found in loop above"); - } - } - self.declarations.iter().enumerate() - .find(|&(_, ref decl)| decl.id() == declaration.id()) - .map_or(DeclarationUpdate::Append, |(pos, decl)| { - let important = self.declarations_importance[pos]; - if decl == declaration && important == importance.important() { - DeclarationUpdate::None - } else { - DeclarationUpdate::UpdateInPlace { pos } - } + }) }) - }).inspect(|update| { - if matches!(update, DeclarationUpdate::None) { - return; - } - any_update = true; - match update { - DeclarationUpdate::Append => { - *new_count += 1; - } - DeclarationUpdate::AppendAndRemove { .. } => { - *any_removal = true; - } - _ => {} - } - })); + .inspect(|update| { + if matches!(update, DeclarationUpdate::None) { + return; + } + any_update = true; + match update { + DeclarationUpdate::Append => { + *new_count += 1; + }, + DeclarationUpdate::AppendAndRemove { .. } => { + *any_removal = true; + }, + _ => {}, + } + }), + ); any_update } @@ -590,8 +629,12 @@ impl PropertyDeclarationBlock { self.declarations_importance.push(important); self.longhands.insert(longhand_id); } else { - let (idx, slot) = self.declarations.iter_mut() - .enumerate().find(|&(_, ref d)| d.id() == decl.id()).unwrap(); + let (idx, slot) = self + .declarations + .iter_mut() + .enumerate() + .find(|&(_, ref d)| d.id() == decl.id()) + .unwrap(); *slot = decl; self.declarations_importance.set(idx, important); } @@ -607,19 +650,24 @@ impl PropertyDeclarationBlock { pos: usize, remove: bool, } - let mut updates_and_removals: SubpropertiesVec = - updates.updates.iter_mut().filter_map(|item| { + let mut updates_and_removals: SubpropertiesVec = updates + .updates + .iter_mut() + .filter_map(|item| { let (pos, remove) = match *item { DeclarationUpdate::UpdateInPlace { pos } => (pos, false), DeclarationUpdate::AppendAndRemove { pos } => (pos, true), _ => return None, }; Some(UpdateOrRemoval { item, pos, remove }) - }).collect(); + }) + .collect(); // Execute removals. It's important to do it in reverse index order, // so that removing doesn't invalidate following positions. updates_and_removals.sort_unstable_by_key(|update| update.pos); - updates_and_removals.iter().rev() + updates_and_removals + .iter() + .rev() .filter(|update| update.remove) .for_each(|update| { self.declarations.remove(update.pos); @@ -637,7 +685,7 @@ impl PropertyDeclarationBlock { DeclarationUpdate::UpdateInPlace { pos: update.pos } ); *update.item = DeclarationUpdate::UpdateInPlace { - pos: update.pos - removed_count + pos: update.pos - removed_count, }; } } @@ -645,18 +693,17 @@ impl PropertyDeclarationBlock { for (decl, update) in drain.declarations.zip_eq(updates.updates.iter()) { match *update { DeclarationUpdate::None => {}, - DeclarationUpdate::Append | - DeclarationUpdate::AppendAndRemove { .. } => { + DeclarationUpdate::Append | DeclarationUpdate::AppendAndRemove { .. } => { if let Some(id) = decl.id().as_longhand() { self.longhands.insert(id); } self.declarations.push(decl); self.declarations_importance.push(important); - } + }, DeclarationUpdate::UpdateInPlace { pos } => { self.declarations[pos] = decl; self.declarations_importance.set(pos, important); - } + }, } } updates.updates.clear(); @@ -665,19 +712,16 @@ impl PropertyDeclarationBlock { /// Returns the first declaration that would be removed by removing /// `property`. #[inline] - pub fn first_declaration_to_remove( - &self, - property: &PropertyId, - ) -> Option { + pub fn first_declaration_to_remove(&self, property: &PropertyId) -> Option { if let Some(id) = property.longhand_id() { if !self.longhands.contains(id) { return None; } } - self.declarations.iter().position(|declaration| { - declaration.id().is_or_is_longhand_of(property) - }) + self.declarations + .iter() + .position(|declaration| declaration.id().is_or_is_longhand_of(property)) } /// Removes a given declaration at a given index. @@ -698,16 +742,14 @@ impl PropertyDeclarationBlock { /// `first_declaration` needs to be the result of /// `first_declaration_to_remove`. #[inline] - pub fn remove_property( - &mut self, - property: &PropertyId, - first_declaration: usize, - ) { + pub fn remove_property(&mut self, property: &PropertyId, first_declaration: usize) { debug_assert_eq!( Some(first_declaration), self.first_declaration_to_remove(property) ); - debug_assert!(self.declarations[first_declaration].id().is_or_is_longhand_of(property)); + debug_assert!(self.declarations[first_declaration] + .id() + .is_or_is_longhand_of(property)); self.remove_declaration_at(first_declaration); @@ -776,16 +818,16 @@ impl PropertyDeclarationBlock { // getKeyframes() implementation for CSS animations, if // |computed_values| is supplied, we use it to expand such variable // declarations. This will be fixed properly in Gecko bug 1391537. - ( - &PropertyDeclaration::WithVariables(ref declaration), - Some(ref _computed_values), - ) => { - declaration.value.substitute_variables( - declaration.id, - custom_properties.as_ref(), - QuirksMode::NoQuirks, - &env, - ).to_css(dest) + (&PropertyDeclaration::WithVariables(ref declaration), Some(ref _computed_values)) => { + declaration + .value + .substitute_variables( + declaration.id, + custom_properties.as_ref(), + QuirksMode::NoQuirks, + &env, + ) + .to_css(dest) }, (ref d, _) => d.to_css(dest), } @@ -798,8 +840,8 @@ impl PropertyDeclarationBlock { let mut longhands = LonghandIdSet::new(); for (property, animation_value) in animation_value_map.iter() { - longhands.insert(*property); - declarations.push(animation_value.uncompute()); + longhands.insert(*property); + declarations.push(animation_value.uncompute()); } PropertyDeclarationBlock { @@ -814,13 +856,12 @@ impl PropertyDeclarationBlock { pub fn has_css_wide_keyword(&self, property: &PropertyId) -> bool { if let Some(id) = property.longhand_id() { if !self.longhands.contains(id) { - return false + return false; } } - self.declarations.iter().any(|decl| - decl.id().is_or_is_longhand_of(property) && - decl.get_css_wide_keyword().is_some() - ) + self.declarations.iter().any(|decl| { + decl.id().is_or_is_longhand_of(property) && decl.get_css_wide_keyword().is_some() + }) } /// Returns a custom properties map which is the result of cascading custom @@ -844,10 +885,7 @@ impl PropertyDeclarationBlock { inherited_custom_properties: Option<&Arc>, environment: &CssEnvironment, ) -> Option> { - let mut builder = CustomPropertiesBuilder::new( - inherited_custom_properties, - environment, - ); + let mut builder = CustomPropertiesBuilder::new(inherited_custom_properties, environment); for declaration in self.normal_declaration_iter() { if let PropertyDeclaration::Custom(ref declaration) = *declaration { @@ -899,7 +937,7 @@ impl PropertyDeclarationBlock { &mut is_first_serialization, )?; continue; - } + }, }; // Step 3.2 @@ -928,22 +966,19 @@ impl PropertyDeclarationBlock { let mut important_count = 0; let mut found_system = None; - let is_system_font = - shorthand == ShorthandId::Font && - self.declarations.iter().any(|l| { - match l.id() { - PropertyDeclarationId::Longhand(id) => { - if already_serialized.contains(id.into()) { - return false; - } + let is_system_font = shorthand == ShorthandId::Font && + self.declarations.iter().any(|l| match l.id() { + PropertyDeclarationId::Longhand(id) => { + if already_serialized.contains(id.into()) { + return false; + } - l.get_system().is_some() - } - PropertyDeclarationId::Custom(..) => { - debug_assert!(l.get_system().is_none()); - false - } - } + l.get_system().is_some() + }, + PropertyDeclarationId::Custom(..) => { + debug_assert!(l.get_system().is_none()); + false + }, }); if is_system_font { @@ -951,7 +986,7 @@ impl PropertyDeclarationBlock { if longhand.get_system().is_some() || longhand.is_default_line_height() { current_longhands.push(longhand); if found_system.is_none() { - found_system = longhand.get_system(); + found_system = longhand.get_system(); } if importance.important() { important_count += 1; @@ -967,11 +1002,11 @@ impl PropertyDeclarationBlock { if importance.important() { important_count += 1; } - } + }, None => { contains_all_longhands = false; break; - } + }, } } @@ -994,23 +1029,30 @@ impl PropertyDeclarationBlock { // Substep 5 - Let value be the result of invoking serialize // a CSS value of current longhands. - let appendable_value = - match shorthand.get_shorthand_appendable_value(current_longhands.iter().cloned()) { - None => continue, - Some(appendable_value) => appendable_value, - }; + let appendable_value = match shorthand + .get_shorthand_appendable_value(current_longhands.iter().cloned()) + { + None => continue, + Some(appendable_value) => appendable_value, + }; // We avoid re-serializing if we're already an // AppendableValue::Css. let mut v = CssString::new(); let value = match (appendable_value, found_system) { - (AppendableValue::Css { css, with_variables }, _) => { + ( + AppendableValue::Css { + css, + with_variables, + }, + _, + ) => { debug_assert!(!css.is_empty()); AppendableValue::Css { css: css, with_variables: with_variables, } - } + }, #[cfg(feature = "gecko")] (_, Some(sys)) => { sys.to_css(&mut CssWriter::new(&mut v))?; @@ -1018,7 +1060,7 @@ impl PropertyDeclarationBlock { css: CssStringBorrow::from(&v), with_variables: false, } - } + }, (other, _) => { append_declaration_value(&mut v, other)?; @@ -1031,7 +1073,7 @@ impl PropertyDeclarationBlock { css: CssStringBorrow::from(&v), with_variables: false, } - } + }, }; // Substeps 7 and 8 @@ -1091,7 +1133,8 @@ impl PropertyDeclarationBlock { /// A convenient enum to represent different kinds of stuff that can represent a /// _value_ in the serialization of a property declaration. pub enum AppendableValue<'a, I> - where I: Iterator, +where + I: Iterator, { /// A given declaration, of which we'll serialize just the value. Declaration(&'a PropertyDeclaration), @@ -1107,14 +1150,11 @@ pub enum AppendableValue<'a, I> css: CssStringBorrow<'a>, /// Whether the original serialization contained variables or not. with_variables: bool, - } + }, } /// Potentially appends whitespace after the first (property: value;) pair. -fn handle_first_serialization( - dest: &mut W, - is_first_serialization: &mut bool, -) -> fmt::Result +fn handle_first_serialization(dest: &mut W, is_first_serialization: &mut bool) -> fmt::Result where W: Write, { @@ -1132,18 +1172,14 @@ pub fn append_declaration_value<'a, I>( appendable_value: AppendableValue<'a, I>, ) -> fmt::Result where - I: Iterator, + I: Iterator, { match appendable_value { - AppendableValue::Css { css, .. } => { - css.append_to(dest) - }, - AppendableValue::Declaration(decl) => { - decl.to_css(dest) - }, + AppendableValue::Css { css, .. } => css.append_to(dest), + AppendableValue::Declaration(decl) => decl.to_css(dest), AppendableValue::DeclarationsForShorthand(shorthand, decls) => { shorthand.longhands_to_css(decls, &mut CssWriter::new(dest)) - } + }, } } @@ -1153,10 +1189,10 @@ pub fn append_serialization<'a, I, N>( property_name: &N, appendable_value: AppendableValue<'a, I>, importance: Importance, - is_first_serialization: &mut bool + is_first_serialization: &mut bool, ) -> fmt::Result where - I: Iterator, + I: Iterator, N: ToCss, { handle_first_serialization(dest, is_first_serialization)?; @@ -1176,7 +1212,7 @@ where if !with_variables { dest.write_str(" ")? } - } + }, // Currently append_serialization is only called with a Css or // a Declaration AppendableValue. AppendableValue::DeclarationsForShorthand(..) => unreachable!(), @@ -1228,7 +1264,7 @@ pub fn parse_one_declaration_into( url_data: &UrlExtraData, error_reporter: Option<&dyn ParseErrorReporter>, parsing_mode: ParsingMode, - quirks_mode: QuirksMode + quirks_mode: QuirksMode, ) -> Result<(), ()> { let context = ParserContext::new( Origin::Author, @@ -1249,20 +1285,22 @@ pub fn parse_one_declaration_into( let mut input = ParserInput::new(input); let mut parser = Parser::new(&mut input); let start_position = parser.position(); - parser.parse_entirely(|parser| { - PropertyDeclaration::parse_into(declarations, id, &context, parser) - }).map_err(|err| { - if context.error_reporting_enabled() { - report_one_css_error( - &context, - None, - None, - err, - parser.slice_from(start_position), - property_id_for_error_reporting, - ) - } - }) + parser + .parse_entirely(|parser| { + PropertyDeclaration::parse_into(declarations, id, &context, parser) + }) + .map_err(|err| { + if context.error_reporting_enabled() { + report_one_css_error( + &context, + None, + None, + err, + parser.slice_from(start_position), + property_id_for_error_reporting, + ) + } + }) } /// A struct to parse property declarations. @@ -1273,7 +1311,6 @@ struct PropertyDeclarationParser<'a, 'b: 'a> { last_parsed_property_id: Option, } - /// Default methods reject all at rules. impl<'a, 'b, 'i> AtRuleParser<'i> for PropertyDeclarationParser<'a, 'b> { type PreludeNoBlock = (); @@ -1284,8 +1321,7 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for PropertyDeclarationParser<'a, 'b> { /// Based on NonMozillaVendorIdentifier from Gecko's CSS parser. fn is_non_mozilla_vendor_identifier(name: &str) -> bool { - (name.starts_with("-") && !name.starts_with("-moz-")) || - name.starts_with("_") + (name.starts_with("-") && !name.starts_with("-moz-")) || name.starts_with("_") } impl<'a, 'b, 'i> DeclarationParser<'i> for PropertyDeclarationParser<'a, 'b> { @@ -1301,10 +1337,8 @@ impl<'a, 'b, 'i> DeclarationParser<'i> for PropertyDeclarationParser<'a, 'b> { Ok(id) => id, Err(..) => { self.last_parsed_property_id = None; - return Err(input.new_custom_error( - StyleParseErrorKind::UnknownProperty(name) - )); - } + return Err(input.new_custom_error(StyleParseErrorKind::UnknownProperty(name))); + }, }; if self.context.error_reporting_enabled() { self.last_parsed_property_id = Some(id.clone()); @@ -1344,10 +1378,8 @@ fn report_one_css_error<'i>( fn all_properties_in_block(block: &PropertyDeclarationBlock, property: &PropertyId) -> bool { match *property { - PropertyId::LonghandAlias(id, _) | - PropertyId::Longhand(id) => block.contains(id), - PropertyId::ShorthandAlias(id, _) | - PropertyId::Shorthand(id) => { + PropertyId::LonghandAlias(id, _) | PropertyId::Longhand(id) => block.contains(id), + PropertyId::ShorthandAlias(id, _) | PropertyId::Shorthand(id) => { id.longhands().all(|longhand| block.contains(longhand)) }, // NOTE(emilio): We could do this, but it seems of limited utility, @@ -1382,7 +1414,9 @@ fn report_one_css_error<'i>( } } error = match *property { - PropertyId::Custom(ref c) => StyleParseErrorKind::new_invalid(format!("--{}", c), error), + PropertyId::Custom(ref c) => { + StyleParseErrorKind::new_invalid(format!("--{}", c), error) + }, _ => StyleParseErrorKind::new_invalid(property.non_custom_id().unwrap().name(), error), }; } @@ -1409,7 +1443,7 @@ fn report_css_errors( pub fn parse_property_declaration_list( context: &ParserContext, input: &mut Parser, - selectors: Option<&SelectorList> + selectors: Option<&SelectorList>, ) -> PropertyDeclarationBlock { let mut declarations = SourcePropertyDeclaration::new(); let mut block = PropertyDeclarationBlock::new(); @@ -1423,11 +1457,8 @@ pub fn parse_property_declaration_list( while let Some(declaration) = iter.next() { match declaration { Ok(importance) => { - block.extend( - iter.parser.declarations.drain(), - importance, - ); - } + block.extend(iter.parser.declarations.drain(), importance); + }, Err((error, slice)) => { iter.parser.declarations.clear(); @@ -1435,7 +1466,7 @@ pub fn parse_property_declaration_list( let property = iter.parser.last_parsed_property_id.take(); errors.push((error, slice, property)); } - } + }, } } From 53cd37ce39213cc49c058a4cfa576c0499a86c00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 26 Aug 2019 20:09:19 +0000 Subject: [PATCH 11/24] style: Remove old warning allow which is no longer relevant. -moz-appearance uses cbindgen since a long time ago. Differential Revision: https://phabricator.services.mozilla.com/D43472 --- components/style/properties/gecko.mako.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 7739014410e..8de2861f263 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -345,10 +345,7 @@ def set_gecko_property(ffi_name, expr): <%def name="impl_keyword_clone(ident, gecko_ffi_name, keyword, cast_type='u8')"> - // FIXME: We introduced non_upper_case_globals for -moz-appearance only - // since the prefix of Gecko value starts with ThemeWidgetType_NS_THEME. - // We should remove this after fix bug 1371809. - #[allow(non_snake_case, non_upper_case_globals)] + #[allow(non_snake_case)] pub fn clone_${ident}(&self) -> longhands::${ident}::computed_value::T { use crate::properties::longhands::${ident}::computed_value::T as Keyword; // FIXME(bholley): Align binary representations and ditch |match| for cast + static_asserts From b2386986918b0874a67b494fef7a1d0b9836f507 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 26 Aug 2019 23:09:47 +0000 Subject: [PATCH 12/24] style: Use cbindgen for clip / -moz-image-region. This also fixes some of the issues with -moz-image-region, where we just minted an auto out of the blue. Differential Revision: https://phabricator.services.mozilla.com/D43474 --- components/style/properties/gecko.mako.rs | 170 +----------------- .../style/properties/longhands/list.mako.rs | 1 + components/style/values/computed/mod.rs | 21 +-- components/style/values/generics/mod.rs | 40 ++++- components/style/values/specified/mod.rs | 14 +- 5 files changed, 52 insertions(+), 194 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 8de2861f263..f751e872f3f 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -2162,7 +2162,7 @@ fn static_assert() { <%self:impl_trait style_struct_name="List" - skip_longhands="list-style-image list-style-type -moz-image-region"> + skip_longhands="list-style-image list-style-type"> pub fn set_list_style_image(&mut self, image: longhands::list_style_image::computed_value::T) { match image { @@ -2232,178 +2232,12 @@ fn static_assert() { Either::Second(string) => T::String(string), } } - - #[allow(non_snake_case)] - pub fn set__moz_image_region(&mut self, v: longhands::_moz_image_region::computed_value::T) { - use crate::values::Either; - use crate::values::generics::length::LengthPercentageOrAuto::*; - - match v { - Either::Second(_auto) => { - self.gecko.mImageRegion.x = 0; - self.gecko.mImageRegion.y = 0; - self.gecko.mImageRegion.width = 0; - self.gecko.mImageRegion.height = 0; - } - Either::First(rect) => { - self.gecko.mImageRegion.x = match rect.left { - LengthPercentage(v) => v.to_i32_au(), - Auto => 0, - }; - self.gecko.mImageRegion.y = match rect.top { - LengthPercentage(v) => v.to_i32_au(), - Auto => 0, - }; - self.gecko.mImageRegion.height = match rect.bottom { - LengthPercentage(value) => (Au::from(value) - Au(self.gecko.mImageRegion.y)).0, - Auto => 0, - }; - self.gecko.mImageRegion.width = match rect.right { - LengthPercentage(value) => (Au::from(value) - Au(self.gecko.mImageRegion.x)).0, - Auto => 0, - }; - } - } - } - - #[allow(non_snake_case)] - pub fn clone__moz_image_region(&self) -> longhands::_moz_image_region::computed_value::T { - use crate::values::{Auto, Either}; - use crate::values::generics::length::LengthPercentageOrAuto::*; - use crate::values::computed::ClipRect; - - // There is no ideal way to detect auto type for structs::nsRect and its components, so - // if all components are zero, we use Auto. - if self.gecko.mImageRegion.x == 0 && - self.gecko.mImageRegion.y == 0 && - self.gecko.mImageRegion.width == 0 && - self.gecko.mImageRegion.height == 0 { - return Either::Second(Auto); - } - - Either::First(ClipRect { - top: LengthPercentage(Au(self.gecko.mImageRegion.y).into()), - right: LengthPercentage(Au(self.gecko.mImageRegion.width + self.gecko.mImageRegion.x).into()), - bottom: LengthPercentage(Au(self.gecko.mImageRegion.height + self.gecko.mImageRegion.y).into()), - left: LengthPercentage(Au(self.gecko.mImageRegion.x).into()), - }) - } - - ${impl_simple_copy('_moz_image_region', 'mImageRegion')} - <%self:impl_trait style_struct_name="Table"> -<%self:impl_trait style_struct_name="Effects" skip_longhands="clip"> - pub fn set_clip(&mut self, v: longhands::clip::computed_value::T) { - use crate::gecko_bindings::structs::NS_STYLE_CLIP_AUTO; - use crate::gecko_bindings::structs::NS_STYLE_CLIP_RECT; - use crate::gecko_bindings::structs::NS_STYLE_CLIP_LEFT_AUTO; - use crate::gecko_bindings::structs::NS_STYLE_CLIP_TOP_AUTO; - use crate::gecko_bindings::structs::NS_STYLE_CLIP_RIGHT_AUTO; - use crate::gecko_bindings::structs::NS_STYLE_CLIP_BOTTOM_AUTO; - use crate::values::generics::length::LengthPercentageOrAuto::*; - use crate::values::Either; - - match v { - Either::First(rect) => { - self.gecko.mClipFlags = NS_STYLE_CLIP_RECT as u8; - self.gecko.mClip.x = match rect.left { - LengthPercentage(l) => l.to_i32_au(), - Auto => { - self.gecko.mClipFlags |= NS_STYLE_CLIP_LEFT_AUTO as u8; - 0 - } - }; - - self.gecko.mClip.y = match rect.top { - LengthPercentage(l) => l.to_i32_au(), - Auto => { - self.gecko.mClipFlags |= NS_STYLE_CLIP_TOP_AUTO as u8; - 0 - } - }; - - self.gecko.mClip.height = match rect.bottom { - LengthPercentage(l) => (Au::from(l) - Au(self.gecko.mClip.y)).0, - Auto => { - self.gecko.mClipFlags |= NS_STYLE_CLIP_BOTTOM_AUTO as u8; - 1 << 30 // NS_MAXSIZE - } - }; - - self.gecko.mClip.width = match rect.right { - LengthPercentage(l) => (Au::from(l) - Au(self.gecko.mClip.x)).0, - Auto => { - self.gecko.mClipFlags |= NS_STYLE_CLIP_RIGHT_AUTO as u8; - 1 << 30 // NS_MAXSIZE - } - }; - }, - Either::Second(_auto) => { - self.gecko.mClipFlags = NS_STYLE_CLIP_AUTO as u8; - self.gecko.mClip.x = 0; - self.gecko.mClip.y = 0; - self.gecko.mClip.width = 0; - self.gecko.mClip.height = 0; - } - } - } - - pub fn copy_clip_from(&mut self, other: &Self) { - self.gecko.mClip = other.gecko.mClip; - self.gecko.mClipFlags = other.gecko.mClipFlags; - } - - pub fn reset_clip(&mut self, other: &Self) { - self.copy_clip_from(other) - } - - pub fn clone_clip(&self) -> longhands::clip::computed_value::T { - use crate::gecko_bindings::structs::NS_STYLE_CLIP_AUTO; - use crate::gecko_bindings::structs::NS_STYLE_CLIP_BOTTOM_AUTO; - use crate::gecko_bindings::structs::NS_STYLE_CLIP_LEFT_AUTO; - use crate::gecko_bindings::structs::NS_STYLE_CLIP_RIGHT_AUTO; - use crate::gecko_bindings::structs::NS_STYLE_CLIP_TOP_AUTO; - use crate::values::generics::length::LengthPercentageOrAuto::*; - use crate::values::computed::{ClipRect, ClipRectOrAuto}; - use crate::values::Either; - - if self.gecko.mClipFlags == NS_STYLE_CLIP_AUTO as u8 { - return ClipRectOrAuto::auto() - } - let left = if self.gecko.mClipFlags & NS_STYLE_CLIP_LEFT_AUTO as u8 != 0 { - debug_assert_eq!(self.gecko.mClip.x, 0); - Auto - } else { - LengthPercentage(Au(self.gecko.mClip.x).into()) - }; - - let top = if self.gecko.mClipFlags & NS_STYLE_CLIP_TOP_AUTO as u8 != 0 { - debug_assert_eq!(self.gecko.mClip.y, 0); - Auto - } else { - LengthPercentage(Au(self.gecko.mClip.y).into()) - }; - - let bottom = if self.gecko.mClipFlags & NS_STYLE_CLIP_BOTTOM_AUTO as u8 != 0 { - debug_assert_eq!(self.gecko.mClip.height, 1 << 30); // NS_MAXSIZE - Auto - } else { - LengthPercentage(Au(self.gecko.mClip.y + self.gecko.mClip.height).into()) - }; - - let right = if self.gecko.mClipFlags & NS_STYLE_CLIP_RIGHT_AUTO as u8 != 0 { - debug_assert_eq!(self.gecko.mClip.width, 1 << 30); // NS_MAXSIZE - Auto - } else { - LengthPercentage(Au(self.gecko.mClip.x + self.gecko.mClip.width).into()) - }; - - Either::First(ClipRect { top, right, bottom, left }) - } +<%self:impl_trait style_struct_name="Effects"> <%self:impl_trait style_struct_name="InheritedBox"> diff --git a/components/style/properties/longhands/list.mako.rs b/components/style/properties/longhands/list.mako.rs index 0de69c952de..202c7839d69 100644 --- a/components/style/properties/longhands/list.mako.rs +++ b/components/style/properties/longhands/list.mako.rs @@ -76,6 +76,7 @@ ${helpers.predefined_type( "ClipRectOrAuto", "computed::ClipRectOrAuto::auto()", engines="gecko", + gecko_ffi_name="mImageRegion", animation_value_type="ComputedValue", boxed=True, spec="Nonstandard (https://developer.mozilla.org/en-US/docs/Web/CSS/-moz-image-region)", diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index a565fc5f93b..aafb42b284a 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -683,11 +683,11 @@ impl From for PositiveInteger { /// A computed positive `` value or `none`. pub type PositiveIntegerOrNone = Either; -/// rect(...) -pub type ClipRect = generics::ClipRect; +/// rect(...) | auto +pub type ClipRect = generics::GenericClipRect; /// rect(...) | auto -pub type ClipRectOrAuto = Either; +pub type ClipRectOrAuto = generics::GenericClipRectOrAuto; /// The computed value of a grid `` pub type TrackBreadth = GenericTrackBreadth; @@ -707,18 +707,3 @@ pub type GridLine = GenericGridLine; /// ` | ` pub type GridTemplateComponent = GenericGridTemplateComponent; - -impl ClipRectOrAuto { - /// Return an auto (default for clip-rect and image-region) value - pub fn auto() -> Self { - Either::Second(Auto) - } - - /// Check if it is auto - pub fn is_auto(&self) -> bool { - match *self { - Either::Second(_) => true, - _ => false, - } - } -} diff --git a/components/style/values/generics/mod.rs b/components/style/values/generics/mod.rs index 343aad87694..3bc564dd385 100644 --- a/components/style/values/generics/mod.rs +++ b/components/style/values/generics/mod.rs @@ -272,9 +272,47 @@ pub struct ZeroToOne(pub T); ToShmem, )] #[css(function = "rect", comma)] -pub struct ClipRect { +#[repr(C)] +pub struct GenericClipRect { pub top: LengthOrAuto, pub right: LengthOrAuto, pub bottom: LengthOrAuto, pub left: LengthOrAuto, } + +pub use self::GenericClipRect as ClipRect; + +/// Either a clip-rect or `auto`. +#[allow(missing_docs)] +#[derive( + Animate, + Clone, + ComputeSquaredDistance, + Copy, + Debug, + MallocSizeOf, + Parse, + PartialEq, + SpecifiedValueInfo, + ToAnimatedValue, + ToAnimatedZero, + ToComputedValue, + ToCss, + ToResolvedValue, + ToShmem, +)] +#[repr(C, u8)] +pub enum GenericClipRectOrAuto { + Auto, + Rect(R), +} + +pub use self::GenericClipRectOrAuto as ClipRectOrAuto; + +impl ClipRectOrAuto { + /// Returns the `auto` value. + #[inline] + pub fn auto() -> Self { + ClipRectOrAuto::Auto + } +} diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index ddb890ba395..bb10a7e540c 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -13,7 +13,7 @@ use super::generics::grid::{GridLine as GenericGridLine, TrackBreadth as Generic use super::generics::grid::{TrackList as GenericTrackList, TrackSize as GenericTrackSize}; use super::generics::transform::IsParallelTo; use super::generics::{self, GreaterThanOrEqualToOne, NonNegative}; -use super::{Auto, CSSFloat, CSSInteger, Either, None_}; +use super::{CSSFloat, CSSInteger, Either, None_}; use crate::context::QuirksMode; use crate::parser::{Parse, ParserContext}; use crate::values::serialize_atom_identifier; @@ -639,7 +639,7 @@ pub type GridLine = GenericGridLine; pub type GridTemplateComponent = GenericGridTemplateComponent; /// rect(...) -pub type ClipRect = generics::ClipRect; +pub type ClipRect = generics::GenericClipRect; impl Parse for ClipRect { fn parse<'i, 't>( @@ -652,7 +652,7 @@ impl Parse for ClipRect { impl ClipRect { /// Parses a rect(, , , ), allowing quirks. - pub fn parse_quirky<'i, 't>( + fn parse_quirky<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, allow_quirks: AllowQuirks, @@ -696,7 +696,7 @@ impl ClipRect { } /// rect(...) | auto -pub type ClipRectOrAuto = Either; +pub type ClipRectOrAuto = generics::GenericClipRectOrAuto; impl ClipRectOrAuto { /// Parses a ClipRect or Auto, allowing quirks. @@ -706,10 +706,10 @@ impl ClipRectOrAuto { allow_quirks: AllowQuirks, ) -> Result> { if let Ok(v) = input.try(|i| ClipRect::parse_quirky(context, i, allow_quirks)) { - Ok(Either::First(v)) - } else { - Auto::parse(context, input).map(Either::Second) + return Ok(generics::GenericClipRectOrAuto::Rect(v)) } + input.expect_ident_matching("auto")?; + Ok(generics::GenericClipRectOrAuto::Auto) } } From 54ce45f3eeb7c2b3069c093cefa4730484e39463 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 26 Aug 2019 23:18:46 +0000 Subject: [PATCH 13/24] style: Update cbindgen. This cleans up the pattern of "Use a private dtor so that the helper functions do the right thing" by enabling it everywhere using: https://github.com/eqrion/cbindgen/pull/377 It also caught some uninitialized value issues. I think they're mostly harmless since we zero-initialize our structs: https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/servo/components/style/properties/gecko.mako.rs#632 And since we override the clip rect, which is the other bit of code that was failing to build with this change. Differential Revision: https://phabricator.services.mozilla.com/D43491 --- components/style/values/computed/text.rs | 2 -- components/style/values/generics/effects.rs | 2 -- components/style/values/generics/grid.rs | 8 -------- components/style/values/generics/svg.rs | 5 ----- components/style/values/generics/transform.rs | 1 - components/style/values/generics/url.rs | 2 -- components/style/values/specified/font.rs | 2 -- components/style/values/specified/list.rs | 2 -- components/style/values/specified/motion.rs | 1 - components/style/values/specified/position.rs | 2 -- components/style/values/specified/text.rs | 1 - 11 files changed, 28 deletions(-) diff --git a/components/style/values/computed/text.rs b/components/style/values/computed/text.rs index bdb2ac507bc..4cccd248d1c 100644 --- a/components/style/values/computed/text.rs +++ b/components/style/values/computed/text.rs @@ -198,8 +198,6 @@ impl TextDecorationsInEffect { } /// Computed value for the text-emphasis-style property -/// -/// cbindgen:derive-tagged-enum-copy-constructor=true #[derive(Clone, Debug, MallocSizeOf, PartialEq, ToCss, ToResolvedValue)] #[allow(missing_docs)] #[repr(C, u8)] diff --git a/components/style/values/generics/effects.rs b/components/style/values/generics/effects.rs index 724f484f283..dd9da8759be 100644 --- a/components/style/values/generics/effects.rs +++ b/components/style/values/generics/effects.rs @@ -34,8 +34,6 @@ pub struct GenericBoxShadow { pub use self::GenericBoxShadow as BoxShadow; /// A generic value for a single `filter`. -/// -/// cbindgen:derive-tagged-enum-copy-constructor=true #[cfg_attr(feature = "servo", derive(Deserialize, Serialize))] #[animation(no_bound(U))] #[derive( diff --git a/components/style/values/generics/grid.rs b/components/style/values/generics/grid.rs index b9e173ea50f..29f634d10fe 100644 --- a/components/style/values/generics/grid.rs +++ b/components/style/values/generics/grid.rs @@ -185,8 +185,6 @@ impl Parse for GridLine { /// avoid re-implementing it for the computed type. /// /// -/// -/// cbindgen:derive-tagged-enum-copy-constructor=true #[derive( Animate, Clone, @@ -230,8 +228,6 @@ impl TrackBreadth { /// generic only to avoid code bloat. It only takes `` /// /// -/// -/// cbindgen:derive-tagged-enum-copy-constructor=true #[derive( Clone, Debug, @@ -494,8 +490,6 @@ impl ToCss for TrackRepeat { } /// Track list values. Can be or -/// -/// cbindgen:derive-tagged-enum-copy-constructor=true #[derive( Animate, Clone, @@ -729,8 +723,6 @@ impl ToCss for LineNameList { } /// Variants for ` | ` -/// -/// cbindgen:derive-tagged-enum-copy-constructor=true #[derive( Animate, Clone, diff --git a/components/style/values/generics/svg.rs b/components/style/values/generics/svg.rs index 4a1fc42551d..82183c30564 100644 --- a/components/style/values/generics/svg.rs +++ b/components/style/values/generics/svg.rs @@ -9,7 +9,6 @@ use cssparser::Parser; use style_traits::ParseError; /// The fallback of an SVG paint server value. -/// cbindgen:derive-tagged-enum-copy-constructor=true #[derive( Animate, Clone, @@ -43,8 +42,6 @@ pub use self::GenericSVGPaintFallback as SVGPaintFallback; /// An SVG paint value /// /// -/// -/// cbindgen:derive-tagged-enum-copy-constructor=true #[animation(no_bound(Url))] #[derive( Animate, @@ -84,8 +81,6 @@ impl Default for SVGPaint { /// /// Whereas the spec only allows PaintServer to have a fallback, Gecko lets the /// context properties have a fallback as well. -/// -/// cbindgen:derive-tagged-enum-copy-constructor=true #[animation(no_bound(U))] #[derive( Animate, diff --git a/components/style/values/generics/transform.rs b/components/style/values/generics/transform.rs index 8dbbdfcd168..4a8dc7f63fa 100644 --- a/components/style/values/generics/transform.rs +++ b/components/style/values/generics/transform.rs @@ -151,7 +151,6 @@ fn is_same(x: &N, y: &N) -> bool { )] #[repr(C, u8)] /// A single operation in the list of a `transform` value -/// cbindgen:derive-tagged-enum-copy-constructor=true pub enum GenericTransformOperation where Angle: Zero, diff --git a/components/style/values/generics/url.rs b/components/style/values/generics/url.rs index 1f271033036..46ed453e82d 100644 --- a/components/style/values/generics/url.rs +++ b/components/style/values/generics/url.rs @@ -5,8 +5,6 @@ //! Generic types for url properties. /// An image url or none, used for example in list-style-image -/// -/// cbindgen:derive-tagged-enum-copy-constructor=true #[derive( Animate, Clone, diff --git a/components/style/values/specified/font.rs b/components/style/values/specified/font.rs index 9e680ec7758..45af8df23bf 100644 --- a/components/style/values/specified/font.rs +++ b/components/style/values/specified/font.rs @@ -1091,8 +1091,6 @@ bitflags! { )] #[repr(C, u8)] /// Set of variant alternates -/// -/// cbindgen:derive-tagged-enum-copy-constructor=true pub enum VariantAlternates { /// Enables display of stylistic alternates #[css(function)] diff --git a/components/style/values/specified/list.rs b/components/style/values/specified/list.rs index 3946d78652e..084889a3a22 100644 --- a/components/style/values/specified/list.rs +++ b/components/style/values/specified/list.rs @@ -126,8 +126,6 @@ pub struct QuoteList( /// Specified and computed `quotes` property: `auto`, `none`, or a list /// of characters. -/// -/// cbindgen:derive-tagged-enum-copy-constructor=true #[derive( Clone, Debug, diff --git a/components/style/values/specified/motion.rs b/components/style/values/specified/motion.rs index 97609ea20b0..8d6f7809fdb 100644 --- a/components/style/values/specified/motion.rs +++ b/components/style/values/specified/motion.rs @@ -15,7 +15,6 @@ use style_traits::{ParseError, StyleParseErrorKind}; /// The offset-path value. /// /// https://drafts.fxtf.org/motion-1/#offset-path-property -/// cbindgen:derive-tagged-enum-copy-constructor=true #[derive( Animate, Clone, diff --git a/components/style/values/specified/position.rs b/components/style/values/specified/position.rs index ce3bab1047b..d3476bb9f65 100644 --- a/components/style/values/specified/position.rs +++ b/components/style/values/specified/position.rs @@ -707,8 +707,6 @@ fn is_name_code_point(c: char) -> bool { /// The syntax of this property also provides a visualization of the structure /// of the grid, making the overall layout of the grid container easier to /// understand. -/// -/// cbindgen:derive-tagged-enum-copy-constructor=true #[repr(C, u8)] #[derive( Clone, diff --git a/components/style/values/specified/text.rs b/components/style/values/specified/text.rs index ebc8e8f86ee..1505192f86f 100644 --- a/components/style/values/specified/text.rs +++ b/components/style/values/specified/text.rs @@ -132,7 +132,6 @@ impl ToComputedValue for LineHeight { } /// A generic value for the `text-overflow` property. -/// cbindgen:derive-tagged-enum-copy-constructor=true #[derive(Clone, Debug, Eq, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss, ToShmem)] #[repr(C, u8)] pub enum TextOverflowSide { From 1406ae7f39710d4c229e45eb1aea11fbadae9269 Mon Sep 17 00:00:00 2001 From: Joel Olsson Date: Thu, 29 Aug 2019 10:48:52 +0000 Subject: [PATCH 14/24] style: Media Query - Enable single and / for . Differential Revision: https://phabricator.services.mozilla.com/D41557 --- components/style/gecko/media_features.rs | 6 +-- .../style/media_queries/media_feature.rs | 4 +- .../media_queries/media_feature_expression.rs | 45 +++++++++++-------- 3 files changed, 32 insertions(+), 23 deletions(-) diff --git a/components/style/gecko/media_features.rs b/components/style/gecko/media_features.rs index a80c486a6c3..ce084c9d16b 100644 --- a/components/style/gecko/media_features.rs +++ b/components/style/gecko/media_features.rs @@ -104,7 +104,7 @@ where }; let size = get_size(device); - let value = AspectRatio(size.width.0 as u32, size.height.0 as u32); + let value = AspectRatio(size.width.0 as f32, size.height.0 as f32); RangeOrOperator::evaluate_with_query_value(range_or_operator, query_value, value) } @@ -559,7 +559,7 @@ lazy_static! { feature!( atom!("aspect-ratio"), AllowsRanges::Yes, - Evaluator::IntRatio(eval_aspect_ratio), + Evaluator::NumberRatio(eval_aspect_ratio), ParsingRequirements::empty(), ), feature!( @@ -583,7 +583,7 @@ lazy_static! { feature!( atom!("device-aspect-ratio"), AllowsRanges::Yes, - Evaluator::IntRatio(eval_device_aspect_ratio), + Evaluator::NumberRatio(eval_device_aspect_ratio), ParsingRequirements::empty(), ), feature!( diff --git a/components/style/media_queries/media_feature.rs b/components/style/media_queries/media_feature.rs index d5e015b3b07..c5251f7419f 100644 --- a/components/style/media_queries/media_feature.rs +++ b/components/style/media_queries/media_feature.rs @@ -44,8 +44,8 @@ pub enum Evaluator { Integer(MediaFeatureEvaluator), Float(MediaFeatureEvaluator), BoolInteger(MediaFeatureEvaluator), - /// An integer ratio, such as the one from device-pixel-ratio. - IntRatio(MediaFeatureEvaluator), + /// A non-negative number ratio, such as the one from device-pixel-ratio. + NumberRatio(MediaFeatureEvaluator), /// A resolution. Resolution(MediaFeatureEvaluator), /// A keyword value. diff --git a/components/style/media_queries/media_feature_expression.rs b/components/style/media_queries/media_feature_expression.rs index 98c7d59c7e1..3c106a4471b 100644 --- a/components/style/media_queries/media_feature_expression.rs +++ b/components/style/media_queries/media_feature_expression.rs @@ -16,7 +16,7 @@ use crate::parser::{Parse, ParserContext}; use crate::servo::media_queries::MEDIA_FEATURES; use crate::str::{starts_with_ignore_ascii_case, string_as_ascii_lowercase}; use crate::values::computed::{self, ToComputedValue}; -use crate::values::specified::{Integer, Length, Number, Resolution}; +use crate::values::specified::{Integer, NonNegativeNumber, Length, Number, Resolution}; use crate::values::{serialize_atom_identifier, CSSFloat}; use crate::{Atom, Zero}; use cssparser::{Parser, Token}; @@ -25,8 +25,8 @@ use std::fmt::{self, Write}; use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss}; /// An aspect ratio, with a numerator and denominator. -#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, ToShmem)] -pub struct AspectRatio(pub u32, pub u32); +#[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, ToShmem)] +pub struct AspectRatio(pub CSSFloat, pub CSSFloat); impl ToCss for AspectRatio { fn to_css(&self, dest: &mut CssWriter) -> fmt::Result @@ -41,9 +41,9 @@ impl ToCss for AspectRatio { impl PartialOrd for AspectRatio { fn partial_cmp(&self, other: &AspectRatio) -> Option { - u64::partial_cmp( - &(self.0 as u64 * other.1 as u64), - &(self.1 as u64 * other.0 as u64), + f64::partial_cmp( + &(self.0 as f64 * other.1 as f64), + &(self.1 as f64 * other.0 as f64), ) } } @@ -429,8 +429,8 @@ impl MediaFeatureExpression { eval(device, expect!(Integer).cloned(), self.range_or_operator) }, Evaluator::Float(eval) => eval(device, expect!(Float).cloned(), self.range_or_operator), - Evaluator::IntRatio(eval) => { - eval(device, expect!(IntRatio).cloned(), self.range_or_operator) + Evaluator::NumberRatio(eval) => { + eval(device, expect!(NumberRatio).cloned(), self.range_or_operator) }, Evaluator::Resolution(eval) => { let computed = expect!(Resolution).map(|specified| { @@ -456,7 +456,7 @@ impl MediaFeatureExpression { /// A value found or expected in a media expression. /// /// FIXME(emilio): How should calc() serialize in the Number / Integer / -/// BoolInteger / IntRatio case, as computed or as specified value? +/// BoolInteger / NumberRatio case, as computed or as specified value? /// /// If the first, this would need to store the relevant values. /// @@ -471,9 +471,9 @@ pub enum MediaExpressionValue { Float(CSSFloat), /// A boolean value, specified as an integer (i.e., either 0 or 1). BoolInteger(bool), - /// Two integers separated by '/', with optional whitespace on either side - /// of the '/'. - IntRatio(AspectRatio), + /// A single non-negative number or two non-negative numbers separated by '/', + /// with optional whitespace on either side of the '/'. + NumberRatio(AspectRatio), /// A resolution. Resolution(Resolution), /// An enumerated value, defined by the variant keyword table in the @@ -493,7 +493,7 @@ impl MediaExpressionValue { MediaExpressionValue::Integer(v) => v.to_css(dest), MediaExpressionValue::Float(v) => v.to_css(dest), MediaExpressionValue::BoolInteger(v) => dest.write_str(if v { "1" } else { "0" }), - MediaExpressionValue::IntRatio(ratio) => ratio.to_css(dest), + MediaExpressionValue::NumberRatio(ratio) => ratio.to_css(dest), MediaExpressionValue::Resolution(ref r) => r.to_css(dest), MediaExpressionValue::Ident(ref ident) => serialize_atom_identifier(ident, dest), MediaExpressionValue::Enumerated(value) => match for_expr.feature().evaluator { @@ -529,11 +529,20 @@ impl MediaExpressionValue { let number = Number::parse(context, input)?; MediaExpressionValue::Float(number.get()) }, - Evaluator::IntRatio(..) => { - let a = Integer::parse_positive(context, input)?; - input.expect_delim('/')?; - let b = Integer::parse_positive(context, input)?; - MediaExpressionValue::IntRatio(AspectRatio(a.value() as u32, b.value() as u32)) + Evaluator::NumberRatio(..) => { + if static_prefs::pref!("layout.css.aspect-ratio-number.enabled") { + let a = NonNegativeNumber::parse(context, input)?.0.get(); + let b = match input.try_parse(|input| input.expect_delim('/')) { + Ok(()) => NonNegativeNumber::parse(context, input)?.0.get(), + _ => 1.0, + }; + MediaExpressionValue::NumberRatio(AspectRatio(a, b)) + } else { + let a = Integer::parse_positive(context, input)?; + input.expect_delim('/')?; + let b = Integer::parse_positive(context, input)?; + MediaExpressionValue::NumberRatio(AspectRatio(a.value() as CSSFloat, b.value() as CSSFloat)) + } }, Evaluator::Resolution(..) => { MediaExpressionValue::Resolution(Resolution::parse(context, input)?) From f7a87c49b1e18803caa43662b10d21b318a81042 Mon Sep 17 00:00:00 2001 From: Mats Palmgren Date: Fri, 30 Aug 2019 00:15:37 +0000 Subject: [PATCH 15/24] style: [css-lists-3] Make 'none' invalid as a in counter()/counters(). CSSWG resolution: https://github.com/w3c/csswg-drafts/issues/4163#issuecomment-521331100 Spec: https://drafts.csswg.org/css-lists-3/#counter-functions Differential Revision: https://phabricator.services.mozilla.com/D43893 --- components/style/gecko/values.rs | 23 +++++++--------- components/style/properties/gecko.mako.rs | 27 ++++++++++++++----- .../style/properties/shorthands/list.mako.rs | 16 +++-------- components/style/values/generics/counters.rs | 6 ++--- components/style/values/generics/mod.rs | 26 +++++++----------- components/style/values/specified/counters.rs | 8 +++--- components/style/values/specified/list.rs | 20 ++++++++------ 7 files changed, 63 insertions(+), 63 deletions(-) diff --git a/components/style/gecko/values.rs b/components/style/gecko/values.rs index f0c998f8f08..b31485345b6 100644 --- a/components/style/gecko/values.rs +++ b/components/style/gecko/values.rs @@ -8,7 +8,7 @@ use crate::counter_style::{Symbol, Symbols}; use crate::gecko_bindings::structs::CounterStylePtr; -use crate::values::generics::CounterStyleOrNone; +use crate::values::generics::CounterStyle; use crate::values::Either; use crate::Atom; use app_units::Au; @@ -49,19 +49,17 @@ pub fn round_border_to_device_pixels(width: Au, au_per_device_px: Au) -> Au { } } -impl CounterStyleOrNone { +impl CounterStyle { /// Convert this counter style to a Gecko CounterStylePtr. pub fn to_gecko_value(self, gecko_value: &mut CounterStylePtr) { use crate::gecko_bindings::bindings::Gecko_SetCounterStyleToName as set_name; use crate::gecko_bindings::bindings::Gecko_SetCounterStyleToSymbols as set_symbols; match self { - CounterStyleOrNone::None => unsafe { - set_name(gecko_value, atom!("none").into_addrefed()); - }, - CounterStyleOrNone::Name(name) => unsafe { + CounterStyle::Name(name) => unsafe { + debug_assert_ne!(name.0, atom!("none")); set_name(gecko_value, name.0.into_addrefed()); }, - CounterStyleOrNone::Symbols(symbols_type, symbols) => { + CounterStyle::Symbols(symbols_type, symbols) => { let symbols: Vec<_> = symbols .0 .iter() @@ -86,7 +84,7 @@ impl CounterStyleOrNone { } } - /// Convert Gecko CounterStylePtr to CounterStyleOrNone or String. + /// Convert Gecko CounterStylePtr to CounterStyle or String. pub fn from_gecko_value(gecko_value: &CounterStylePtr) -> Either { use crate::gecko_bindings::bindings; use crate::values::generics::SymbolsType; @@ -95,11 +93,8 @@ impl CounterStyleOrNone { let name = unsafe { bindings::Gecko_CounterStyle_GetName(gecko_value) }; if !name.is_null() { let name = unsafe { Atom::from_raw(name) }; - if name == atom!("none") { - Either::First(CounterStyleOrNone::None) - } else { - Either::First(CounterStyleOrNone::Name(CustomIdent(name))) - } + debug_assert_ne!(name, atom!("none")); + Either::First(CounterStyle::Name(CustomIdent(name))) } else { let anonymous = unsafe { bindings::Gecko_CounterStyle_GetAnonymous(gecko_value).as_ref() }.unwrap(); @@ -113,7 +108,7 @@ impl CounterStyleOrNone { .iter() .map(|gecko_symbol| Symbol::String(gecko_symbol.to_string().into())) .collect(); - Either::First(CounterStyleOrNone::Symbols(symbol_type, Symbols(symbols))) + Either::First(CounterStyle::Symbols(symbol_type, Symbols(symbols))) } } } diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index f751e872f3f..526304d87ef 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -2199,10 +2199,15 @@ fn static_assert() { } pub fn set_list_style_type(&mut self, v: longhands::list_style_type::computed_value::T) { + use crate::gecko_bindings::bindings::Gecko_SetCounterStyleToName; use crate::gecko_bindings::bindings::Gecko_SetCounterStyleToString; use nsstring::{nsACString, nsCStr}; use self::longhands::list_style_type::computed_value::T; match v { + T::None => unsafe { + Gecko_SetCounterStyleToName(&mut self.gecko.mCounterStyle, + atom!("none").into_addrefed()); + } T::CounterStyle(s) => s.to_gecko_value(&mut self.gecko.mCounterStyle), T::String(s) => unsafe { Gecko_SetCounterStyleToString(&mut self.gecko.mCounterStyle, @@ -2224,9 +2229,19 @@ fn static_assert() { pub fn clone_list_style_type(&self) -> longhands::list_style_type::computed_value::T { use self::longhands::list_style_type::computed_value::T; use crate::values::Either; - use crate::values::generics::CounterStyleOrNone; + use crate::values::generics::CounterStyle; + use crate::gecko_bindings::bindings; - let result = CounterStyleOrNone::from_gecko_value(&self.gecko.mCounterStyle); + let name = unsafe { + bindings::Gecko_CounterStyle_GetName(&self.gecko.mCounterStyle) + }; + if !name.is_null() { + let name = unsafe { Atom::from_raw(name) }; + if name == atom!("none") { + return T::None; + } + } + let result = CounterStyle::from_gecko_value(&self.gecko.mCounterStyle); match result { Either::First(counter_style) => T::CounterStyle(counter_style), Either::Second(string) => T::String(string), @@ -2573,7 +2588,7 @@ clip-path pub fn set_content(&mut self, v: longhands::content::computed_value::T) { use crate::values::CustomIdent; use crate::values::generics::counters::{Content, ContentItem}; - use crate::values::generics::CounterStyleOrNone; + use crate::values::generics::CounterStyle; use crate::gecko_bindings::structs::nsStyleContentData; use crate::gecko_bindings::structs::nsStyleContentAttr; use crate::gecko_bindings::structs::StyleContentType; @@ -2594,7 +2609,7 @@ clip-path content_type: StyleContentType, name: CustomIdent, sep: &str, - style: CounterStyleOrNone, + style: CounterStyle, ) { debug_assert!(content_type == StyleContentType::Counter || content_type == StyleContentType::Counters); @@ -2724,7 +2739,7 @@ clip-path use crate::gecko_bindings::structs::StyleContentType; use crate::values::generics::counters::{Content, ContentItem}; use crate::values::{CustomIdent, Either}; - use crate::values::generics::CounterStyleOrNone; + use crate::values::generics::CounterStyle; use crate::values::specified::Attr; if self.gecko.mContents.is_empty() { @@ -2769,7 +2784,7 @@ clip-path Atom::from_raw(gecko_function.mIdent.mRawPtr) }); let style = - CounterStyleOrNone::from_gecko_value(&gecko_function.mCounterStyle); + CounterStyle::from_gecko_value(&gecko_function.mCounterStyle); let style = match style { Either::First(counter_style) => counter_style, Either::Second(_) => diff --git a/components/style/properties/shorthands/list.mako.rs b/components/style/properties/shorthands/list.mako.rs index f2e8e28b19d..8e842cd153e 100644 --- a/components/style/properties/shorthands/list.mako.rs +++ b/components/style/properties/shorthands/list.mako.rs @@ -61,31 +61,23 @@ let position = unwrap_or_initial!(list_style_position, position); - fn list_style_type_none() -> list_style_type::SpecifiedValue { - % if engine == "gecko": - use crate::values::generics::CounterStyleOrNone; - list_style_type::SpecifiedValue::CounterStyle(CounterStyleOrNone::None) - % else: - list_style_type::SpecifiedValue::None - % endif - } - // If there are two `none`s, then we can't have a type or image; if there is one `none`, // then we can't have both a type *and* an image; if there is no `none` then we're fine as // long as we parsed something. + use self::list_style_type::SpecifiedValue as ListStyleType; match (any, nones, list_style_type, image) { (true, 2, None, None) => { Ok(expanded! { list_style_position: position, list_style_image: ImageUrlOrNone::none(), - list_style_type: list_style_type_none(), + list_style_type: ListStyleType::None, }) } (true, 1, None, Some(image)) => { Ok(expanded! { list_style_position: position, list_style_image: image, - list_style_type: list_style_type_none(), + list_style_type: ListStyleType::None, }) } (true, 1, Some(list_style_type), None) => { @@ -99,7 +91,7 @@ Ok(expanded! { list_style_position: position, list_style_image: ImageUrlOrNone::none(), - list_style_type: list_style_type_none(), + list_style_type: ListStyleType::None, }) } (true, 0, list_style_type, image) => { diff --git a/components/style/values/generics/counters.rs b/components/style/values/generics/counters.rs index fbb6927b9f1..9f3135d867d 100644 --- a/components/style/values/generics/counters.rs +++ b/components/style/values/generics/counters.rs @@ -7,7 +7,7 @@ #[cfg(feature = "servo")] use crate::computed_values::list_style_type::T as ListStyleType; #[cfg(feature = "gecko")] -use crate::values::generics::CounterStyleOrNone; +use crate::values::generics::CounterStyle; #[cfg(feature = "gecko")] use crate::values::specified::Attr; use crate::values::CustomIdent; @@ -127,7 +127,7 @@ impl Counters { type CounterStyleType = ListStyleType; #[cfg(feature = "gecko")] -type CounterStyleType = CounterStyleOrNone; +type CounterStyleType = CounterStyle; #[cfg(feature = "servo")] #[inline] @@ -138,7 +138,7 @@ fn is_decimal(counter_type: &CounterStyleType) -> bool { #[cfg(feature = "gecko")] #[inline] fn is_decimal(counter_type: &CounterStyleType) -> bool { - *counter_type == CounterStyleOrNone::decimal() + *counter_type == CounterStyle::decimal() } /// The specified value for the `content` property. diff --git a/components/style/values/generics/mod.rs b/components/style/values/generics/mod.rs index 3bc564dd385..933ad38f7db 100644 --- a/components/style/values/generics/mod.rs +++ b/components/style/values/generics/mod.rs @@ -92,13 +92,10 @@ impl SymbolsType { /// /// -/// Since wherever is used, 'none' is a valid value as -/// well, we combine them into one type to make code simpler. +/// Note that 'none' is not a valid name. #[cfg_attr(feature = "gecko", derive(MallocSizeOf))] #[derive(Clone, Debug, Eq, PartialEq, ToComputedValue, ToCss, ToResolvedValue, ToShmem)] -pub enum CounterStyleOrNone { - /// `none` - None, +pub enum CounterStyle { /// `` Name(CustomIdent), /// `symbols()` @@ -111,28 +108,25 @@ fn is_symbolic(symbols_type: &SymbolsType) -> bool { *symbols_type == SymbolsType::Symbolic } -impl CounterStyleOrNone { +impl CounterStyle { /// disc value pub fn disc() -> Self { - CounterStyleOrNone::Name(CustomIdent(atom!("disc"))) + CounterStyle::Name(CustomIdent(atom!("disc"))) } /// decimal value pub fn decimal() -> Self { - CounterStyleOrNone::Name(CustomIdent(atom!("decimal"))) + CounterStyle::Name(CustomIdent(atom!("decimal"))) } } -impl Parse for CounterStyleOrNone { +impl Parse for CounterStyle { fn parse<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { if let Ok(name) = input.try(|i| parse_counter_style_name(i)) { - return Ok(CounterStyleOrNone::Name(name)); - } - if input.try(|i| i.expect_ident_matching("none")).is_ok() { - return Ok(CounterStyleOrNone::None); + return Ok(CounterStyle::Name(name)); } input.expect_function_matching("symbols")?; input.parse_nested_block(|input| { @@ -151,12 +145,12 @@ impl Parse for CounterStyleOrNone { if symbols.0.iter().any(|sym| !sym.is_allowed_in_symbols()) { return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)); } - Ok(CounterStyleOrNone::Symbols(symbols_type, symbols)) + Ok(CounterStyle::Symbols(symbols_type, symbols)) }) } } -impl SpecifiedValueInfo for CounterStyleOrNone { +impl SpecifiedValueInfo for CounterStyle { fn collect_completion_keywords(f: KeywordsCollectFn) { // XXX The best approach for implementing this is probably // having a CounterStyleName type wrapping CustomIdent, and @@ -165,7 +159,7 @@ impl SpecifiedValueInfo for CounterStyleOrNone { // approach here. macro_rules! predefined { ($($name:expr,)+) => { - f(&["none", "symbols", $($name,)+]); + f(&["symbols", $($name,)+]); } } include!("../../counter_style/predefined.rs"); diff --git a/components/style/values/specified/counters.rs b/components/style/values/specified/counters.rs index 262e7765a4a..98e9a27752c 100644 --- a/components/style/values/specified/counters.rs +++ b/components/style/values/specified/counters.rs @@ -12,7 +12,7 @@ use crate::values::generics::counters::CounterIncrement as GenericCounterIncreme use crate::values::generics::counters::CounterPair; use crate::values::generics::counters::CounterSetOrReset as GenericCounterSetOrReset; #[cfg(feature = "gecko")] -use crate::values::generics::CounterStyleOrNone; +use crate::values::generics::CounterStyle; use crate::values::specified::url::SpecifiedImageUrl; #[cfg(feature = "gecko")] use crate::values::specified::Attr; @@ -98,13 +98,13 @@ impl Content { } #[cfg(feature = "gecko")] - fn parse_counter_style(context: &ParserContext, input: &mut Parser) -> CounterStyleOrNone { + fn parse_counter_style(context: &ParserContext, input: &mut Parser) -> CounterStyle { input .try(|input| { input.expect_comma()?; - CounterStyleOrNone::parse(context, input) + CounterStyle::parse(context, input) }) - .unwrap_or(CounterStyleOrNone::decimal()) + .unwrap_or(CounterStyle::decimal()) } } diff --git a/components/style/values/specified/list.rs b/components/style/values/specified/list.rs index 084889a3a22..1c93f8e8a58 100644 --- a/components/style/values/specified/list.rs +++ b/components/style/values/specified/list.rs @@ -6,7 +6,7 @@ use crate::parser::{Parse, ParserContext}; #[cfg(feature = "gecko")] -use crate::values::generics::CounterStyleOrNone; +use crate::values::generics::CounterStyle; #[cfg(feature = "gecko")] use crate::values::CustomIdent; use cssparser::{Parser, Token}; @@ -27,8 +27,10 @@ use style_traits::{ParseError, StyleParseErrorKind}; ToShmem, )] pub enum ListStyleType { - /// | none - CounterStyle(CounterStyleOrNone), + /// `none` + None, + /// + CounterStyle(CounterStyle), /// String(String), } @@ -38,7 +40,7 @@ impl ListStyleType { /// Initial specified value for `list-style-type`. #[inline] pub fn disc() -> Self { - ListStyleType::CounterStyle(CounterStyleOrNone::disc()) + ListStyleType::CounterStyle(CounterStyle::disc()) } /// Convert from gecko keyword to list-style-type. @@ -50,10 +52,10 @@ impl ListStyleType { use crate::gecko_bindings::structs; if value == structs::NS_STYLE_LIST_STYLE_NONE { - return ListStyleType::CounterStyle(CounterStyleOrNone::None); + return ListStyleType::None; } - ListStyleType::CounterStyle(CounterStyleOrNone::Name(CustomIdent(match value { + ListStyleType::CounterStyle(CounterStyle::Name(CustomIdent(match value { structs::NS_STYLE_LIST_STYLE_DISC => atom!("disc"), structs::NS_STYLE_LIST_STYLE_CIRCLE => atom!("circle"), structs::NS_STYLE_LIST_STYLE_SQUARE => atom!("square"), @@ -73,10 +75,12 @@ impl Parse for ListStyleType { context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { - if let Ok(style) = input.try(|i| CounterStyleOrNone::parse(context, i)) { + if let Ok(style) = input.try(|i| CounterStyle::parse(context, i)) { return Ok(ListStyleType::CounterStyle(style)); } - + if input.try(|i| i.expect_ident_matching("none")).is_ok() { + return Ok(ListStyleType::None); + } Ok(ListStyleType::String( input.expect_string()?.as_ref().to_owned(), )) From c3a3bd48d785172009ff570f123185243fa541dd Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Thu, 29 Aug 2019 23:40:13 +0000 Subject: [PATCH 16/24] style: Support css use counters for unimplemented properties. For developing properties, we will handle them in an other bug. Besides, I use an iframe for the test because we create a use counter in the constructor of Document, which use the prefs to decide what kind of properties we want to record. So, in the test, we have to reload iframe to make sure we re-create the document, so does the use counter, to make sure the prefs work properly. The two prefs affect the css use counters: 1. layout.css.use-counters.enabled: Allocate use counters, and record non-custom properties. 2. layout.css.use-counters-unimplemented.enabled: Record all unimplmented properties into the use counters. If we disable layout.css.use-counters.enblaed, we don't create use counters object, so layout.css.use-counters-unimplemented.enabled doesn't work, either. Differential Revision: https://phabricator.services.mozilla.com/D43860 --- .../properties/counted_unknown_properties.py | 128 ++++++++++++++++++ components/style/properties/data.py | 8 ++ .../style/properties/properties.mako.rs | 97 +++++++++++-- components/style/use_counters/mod.rs | 89 +++++++----- 4 files changed, 278 insertions(+), 44 deletions(-) create mode 100644 components/style/properties/counted_unknown_properties.py diff --git a/components/style/properties/counted_unknown_properties.py b/components/style/properties/counted_unknown_properties.py new file mode 100644 index 00000000000..628d7463ded --- /dev/null +++ b/components/style/properties/counted_unknown_properties.py @@ -0,0 +1,128 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# 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/. + +# The following properties are under development, so they are not in this list. +# FIXME: We should handle the developing properties properly by Bug 1577358: +# "backdrop-filter", +# "text-decoration-skip-ink", +# "column-span", +# "offset-distance", +# "offset-path", +# "offset-rotate" +COUNTED_UNKNOWN_PROPERTIES = [ + "-webkit-font-smoothing", + "zoom", + "-webkit-tap-highlight-color", + "speak", + "text-size-adjust", + "-webkit-font-feature-settings", + "-webkit-user-drag", + "size", + "-webkit-clip-path", + "orphans", + "widows", + "-webkit-user-modify", + "-webkit-margin-before", + "-webkit-margin-after", + "tab-size", + "-webkit-margin-start", + "-webkit-column-break-inside", + "-webkit-padding-start", + "-webkit-margin-end", + "-webkit-box-reflect", + "-webkit-print-color-adjust", + "-webkit-mask-box-image", + "-webkit-line-break", + "-webkit-text-security", + "alignment-baseline", + "-webkit-writing-mode", + "baseline-shift", + "-webkit-hyphenate-character", + "page", + "text-underline-position", + "-webkit-highlight", + "background-repeat-x", + "-webkit-padding-end", + "background-repeat-y", + "-webkit-text-emphasis-color", + "-webkit-margin-top-collapse", + "-webkit-rtl-ordering", + "-webkit-padding-before", + "-webkit-text-decorations-in-effect", + "-webkit-border-vertical-spacing", + "-webkit-locale", + "-webkit-padding-after", + "-webkit-border-horizontal-spacing", + "color-rendering", + "-webkit-column-break-before", + "-webkit-transform-origin-x", + "-webkit-transform-origin-y", + "-webkit-text-emphasis-position", + "buffered-rendering", + "-webkit-text-orientation", + "-webkit-text-combine", + "-webkit-text-emphasis-style", + "-webkit-text-emphasis", + "d", + "-webkit-mask-box-image-width", + "-webkit-mask-box-image-source", + "-webkit-mask-box-image-outset", + "-webkit-mask-box-image-slice", + "-webkit-mask-box-image-repeat", + "-webkit-margin-after-collapse", + "-webkit-border-before-color", + "-webkit-border-before-width", + "-webkit-perspective-origin-x", + "-webkit-perspective-origin-y", + "-webkit-margin-before-collapse", + "-webkit-border-before-style", + "scroll-snap-stop", + "-webkit-margin-bottom-collapse", + "-webkit-ruby-position", + "-webkit-column-break-after", + "-webkit-margin-collapse", + "offset", + "-webkit-border-before", + "-webkit-border-end", + "-webkit-border-after", + "-webkit-border-start", + "-webkit-min-logical-width", + "-webkit-logical-height", + "-webkit-transform-origin-z", + "-webkit-font-size-delta", + "-webkit-logical-width", + "-webkit-max-logical-width", + "-webkit-min-logical-height", + "-webkit-max-logical-height", + "-webkit-border-end-color", + "-webkit-border-end-width", + "-webkit-border-start-color", + "-webkit-border-start-width", + "-webkit-border-after-color", + "-webkit-border-after-width", + "-webkit-border-end-style", + "-webkit-border-after-style", + "-webkit-border-start-style", + "-webkit-mask-repeat-x", + "-webkit-mask-repeat-y", + "user-zoom", + "min-zoom", + "-webkit-box-decoration-break", + "orientation", + "max-zoom", + "-webkit-app-region", + "-webkit-column-rule", + "-webkit-column-span", + "-webkit-column-gap", + "-webkit-shape-outside", + "-webkit-column-rule-width", + "-webkit-column-count", + "-webkit-opacity", + "-webkit-column-width", + "-webkit-shape-image-threshold", + "-webkit-column-rule-style", + "-webkit-columns", + "-webkit-column-rule-color", + "-webkit-shape-margin", +] diff --git a/components/style/properties/data.py b/components/style/properties/data.py index d1e78e6648c..7c2330e47d8 100644 --- a/components/style/properties/data.py +++ b/components/style/properties/data.py @@ -3,6 +3,7 @@ # file, You can obtain one at https://mozilla.org/MPL/2.0/. import re +from counted_unknown_properties import COUNTED_UNKNOWN_PROPERTIES PHYSICAL_SIDES = ["top", "right", "bottom", "left"] LOGICAL_SIDES = ["block-start", "block-end", "inline-start", "inline-end"] @@ -574,6 +575,7 @@ class PropertiesData(object): self.shorthands = [] self.shorthands_by_name = {} self.shorthand_aliases = [] + self.counted_unknown_properties = [CountedUnknownProperty(p) for p in COUNTED_UNKNOWN_PROPERTIES] def new_style_struct(self, *args, **kwargs): style_struct = StyleStruct(*args, **kwargs) @@ -794,3 +796,9 @@ class PropertyRestrictions: + PropertyRestrictions.shorthand(data, "background") + PropertyRestrictions.shorthand(data, "outline") + PropertyRestrictions.shorthand(data, "font")) + +class CountedUnknownProperty: + def __init__(self, name): + self.name = name + self.ident = to_rust_ident(name) + self.camel_case = to_camel_case(self.ident) diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 72ae381f5d9..a1b4cedcfed 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -38,6 +38,7 @@ use style_traits::{CssWriter, KeywordsCollectFn, ParseError, ParsingMode}; use style_traits::{SpecifiedValueInfo, StyleParseErrorKind, ToCss}; use to_shmem::impl_trivial_to_shmem; use crate::stylesheets::{CssRuleType, Origin, UrlExtraData}; +use crate::use_counters::UseCounters; use crate::values::generics::text::LineHeight; use crate::values::{computed, resolved}; use crate::values::computed::NonNegativeLength; @@ -427,6 +428,9 @@ pub struct NonCustomPropertyId(usize); pub const NON_CUSTOM_PROPERTY_ID_COUNT: usize = ${len(data.longhands) + len(data.shorthands) + len(data.all_aliases())}; +/// The length of all counted unknown properties. +pub const COUNTED_UNKNOWN_PROPERTY_COUNT: usize = ${len(data.counted_unknown_properties)}; + % if engine == "gecko": #[allow(dead_code)] unsafe fn static_assert_nscsspropertyid() { @@ -1788,6 +1792,45 @@ impl ToCss for PropertyId { } } +/// The counted unknown property list which is used for css use counters. +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] +#[repr(u8)] +pub enum CountedUnknownProperty { + % for prop in data.counted_unknown_properties: + /// ${prop.name} + ${prop.camel_case}, + % endfor +} + +impl CountedUnknownProperty { + /// Parse the counted unknown property. + pub fn parse_for_test(property_name: &str) -> Option { + ascii_case_insensitive_phf_map! { + unknown_id -> CountedUnknownProperty = { + % for property in data.counted_unknown_properties: + "${property.name}" => CountedUnknownProperty::${property.camel_case}, + % endfor + } + } + unknown_id(property_name).cloned() + } + + /// Returns the underlying index, used for use counter. + pub fn bit(self) -> usize { + self as usize + } +} + +#[cfg(feature = "gecko")] +fn is_counted_unknown_use_counters_enabled() -> bool { + static_prefs::pref!("layout.css.use-counters-unimplemented.enabled") +} + +#[cfg(feature = "servo")] +fn is_counted_unknown_use_counters_enabled() -> bool { + false +} + impl PropertyId { /// Return the longhand id that this property id represents. #[inline] @@ -1801,16 +1844,30 @@ impl PropertyId { /// Returns a given property from the string `s`. /// - /// Returns Err(()) for unknown non-custom properties. - fn parse_unchecked(property_name: &str) -> Result { + /// Returns Err(()) for unknown properties. + fn parse_unchecked( + property_name: &str, + use_counters: Option< &UseCounters>, + ) -> Result { + // A special id for css use counters. + // ShorthandAlias is not used in the Servo build. + // That's why we need to allow dead_code. + #[allow(dead_code)] + pub enum StaticId { + Longhand(LonghandId), + Shorthand(ShorthandId), + LonghandAlias(LonghandId, AliasId), + ShorthandAlias(ShorthandId, AliasId), + CountedUnknown(CountedUnknownProperty), + } ascii_case_insensitive_phf_map! { - property_id -> PropertyId = { + static_id -> StaticId = { % for (kind, properties) in [("Longhand", data.longhands), ("Shorthand", data.shorthands)]: % for property in properties: - "${property.name}" => PropertyId::${kind}(${kind}Id::${property.camel_case}), + "${property.name}" => StaticId::${kind}(${kind}Id::${property.camel_case}), % for alias in property.alias: "${alias.name}" => { - PropertyId::${kind}Alias( + StaticId::${kind}Alias( ${kind}Id::${property.camel_case}, AliasId::${alias.camel_case}, ) @@ -1818,11 +1875,31 @@ impl PropertyId { % endfor % endfor % endfor + % for property in data.counted_unknown_properties: + "${property.name}" => { + StaticId::CountedUnknown(CountedUnknownProperty::${property.camel_case}) + }, + % endfor } } - if let Some(id) = property_id(property_name) { - return Ok(id.clone()) + if let Some(id) = static_id(property_name) { + return Ok(match *id { + StaticId::Longhand(id) => PropertyId::Longhand(id), + StaticId::Shorthand(id) => PropertyId::Shorthand(id), + StaticId::LonghandAlias(id, alias) => PropertyId::LonghandAlias(id, alias), + StaticId::ShorthandAlias(id, alias) => PropertyId::ShorthandAlias(id, alias), + StaticId::CountedUnknown(unknown_prop) => { + if is_counted_unknown_use_counters_enabled() { + if let Some(counters) = use_counters { + counters.counted_unknown_properties.record(unknown_prop); + } + } + + // Always return Err(()) because these aren't valid custom property names. + return Err(()); + } + }); } let name = crate::custom_properties::parse_name(property_name)?; @@ -1833,7 +1910,7 @@ impl PropertyId { /// enabled for all content. #[inline] pub fn parse_enabled_for_all_content(name: &str) -> Result { - let id = Self::parse_unchecked(name)?; + let id = Self::parse_unchecked(name, None)?; if !id.enabled_for_all_content() { return Err(()); @@ -1847,7 +1924,7 @@ impl PropertyId { /// allowed in this context. #[inline] pub fn parse(name: &str, context: &ParserContext) -> Result { - let id = Self::parse_unchecked(name)?; + let id = Self::parse_unchecked(name, context.use_counters)?; if !id.allowed_in(context) { return Err(()); @@ -1865,7 +1942,7 @@ impl PropertyId { name: &str, context: &ParserContext, ) -> Result { - let id = Self::parse_unchecked(name)?; + let id = Self::parse_unchecked(name, None)?; if !id.allowed_in_ignoring_rule_type(context) { return Err(()); diff --git a/components/style/use_counters/mod.rs b/components/style/use_counters/mod.rs index c9a4b449ea0..bb8e92594a6 100644 --- a/components/style/use_counters/mod.rs +++ b/components/style/use_counters/mod.rs @@ -6,6 +6,7 @@ #[cfg(feature = "gecko")] use crate::gecko_bindings::sugar::ownership::{HasBoxFFI, HasFFI, HasSimpleFFI}; +use crate::properties::{CountedUnknownProperty, COUNTED_UNKNOWN_PROPERTY_COUNT}; use crate::properties::{NonCustomPropertyId, NON_CUSTOM_PROPERTY_ID_COUNT}; use std::cell::Cell; @@ -15,46 +16,62 @@ const BITS_PER_ENTRY: usize = 64; #[cfg(target_pointer_width = "32")] const BITS_PER_ENTRY: usize = 32; +/// One bit per each non-custom CSS property. +#[derive(Default)] +pub struct CountedUnknownPropertyUseCounters { + storage: [Cell; (COUNTED_UNKNOWN_PROPERTY_COUNT - 1 + BITS_PER_ENTRY) / BITS_PER_ENTRY], +} + /// One bit per each non-custom CSS property. #[derive(Default)] pub struct NonCustomPropertyUseCounters { storage: [Cell; (NON_CUSTOM_PROPERTY_ID_COUNT - 1 + BITS_PER_ENTRY) / BITS_PER_ENTRY], } -impl NonCustomPropertyUseCounters { - /// Returns the bucket a given property belongs in, and the bitmask for that - /// property. - #[inline(always)] - fn bucket_and_pattern(id: NonCustomPropertyId) -> (usize, usize) { - let bit = id.bit(); - let bucket = bit / BITS_PER_ENTRY; - let bit_in_bucket = bit % BITS_PER_ENTRY; - (bucket, 1 << bit_in_bucket) - } - - /// Record that a given non-custom property ID has been parsed. - #[inline] - pub fn record(&self, id: NonCustomPropertyId) { - let (bucket, pattern) = Self::bucket_and_pattern(id); - let bucket = &self.storage[bucket]; - bucket.set(bucket.get() | pattern) - } - - /// Returns whether a given non-custom property ID has been recorded - /// earlier. - #[inline] - pub fn recorded(&self, id: NonCustomPropertyId) -> bool { - let (bucket, pattern) = Self::bucket_and_pattern(id); - self.storage[bucket].get() & pattern != 0 - } - - /// Merge `other` into `self`. - #[inline] - fn merge(&self, other: &Self) { - for (bucket, other_bucket) in self.storage.iter().zip(other.storage.iter()) { - bucket.set(bucket.get() | other_bucket.get()) +macro_rules! property_use_counters_methods { + ($id: ident) => { + /// Returns the bucket a given property belongs in, and the bitmask for that + /// property. + #[inline(always)] + fn bucket_and_pattern(id: $id) -> (usize, usize) { + let bit = id.bit(); + let bucket = bit / BITS_PER_ENTRY; + let bit_in_bucket = bit % BITS_PER_ENTRY; + (bucket, 1 << bit_in_bucket) } - } + + /// Record that a given property ID has been parsed. + #[inline] + pub fn record(&self, id: $id) { + let (bucket, pattern) = Self::bucket_and_pattern(id); + let bucket = &self.storage[bucket]; + bucket.set(bucket.get() | pattern) + } + + /// Returns whether a given property ID has been recorded + /// earlier. + #[inline] + pub fn recorded(&self, id: $id) -> bool { + let (bucket, pattern) = Self::bucket_and_pattern(id); + self.storage[bucket].get() & pattern != 0 + } + + /// Merge `other` into `self`. + #[inline] + fn merge(&self, other: &Self) { + for (bucket, other_bucket) in self.storage.iter().zip(other.storage.iter()) { + bucket.set(bucket.get() | other_bucket.get()) + } + } + }; +} + +impl CountedUnknownPropertyUseCounters { + property_use_counters_methods!(CountedUnknownProperty); +} + +impl NonCustomPropertyUseCounters { + property_use_counters_methods!(NonCustomPropertyId); } /// The use-counter data related to a given document we want to store. @@ -63,6 +80,8 @@ pub struct UseCounters { /// The counters for non-custom properties that have been parsed in the /// document's stylesheets. pub non_custom_properties: NonCustomPropertyUseCounters, + /// The counters for css properties which we haven't implemented yet. + pub counted_unknown_properties: CountedUnknownPropertyUseCounters, } impl UseCounters { @@ -72,7 +91,9 @@ impl UseCounters { #[inline] pub fn merge(&self, other: &Self) { self.non_custom_properties - .merge(&other.non_custom_properties) + .merge(&other.non_custom_properties); + self.counted_unknown_properties + .merge(&other.counted_unknown_properties); } } From c7add815174831975ebfec3d65536c8ea5e1d9ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 30 Aug 2019 14:35:34 +0000 Subject: [PATCH 17/24] style: Account for user stylesheets for Shadow DOM invalidation. Differential Revision: https://phabricator.services.mozilla.com/D43992 --- components/style/invalidation/element/state_and_attributes.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/style/invalidation/element/state_and_attributes.rs b/components/style/invalidation/element/state_and_attributes.rs index eccc5f60bbf..30521fb8c00 100644 --- a/components/style/invalidation/element/state_and_attributes.rs +++ b/components/style/invalidation/element/state_and_attributes.rs @@ -16,7 +16,7 @@ use crate::invalidation::element::invalidator::{Invalidation, InvalidationProces use crate::invalidation::element::restyle_hints::RestyleHint; use crate::selector_map::SelectorMap; use crate::selector_parser::Snapshot; -use crate::stylesheets::origin::{Origin, OriginSet}; +use crate::stylesheets::origin::OriginSet; use crate::{Atom, WeakAtom}; use selectors::attr::CaseSensitivity; use selectors::matching::matches_selector; @@ -246,7 +246,7 @@ where }; let document_origins = if !matches_document_author_rules { - Origin::UserAgent.into() + OriginSet::ORIGIN_USER_AGENT | OriginSet::ORIGIN_USER } else { OriginSet::all() }; From 3fcd23dcdf0c8f8e32a1ae0b0eafd7640f63b10b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Sep 2019 11:55:28 +0200 Subject: [PATCH 18/24] style: Add a comment about a Send bound that may look confusing. --- components/style/scoped_tls.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/components/style/scoped_tls.rs b/components/style/scoped_tls.rs index b4eed7509c9..e1e5209a058 100644 --- a/components/style/scoped_tls.rs +++ b/components/style/scoped_tls.rs @@ -15,6 +15,9 @@ use std::ops::DerefMut; /// /// We use this on Servo to construct thread-local contexts, but clear them once /// we're done with restyling. +/// +/// Note that the cleanup is done on the thread that owns the scoped TLS, thus +/// the Send bound. pub struct ScopedTLS<'scope, T: Send> { pool: &'scope rayon::ThreadPool, slots: Box<[RefCell>]>, From 987a1eeb62c218fddd6d5ddfdfcaba016db8c150 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 2 Sep 2019 23:11:26 +0000 Subject: [PATCH 19/24] style: Use cbindgen for counters. Differential Revision: https://phabricator.services.mozilla.com/D44403 --- components/style/properties/gecko.mako.rs | 48 +------------------ components/style/values/generics/counters.rs | 30 ++++++------ components/style/values/specified/counters.rs | 4 +- 3 files changed, 17 insertions(+), 65 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 526304d87ef..97321fcbd38 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -2579,8 +2579,7 @@ clip-path ${impl_simple('column_rule_style', 'mColumnRuleStyle')} -<%self:impl_trait style_struct_name="Counters" - skip_longhands="content counter-increment counter-reset counter-set"> +<%self:impl_trait style_struct_name="Counters" skip_longhands="content"> pub fn ineffective_content_property(&self) -> bool { self.gecko.mContents.is_empty() } @@ -2811,51 +2810,6 @@ clip-path }).collect::>().into_boxed_slice() ) } - - % for counter_property in ["Increment", "Reset", "Set"]: - pub fn set_counter_${counter_property.lower()}( - &mut self, - v: longhands::counter_${counter_property.lower()}::computed_value::T - ) { - unsafe { - bindings::Gecko_ClearAndResizeCounter${counter_property}s(&mut *self.gecko, v.len() as u32); - for (i, pair) in v.0.into_vec().into_iter().enumerate() { - self.gecko.m${counter_property}s[i].mCounter.set_move( - RefPtr::from_addrefed(pair.name.0.into_addrefed()) - ); - self.gecko.m${counter_property}s[i].mValue = pair.value; - } - } - } - - pub fn copy_counter_${counter_property.lower()}_from(&mut self, other: &Self) { - unsafe { - bindings::Gecko_CopyCounter${counter_property}sFrom(&mut *self.gecko, &*other.gecko) - } - } - - pub fn reset_counter_${counter_property.lower()}(&mut self, other: &Self) { - self.copy_counter_${counter_property.lower()}_from(other) - } - - pub fn clone_counter_${counter_property.lower()}( - &self - ) -> longhands::counter_${counter_property.lower()}::computed_value::T { - use crate::values::generics::counters::CounterPair; - use crate::values::CustomIdent; - - longhands::counter_${counter_property.lower()}::computed_value::T::new( - self.gecko.m${counter_property}s.iter().map(|ref gecko_counter| { - CounterPair { - name: CustomIdent(unsafe { - Atom::from_raw(gecko_counter.mCounter.mRawPtr) - }), - value: gecko_counter.mValue, - } - }).collect() - ) - } - % endfor <%self:impl_trait style_struct_name="UI" skip_longhands="-moz-force-broken-image-icon"> diff --git a/components/style/values/generics/counters.rs b/components/style/values/generics/counters.rs index 9f3135d867d..391da7e33cc 100644 --- a/components/style/values/generics/counters.rs +++ b/components/style/values/generics/counters.rs @@ -25,12 +25,14 @@ use std::ops::Deref; ToResolvedValue, ToShmem, )] -pub struct CounterPair { +#[repr(C)] +pub struct GenericCounterPair { /// The name of the counter. pub name: CustomIdent, /// The value of the counter / increment / etc. pub value: Integer, } +pub use self::GenericCounterPair as CounterPair; /// A generic value for the `counter-increment` property. #[derive( @@ -45,13 +47,15 @@ pub struct CounterPair { ToResolvedValue, ToShmem, )] -pub struct CounterIncrement(pub Counters); +#[repr(transparent)] +pub struct GenericCounterIncrement(pub GenericCounters); +pub use self::GenericCounterIncrement as CounterIncrement; impl CounterIncrement { /// Returns a new value for `counter-increment`. #[inline] pub fn new(counters: Vec>) -> Self { - CounterIncrement(Counters(counters.into_boxed_slice())) + CounterIncrement(Counters(counters.into())) } } @@ -77,13 +81,15 @@ impl Deref for CounterIncrement { ToResolvedValue, ToShmem, )] -pub struct CounterSetOrReset(pub Counters); +#[repr(transparent)] +pub struct GenericCounterSetOrReset(pub GenericCounters); +pub use self::GenericCounterSetOrReset as CounterSetOrReset; impl CounterSetOrReset { /// Returns a new value for `counter-set` / `counter-reset`. #[inline] pub fn new(counters: Vec>) -> Self { - CounterSetOrReset(Counters(counters.into_boxed_slice())) + CounterSetOrReset(Counters(counters.into())) } } @@ -111,17 +117,9 @@ impl Deref for CounterSetOrReset { ToResolvedValue, ToShmem, )] -pub struct Counters(#[css(iterable, if_empty = "none")] Box<[CounterPair]>); - -impl Counters { - /// Move out the Box into a vector. This could just return the Box<>, but - /// Vec<> is a bit more convenient because Box<[T]> doesn't implement - /// IntoIter: https://github.com/rust-lang/rust/issues/59878 - #[inline] - pub fn into_vec(self) -> Vec> { - self.0.into_vec() - } -} +#[repr(transparent)] +pub struct GenericCounters(#[css(iterable, if_empty = "none")] crate::OwnedSlice>); +pub use self::GenericCounters as Counters; #[cfg(feature = "servo")] type CounterStyleType = ListStyleType; diff --git a/components/style/values/specified/counters.rs b/components/style/values/specified/counters.rs index 98e9a27752c..39ca07e3563 100644 --- a/components/style/values/specified/counters.rs +++ b/components/style/values/specified/counters.rs @@ -8,9 +8,9 @@ use crate::computed_values::list_style_type::T as ListStyleType; use crate::parser::{Parse, ParserContext}; use crate::values::generics::counters as generics; -use crate::values::generics::counters::CounterIncrement as GenericCounterIncrement; +use crate::values::generics::counters::GenericCounterIncrement; use crate::values::generics::counters::CounterPair; -use crate::values::generics::counters::CounterSetOrReset as GenericCounterSetOrReset; +use crate::values::generics::counters::GenericCounterSetOrReset; #[cfg(feature = "gecko")] use crate::values::generics::CounterStyle; use crate::values::specified::url::SpecifiedImageUrl; From 3c2e4d1e98dca2e7be4bf52252326911d11b5d81 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Mon, 9 Sep 2019 04:49:55 +0000 Subject: [PATCH 20/24] style: Align the Rust and C++ representations of WritingMode. Differential Revision: https://phabricator.services.mozilla.com/D44412 --- components/style/logical_geometry.rs | 103 ++++++++++++++++++++------- components/style/style_adjuster.rs | 3 + 2 files changed, 81 insertions(+), 25 deletions(-) diff --git a/components/style/logical_geometry.rs b/components/style/logical_geometry.rs index 3817c12dcf0..38ffb25a740 100644 --- a/components/style/logical_geometry.rs +++ b/components/style/logical_geometry.rs @@ -26,16 +26,48 @@ pub enum InlineBaseDirection { // TODO: improve the readability of the WritingMode serialization, refer to the Debug:fmt() bitflags!( #[cfg_attr(feature = "servo", derive(MallocSizeOf, Serialize))] + #[repr(C)] pub struct WritingMode: u8 { - const RTL = 1 << 0; - const VERTICAL = 1 << 1; + /// A vertical writing mode; writing-mode is vertical-rl, + /// vertical-lr, sideways-lr, or sideways-rl. + const VERTICAL = 1 << 0; + /// The inline flow direction is reversed against the physical + /// direction (i.e. right-to-left or bottom-to-top); writing-mode is + /// sideways-lr or direction is rtl (but not both). + /// + /// (This bit can be derived from the others, but we store it for + /// convenience.) + const INLINE_REVERSED = 1 << 1; + /// A vertical writing mode whose block progression direction is left- + /// to-right; writing-mode is vertical-lr or sideways-lr. + /// + /// Never set without VERTICAL. const VERTICAL_LR = 1 << 2; - /// For vertical writing modes only. When set, line-over/line-under - /// sides are inverted from block-start/block-end. This flag is - /// set when sideways-lr is used. + /// The line-over/line-under sides are inverted with respect to the + /// block-start/block-end edge; writing-mode is vertical-lr. + /// + /// Never set without VERTICAL and VERTICAL_LR. const LINE_INVERTED = 1 << 3; - const SIDEWAYS = 1 << 4; - const UPRIGHT = 1 << 5; + /// direction is rtl. + const RTL = 1 << 4; + /// Horizontal text within a vertical writing mode is displayed sideways + /// and runs top-to-bottom or bottom-to-top; set in these cases: + /// + /// * writing-mode: vertical-rl; text-orientation: sideways; + /// * writing-mode: vertical-lr; text-orientation: sideways; + /// * writing-mode: sideways-rl; + /// * writing-mode: sideways-lr; + /// + /// Never set without VERTICAL. + const SIDEWAYS = 1 << 5; + /// Horizontal text within a vertical writing mode is displayed with each + /// glyph upright; set in these cases: + /// + /// * writing-mode: vertical-rl; text-orientation: upright; + /// * writing-mode: vertical-lr: text-orientation: upright; + /// + /// Never set without VERTICAL. + const UPRIGHT = 1 << 6; } ); @@ -47,33 +79,52 @@ impl WritingMode { let mut flags = WritingMode::empty(); - match inheritedbox_style.clone_direction() { + let direction = inheritedbox_style.clone_direction(); + let writing_mode = inheritedbox_style.clone_writing_mode(); + + match direction { Direction::Ltr => {}, Direction::Rtl => { flags.insert(WritingMode::RTL); }, } - match inheritedbox_style.clone_writing_mode() { - SpecifiedWritingMode::HorizontalTb => {}, + match writing_mode { + SpecifiedWritingMode::HorizontalTb => { + if direction == Direction::Rtl { + flags.insert(WritingMode::INLINE_REVERSED); + } + }, SpecifiedWritingMode::VerticalRl => { flags.insert(WritingMode::VERTICAL); + if direction == Direction::Rtl { + flags.insert(WritingMode::INLINE_REVERSED); + } }, SpecifiedWritingMode::VerticalLr => { flags.insert(WritingMode::VERTICAL); flags.insert(WritingMode::VERTICAL_LR); + flags.insert(WritingMode::LINE_INVERTED); + if direction == Direction::Rtl { + flags.insert(WritingMode::INLINE_REVERSED); + } }, #[cfg(feature = "gecko")] SpecifiedWritingMode::SidewaysRl => { flags.insert(WritingMode::VERTICAL); flags.insert(WritingMode::SIDEWAYS); + if direction == Direction::Rtl { + flags.insert(WritingMode::INLINE_REVERSED); + } }, #[cfg(feature = "gecko")] SpecifiedWritingMode::SidewaysLr => { flags.insert(WritingMode::VERTICAL); flags.insert(WritingMode::VERTICAL_LR); - flags.insert(WritingMode::LINE_INVERTED); flags.insert(WritingMode::SIDEWAYS); + if direction == Direction::Ltr { + flags.insert(WritingMode::INLINE_REVERSED); + } }, } @@ -81,19 +132,21 @@ impl WritingMode { { use crate::properties::longhands::text_orientation::computed_value::T as TextOrientation; - // If FLAG_SIDEWAYS is already set, this means writing-mode is - // either sideways-rl or sideways-lr, and for both of these values, - // text-orientation has no effect. - if !flags.intersects(WritingMode::SIDEWAYS) { - match inheritedbox_style.clone_text_orientation() { - TextOrientation::Mixed => {}, - TextOrientation::Upright => { - flags.insert(WritingMode::UPRIGHT); - }, - TextOrientation::Sideways => { - flags.insert(WritingMode::SIDEWAYS); - }, - } + // text-orientation only has an effect for vertical-rl and + // vertical-lr values of writing-mode. + match writing_mode { + SpecifiedWritingMode::VerticalRl | SpecifiedWritingMode::VerticalLr => { + match inheritedbox_style.clone_text_orientation() { + TextOrientation::Mixed => {}, + TextOrientation::Upright => { + flags.insert(WritingMode::UPRIGHT); + }, + TextOrientation::Sideways => { + flags.insert(WritingMode::SIDEWAYS); + }, + } + }, + _ => {}, } } @@ -115,7 +168,7 @@ impl WritingMode { #[inline] pub fn is_inline_tb(&self) -> bool { // https://drafts.csswg.org/css-writing-modes-3/#logical-to-physical - self.intersects(WritingMode::RTL) == self.intersects(WritingMode::LINE_INVERTED) + !self.intersects(WritingMode::INLINE_REVERSED) } #[inline] diff --git a/components/style/style_adjuster.rs b/components/style/style_adjuster.rs index 577aa1a0267..9f274917b08 100644 --- a/components/style/style_adjuster.rs +++ b/components/style/style_adjuster.rs @@ -283,6 +283,7 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { fn adjust_for_text_combine_upright(&mut self) { use crate::computed_values::text_combine_upright::T as TextCombineUpright; use crate::computed_values::writing_mode::T as WritingMode; + use crate::logical_geometry; let writing_mode = self.style.get_inherited_box().clone_writing_mode(); let text_combine_upright = self.style.get_inherited_text().clone_text_combine_upright(); @@ -294,6 +295,8 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { self.style .mutate_inherited_box() .set_writing_mode(WritingMode::HorizontalTb); + self.style.writing_mode = + logical_geometry::WritingMode::new(self.style.get_inherited_box()); } } From eb8195dfa4c700a62d9dec62282841e06c0edb02 Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Tue, 10 Sep 2019 21:51:54 +0000 Subject: [PATCH 21/24] style: Tweak the serialization of text-decoration. We are trying to not serialize `text-decoration-line: none` if there are other non-default values for the longhands. Differential Revision: https://phabricator.services.mozilla.com/D44908 --- .../style/properties/shorthands/text.mako.rs | 44 ++++++++++++++----- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/components/style/properties/shorthands/text.mako.rs b/components/style/properties/shorthands/text.mako.rs index 33b4a074892..77b15d9a722 100644 --- a/components/style/properties/shorthands/text.mako.rs +++ b/components/style/properties/shorthands/text.mako.rs @@ -74,25 +74,49 @@ impl<'a> ToCss for LonghandsToSerialize<'a> { fn to_css(&self, dest: &mut CssWriter) -> fmt::Result where W: fmt::Write { - self.text_decoration_line.to_css(dest)?; + use crate::values::specified::TextDecorationLine; + + let (is_solid_style, is_current_color, is_auto_thickness) = + ( + % if engine == "gecko": + *self.text_decoration_style == text_decoration_style::SpecifiedValue::Solid, + *self.text_decoration_color == specified::Color::CurrentColor, + self.text_decoration_thickness.map_or(true, |t| t.is_auto()) + % else: + true, true, true + % endif + ); + + let mut has_value = false; + let is_none = *self.text_decoration_line == TextDecorationLine::none(); + if (is_solid_style && is_current_color && is_auto_thickness) || !is_none { + self.text_decoration_line.to_css(dest)?; + has_value = true; + } % if engine == "gecko": - if *self.text_decoration_style != text_decoration_style::SpecifiedValue::Solid { + if !is_solid_style { + if has_value { dest.write_str(" ")?; - self.text_decoration_style.to_css(dest)?; } + self.text_decoration_style.to_css(dest)?; + has_value = true; + } - if *self.text_decoration_color != specified::Color::CurrentColor { + if !is_current_color { + if has_value { dest.write_str(" ")?; - self.text_decoration_color.to_css(dest)?; } + self.text_decoration_color.to_css(dest)?; + has_value = true; + } - if let Some(text_decoration_thickness) = self.text_decoration_thickness { - if !text_decoration_thickness.is_auto() { - dest.write_str(" ")?; - self.text_decoration_thickness.to_css(dest)?; - } + if !is_auto_thickness { + if has_value { + dest.write_str(" ")?; } + self.text_decoration_thickness.to_css(dest)?; + } % endif Ok(()) From d54d1bcb17108710e4339952f71b0e989995f642 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Mon, 9 Sep 2019 17:23:20 +0000 Subject: [PATCH 22/24] style: Remove duplicated logic from stylo's Gecko wrapper. Stylo's Gecko wrapper duplicated some logic from the C++ side so that the Rust compiler would be able to optimize better. Now that we have xLTO, this kind of manual inlining should not be neccessary anymore. Differential Revision: https://phabricator.services.mozilla.com/D43765 --- components/style/gecko/wrapper.rs | 50 ++----------------------------- 1 file changed, 2 insertions(+), 48 deletions(-) diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 39e3678eebc..e2ae42fcb66 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -706,44 +706,6 @@ impl<'le> GeckoElement<'le> { !self.xbl_binding_with_content().is_none() } - /// This duplicates the logic in Gecko's virtual nsINode::GetBindingParent - /// function, which only has two implementations: one for XUL elements, and - /// one for other elements. - /// - /// We just hard code in our knowledge of those two implementations here. - fn xbl_binding_parent(&self) -> Option { - if self.is_xul_element() { - // FIXME(heycam): Having trouble with bindgen on nsXULElement, - // where the binding parent is stored in a member variable - // rather than in slots. So just get it through FFI for now. - unsafe { - bindings::Gecko_GetBindingParent(self.0) - .as_ref() - .map(GeckoElement) - } - } else { - let binding_parent = - unsafe { self.non_xul_xbl_binding_parent().as_ref() }.map(GeckoElement); - - debug_assert!( - binding_parent == - unsafe { - bindings::Gecko_GetBindingParent(self.0) - .as_ref() - .map(GeckoElement) - } - ); - binding_parent - } - } - - #[inline] - fn non_xul_xbl_binding_parent(&self) -> *mut RawGeckoElement { - debug_assert!(!self.is_xul_element()); - self.extended_slots() - .map_or(ptr::null_mut(), |slots| slots._base.mBindingParent.mRawPtr) - } - #[inline] fn namespace_id(&self) -> i32 { self.as_node().node_info().mInner.mNamespaceID @@ -865,19 +827,11 @@ impl<'le> GeckoElement<'le> { return self.flags() & (NODE_IS_NATIVE_ANONYMOUS_ROOT as u32) != 0; } - /// This logic is duplicated in Gecko's nsIContent::IsInAnonymousSubtree. #[inline] fn is_in_anonymous_subtree(&self) -> bool { - if self.is_in_native_anonymous_subtree() { - return true; + unsafe { + bindings::Gecko_IsInAnonymousSubtree(self.0) } - - let binding_parent = match self.xbl_binding_parent() { - Some(p) => p, - None => return false, - }; - - binding_parent.shadow_root().is_none() } /// Returns true if this node is the shadow root of an use-element shadow tree. From 9eaadc68600637268f19accf2ad7dec93d536285 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 9 Sep 2019 22:39:46 +0000 Subject: [PATCH 23/24] style: Shutdown Servo's thread-pool in leak-checking builds, leak the atom table elsewhere. Differential Revision: https://phabricator.services.mozilla.com/D44217 --- components/selectors/bloom.rs | 6 +-- components/style/bloom.rs | 15 +++++-- components/style/global_style_data.rs | 56 +++++++++++++++++++++++++-- components/style/sharing/mod.rs | 11 +++--- 4 files changed, 70 insertions(+), 18 deletions(-) diff --git a/components/selectors/bloom.rs b/components/selectors/bloom.rs index c4b19e59ea8..98461d1ba24 100644 --- a/components/selectors/bloom.rs +++ b/components/selectors/bloom.rs @@ -64,7 +64,7 @@ pub type BloomFilter = CountingBloomFilter; /// Similarly, using a KeySize of 10 would lead to a 4% false /// positive rate for N == 100 and to quite bad false positive /// rates for larger N. -#[derive(Clone)] +#[derive(Clone, Default)] pub struct CountingBloomFilter where S: BloomStorage, @@ -79,9 +79,7 @@ where /// Creates a new bloom filter. #[inline] pub fn new() -> Self { - CountingBloomFilter { - storage: Default::default(), - } + Default::default() } #[inline] diff --git a/components/style/bloom.rs b/components/style/bloom.rs index 23493e3e86a..a24dac42b2a 100644 --- a/components/style/bloom.rs +++ b/components/style/bloom.rs @@ -13,13 +13,20 @@ use owning_ref::OwningHandle; use selectors::bloom::BloomFilter; use servo_arc::Arc; use smallvec::SmallVec; +use std::mem::ManuallyDrop; thread_local! { /// Bloom filters are large allocations, so we store them in thread-local storage /// such that they can be reused across style traversals. StyleBloom is responsible /// for ensuring that the bloom filter is zeroed when it is dropped. - static BLOOM_KEY: Arc> = - Arc::new_leaked(AtomicRefCell::new(BloomFilter::new())); + /// + /// We intentionally leak this from TLS because we don't have the guarantee + /// of TLS destructors to run in worker threads. + /// + /// We could change this once https://github.com/rayon-rs/rayon/issues/688 + /// is fixed, hopefully. + static BLOOM_KEY: ManuallyDrop>> = + ManuallyDrop::new(Arc::new_leaked(Default::default())); } /// A struct that allows us to fast-reject deep descendant selectors avoiding @@ -128,7 +135,7 @@ impl StyleBloom { // See https://github.com/servo/servo/pull/18420#issuecomment-328769322 #[inline(never)] pub fn new() -> Self { - let bloom_arc = BLOOM_KEY.with(|b| b.clone()); + let bloom_arc = BLOOM_KEY.with(|b| Arc::clone(&*b)); let filter = OwningHandle::new_with_fn(bloom_arc, |x| unsafe { x.as_ref() }.unwrap().borrow_mut()); debug_assert!( @@ -136,7 +143,7 @@ impl StyleBloom { "Forgot to zero the bloom filter last time" ); StyleBloom { - filter: filter, + filter, elements: Default::default(), pushed_hashes: Default::default(), } diff --git a/components/style/global_style_data.rs b/components/style/global_style_data.rs index eff53078c9e..504483ae5c8 100644 --- a/components/style/global_style_data.rs +++ b/components/style/global_style_data.rs @@ -12,6 +12,8 @@ use crate::shared_lock::SharedRwLock; use crate::thread_state; use rayon; use std::env; +use std::sync::atomic::{AtomicUsize, Ordering}; +use parking_lot::{RwLock, RwLockReadGuard}; /// Global style data pub struct GlobalStyleData { @@ -22,20 +24,28 @@ pub struct GlobalStyleData { pub options: StyleSystemOptions, } -/// Global thread pool +/// Global thread pool. pub struct StyleThreadPool { /// How many threads parallel styling can use. pub num_threads: usize, /// The parallel styling thread pool. - pub style_thread_pool: Option, + /// + /// For leak-checking purposes, we want to terminate the thread-pool, which + /// waits for all the async jobs to complete. Thus the RwLock. + style_thread_pool: RwLock>, } fn thread_name(index: usize) -> String { format!("StyleThread#{}", index) } +// A counter so that we can wait for shutdown of all threads. See +// StyleThreadPool::shutdown. +static ALIVE_WORKER_THREADS: AtomicUsize = AtomicUsize::new(0); + fn thread_startup(_index: usize) { + ALIVE_WORKER_THREADS.fetch_add(1, Ordering::Relaxed); thread_state::initialize_layout_worker_thread(); #[cfg(feature = "gecko")] unsafe { @@ -55,6 +65,43 @@ fn thread_shutdown(_: usize) { bindings::Gecko_UnregisterProfilerThread(); bindings::Gecko_SetJemallocThreadLocalArena(false); } + ALIVE_WORKER_THREADS.fetch_sub(1, Ordering::Relaxed); +} + +impl StyleThreadPool { + /// Shuts down the thread pool, waiting for all work to complete. + pub fn shutdown() { + if ALIVE_WORKER_THREADS.load(Ordering::Relaxed) == 0 { + return; + } + { + // Drop the pool. + let _ = STYLE_THREAD_POOL.style_thread_pool.write().take(); + } + // Spin until all our threads are done. This will usually be pretty + // fast, as on shutdown there should be basically no threads left + // running. + // + // This still _technically_ doesn't give us the guarantee of TLS + // destructors running on the worker threads. For that we'd need help + // from rayon to properly join the threads. + // + // See https://github.com/rayon-rs/rayon/issues/688 + // + // So we instead intentionally leak TLS stuff (see BLOOM_KEY and co) for + // now until that's fixed. + while ALIVE_WORKER_THREADS.load(Ordering::Relaxed) != 0 { + std::thread::yield_now(); + } + } + + /// Returns a reference to the thread pool. + /// + /// We only really want to give read-only access to the pool, except + /// for shutdown(). + pub fn pool(&self) -> RwLockReadGuard> { + self.style_thread_pool.read() + } } lazy_static! { @@ -113,10 +160,11 @@ lazy_static! { }; StyleThreadPool { - num_threads: num_threads, - style_thread_pool: pool, + num_threads, + style_thread_pool: RwLock::new(pool), } }; + /// Global style data pub static ref GLOBAL_STYLE_DATA: GlobalStyleData = GlobalStyleData { shared_lock: SharedRwLock::new_leaked(), diff --git a/components/style/sharing/mod.rs b/components/style/sharing/mod.rs index 44fd51362d2..38fd6283436 100644 --- a/components/style/sharing/mod.rs +++ b/components/style/sharing/mod.rs @@ -82,7 +82,7 @@ use servo_arc::Arc; use smallbitvec::SmallBitVec; use smallvec::SmallVec; use std::marker::PhantomData; -use std::mem; +use std::mem::{self, ManuallyDrop}; use std::ops::Deref; use std::ptr::NonNull; use uluru::{Entry, LRUCache}; @@ -477,10 +477,9 @@ type TypelessSharingCache = SharingCacheBase; type StoredSharingCache = Arc>; thread_local! { - // TODO(emilio): Looks like a few of these should just be Rc> or - // something. No need for atomics in the thread-local code. - static SHARING_CACHE_KEY: StoredSharingCache = - Arc::new_leaked(AtomicRefCell::new(TypelessSharingCache::default())); + // See the comment on bloom.rs about why do we leak this. + static SHARING_CACHE_KEY: ManuallyDrop = + ManuallyDrop::new(Arc::new_leaked(Default::default())); } /// An LRU cache of the last few nodes seen, so that we can aggressively try to @@ -533,7 +532,7 @@ impl StyleSharingCache { mem::align_of::>(), mem::align_of::() ); - let cache_arc = SHARING_CACHE_KEY.with(|c| c.clone()); + let cache_arc = SHARING_CACHE_KEY.with(|c| Arc::clone(&*c)); let cache = OwningHandle::new_with_fn(cache_arc, |x| unsafe { x.as_ref() }.unwrap().borrow_mut()); debug_assert!(cache.is_empty()); From cd3b0c23fa8971b312e2528711d220c834200061 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 12 Sep 2019 22:44:31 +0200 Subject: [PATCH 24/24] style: Fix Servo build, and rustfmt recent changes. --- components/layout/display_list/builder.rs | 8 +-- components/layout_thread/lib.rs | 11 ++-- components/style/gecko/wrapper.rs | 4 +- components/style/global_style_data.rs | 2 +- .../media_queries/media_feature_expression.rs | 42 +++++++++------ components/style/properties/data.py | 1 + .../style/properties/shorthands/text.mako.rs | 1 + components/style/values/computed/font.rs | 10 ++-- components/style/values/generics/counters.rs | 4 +- components/style/values/generics/font.rs | 1 - components/style/values/generics/mod.rs | 6 +++ components/style/values/specified/counters.rs | 2 +- components/style/values/specified/font.rs | 53 +++++++++++-------- components/style/values/specified/mod.rs | 2 +- components/style/values/specified/text.rs | 3 +- 15 files changed, 90 insertions(+), 60 deletions(-) diff --git a/components/layout/display_list/builder.rs b/components/layout/display_list/builder.rs index cba26fe05a4..f951bd3f8f2 100644 --- a/components/layout/display_list/builder.rs +++ b/components/layout/display_list/builder.rs @@ -61,11 +61,11 @@ use style::properties::{style_structs, ComputedValues}; use style::servo::restyle_damage::ServoRestyleDamage; use style::values::computed::effects::SimpleShadow; use style::values::computed::image::{Image, ImageLayer}; -use style::values::computed::{Gradient, LengthOrAuto}; +use style::values::computed::{ClipRectOrAuto, Gradient, LengthOrAuto}; use style::values::generics::background::BackgroundSize; use style::values::generics::image::{GradientKind, PaintWorklet}; use style::values::specified::ui::CursorKind; -use style::values::{Either, RGBA}; +use style::values::RGBA; use style_traits::ToCss; use webrender_api::units::{LayoutRect, LayoutSize, LayoutTransform, LayoutVector2D}; use webrender_api::{self, BorderDetails, BorderRadius, BorderSide, BoxShadowClipMode, ColorF}; @@ -2632,8 +2632,8 @@ impl BlockFlow { ) { // Account for `clip` per CSS 2.1 ยง 11.1.2. let style_clip_rect = match self.fragment.style().get_effects().clip { - Either::First(style_clip_rect) => style_clip_rect, - _ => return, + ClipRectOrAuto::Rect(ref r) => r, + ClipRectOrAuto::Auto => return, }; // CSS `clip` should only apply to position:absolute or positione:fixed elements. diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index e24360db2dc..f03ca9f9d03 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -1497,11 +1497,10 @@ impl LayoutThread { // Create a layout context for use throughout the following passes. let mut layout_context = self.build_layout_context(guards.clone(), true, &map); + let pool; let (thread_pool, num_threads) = if self.parallel_flag { - ( - STYLE_THREAD_POOL.style_thread_pool.as_ref(), - STYLE_THREAD_POOL.num_threads, - ) + pool = STYLE_THREAD_POOL.pool(); + (pool.as_ref(), STYLE_THREAD_POOL.num_threads) } else { (None, 1) }; @@ -1859,8 +1858,10 @@ impl LayoutThread { || { let profiler_metadata = self.profiler_metadata(); + let pool; let thread_pool = if self.parallel_flag { - STYLE_THREAD_POOL.style_thread_pool.as_ref() + pool = STYLE_THREAD_POOL.pool(); + pool.as_ref() } else { None }; diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index e2ae42fcb66..68c0473bf12 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -829,9 +829,7 @@ impl<'le> GeckoElement<'le> { #[inline] fn is_in_anonymous_subtree(&self) -> bool { - unsafe { - bindings::Gecko_IsInAnonymousSubtree(self.0) - } + unsafe { bindings::Gecko_IsInAnonymousSubtree(self.0) } } /// Returns true if this node is the shadow root of an use-element shadow tree. diff --git a/components/style/global_style_data.rs b/components/style/global_style_data.rs index 504483ae5c8..a9e79bf6820 100644 --- a/components/style/global_style_data.rs +++ b/components/style/global_style_data.rs @@ -10,10 +10,10 @@ use crate::gecko_bindings::bindings; use crate::parallel::STYLE_THREAD_STACK_SIZE_KB; use crate::shared_lock::SharedRwLock; use crate::thread_state; +use parking_lot::{RwLock, RwLockReadGuard}; use rayon; use std::env; use std::sync::atomic::{AtomicUsize, Ordering}; -use parking_lot::{RwLock, RwLockReadGuard}; /// Global style data pub struct GlobalStyleData { diff --git a/components/style/media_queries/media_feature_expression.rs b/components/style/media_queries/media_feature_expression.rs index 3c106a4471b..1eef9e57ebc 100644 --- a/components/style/media_queries/media_feature_expression.rs +++ b/components/style/media_queries/media_feature_expression.rs @@ -16,7 +16,9 @@ use crate::parser::{Parse, ParserContext}; use crate::servo::media_queries::MEDIA_FEATURES; use crate::str::{starts_with_ignore_ascii_case, string_as_ascii_lowercase}; use crate::values::computed::{self, ToComputedValue}; -use crate::values::specified::{Integer, NonNegativeNumber, Length, Number, Resolution}; +#[cfg(feature = "gecko")] +use crate::values::specified::NonNegativeNumber; +use crate::values::specified::{Integer, Length, Number, Resolution}; use crate::values::{serialize_atom_identifier, CSSFloat}; use crate::{Atom, Zero}; use cssparser::{Parser, Token}; @@ -429,9 +431,11 @@ impl MediaFeatureExpression { eval(device, expect!(Integer).cloned(), self.range_or_operator) }, Evaluator::Float(eval) => eval(device, expect!(Float).cloned(), self.range_or_operator), - Evaluator::NumberRatio(eval) => { - eval(device, expect!(NumberRatio).cloned(), self.range_or_operator) - }, + Evaluator::NumberRatio(eval) => eval( + device, + expect!(NumberRatio).cloned(), + self.range_or_operator, + ), Evaluator::Resolution(eval) => { let computed = expect!(Resolution).map(|specified| { computed::Context::for_media_query_evaluation(device, quirks_mode, |context| { @@ -530,19 +534,25 @@ impl MediaExpressionValue { MediaExpressionValue::Float(number.get()) }, Evaluator::NumberRatio(..) => { - if static_prefs::pref!("layout.css.aspect-ratio-number.enabled") { - let a = NonNegativeNumber::parse(context, input)?.0.get(); - let b = match input.try_parse(|input| input.expect_delim('/')) { - Ok(()) => NonNegativeNumber::parse(context, input)?.0.get(), - _ => 1.0, - }; - MediaExpressionValue::NumberRatio(AspectRatio(a, b)) - } else { - let a = Integer::parse_positive(context, input)?; - input.expect_delim('/')?; - let b = Integer::parse_positive(context, input)?; - MediaExpressionValue::NumberRatio(AspectRatio(a.value() as CSSFloat, b.value() as CSSFloat)) + #[cfg(feature = "gecko")] + { + if static_prefs::pref!("layout.css.aspect-ratio-number.enabled") { + let a = NonNegativeNumber::parse(context, input)?.0.get(); + let b = match input.try_parse(|input| input.expect_delim('/')) { + Ok(()) => NonNegativeNumber::parse(context, input)?.0.get(), + _ => 1.0, + }; + return Ok(MediaExpressionValue::NumberRatio(AspectRatio(a, b))); + } } + + let a = Integer::parse_positive(context, input)?; + input.expect_delim('/')?; + let b = Integer::parse_positive(context, input)?; + MediaExpressionValue::NumberRatio(AspectRatio( + a.value() as CSSFloat, + b.value() as CSSFloat, + )) }, Evaluator::Resolution(..) => { MediaExpressionValue::Resolution(Resolution::parse(context, input)?) diff --git a/components/style/properties/data.py b/components/style/properties/data.py index 7c2330e47d8..d1876a5ea02 100644 --- a/components/style/properties/data.py +++ b/components/style/properties/data.py @@ -797,6 +797,7 @@ class PropertyRestrictions: + PropertyRestrictions.shorthand(data, "outline") + PropertyRestrictions.shorthand(data, "font")) + class CountedUnknownProperty: def __init__(self, name): self.name = name diff --git a/components/style/properties/shorthands/text.mako.rs b/components/style/properties/shorthands/text.mako.rs index 77b15d9a722..79f7f33d850 100644 --- a/components/style/properties/shorthands/text.mako.rs +++ b/components/style/properties/shorthands/text.mako.rs @@ -73,6 +73,7 @@ } impl<'a> ToCss for LonghandsToSerialize<'a> { + #[allow(unused)] fn to_css(&self, dest: &mut CssWriter) -> fmt::Result where W: fmt::Write { use crate::values::specified::TextDecorationLine; diff --git a/components/style/values/computed/font.rs b/components/style/values/computed/font.rs index fee8c24ee3b..a4fa6013277 100644 --- a/components/style/values/computed/font.rs +++ b/components/style/values/computed/font.rs @@ -9,11 +9,15 @@ use crate::gecko_bindings::sugar::refptr::RefPtr; #[cfg(feature = "gecko")] use crate::gecko_bindings::{bindings, structs}; use crate::values::animated::{ToAnimatedValue, ToAnimatedZero}; -use crate::values::computed::{Angle, Context, Integer, Length, NonNegativeLength, NonNegativePercentage}; +use crate::values::computed::{ + Angle, Context, Integer, Length, NonNegativeLength, NonNegativePercentage, +}; use crate::values::computed::{Number, Percentage, ToComputedValue}; -use crate::values::generics::{font as generics, NonNegative}; use crate::values::generics::font::{FeatureTagValue, FontSettings, VariationValue}; -use crate::values::specified::font::{self as specified, KeywordInfo, MAX_FONT_WEIGHT, MIN_FONT_WEIGHT}; +use crate::values::generics::{font as generics, NonNegative}; +use crate::values::specified::font::{ + self as specified, KeywordInfo, MAX_FONT_WEIGHT, MIN_FONT_WEIGHT, +}; use crate::values::specified::length::{FontBaseSize, NoCalcLength}; use crate::values::CSSFloat; use crate::Atom; diff --git a/components/style/values/generics/counters.rs b/components/style/values/generics/counters.rs index 391da7e33cc..69893c75561 100644 --- a/components/style/values/generics/counters.rs +++ b/components/style/values/generics/counters.rs @@ -118,7 +118,9 @@ impl Deref for CounterSetOrReset { ToShmem, )] #[repr(transparent)] -pub struct GenericCounters(#[css(iterable, if_empty = "none")] crate::OwnedSlice>); +pub struct GenericCounters( + #[css(iterable, if_empty = "none")] crate::OwnedSlice>, +); pub use self::GenericCounters as Counters; #[cfg(feature = "servo")] diff --git a/components/style/values/generics/font.rs b/components/style/values/generics/font.rs index 2972fa01c68..2b29104ff81 100644 --- a/components/style/values/generics/font.rs +++ b/components/style/values/generics/font.rs @@ -171,7 +171,6 @@ impl Parse for FontTag { } } - /// A generic value for the `font-style` property. /// /// https://drafts.csswg.org/css-fonts-4/#font-style-prop diff --git a/components/style/values/generics/mod.rs b/components/style/values/generics/mod.rs index 933ad38f7db..04de85cbcf2 100644 --- a/components/style/values/generics/mod.rs +++ b/components/style/values/generics/mod.rs @@ -309,4 +309,10 @@ impl ClipRectOrAuto { pub fn auto() -> Self { ClipRectOrAuto::Auto } + + /// Returns whether this value is the `auto` value. + #[inline] + pub fn is_auto(&self) -> bool { + matches!(*self, ClipRectOrAuto::Auto) + } } diff --git a/components/style/values/specified/counters.rs b/components/style/values/specified/counters.rs index 39ca07e3563..54690b53105 100644 --- a/components/style/values/specified/counters.rs +++ b/components/style/values/specified/counters.rs @@ -8,8 +8,8 @@ use crate::computed_values::list_style_type::T as ListStyleType; use crate::parser::{Parse, ParserContext}; use crate::values::generics::counters as generics; -use crate::values::generics::counters::GenericCounterIncrement; use crate::values::generics::counters::CounterPair; +use crate::values::generics::counters::GenericCounterIncrement; use crate::values::generics::counters::GenericCounterSetOrReset; #[cfg(feature = "gecko")] use crate::values::generics::CounterStyle; diff --git a/components/style/values/specified/font.rs b/components/style/values/specified/font.rs index 45af8df23bf..90426f80770 100644 --- a/components/style/values/specified/font.rs +++ b/components/style/values/specified/font.rs @@ -12,8 +12,8 @@ use crate::values::computed::font::{FamilyName, FontFamilyList, FontStyleAngle, use crate::values::computed::{font as computed, Length, NonNegativeLength}; use crate::values::computed::{Angle as ComputedAngle, Percentage as ComputedPercentage}; use crate::values::computed::{CSSPixelLength, Context, ToComputedValue}; -use crate::values::generics::font::{self as generics, FeatureTagValue, FontSettings, FontTag}; use crate::values::generics::font::VariationValue; +use crate::values::generics::font::{self as generics, FeatureTagValue, FontSettings, FontTag}; use crate::values::generics::NonNegative; use crate::values::specified::length::{FontBaseSize, AU_PER_PT, AU_PER_PX}; use crate::values::specified::{AllowQuirks, Angle, Integer, LengthPercentage}; @@ -877,19 +877,17 @@ impl ToComputedValue for KeywordSize { impl FontSize { /// pub fn from_html_size(size: u8) -> Self { - FontSize::Keyword( - KeywordInfo::new(match size { - // If value is less than 1, let it be 1. - 0 | 1 => KeywordSize::XSmall, - 2 => KeywordSize::Small, - 3 => KeywordSize::Medium, - 4 => KeywordSize::Large, - 5 => KeywordSize::XLarge, - 6 => KeywordSize::XXLarge, - // If value is greater than 7, let it be 7. - _ => KeywordSize::XXXLarge, - }) - ) + FontSize::Keyword(KeywordInfo::new(match size { + // If value is less than 1, let it be 1. + 0 | 1 => KeywordSize::XSmall, + 2 => KeywordSize::Small, + 3 => KeywordSize::Medium, + 4 => KeywordSize::Large, + 5 => KeywordSize::XLarge, + 6 => KeywordSize::XXLarge, + // If value is greater than 7, let it be 7. + _ => KeywordSize::XXXLarge, + })) } /// Compute it against a given base font size @@ -924,9 +922,7 @@ impl FontSize { FontSize::Length(LengthPercentage::Length(NoCalcLength::Absolute(ref l))) => { context.maybe_zoom_text(l.to_computed_value(context)) }, - FontSize::Length(LengthPercentage::Length(ref l)) => { - l.to_computed_value(context) - }, + FontSize::Length(LengthPercentage::Length(ref l)) => l.to_computed_value(context), FontSize::Length(LengthPercentage::Percentage(pc)) => { // If the parent font was keyword-derived, this is too. // Tack the % onto the factor @@ -983,8 +979,7 @@ impl FontSize { }, FontSize::Larger => { info = compose_keyword(LARGER_FONT_SIZE_RATIO); - FontRelativeLength::Em(LARGER_FONT_SIZE_RATIO) - .to_computed_value(context, base_size) + FontRelativeLength::Em(LARGER_FONT_SIZE_RATIO).to_computed_value(context, base_size) }, FontSize::System(_) => { @@ -994,7 +989,13 @@ impl FontSize { } #[cfg(feature = "gecko")] { - context.cached_system_font.as_ref().unwrap().font_size.size.0 + context + .cached_system_font + .as_ref() + .unwrap() + .font_size + .size + .0 } }, }; @@ -1115,7 +1116,15 @@ pub enum VariantAlternates { } #[derive( - Clone, Debug, Default, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss, ToResolvedValue, ToShmem, + Clone, + Debug, + Default, + MallocSizeOf, + PartialEq, + SpecifiedValueInfo, + ToCss, + ToResolvedValue, + ToShmem, )] #[repr(transparent)] /// List of Variant Alternates @@ -1190,7 +1199,7 @@ impl Parse for FontVariantAlternates { .try(|input| input.expect_ident_matching("normal")) .is_ok() { - return Ok(FontVariantAlternates::Value(Default::default())) + return Ok(FontVariantAlternates::Value(Default::default())); } let mut alternates = Vec::new(); diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index bb10a7e540c..5daecf353bf 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -706,7 +706,7 @@ impl ClipRectOrAuto { allow_quirks: AllowQuirks, ) -> Result> { if let Ok(v) = input.try(|i| ClipRect::parse_quirky(context, i, allow_quirks)) { - return Ok(generics::GenericClipRectOrAuto::Rect(v)) + return Ok(generics::GenericClipRectOrAuto::Rect(v)); } input.expect_ident_matching("auto")?; Ok(generics::GenericClipRectOrAuto::Auto) diff --git a/components/style/values/specified/text.rs b/components/style/values/specified/text.rs index 1505192f86f..abe44aa8f03 100644 --- a/components/style/values/specified/text.rs +++ b/components/style/values/specified/text.rs @@ -89,8 +89,7 @@ impl ToComputedValue for LineHeight { GenericLineHeight::Length(ref non_negative_lp) => { let result = match non_negative_lp.0 { LengthPercentage::Length(NoCalcLength::Absolute(ref abs)) => { - context - .maybe_zoom_text(abs.to_computed_value(context)) + context.maybe_zoom_text(abs.to_computed_value(context)) }, LengthPercentage::Length(ref length) => length.to_computed_value(context), LengthPercentage::Percentage(ref p) => FontRelativeLength::Em(p.0)