From 8c322b9c1ed91f2a5dd96e6b2d8abb3b155e051e Mon Sep 17 00:00:00 2001 From: Xidorn Quan Date: Sun, 29 Apr 2018 21:17:26 +1000 Subject: [PATCH 01/19] style: Make KTableEntry an independent type. Bug: 1448759 Reviewed-by: heycam MozReview-Commit-ID: oZfJAigThN --- components/style/gecko/media_queries.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/components/style/gecko/media_queries.rs b/components/style/gecko/media_queries.rs index e488fdfa00a..7d9724cc6c5 100644 --- a/components/style/gecko/media_queries.rs +++ b/components/style/gecko/media_queries.rs @@ -13,7 +13,7 @@ use euclid::TypedScale; use gecko::values::{convert_nscolor_to_rgba, convert_rgba_to_nscolor}; use gecko_bindings::bindings; use gecko_bindings::structs; -use gecko_bindings::structs::{nsCSSKeyword, nsCSSProps_KTableEntry, nsCSSUnit, nsCSSValue}; +use gecko_bindings::structs::{nsCSSKTableEntry, nsCSSKeyword, nsCSSUnit, nsCSSValue}; use gecko_bindings::structs::{nsMediaFeature, nsMediaFeature_RangeType}; use gecko_bindings::structs::{nsMediaFeature_ValueType, nsPresContext}; use gecko_bindings::structs::RawGeckoPresContextOwned; @@ -479,7 +479,7 @@ where } unsafe fn find_in_table( - mut current_entry: *const nsCSSProps_KTableEntry, + mut current_entry: *const nsCSSKTableEntry, mut f: F, ) -> Option<(nsCSSKeyword, i16)> where @@ -544,7 +544,7 @@ fn parse_feature_value<'i, 't>( bindings::Gecko_LookupCSSKeyword(keyword.as_bytes().as_ptr(), keyword.len() as u32) }; - let first_table_entry: *const nsCSSProps_KTableEntry = + let first_table_entry: *const nsCSSKTableEntry = unsafe { *feature.mData.mKeywordTable.as_ref() }; let value = match unsafe { find_in_table(first_table_entry, |kw, _| kw == keyword) } { From a375baf84b029c56222d3fe8a304f374ab1c9549 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 29 Apr 2018 05:12:57 +0200 Subject: [PATCH 02/19] style: Derive ToCss for Counters. Bug: 1457332 Reviewed-by: xidorn MozReview-Commit-ID: 1jOglcqt1Dd --- components/style/properties/gecko.mako.rs | 12 +++-- components/style/values/generics/counters.rs | 52 ++++++------------- components/style/values/specified/counters.rs | 9 ++-- 3 files changed, 29 insertions(+), 44 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index a3d316df321..4479d040564 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -5697,9 +5697,9 @@ clip-path ) { unsafe { bindings::Gecko_ClearAndResizeCounter${counter_property}s(&mut self.gecko, v.len() as u32); - for (i, &(ref name, value)) in v.iter().enumerate() { - self.gecko.m${counter_property}s[i].mCounter.assign(name.0.as_slice()); - self.gecko.m${counter_property}s[i].mValue = value; + for (i, ref pair) in v.iter().enumerate() { + self.gecko.m${counter_property}s[i].mCounter.assign(pair.name.0.as_slice()); + self.gecko.m${counter_property}s[i].mValue = pair.value; } } } @@ -5717,12 +5717,16 @@ clip-path pub fn clone_counter_${counter_property.lower()}( &self ) -> longhands::counter_${counter_property.lower()}::computed_value::T { + use values::generics::counters::CounterPair; use values::CustomIdent; use gecko_string_cache::Atom; longhands::counter_${counter_property.lower()}::computed_value::T::new( self.gecko.m${counter_property}s.iter().map(|ref gecko_counter| { - (CustomIdent(Atom::from(gecko_counter.mCounter.to_string())), gecko_counter.mValue) + CounterPair { + name: CustomIdent(Atom::from(gecko_counter.mCounter.to_string())), + value: gecko_counter.mValue, + } }).collect() ) } diff --git a/components/style/values/generics/counters.rs b/components/style/values/generics/counters.rs index 7373cd8e947..9f50ab1e2b0 100644 --- a/components/style/values/generics/counters.rs +++ b/components/style/values/generics/counters.rs @@ -4,12 +4,19 @@ //! Generic types for counters-related CSS values. -use std::fmt; -use std::fmt::Write; use std::ops::Deref; -use style_traits::{CssWriter, ToCss}; use values::CustomIdent; +/// A name / value pair for counters. +#[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, + ToComputedValue, ToCss)] +pub struct CounterPair { + /// The name of the counter. + pub name: CustomIdent, + /// The value of the counter / increment / etc. + pub value: Integer, +} + /// A generic value for the `counter-increment` property. #[derive(Clone, Debug, Default, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToComputedValue, ToCss)] @@ -18,13 +25,13 @@ pub struct CounterIncrement(Counters); impl CounterIncrement { /// Returns a new value for `counter-increment`. #[inline] - pub fn new(counters: Vec<(CustomIdent, I)>) -> Self { + pub fn new(counters: Vec>) -> Self { CounterIncrement(Counters(counters.into_boxed_slice())) } } impl Deref for CounterIncrement { - type Target = [(CustomIdent, I)]; + type Target = [CounterPair]; #[inline] fn deref(&self) -> &Self::Target { @@ -40,13 +47,13 @@ pub struct CounterReset(Counters); impl CounterReset { /// Returns a new value for `counter-reset`. #[inline] - pub fn new(counters: Vec<(CustomIdent, I)>) -> Self { + pub fn new(counters: Vec>) -> Self { CounterReset(Counters(counters.into_boxed_slice())) } } impl Deref for CounterReset { - type Target = [(CustomIdent, I)]; + type Target = [CounterPair]; #[inline] fn deref(&self) -> &Self::Target { @@ -58,8 +65,8 @@ impl Deref for CounterReset { /// /// Keyword `none` is represented by an empty vector. #[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, - ToComputedValue)] -pub struct Counters(#[css(if_empty = "none")] Box<[(CustomIdent, I)]>); + ToComputedValue, ToCss)] +pub struct Counters(#[css(iterable, if_empty = "none")] Box<[CounterPair]>); impl Default for Counters { #[inline] @@ -67,30 +74,3 @@ impl Default for Counters { Counters(vec![].into_boxed_slice()) } } - -impl ToCss for Counters -where - I: ToCss, -{ - #[inline] - fn to_css(&self, dest: &mut CssWriter) -> fmt::Result - where - W: fmt::Write, - { - if self.0.is_empty() { - return dest.write_str("none"); - } - - let mut first = true; - for &(ref name, ref value) in &*self.0 { - if !first { - dest.write_str(" ")?; - } - first = false; - name.to_css(dest)?; - dest.write_str(" ")?; - value.to_css(dest)?; - } - Ok(()) - } -} diff --git a/components/style/values/specified/counters.rs b/components/style/values/specified/counters.rs index 8a8e25ab065..64bdd04becc 100644 --- a/components/style/values/specified/counters.rs +++ b/components/style/values/specified/counters.rs @@ -12,6 +12,7 @@ use style_traits::{ParseError, StyleParseErrorKind}; use values::CustomIdent; #[cfg(feature = "gecko")] use values::generics::CounterStyleOrNone; +use values::generics::counters::CounterPair; use values::generics::counters::CounterIncrement as GenericCounterIncrement; use values::generics::counters::CounterReset as GenericCounterReset; #[cfg(feature = "gecko")] @@ -48,7 +49,7 @@ fn parse_counters<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, default_value: i32, -) -> Result, ParseError<'i>> { +) -> Result>, ParseError<'i>> { if input .try(|input| input.expect_ident_matching("none")) .is_ok() @@ -59,16 +60,16 @@ fn parse_counters<'i, 't>( let mut counters = Vec::new(); loop { let location = input.current_source_location(); - let counter_name = match input.next() { + let name = match input.next() { Ok(&Token::Ident(ref ident)) => CustomIdent::from_ident(location, ident, &["none"])?, Ok(t) => return Err(location.new_unexpected_token_error(t.clone())), Err(_) => break, }; - let counter_delta = input + let value = input .try(|input| Integer::parse(context, input)) .unwrap_or(Integer::new(default_value)); - counters.push((counter_name, counter_delta)) + counters.push(CounterPair { name, value }); } if !counters.is_empty() { From c508d8576dafb13a4f0d51a837508f7984e4c1a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 29 Apr 2018 04:48:47 +0200 Subject: [PATCH 03/19] style: Move represents_keyword to the css attributes. Bug: 1457635 Reviewed-by: xidorn MozReview-Commit-ID: 21yuU4h34AQ --- components/style/values/generics/border.rs | 21 ++----------- components/style/values/generics/effects.rs | 30 ++----------------- components/style/values/specified/font.rs | 4 +-- components/style/values/specified/position.rs | 18 ++--------- .../style_derive/specified_value_info.rs | 5 ++-- components/style_derive/to_css.rs | 10 +++++++ .../style_traits/specified_value_info.rs | 2 -- components/style_traits/values.rs | 3 ++ 8 files changed, 24 insertions(+), 69 deletions(-) diff --git a/components/style/values/generics/border.rs b/components/style/values/generics/border.rs index 6230a1cf36a..c25560ed0f1 100644 --- a/components/style/values/generics/border.rs +++ b/components/style/values/generics/border.rs @@ -23,12 +23,13 @@ pub enum BorderImageSideWidth { /// A generic value for the `border-image-slice` property. #[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, - ToComputedValue)] + ToComputedValue, ToCss)] pub struct BorderImageSlice { /// The offsets. + #[css(field_bound)] pub offsets: Rect, /// Whether to fill the middle part. - #[value_info(represents_keyword)] + #[css(represents_keyword)] pub fill: bool, } @@ -86,22 +87,6 @@ where } } -impl ToCss for BorderImageSlice -where - N: PartialEq + ToCss, -{ - fn to_css(&self, dest: &mut CssWriter) -> fmt::Result - where - W: Write, - { - self.offsets.to_css(dest)?; - if self.fill { - dest.write_str(" fill")?; - } - Ok(()) - } -} - impl BorderRadius { /// Returns a new `BorderRadius`. #[inline] diff --git a/components/style/values/generics/effects.rs b/components/style/values/generics/effects.rs index 7a76cdeb7bc..e5ee10fcada 100644 --- a/components/style/values/generics/effects.rs +++ b/components/style/values/generics/effects.rs @@ -4,14 +4,12 @@ //! Generic types for CSS values related to effects. -use std::fmt::{self, Write}; -use style_traits::values::{CssWriter, SequenceWriter, ToCss}; #[cfg(feature = "gecko")] use values::specified::url::SpecifiedUrl; /// A generic value for a single `box-shadow`. #[derive(Animate, Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, - ToAnimatedValue, ToAnimatedZero)] + ToAnimatedValue, ToAnimatedZero, ToCss)] pub struct BoxShadow { /// The base shadow. pub base: SimpleShadow, @@ -19,7 +17,7 @@ pub struct BoxShadow { pub spread: ShapeLength, /// Whether this is an inset box shadow. #[animation(constant)] - #[value_info(represents_keyword)] + #[css(represents_keyword)] pub inset: bool, } @@ -80,27 +78,3 @@ pub struct SimpleShadow { /// Blur radius. pub blur: ShapeLength, } - -impl ToCss - for BoxShadow -where - Color: ToCss, - SizeLength: ToCss, - BlurShapeLength: ToCss, - ShapeLength: ToCss, -{ - fn to_css(&self, dest: &mut CssWriter) -> fmt::Result - where - W: Write, - { - { - let mut writer = SequenceWriter::new(&mut *dest, " "); - writer.item(&self.base)?; - writer.item(&self.spread)?; - } - if self.inset { - dest.write_str(" inset")?; - } - Ok(()) - } -} diff --git a/components/style/values/specified/font.rs b/components/style/values/specified/font.rs index 8ec4354ab7a..266d7c52c09 100644 --- a/components/style/values/specified/font.rs +++ b/components/style/values/specified/font.rs @@ -1947,11 +1947,11 @@ pub struct FontSynthesis { /// If a `font-weight` is requested that the font family does not contain, /// the user agent may synthesize the requested weight from the weights /// that do exist in the font family. - #[value_info(represents_keyword)] + #[css(represents_keyword)] pub weight: bool, /// If a font-style is requested that the font family does not contain, /// the user agent may synthesize the requested style from the normal face in the font family. - #[value_info(represents_keyword)] + #[css(represents_keyword)] pub style: bool, } diff --git a/components/style/values/specified/position.rs b/components/style/values/specified/position.rs index 0f2669f0a02..17a5db17bcb 100644 --- a/components/style/values/specified/position.rs +++ b/components/style/values/specified/position.rs @@ -424,14 +424,14 @@ pub enum AutoFlow { } #[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, SpecifiedValueInfo, - ToComputedValue)] + ToComputedValue, ToCss)] /// Controls how the auto-placement algorithm works /// specifying exactly how auto-placed items get flowed into the grid pub struct GridAutoFlow { /// Specifiy how auto-placement algorithm fills each `row` or `column` in turn pub autoflow: AutoFlow, /// Specify use `dense` packing algorithm or not - #[value_info(represents_keyword)] + #[css(represents_keyword)] pub dense: bool, } @@ -446,20 +446,6 @@ impl GridAutoFlow { } } -impl ToCss for GridAutoFlow { - fn to_css(&self, dest: &mut CssWriter) -> fmt::Result - where - W: Write, - { - self.autoflow.to_css(dest)?; - - if self.dense { - dest.write_str(" dense")?; - } - Ok(()) - } -} - impl Parse for GridAutoFlow { /// [ row | column ] || dense fn parse<'i, 't>( diff --git a/components/style_derive/specified_value_info.rs b/components/style_derive/specified_value_info.rs index 11ad7a0d1b4..7aae081641d 100644 --- a/components/style_derive/specified_value_info.rs +++ b/components/style_derive/specified_value_info.rs @@ -140,13 +140,13 @@ fn derive_struct_fields<'a>( values.push(value.to_string()); } } - if info_attrs.represents_keyword { + let css_attrs = cg::parse_field_attrs::(field); + if css_attrs.represents_keyword { let ident = field.ident.as_ref() .expect("only named field should use represents_keyword"); values.push(cg::to_css_identifier(ident.as_ref())); return None; } - let css_attrs = cg::parse_field_attrs::(field); if let Some(if_empty) = css_attrs.if_empty { values.push(if_empty); } @@ -176,6 +176,5 @@ struct ValueInfoVariantAttrs { #[darling(attributes(value_info), default)] #[derive(Default, FromField)] struct ValueInfoFieldAttrs { - represents_keyword: bool, other_values: Option, } diff --git a/components/style_derive/to_css.rs b/components/style_derive/to_css.rs index 722107da0cd..7ac7d3a1ae7 100644 --- a/components/style_derive/to_css.rs +++ b/components/style_derive/to_css.rs @@ -189,6 +189,15 @@ fn derive_single_field_expr( writer.item(&item)?; } } + } else if attrs.represents_keyword { + let ident = + field.ast().ident.as_ref().expect("Unnamed field with represents_keyword?"); + let ident = cg::to_css_identifier(ident.as_ref()); + quote! { + if *#field { + writer.raw_item(#ident)?; + } + } } else { if attrs.field_bound { let ty = &field.ast().ty; @@ -236,5 +245,6 @@ pub struct CssFieldAttrs { pub field_bound: bool, pub iterable: bool, pub skip: bool, + pub represents_keyword: bool, pub skip_if: Option, } diff --git a/components/style_traits/specified_value_info.rs b/components/style_traits/specified_value_info.rs index 28e1eef25e6..d9f4578b1a8 100644 --- a/components/style_traits/specified_value_info.rs +++ b/components/style_traits/specified_value_info.rs @@ -56,8 +56,6 @@ pub type KeywordsCollectFn<'a> = &'a mut FnMut(&[&'static str]); /// * `#[value_info(starts_with_keyword)]` can be used on variants to /// add the name of a non-unit variant (serialized like `ToCss`) into /// `collect_completion_keywords`. -/// * `#[value_info(represents_keyword)]` can be used on fields into -/// `collect_completion_keywords`. pub trait SpecifiedValueInfo { /// Supported CssTypes by the given value type. /// diff --git a/components/style_traits/values.rs b/components/style_traits/values.rs index 481eb1aff6d..9a618067fd7 100644 --- a/components/style_traits/values.rs +++ b/components/style_traits/values.rs @@ -37,6 +37,9 @@ use std::fmt::{self, Write}; /// * if `#[css(skip_if = "function")]` is found on a field, the `ToCss` call /// for that field is skipped if `function` returns true. This function is /// provided the field as an argument; +/// * `#[css(represents_keyword)]` can be used on bool fields in order to +/// serialize the field name if the field is true, or nothing otherwise. It +/// also collects those keywords for `SpecifiedValueInfo`. /// * finally, one can put `#[css(derive_debug)]` on the whole type, to /// implement `Debug` by a single call to `ToCss::to_css`. pub trait ToCss { From 40a616920c8751baebadf03345ac4e618c7e3e01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 29 Apr 2018 05:34:28 +0200 Subject: [PATCH 04/19] style: Remove values::Verbatim. No point of having two things that do the same. Bug: 1457635 Reviewed-by: xidorn MozReview-Commit-ID: Do1L4bvOeVQ --- components/style_derive/to_css.rs | 2 +- components/style_traits/values.rs | 15 --------------- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/components/style_derive/to_css.rs b/components/style_derive/to_css.rs index 7ac7d3a1ae7..0caef8b01a7 100644 --- a/components/style_derive/to_css.rs +++ b/components/style_derive/to_css.rs @@ -175,7 +175,7 @@ fn derive_single_field_expr( { let mut iter = #field.iter().peekable(); if iter.peek().is_none() { - writer.item(&::style_traits::values::Verbatim(#if_empty))?; + writer.raw_item(#if_empty)?; } else { for item in iter { writer.item(&item)?; diff --git a/components/style_traits/values.rs b/components/style_traits/values.rs index 9a618067fd7..29b8eee58ea 100644 --- a/components/style_traits/values.rs +++ b/components/style_traits/values.rs @@ -235,21 +235,6 @@ where } } -/// A wrapper type that implements `ToCss` by printing its inner field. -pub struct Verbatim<'a, T>(pub &'a T) -where - T: ?Sized + 'a; - -impl<'a, T> ToCss for Verbatim<'a, T> -where - T: AsRef + ?Sized + 'a, -{ - #[inline] - fn to_css(&self, dest: &mut CssWriter) -> fmt::Result where W: Write { - dest.write_str(self.0.as_ref()) - } -} - /// Type used as the associated type in the `OneOrMoreSeparated` trait on a /// type to indicate that a serialized list of elements of this type is /// separated by commas. From 4f161d9339336ea265b9b4009e7f48e559ab4a41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 1 May 2018 07:09:12 +0200 Subject: [PATCH 05/19] style: Handle Shadow DOM in keyframes lookup. Bug: 1018269 Reviewed-by: heycam MozReview-Commit-ID: AeUmsOHOUYR --- components/style/stylist.rs | 52 ++++++++++++++++++++++++++++++++----- ports/geckolib/glue.rs | 7 ++--- 2 files changed, 49 insertions(+), 10 deletions(-) diff --git a/components/style/stylist.rs b/components/style/stylist.rs index a76a461b73f..732edc212b5 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -1421,14 +1421,52 @@ impl Stylist { } /// Returns the registered `@keyframes` animation for the specified name. - /// - /// FIXME(emilio): This needs to account for the element rules. #[inline] - pub fn get_animation(&self, name: &Atom) -> Option<&KeyframesAnimation> { - self.cascade_data - .iter_origins() - .filter_map(|(d, _)| d.animations.get(name)) - .next() + pub fn get_animation<'a, E>( + &'a self, + name: &Atom, + element: E, + ) -> Option<&'a KeyframesAnimation> + where + E: TElement + 'a, + { + macro_rules! try_find_in { + ($data:expr) => { + if let Some(animation) = $data.animations.get(name) { + return Some(animation); + } + } + } + + // NOTE(emilio): We implement basically what Blink does for this case, + // which is [1] as of this writing. + // + // See [2] for the spec discussion about what to do about this. WebKit's + // behavior makes a bit more sense off-hand, but it's way more complex + // to implement, and it makes value computation having to thread around + // the cascade level, which is not great. Also, it breaks if you inherit + // animation-name from an element in a different tree. + // + // See [3] for the bug to implement whatever gets resolved, and related + // bugs for a bit more context. + // + // [1]: https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/css/resolver/style_resolver.cc?l=1267&rcl=90f9f8680ebb4a87d177f3b0833372ae4e0c88d8 + // [2]: https://github.com/w3c/csswg-drafts/issues/1995 + // [3]: https://bugzil.la/1458189 + if let Some(shadow) = element.shadow_root() { + try_find_in!(shadow.style_data()); + } + + if let Some(shadow) = element.containing_shadow() { + try_find_in!(shadow.style_data()); + } else { + try_find_in!(self.cascade_data.author); + } + + try_find_in!(self.cascade_data.user); + try_find_in!(self.cascade_data.user_agent.cascade_data); + + None } /// Computes the match results of a given element against the set of diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index e602401c954..96c93209340 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -4768,17 +4768,18 @@ fn fill_in_missing_keyframe_values( #[no_mangle] pub unsafe extern "C" fn Servo_StyleSet_GetKeyframesForName( raw_data: RawServoStyleSetBorrowed, + element: RawGeckoElementBorrowed, name: *mut nsAtom, inherited_timing_function: nsTimingFunctionBorrowed, keyframes: RawGeckoKeyframeListBorrowedMut, ) -> bool { - debug_assert!(keyframes.len() == 0, - "keyframes should be initially empty"); + debug_assert!(keyframes.len() == 0, "keyframes should be initially empty"); + let element = GeckoElement(element); let data = PerDocumentStyleData::from_ffi(raw_data).borrow(); let name = Atom::from_raw(name); - let animation = match data.stylist.get_animation(&name) { + let animation = match data.stylist.get_animation(&name, element) { Some(animation) => animation, None => return false, }; From 9ed48952f5f64b5666fb4c672fa8e8c468113d26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 30 Apr 2018 17:50:03 +0200 Subject: [PATCH 06/19] style: Merge ServoStyleSheet and StyleSheet. These are the most minimal changes I could make. More cleanups incoming. Bug: 1457920 Reviewed-by: xidorn MozReview-Commit-ID: AdMOA1acQIH --- components/style/gecko/data.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/components/style/gecko/data.rs b/components/style/gecko/data.rs index 567a9b7f6ca..ac41eaa14c3 100644 --- a/components/style/gecko/data.rs +++ b/components/style/gecko/data.rs @@ -9,7 +9,7 @@ use context::QuirksMode; use dom::TElement; use gecko_bindings::bindings::{self, RawServoStyleSet}; use gecko_bindings::structs::{self, RawGeckoPresContextOwned, ServoStyleSetSizes, ServoStyleSheet}; -use gecko_bindings::structs::{ServoStyleSheetInner, StyleSheetInfo, nsIDocument}; +use gecko_bindings::structs::{StyleSheetInfo, nsIDocument}; use gecko_bindings::sugar::ownership::{HasArcFFI, HasBoxFFI, HasFFI, HasSimpleFFI}; use invalidation::media_queries::{MediaListKey, ToMediaListKey}; use malloc_size_of::MallocSizeOfOps; @@ -54,9 +54,9 @@ impl GeckoStyleSheet { unsafe { &*self.0 } } - fn inner(&self) -> &ServoStyleSheetInner { + fn inner(&self) -> &StyleSheetInfo { unsafe { - &*(self.raw()._base.mInner as *const StyleSheetInfo as *const ServoStyleSheetInner) + &*(self.raw().mInner as *const StyleSheetInfo) } } @@ -98,7 +98,7 @@ impl StylesheetInDocument for GeckoStyleSheet { use std::mem; unsafe { - let dom_media_list = self.raw()._base.mMedia.mRawPtr as *const DomMediaList; + let dom_media_list = self.raw().mMedia.mRawPtr as *const DomMediaList; if dom_media_list.is_null() { return None; } From 9d721072f585c10a27d72ae52ea7a5c77e3f6eba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 30 Apr 2018 19:23:14 +0200 Subject: [PATCH 07/19] style: Remove ServoStyleSheet usage. Bug: 1457920 Reviewed-by: xidorn MozReview-Commit-ID: LIBkovuQ6MB --- components/style/gecko/data.rs | 16 +++++++-------- components/style/shared_lock.rs | 2 +- ports/geckolib/error_reporter.rs | 10 +++++---- ports/geckolib/glue.rs | 32 ++++++++++++++--------------- ports/geckolib/stylesheet_loader.rs | 14 +++++++------ 5 files changed, 39 insertions(+), 35 deletions(-) diff --git a/components/style/gecko/data.rs b/components/style/gecko/data.rs index ac41eaa14c3..a4ed8294ac2 100644 --- a/components/style/gecko/data.rs +++ b/components/style/gecko/data.rs @@ -8,7 +8,7 @@ use atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut}; use context::QuirksMode; use dom::TElement; use gecko_bindings::bindings::{self, RawServoStyleSet}; -use gecko_bindings::structs::{self, RawGeckoPresContextOwned, ServoStyleSetSizes, ServoStyleSheet}; +use gecko_bindings::structs::{self, RawGeckoPresContextOwned, ServoStyleSetSizes, StyleSheet as DomStyleSheet}; use gecko_bindings::structs::{StyleSheetInfo, nsIDocument}; use gecko_bindings::sugar::ownership::{HasArcFFI, HasBoxFFI, HasFFI, HasSimpleFFI}; use invalidation::media_queries::{MediaListKey, ToMediaListKey}; @@ -23,7 +23,7 @@ use stylist::Stylist; /// Little wrapper to a Gecko style sheet. #[derive(Debug, Eq, PartialEq)] -pub struct GeckoStyleSheet(*const ServoStyleSheet); +pub struct GeckoStyleSheet(*const DomStyleSheet); impl ToMediaListKey for ::gecko::data::GeckoStyleSheet { fn to_media_list_key(&self) -> MediaListKey { @@ -33,24 +33,24 @@ impl ToMediaListKey for ::gecko::data::GeckoStyleSheet { } impl GeckoStyleSheet { - /// Create a `GeckoStyleSheet` from a raw `ServoStyleSheet` pointer. + /// Create a `GeckoStyleSheet` from a raw `DomStyleSheet` pointer. #[inline] - pub unsafe fn new(s: *const ServoStyleSheet) -> Self { + pub unsafe fn new(s: *const DomStyleSheet) -> Self { debug_assert!(!s.is_null()); bindings::Gecko_StyleSheet_AddRef(s); Self::from_addrefed(s) } - /// Create a `GeckoStyleSheet` from a raw `ServoStyleSheet` pointer that + /// Create a `GeckoStyleSheet` from a raw `DomStyleSheet` pointer that /// already holds a strong reference. #[inline] - pub unsafe fn from_addrefed(s: *const ServoStyleSheet) -> Self { + pub unsafe fn from_addrefed(s: *const DomStyleSheet) -> Self { debug_assert!(!s.is_null()); GeckoStyleSheet(s) } - /// Get the raw `ServoStyleSheet` that we're wrapping. - pub fn raw(&self) -> &ServoStyleSheet { + /// Get the raw `StyleSheet` that we're wrapping. + pub fn raw(&self) -> &DomStyleSheet { unsafe { &*self.0 } } diff --git a/components/style/shared_lock.rs b/components/style/shared_lock.rs index 9bb54a274c6..e79f684a7bb 100644 --- a/components/style/shared_lock.rs +++ b/components/style/shared_lock.rs @@ -237,7 +237,7 @@ pub trait ToCssWithGuard { #[cfg(feature = "gecko")] pub struct DeepCloneParams { /// The new sheet we're cloning rules into. - pub reference_sheet: *const ::gecko_bindings::structs::ServoStyleSheet, + pub reference_sheet: *const ::gecko_bindings::structs::StyleSheet, } /// Parameters needed for deep clones. diff --git a/ports/geckolib/error_reporter.rs b/ports/geckolib/error_reporter.rs index 7e59f416ff2..3faddf7dcd2 100644 --- a/ports/geckolib/error_reporter.rs +++ b/ports/geckolib/error_reporter.rs @@ -14,7 +14,7 @@ use std::ptr; use style::error_reporting::{ParseErrorReporter, ContextualParseError}; use style::gecko_bindings::bindings::{Gecko_CreateCSSErrorReporter, Gecko_DestroyCSSErrorReporter}; use style::gecko_bindings::bindings::Gecko_ReportUnexpectedCSSError; -use style::gecko_bindings::structs::{Loader, ServoStyleSheet, nsIURI}; +use style::gecko_bindings::structs::{Loader, StyleSheet as DomStyleSheet, nsIURI}; use style::gecko_bindings::structs::ErrorReporter as GeckoErrorReporter; use style::gecko_bindings::structs::URLExtraData as RawUrlExtraData; use style::stylesheets::UrlExtraData; @@ -27,9 +27,11 @@ pub struct ErrorReporter(*mut GeckoErrorReporter); impl ErrorReporter { /// Create a new instance of the Gecko error reporter. - pub fn new(sheet: *mut ServoStyleSheet, - loader: *mut Loader, - extra_data: *mut RawUrlExtraData) -> ErrorReporter { + pub fn new( + sheet: *mut DomStyleSheet, + loader: *mut Loader, + extra_data: *mut RawUrlExtraData, + ) -> Self { unsafe { let url = extra_data.as_ref() .map(|d| d.mBaseURI.raw::()) diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 96c93209340..0b0ca198806 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -92,7 +92,7 @@ use style::gecko_bindings::structs; use style::gecko_bindings::structs::{CallerType, CSSPseudoElementType, CompositeOperation}; use style::gecko_bindings::structs::{Loader, LoaderReusableStyleSheets}; use style::gecko_bindings::structs::{RawServoStyleRule, ComputedStyleStrong, RustString}; -use style::gecko_bindings::structs::{ServoStyleSheet, SheetLoadData, SheetLoadDataHolder}; +use style::gecko_bindings::structs::{StyleSheet as DomStyleSheet, SheetLoadData, SheetLoadDataHolder}; use style::gecko_bindings::structs::{SheetParsingMode, nsAtom, nsCSSPropertyID}; use style::gecko_bindings::structs::{nsCSSFontDesc, nsCSSCounterDesc}; use style::gecko_bindings::structs::{nsRestyleHint, nsChangeHint, PropertyValuePair}; @@ -1194,7 +1194,7 @@ fn mode_to_origin(mode: SheetParsingMode) -> Origin { #[no_mangle] pub extern "C" fn Servo_StyleSheet_FromUTF8Bytes( loader: *mut Loader, - stylesheet: *mut ServoStyleSheet, + stylesheet: *mut DomStyleSheet, load_data: *mut SheetLoadData, bytes: *const nsACString, mode: SheetParsingMode, @@ -1263,7 +1263,7 @@ pub extern "C" fn Servo_StyleSheet_FromUTF8BytesAsync( #[no_mangle] pub extern "C" fn Servo_StyleSet_AppendStyleSheet( raw_data: RawServoStyleSetBorrowed, - sheet: *const ServoStyleSheet, + sheet: *const DomStyleSheet, ) { let global_style_data = &*GLOBAL_STYLE_DATA; let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); @@ -1288,7 +1288,7 @@ pub extern "C" fn Servo_AuthorStyles_Drop( #[no_mangle] pub unsafe extern "C" fn Servo_AuthorStyles_AppendStyleSheet( styles: RawServoAuthorStylesBorrowedMut, - sheet: *const ServoStyleSheet, + sheet: *const DomStyleSheet, ) { let styles = AuthorStyles::::from_ffi_mut(styles); @@ -1301,8 +1301,8 @@ pub unsafe extern "C" fn Servo_AuthorStyles_AppendStyleSheet( #[no_mangle] pub unsafe extern "C" fn Servo_AuthorStyles_InsertStyleSheetBefore( styles: RawServoAuthorStylesBorrowedMut, - sheet: *const ServoStyleSheet, - before_sheet: *const ServoStyleSheet, + sheet: *const DomStyleSheet, + before_sheet: *const DomStyleSheet, ) { let styles = AuthorStyles::::from_ffi_mut(styles); @@ -1319,7 +1319,7 @@ pub unsafe extern "C" fn Servo_AuthorStyles_InsertStyleSheetBefore( #[no_mangle] pub unsafe extern "C" fn Servo_AuthorStyles_RemoveStyleSheet( styles: RawServoAuthorStylesBorrowedMut, - sheet: *const ServoStyleSheet, + sheet: *const DomStyleSheet, ) { let styles = AuthorStyles::::from_ffi_mut(styles); @@ -1454,7 +1454,7 @@ pub unsafe extern "C" fn Servo_StyleSet_MediumFeaturesChanged( #[no_mangle] pub extern "C" fn Servo_StyleSet_PrependStyleSheet( raw_data: RawServoStyleSetBorrowed, - sheet: *const ServoStyleSheet, + sheet: *const DomStyleSheet, ) { let global_style_data = &*GLOBAL_STYLE_DATA; let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); @@ -1467,8 +1467,8 @@ pub extern "C" fn Servo_StyleSet_PrependStyleSheet( #[no_mangle] pub extern "C" fn Servo_StyleSet_InsertStyleSheetBefore( raw_data: RawServoStyleSetBorrowed, - sheet: *const ServoStyleSheet, - before_sheet: *const ServoStyleSheet + sheet: *const DomStyleSheet, + before_sheet: *const DomStyleSheet ) { let global_style_data = &*GLOBAL_STYLE_DATA; let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); @@ -1485,7 +1485,7 @@ pub extern "C" fn Servo_StyleSet_InsertStyleSheetBefore( #[no_mangle] pub extern "C" fn Servo_StyleSet_RemoveStyleSheet( raw_data: RawServoStyleSetBorrowed, - sheet: *const ServoStyleSheet + sheet: *const DomStyleSheet ) { let global_style_data = &*GLOBAL_STYLE_DATA; let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); @@ -1560,7 +1560,7 @@ pub extern "C" fn Servo_StyleSheet_GetRules( #[no_mangle] pub extern "C" fn Servo_StyleSheet_Clone( raw_sheet: RawServoStyleSheetContentsBorrowed, - reference_sheet: *const ServoStyleSheet, + reference_sheet: *const DomStyleSheet, ) -> RawServoStyleSheetContentsStrong { use style::shared_lock::{DeepCloneParams, DeepCloneWithLock}; let global_style_data = &*GLOBAL_STYLE_DATA; @@ -1681,7 +1681,7 @@ pub extern "C" fn Servo_CssRules_InsertRule( index: u32, nested: bool, loader: *mut Loader, - gecko_stylesheet: *mut ServoStyleSheet, + gecko_stylesheet: *mut DomStyleSheet, rule_type: *mut u16, ) -> nsresult { let loader = if loader.is_null() { @@ -2136,16 +2136,16 @@ pub extern "C" fn Servo_ImportRule_GetHref(rule: RawServoImportRuleBorrowed, res #[no_mangle] pub extern "C" fn Servo_ImportRule_GetSheet( rule: RawServoImportRuleBorrowed, -) -> *const ServoStyleSheet { +) -> *const DomStyleSheet { read_locked_arc(rule, |rule: &ImportRule| { - rule.stylesheet.as_sheet().unwrap().raw() as *const ServoStyleSheet + rule.stylesheet.as_sheet().unwrap().raw() as *const DomStyleSheet }) } #[no_mangle] pub extern "C" fn Servo_ImportRule_SetSheet( rule: RawServoImportRuleBorrowed, - sheet: *mut ServoStyleSheet, + sheet: *mut DomStyleSheet, ) { write_locked_arc(rule, |rule: &mut ImportRule| { let sheet = unsafe { GeckoStyleSheet::new(sheet) }; diff --git a/ports/geckolib/stylesheet_loader.rs b/ports/geckolib/stylesheet_loader.rs index 0c320fee897..7ed1db57bc3 100644 --- a/ports/geckolib/stylesheet_loader.rs +++ b/ports/geckolib/stylesheet_loader.rs @@ -12,7 +12,7 @@ use style::gecko::global_style_data::GLOBAL_STYLE_DATA; use style::gecko_bindings::bindings; use style::gecko_bindings::bindings::Gecko_LoadStyleSheet; use style::gecko_bindings::structs::{Loader, LoaderReusableStyleSheets}; -use style::gecko_bindings::structs::{ServoStyleSheet, SheetLoadData, SheetLoadDataHolder}; +use style::gecko_bindings::structs::{StyleSheet as DomStyleSheet, SheetLoadData, SheetLoadDataHolder}; use style::gecko_bindings::structs::URLExtraData; use style::gecko_bindings::sugar::ownership::FFIArcHelpers; use style::gecko_bindings::sugar::refptr::RefPtr; @@ -24,13 +24,15 @@ use style::stylesheets::StylesheetContents; use style::stylesheets::import_rule::ImportSheet; use style::values::CssUrl; -pub struct StylesheetLoader(*mut Loader, *mut ServoStyleSheet, *mut SheetLoadData, *mut LoaderReusableStyleSheets); +pub struct StylesheetLoader(*mut Loader, *mut DomStyleSheet, *mut SheetLoadData, *mut LoaderReusableStyleSheets); impl StylesheetLoader { - pub fn new(loader: *mut Loader, - parent: *mut ServoStyleSheet, - parent_load_data: *mut SheetLoadData, - reusable_sheets: *mut LoaderReusableStyleSheets) -> Self { + pub fn new( + loader: *mut Loader, + parent: *mut DomStyleSheet, + parent_load_data: *mut SheetLoadData, + reusable_sheets: *mut LoaderReusableStyleSheets, + ) -> Self { StylesheetLoader(loader, parent, parent_load_data, reusable_sheets) } } From 0ef70d52f22a0f9a3398866a5db9447dd5407c3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 3 May 2018 10:15:15 +0200 Subject: [PATCH 08/19] style: Fix perspective interpolation. It's not sound to insert random matrices in random positions in the transform operation list. I cannot make any sense of what the old code was trying to do. Bug: 1458715 Reviewed-by: hiro MozReview-Commit-ID: 5BtCiueEPlR --- .../helpers/animated_properties.mako.rs | 25 ++++--------------- components/style/values/computed/transform.rs | 6 ++--- 2 files changed, 8 insertions(+), 23 deletions(-) diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index 57a8ed99880..387507516e8 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -1291,16 +1291,8 @@ impl Animate for ComputedTransformOperation { &TransformOperation::Perspective(ref fd), &TransformOperation::Perspective(ref td), ) => { - let mut fd_matrix = Matrix3D::identity(); - let mut td_matrix = Matrix3D::identity(); - if fd.px() > 0. { - fd_matrix.m34 = -1. / fd.px(); - } - if td.px() > 0. { - td_matrix.m34 = -1. / td.px(); - } - Ok(TransformOperation::Matrix3D( - fd_matrix.animate(&td_matrix, procedure)?, + Ok(TransformOperation::Perspective( + fd.animate(td, procedure)? )) }, _ if self.is_translate() && other.is_translate() => { @@ -2640,16 +2632,7 @@ impl ComputeSquaredDistance for ComputedTransformOperation { &TransformOperation::Perspective(ref fd), &TransformOperation::Perspective(ref td), ) => { - let mut fd_matrix = Matrix3D::identity(); - let mut td_matrix = Matrix3D::identity(); - if fd.px() > 0. { - fd_matrix.m34 = -1. / fd.px(); - } - - if td.px() > 0. { - td_matrix.m34 = -1. / td.px(); - } - fd_matrix.compute_squared_distance(&td_matrix) + fd.compute_squared_distance(td) } ( &TransformOperation::Perspective(ref p), @@ -2658,6 +2641,8 @@ impl ComputeSquaredDistance for ComputedTransformOperation { &TransformOperation::Matrix3D(ref m), &TransformOperation::Perspective(ref p), ) => { + // FIXME(emilio): Is this right? Why interpolating this with + // Perspective but not with anything else? let mut p_matrix = Matrix3D::identity(); if p.px() > 0. { p_matrix.m34 = -1. / p.px(); diff --git a/components/style/values/computed/transform.rs b/components/style/values/computed/transform.rs index 38b45f83669..fd7a7cf5c2d 100644 --- a/components/style/values/computed/transform.rs +++ b/components/style/values/computed/transform.rs @@ -251,11 +251,11 @@ impl ToAnimatedZero for TransformOperation { generic::TransformOperation::Rotate(_) => { Ok(generic::TransformOperation::Rotate(Angle::zero())) }, - generic::TransformOperation::Perspective(..) | + generic::TransformOperation::Perspective(ref l) => { + Ok(generic::TransformOperation::Perspective(l.to_animated_zero()?)) + }, generic::TransformOperation::AccumulateMatrix { .. } | generic::TransformOperation::InterpolateMatrix { .. } => { - // Perspective: We convert a perspective function into an equivalent - // ComputedMatrix, and then decompose/interpolate/recompose these matrices. // AccumulateMatrix/InterpolateMatrix: We do interpolation on // AccumulateMatrix/InterpolateMatrix by reading it as a ComputedMatrix // (with layout information), and then do matrix interpolation. From 6ecc7445fc5fc654cbbf4b818b4549699f07d71a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 3 May 2018 19:24:05 +0200 Subject: [PATCH 09/19] style: Make the font canvas code not mess with the rule tree. Inserting a lot of rules in the root level is super-inefficient. This fixes it by not doing it. It gives the root rule node to the style, but that's fine, since it's useless. All this code-path is already pretty messy. Bug: 1457678 Reviewed-by: xidorn,hiro MozReview-Commit-ID: GoGHI4YJbKr --- components/style/stylist.rs | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 732edc212b5..54d006d4ff8 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -1555,27 +1555,25 @@ impl Stylist { E: TElement, { use font_metrics::get_metrics_provider_for_product; - use std::iter; - // FIXME(emilio): Why do we even need the rule node? We should probably - // just avoid allocating it and calling `apply_declarations` directly, - // maybe... - let rule_node = self.rule_tree.insert_ordered_rules(iter::once(( - StyleSource::from_declarations(declarations), - CascadeLevel::StyleAttributeNormal, - ))); + let block = declarations.read_with(guards.author); + let iter_declarations = || { + block.declaration_importance_iter().map(|(declaration, importance)| { + debug_assert!(!importance.important()); + (declaration, CascadeLevel::StyleAttributeNormal) + }) + }; - // This currently ignores visited styles. It appears to be used for - // font styles in via Servo_StyleSet_ResolveForDeclarations. - // It is unclear if visited styles are meaningful for this case. let metrics = get_metrics_provider_for_product(); - // FIXME(emilio): the pseudo bit looks quite dubious! - properties::cascade::( + // We don't bother inserting these declarations in the rule tree, since + // it'd be quite useless and slow. + properties::apply_declarations::( &self.device, /* pseudo = */ None, - &rule_node, + self.rule_tree.root(), guards, + iter_declarations, Some(parent_style), Some(parent_style), Some(parent_style), From 12913d048f402c3516a299ac2fab3d66bddcdc6a Mon Sep 17 00:00:00 2001 From: Jonathan Watt Date: Wed, 25 Apr 2018 14:54:26 +0100 Subject: [PATCH 10/19] style: Remove the 'property_name' macro. Bug: 1458219 Reviewed-by: xidorn --- components/style/properties/properties.mako.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index a1d2ee9d085..5e3d8eaeaaf 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -56,12 +56,6 @@ use style_adjuster::StyleAdjuster; pub use self::declaration_block::*; -#[cfg(feature = "gecko")] -#[macro_export] -macro_rules! property_name { - ($s: tt) => { atom!($s) } -} - <%! from data import Method, Keyword, to_rust_ident, to_camel_case, SYSTEM_FONT_LONGHANDS import os.path From 09b22ab2dc264cf22647c273ed1c94cdd865f7df Mon Sep 17 00:00:00 2001 From: Xidorn Quan Date: Wed, 2 May 2018 21:07:07 +1000 Subject: [PATCH 11/19] style: Allow 16% false positives in test bloom::create_and_insert_some_stuff. It seems that the result of hash algorithm used in bloom filter depends on the pointer length. On 64bit platforms, there are 135 false positives in the first part of that test, and 8 in the second part. However, on 32bit platforms, the numbers become 157 and 16 correspondingly. 16 is still less than 20% in the second part, so all fine, but 157 is slightly larger than 15% in the test assertion. Given it is what we are shipping, we probably should just accept this and loosen the assertion. Bug: 1457524 Reviewed-by: heycam MozReview-Commit-ID: 9kFXBzLFAzE --- components/selectors/bloom.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/selectors/bloom.rs b/components/selectors/bloom.rs index e06649d73bf..e4cd0a77f98 100644 --- a/components/selectors/bloom.rs +++ b/components/selectors/bloom.rs @@ -335,7 +335,7 @@ fn create_and_insert_some_stuff() { let false_positives = (1001_usize..2000).filter(|i| bf.might_contain(i)).count(); - assert!(false_positives < 150, "{} is not < 150", false_positives); // 15%. + assert!(false_positives < 160, "{} is not < 160", false_positives); // 16%. for i in 0_usize..100 { bf.remove(&i); From 39169ad92ed2d3120b4e76c3658353a9346c8a5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 5 May 2018 16:24:36 +0200 Subject: [PATCH 12/19] style: Fix tidy issues. --- components/style/stylist.rs | 3 ++- components/style/values/specified/counters.rs | 2 +- ports/geckolib/glue.rs | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 54d006d4ff8..34f187e483a 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -1450,7 +1450,8 @@ impl Stylist { // See [3] for the bug to implement whatever gets resolved, and related // bugs for a bit more context. // - // [1]: https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/css/resolver/style_resolver.cc?l=1267&rcl=90f9f8680ebb4a87d177f3b0833372ae4e0c88d8 + // [1]: https://cs.chromium.org/chromium/src/third_party/blink/renderer/ + // core/css/resolver/style_resolver.cc?l=1267&rcl=90f9f8680ebb4a87d177f3b0833372ae4e0c88d8 // [2]: https://github.com/w3c/csswg-drafts/issues/1995 // [3]: https://bugzil.la/1458189 if let Some(shadow) = element.shadow_root() { diff --git a/components/style/values/specified/counters.rs b/components/style/values/specified/counters.rs index 64bdd04becc..3daf8ee56a0 100644 --- a/components/style/values/specified/counters.rs +++ b/components/style/values/specified/counters.rs @@ -12,8 +12,8 @@ use style_traits::{ParseError, StyleParseErrorKind}; use values::CustomIdent; #[cfg(feature = "gecko")] use values::generics::CounterStyleOrNone; -use values::generics::counters::CounterPair; use values::generics::counters::CounterIncrement as GenericCounterIncrement; +use values::generics::counters::CounterPair; use values::generics::counters::CounterReset as GenericCounterReset; #[cfg(feature = "gecko")] use values::specified::Attr; diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 0b0ca198806..93918a1c997 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -92,8 +92,8 @@ use style::gecko_bindings::structs; use style::gecko_bindings::structs::{CallerType, CSSPseudoElementType, CompositeOperation}; use style::gecko_bindings::structs::{Loader, LoaderReusableStyleSheets}; use style::gecko_bindings::structs::{RawServoStyleRule, ComputedStyleStrong, RustString}; -use style::gecko_bindings::structs::{StyleSheet as DomStyleSheet, SheetLoadData, SheetLoadDataHolder}; use style::gecko_bindings::structs::{SheetParsingMode, nsAtom, nsCSSPropertyID}; +use style::gecko_bindings::structs::{StyleSheet as DomStyleSheet, SheetLoadData, SheetLoadDataHolder}; use style::gecko_bindings::structs::{nsCSSFontDesc, nsCSSCounterDesc}; use style::gecko_bindings::structs::{nsRestyleHint, nsChangeHint, PropertyValuePair}; use style::gecko_bindings::structs::AtomArray; From 33b593d32e8bcd4f5575ba46499aa420efe843c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 5 May 2018 17:19:06 +0200 Subject: [PATCH 13/19] style: Fix servo build. --- components/layout/animation.rs | 6 ++-- components/layout/generated_content.rs | 16 ++++----- components/style/animation.rs | 35 +++++++++---------- components/style/matching.rs | 1 + .../style/stylesheets/keyframes_rule.rs | 6 ++-- 5 files changed, 32 insertions(+), 32 deletions(-) diff --git a/components/layout/animation.rs b/components/layout/animation.rs index 3ea59e7b972..05718dda00a 100644 --- a/components/layout/animation.rs +++ b/components/layout/animation.rs @@ -39,14 +39,14 @@ where let mut new_running_animations = vec![]; while let Ok(animation) = new_animations_receiver.try_recv() { let mut should_push = true; - if let Animation::Keyframes(ref node, ref name, ref state) = animation { + if let Animation::Keyframes(ref node, _, ref name, ref state) = animation { // If the animation was already present in the list for the // node, just update its state, else push the new animation to // run. if let Some(ref mut animations) = running_animations.get_mut(node) { // TODO: This being linear is probably not optimal. for anim in animations.iter_mut() { - if let Animation::Keyframes(_, ref anim_name, ref mut anim_state) = *anim { + if let Animation::Keyframes(_, _, ref anim_name, ref mut anim_state) = *anim { if *name == *anim_name { debug!("update_animation_state: Found other animation {}", name); anim_state.update_from_other(&state, timer); @@ -83,7 +83,7 @@ where Animation::Transition(_, started_at, ref frame, _expired) => { now < started_at + frame.duration } - Animation::Keyframes(_, _, ref mut state) => { + Animation::Keyframes(_, _, _, ref mut state) => { // This animation is still running, or we need to keep // iterating. now < state.started_at + state.duration || state.tick() diff --git a/components/layout/generated_content.rs b/components/layout/generated_content.rs index 700e9bc9ddd..da390cc9cec 100644 --- a/components/layout/generated_content.rs +++ b/components/layout/generated_content.rs @@ -272,27 +272,27 @@ impl<'a, 'b> ResolveGeneratedContentFragmentMutator<'a, 'b> { } self.traversal.list_item.truncate_to_level(self.level); - for &(ref counter_name, value) in &*fragment.style().get_counters().counter_reset { - let counter_name = &*counter_name.0; + for pair in &*fragment.style().get_counters().counter_reset { + let counter_name = &*pair.name.0; if let Some(ref mut counter) = self.traversal.counters.get_mut(counter_name) { - counter.reset(self.level, value); + counter.reset(self.level, pair.value); continue } let mut counter = Counter::new(); - counter.reset(self.level, value); + counter.reset(self.level, pair.value); self.traversal.counters.insert(counter_name.to_owned(), counter); } - for &(ref counter_name, value) in &*fragment.style().get_counters().counter_increment { - let counter_name = &*counter_name.0; + for pair in &*fragment.style().get_counters().counter_increment { + let counter_name = &*pair.name.0; if let Some(ref mut counter) = self.traversal.counters.get_mut(counter_name) { - counter.increment(self.level, value); + counter.increment(self.level, pair.value); continue } let mut counter = Counter::new(); - counter.increment(self.level, value); + counter.increment(self.level, pair.value); self.traversal.counters.insert(counter_name.to_owned(), counter); } diff --git a/components/style/animation.rs b/components/style/animation.rs index 9fc93a6cd5b..983078e8d9d 100644 --- a/components/style/animation.rs +++ b/components/style/animation.rs @@ -17,7 +17,7 @@ use properties::longhands::animation_play_state::computed_value::single_value::T use rule_tree::CascadeLevel; use servo_arc::Arc; use std::sync::mpsc::Sender; -use stylesheets::keyframes_rule::{KeyframesStep, KeyframesStepValue}; +use stylesheets::keyframes_rule::{KeyframesAnimation, KeyframesStep, KeyframesStepValue}; use timer::Timer; use values::computed::Time; use values::computed::transform::TimingFunction; @@ -194,7 +194,9 @@ pub enum Animation { Transition(OpaqueNode, f64, AnimationFrame, bool), /// A keyframes animation is identified by a name, and can have a /// node-dependent state (i.e. iteration count, etc.). - Keyframes(OpaqueNode, Atom, KeyframesAnimationState), + /// + /// TODO(emilio): The animation object could be refcounted. + Keyframes(OpaqueNode, KeyframesAnimation, Atom, KeyframesAnimationState), } impl Animation { @@ -204,7 +206,7 @@ impl Animation { debug_assert!(!self.is_expired()); match *self { Animation::Transition(_, _, _, ref mut expired) => *expired = true, - Animation::Keyframes(_, _, ref mut state) => state.expired = true, + Animation::Keyframes(_, _, _, ref mut state) => state.expired = true, } } @@ -213,7 +215,7 @@ impl Animation { pub fn is_expired(&self) -> bool { match *self { Animation::Transition(_, _, _, expired) => expired, - Animation::Keyframes(_, _, ref state) => state.expired, + Animation::Keyframes(_, _, _, ref state) => state.expired, } } @@ -222,7 +224,7 @@ impl Animation { pub fn node(&self) -> &OpaqueNode { match *self { Animation::Transition(ref node, _, _, _) => node, - Animation::Keyframes(ref node, _, _) => node, + Animation::Keyframes(ref node, _, _, _) => node, } } @@ -231,7 +233,7 @@ impl Animation { pub fn is_paused(&self) -> bool { match *self { Animation::Transition(..) => false, - Animation::Keyframes(_, _, ref state) => state.is_paused(), + Animation::Keyframes(_, _, _, ref state) => state.is_paused(), } } @@ -500,12 +502,16 @@ where /// Triggers animations for a given node looking at the animation property /// values. -pub fn maybe_start_animations( +pub fn maybe_start_animations( + element: E, context: &SharedStyleContext, new_animations_sender: &Sender, node: OpaqueNode, new_style: &Arc, -) -> bool { +) -> bool +where + E: TElement, +{ let mut had_animations = false; let box_style = new_style.get_box(); @@ -522,7 +528,7 @@ pub fn maybe_start_animations( continue; } - if let Some(ref anim) = context.stylist.get_animation(name) { + if let Some(anim) = context.stylist.get_animation(name, element) { debug!("maybe_start_animations: animation {} found", name); // If this animation doesn't have any keyframe, we can just continue @@ -561,6 +567,7 @@ pub fn maybe_start_animations( new_animations_sender .send(Animation::Keyframes( node, + anim.clone(), name.clone(), KeyframesAnimationState { started_at: animation_start, @@ -628,7 +635,7 @@ pub fn update_style_for_animation( *style = new_style } }, - Animation::Keyframes(_, ref name, ref state) => { + Animation::Keyframes(_, ref animation, ref name, ref state) => { debug!( "update_style_for_animation: animation found: \"{}\", {:?}", name, state @@ -641,14 +648,6 @@ pub fn update_style_for_animation( KeyframesRunningState::Paused(progress) => started_at + duration * progress, }; - let animation = match context.stylist.get_animation(name) { - None => { - warn!("update_style_for_animation: Animation {:?} not found", name); - return; - }, - Some(animation) => animation, - }; - debug_assert!(!animation.steps.is_empty()); let maybe_index = style diff --git a/components/style/matching.rs b/components/style/matching.rs index 97b31f4f6b0..1d5cbf8d375 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -425,6 +425,7 @@ trait PrivateMatchMethods: TElement { let this_opaque = self.as_node().opaque(); // Trigger any present animations if necessary. animation::maybe_start_animations( + *self, &shared_context, new_animations_sender, this_opaque, diff --git a/components/style/stylesheets/keyframes_rule.rs b/components/style/stylesheets/keyframes_rule.rs index c4649dc135f..4de3acbaff5 100644 --- a/components/style/stylesheets/keyframes_rule.rs +++ b/components/style/stylesheets/keyframes_rule.rs @@ -259,7 +259,7 @@ impl DeepCloneWithLock for Keyframe { /// declarations to apply. /// /// TODO: Find a better name for this? -#[derive(Debug, MallocSizeOf)] +#[derive(Clone, Debug, MallocSizeOf)] pub enum KeyframesStepValue { /// A step formed by a declaration block specified by the CSS. Declarations { @@ -275,7 +275,7 @@ pub enum KeyframesStepValue { } /// A single step from a keyframe animation. -#[derive(Debug, MallocSizeOf)] +#[derive(Clone, Debug, MallocSizeOf)] pub struct KeyframesStep { /// The percentage of the animation duration when this step starts. pub start_percentage: KeyframePercentage, @@ -352,7 +352,7 @@ impl KeyframesStep { /// of keyframes, in order. /// /// It only takes into account animable properties. -#[derive(Debug, MallocSizeOf)] +#[derive(Clone, Debug, MallocSizeOf)] pub struct KeyframesAnimation { /// The difference steps of the animation. pub steps: Vec, From cb48a837bf3bcdd1fd801ecd3a31e5e41139cf66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 5 May 2018 17:39:12 +0200 Subject: [PATCH 14/19] style: Implement Debug for KeyframeAnimationStyle by hand. The ComputedValues format is huge and unneeded. --- components/style/animation.rs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/components/style/animation.rs b/components/style/animation.rs index 983078e8d9d..685d3933512 100644 --- a/components/style/animation.rs +++ b/components/style/animation.rs @@ -16,6 +16,7 @@ use properties::longhands::animation_direction::computed_value::single_value::T use properties::longhands::animation_play_state::computed_value::single_value::T as AnimationPlayState; use rule_tree::CascadeLevel; use servo_arc::Arc; +use std::fmt; use std::sync::mpsc::Sender; use stylesheets::keyframes_rule::{KeyframesAnimation, KeyframesStep, KeyframesStepValue}; use timer::Timer; @@ -52,7 +53,7 @@ pub enum KeyframesRunningState { /// duration, the current and maximum iteration count, and the state (either /// playing or paused). // TODO: unify the use of f32/f64 in this file. -#[derive(Clone, Debug)] +#[derive(Clone)] pub struct KeyframesAnimationState { /// The time this animation started at. pub started_at: f64, @@ -183,6 +184,22 @@ impl KeyframesAnimationState { } } +impl fmt::Debug for KeyframesAnimationState { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.debug_struct("KeyframesAnimationState") + .field("started_at", &self.started_at) + .field("duration", &self.duration) + .field("delay", &self.delay) + .field("iteration_state", &self.iteration_state) + .field("running_state", &self.running_state) + .field("direction", &self.direction) + .field("current_direction", &self.current_direction) + .field("expired", &self.expired) + .field("cascade_style", &()) + .finish() + } +} + /// State relating to an animation. #[derive(Clone, Debug)] pub enum Animation { From 921c3892473ca49bba42ff0ab9b9feb3b913a91d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 5 May 2018 17:28:58 +0200 Subject: [PATCH 15/19] style: Remove some unneeded cfg(..). The less not-compiled code in common builds, the better for everybody. --- components/style/animation.rs | 3 +-- components/style/lib.rs | 1 - .../properties/helpers/animated_properties.mako.rs | 12 +++++++----- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/components/style/animation.rs b/components/style/animation.rs index 685d3933512..bbf847d7f14 100644 --- a/components/style/animation.rs +++ b/components/style/animation.rs @@ -3,7 +3,6 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ //! CSS transitions and animations. -#![deny(missing_docs)] use Atom; use bezier::Bezier; @@ -409,7 +408,6 @@ impl PropertyAnimation { /// Inserts transitions into the queue of running animations as applicable for /// the given style difference. This is called from the layout worker threads. /// Returns true if any animations were kicked off and false otherwise. -#[cfg(feature = "servo")] pub fn start_transitions_if_applicable( new_animations_sender: &Sender, opaque_node: OpaqueNode, @@ -629,6 +627,7 @@ pub fn update_style_for_animation_frame( true } + /// Updates a single animation and associated style based on the current time. pub fn update_style_for_animation( context: &SharedStyleContext, diff --git a/components/style/lib.rs b/components/style/lib.rs index 538dce92b87..20a54c62680 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -103,7 +103,6 @@ extern crate void; #[macro_use] mod macros; -#[cfg(feature = "servo")] pub mod animation; pub mod applicable_declarations; #[allow(missing_docs)] // TODO. diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index 387507516e8..fa6166e17ae 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -196,7 +196,6 @@ pub fn nscsspropertyid_is_transitionable(property: nsCSSPropertyID) -> bool { /// An animated property interpolation between two computed values for that /// property. -#[cfg(feature = "servo")] #[derive(Clone, Debug, PartialEq)] #[cfg_attr(feature = "servo", derive(MallocSizeOf))] pub enum AnimatedProperty { @@ -213,7 +212,6 @@ pub enum AnimatedProperty { % endfor } -#[cfg(feature = "servo")] impl AnimatedProperty { /// Get the name of this property. pub fn name(&self) -> &'static str { @@ -255,9 +253,12 @@ impl AnimatedProperty { /// Update `style` with the proper computed style corresponding to this /// animation at `progress`. + #[cfg_attr(feature = "gecko", allow(unused))] pub fn update(&self, style: &mut ComputedValues, progress: f64) { - match *self { - % for prop in data.longhands: + #[cfg(feature = "servo")] + { + match *self { + % for prop in data.longhands: % if prop.animatable: AnimatedProperty::${prop.camel_case}(ref from, ref to) => { // https://drafts.csswg.org/web-animations/#discrete-animation-type @@ -276,7 +277,8 @@ impl AnimatedProperty { style.mutate_${prop.style_struct.name_lower}().set_${prop.ident}(value); } % endif - % endfor + % endfor + } } } From 953ba58b68e82ce140d34a8253c5ec7f96634b80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 5 May 2018 17:40:55 +0200 Subject: [PATCH 16/19] style: Remove a useless unit test that no longer compiles. --- tests/unit/style/properties/serialization.rs | 29 +------------------- 1 file changed, 1 insertion(+), 28 deletions(-) diff --git a/tests/unit/style/properties/serialization.rs b/tests/unit/style/properties/serialization.rs index e274d8a2a6a..e7b5689e05f 100644 --- a/tests/unit/style/properties/serialization.rs +++ b/tests/unit/style/properties/serialization.rs @@ -7,7 +7,7 @@ use style::computed_values::display::T as Display; use style::properties::{PropertyDeclaration, Importance}; use style::properties::declaration_block::PropertyDeclarationBlock; use style::properties::parse_property_declaration_list; -use style::values::{CustomIdent, RGBA}; +use style::values::RGBA; use style::values::specified::{BorderStyle, BorderSideWidth, Color}; use style::values::specified::{Length, LengthOrPercentage, LengthOrPercentageOrAuto}; use style::values::specified::NoCalcLength; @@ -890,31 +890,4 @@ mod shorthand_serialization { assert_eq!(shadow.to_css_string(), shadow_css); } } - - mod counter_increment { - pub use super::*; - pub use style::properties::longhands::counter_increment::SpecifiedValue as CounterIncrement; - use style::values::specified::Integer; - - #[test] - fn counter_increment_with_properties_should_serialize_correctly() { - let mut properties = Vec::new(); - - properties.push((CustomIdent("counter1".into()), Integer::new(1))); - properties.push((CustomIdent("counter2".into()), Integer::new(-4))); - - let counter_increment = CounterIncrement::new(properties); - let counter_increment_css = "counter1 1 counter2 -4"; - - assert_eq!(counter_increment.to_css_string(), counter_increment_css); - } - - #[test] - fn counter_increment_without_properties_should_serialize_correctly() { - let counter_increment = CounterIncrement::new(Vec::new()); - let counter_increment_css = "none"; - - assert_eq!(counter_increment.to_css_string(), counter_increment_css); - } - } } From 83cb0992db1f0f4f0f8e605d8f131ec506a3ea3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 3 May 2018 18:27:44 +0200 Subject: [PATCH 17/19] style: Make SMIL values not roundtrip through strings. Bug: 1458814 Reviewed-by: hiro MozReview-Commit-ID: DpbFSutIv3t --- ports/geckolib/glue.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 93918a1c997..d4e16a470e5 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -3609,6 +3609,20 @@ pub unsafe extern "C" fn Servo_DeclarationBlock_SetProperty( ) } +#[no_mangle] +pub unsafe extern "C" fn Servo_DeclarationBlock_SetPropertyToAnimationValue( + declarations: RawServoDeclarationBlockBorrowed, + animation_value: RawServoAnimationValueBorrowed, +) -> bool { + write_locked_arc(declarations, |decls: &mut PropertyDeclarationBlock| { + decls.push( + AnimationValue::as_arc(&animation_value).uncompute(), + Importance::Normal, + DeclarationSource::CssOm, + ) + }) +} + #[no_mangle] pub unsafe extern "C" fn Servo_DeclarationBlock_SetPropertyById( declarations: RawServoDeclarationBlockBorrowed, From 56688121d3ea29887e1735670693ce2f40550736 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 3 May 2018 18:41:17 +0200 Subject: [PATCH 18/19] style: Remove a bit of trivially dead code. Bug: 1458814 Reviewed-by: hiro MozReview-Commit-ID: GG41v4TejBU --- ports/geckolib/glue.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index d4e16a470e5..374941161f6 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -4520,7 +4520,6 @@ pub extern "C" fn Servo_GetComputedKeyframeValues( raw_data: RawServoStyleSetBorrowed, computed_keyframes: RawGeckoComputedKeyframeValuesListBorrowedMut ) { - use std::mem; use style::properties::LonghandIdSet; let data = PerDocumentStyleData::from_ffi(raw_data).borrow(); @@ -4581,10 +4580,6 @@ pub extern "C" fn Servo_GetComputedKeyframeValues( // This is safe since we immediately write to the uninitialized values. unsafe { animation_values.set_len((property_index + 1) as u32) }; animation_values[property_index].mProperty = property.to_nscsspropertyid(); - // We only make sure we have enough space for this variable, - // but didn't construct a default value for StyleAnimationValue, - // so we should zero it to avoid getting undefined behaviors. - animation_values[property_index].mValue.mGecko = unsafe { mem::zeroed() }; match value { Some(v) => { animation_values[property_index].mValue.mServo.set_arc_leaky(Arc::new(v)); From 16815bf97fd5a6d84ff03faa8cfaffddd24ca9ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 3 May 2018 18:47:47 +0200 Subject: [PATCH 19/19] style: Remove code that is no longer called. Au revoir! Bug: 1458814 Reviewed-by: hiro MozReview-Commit-ID: DjqszUSIzXs --- ports/geckolib/glue.rs | 28 +--------------------------- 1 file changed, 1 insertion(+), 27 deletions(-) diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 374941161f6..3b341cadbb5 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -73,7 +73,6 @@ use style::gecko_bindings::bindings::RawGeckoCSSPropertyIDListBorrowed; use style::gecko_bindings::bindings::RawGeckoComputedKeyframeValuesListBorrowedMut; use style::gecko_bindings::bindings::RawGeckoComputedTimingBorrowed; use style::gecko_bindings::bindings::RawGeckoFontFaceRuleListBorrowedMut; -use style::gecko_bindings::bindings::RawGeckoServoAnimationValueListBorrowed; use style::gecko_bindings::bindings::RawGeckoServoAnimationValueListBorrowedMut; use style::gecko_bindings::bindings::RawGeckoServoStyleRuleListBorrowedMut; use style::gecko_bindings::bindings::RawServoAnimationValueBorrowed; @@ -129,7 +128,7 @@ use style::invalidation::element::restyle_hints; use style::media_queries::{MediaList, parse_media_query_list}; use style::parser::{Parse, ParserContext, self}; use style::properties::{ComputedValues, DeclarationSource, Importance}; -use style::properties::{LonghandId, LonghandIdSet, PropertyDeclaration, PropertyDeclarationBlock, PropertyId}; +use style::properties::{LonghandId, LonghandIdSet, PropertyDeclarationBlock, PropertyId}; use style::properties::{PropertyDeclarationId, ShorthandId}; use style::properties::{SourcePropertyDeclaration, StyleBuilder}; use style::properties::{parse_one_declaration_into, parse_style_attribute}; @@ -708,31 +707,6 @@ pub extern "C" fn Servo_AnimationValue_Serialize( debug_assert!(rv.is_ok()); } -#[no_mangle] -pub unsafe extern "C" fn Servo_Shorthand_AnimationValues_Serialize( - shorthand_property: nsCSSPropertyID, - values: RawGeckoServoAnimationValueListBorrowed, - buffer: *mut nsAString, -) { - let property_id = get_property_id_from_nscsspropertyid!(shorthand_property, ()); - let shorthand = match property_id.as_shorthand() { - Ok(shorthand) => shorthand, - _ => return, - }; - - // Convert RawServoAnimationValue(s) into a vector of PropertyDeclaration - // so that we can use reference of the PropertyDeclaration without worrying - // about its lifetime. (longhands_to_css() expects &PropertyDeclaration - // iterator.) - let declarations: Vec = - values.iter().map(|v| AnimationValue::as_arc(&&*v.mRawPtr).uncompute()).collect(); - - let _ = shorthand.longhands_to_css( - declarations.iter(), - &mut CssWriter::new(&mut *buffer), - ); -} - #[no_mangle] pub extern "C" fn Servo_AnimationValue_GetOpacity( value: RawServoAnimationValueBorrowed,