From 50bc0980418eb07d932313da0fc4437f63280b2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 1 Jun 2018 11:35:57 +0200 Subject: [PATCH] Bug 1466008: Make will-change honor prefs properly, and clean it up while at it. r=xidorn Will add a test, though in JSConf right now... MozReview-Commit-ID: JyzwaRgf5Ct --- layout/reftests/bugs/1466008-ref.html | 2 + layout/reftests/bugs/1466008.html | 5 + layout/reftests/bugs/reftest.list | 1 + layout/style/nsStyleConsts.h | 2 + .../components/style/properties/gecko.mako.rs | 66 ++----------- .../style/properties/properties.mako.rs | 6 +- .../components/style/values/specified/box.rs | 97 +++++++++++++++++-- 7 files changed, 113 insertions(+), 66 deletions(-) create mode 100644 layout/reftests/bugs/1466008-ref.html create mode 100644 layout/reftests/bugs/1466008.html diff --git a/layout/reftests/bugs/1466008-ref.html b/layout/reftests/bugs/1466008-ref.html new file mode 100644 index 000000000000..7655e88d9f83 --- /dev/null +++ b/layout/reftests/bugs/1466008-ref.html @@ -0,0 +1,2 @@ + +
diff --git a/layout/reftests/bugs/1466008.html b/layout/reftests/bugs/1466008.html new file mode 100644 index 000000000000..dcccfab213de --- /dev/null +++ b/layout/reftests/bugs/1466008.html @@ -0,0 +1,5 @@ + +
+
+
+
diff --git a/layout/reftests/bugs/reftest.list b/layout/reftests/bugs/reftest.list index f81d7613fef4..b586255bb5f9 100644 --- a/layout/reftests/bugs/reftest.list +++ b/layout/reftests/bugs/reftest.list @@ -2076,3 +2076,4 @@ pref(layout.css.moz-document.url-prefix-hack.enabled,true) == 1446470.html 10350 pref(layout.css.moz-document.url-prefix-hack.enabled,false) == 1446470-2.html 1035091-ref.html test-pref(layout.css.prefixes.gradients,false) == 1451874.html 1451874-ref.html fuzzy-if(!(webrender&>kWidget),1-2,17500-17500) == 1412375.html 1412375-ref.html +test-pref(layout.css.contain.enabled,false) == 1466008.html 1466008-ref.html diff --git a/layout/style/nsStyleConsts.h b/layout/style/nsStyleConsts.h index 9eda7b2f4189..9201683099cb 100644 --- a/layout/style/nsStyleConsts.h +++ b/layout/style/nsStyleConsts.h @@ -232,6 +232,8 @@ enum class StyleOrient : uint8_t { }; // See nsStyleDisplay +// +// These need to be in sync with WillChangeBits in box.rs. #define NS_STYLE_WILL_CHANGE_STACKING_CONTEXT (1<<0) #define NS_STYLE_WILL_CHANGE_TRANSFORM (1<<1) #define NS_STYLE_WILL_CHANGE_SCROLL (1<<2) diff --git a/servo/components/style/properties/gecko.mako.rs b/servo/components/style/properties/gecko.mako.rs index 741aa7b86b42..4b902358866a 100644 --- a/servo/components/style/properties/gecko.mako.rs +++ b/servo/components/style/properties/gecko.mako.rs @@ -3515,77 +3515,27 @@ fn static_assert() { pub fn set_will_change(&mut self, v: longhands::will_change::computed_value::T) { use gecko_bindings::bindings::{Gecko_AppendWillChange, Gecko_ClearWillChange}; - use gecko_bindings::structs::NS_STYLE_WILL_CHANGE_OPACITY; - use gecko_bindings::structs::NS_STYLE_WILL_CHANGE_SCROLL; - use gecko_bindings::structs::NS_STYLE_WILL_CHANGE_TRANSFORM; - use properties::PropertyId; use properties::longhands::will_change::computed_value::T; - fn will_change_bitfield_from_prop_flags(prop: LonghandId) -> u8 { - use properties::PropertyFlags; - use gecko_bindings::structs::NS_STYLE_WILL_CHANGE_ABSPOS_CB; - use gecko_bindings::structs::NS_STYLE_WILL_CHANGE_FIXPOS_CB; - use gecko_bindings::structs::NS_STYLE_WILL_CHANGE_STACKING_CONTEXT; - let servo_flags = prop.flags(); - let mut bitfield = 0; - - if servo_flags.contains(PropertyFlags::CREATES_STACKING_CONTEXT) { - bitfield |= NS_STYLE_WILL_CHANGE_STACKING_CONTEXT; - } - if servo_flags.contains(PropertyFlags::FIXPOS_CB) { - bitfield |= NS_STYLE_WILL_CHANGE_FIXPOS_CB; - } - if servo_flags.contains(PropertyFlags::ABSPOS_CB) { - bitfield |= NS_STYLE_WILL_CHANGE_ABSPOS_CB; - } - - bitfield as u8 - } - - self.gecko.mWillChangeBitField = 0; - match v { - T::AnimateableFeatures(features) => { + T::AnimateableFeatures { features, bits } => { unsafe { Gecko_ClearWillChange(&mut self.gecko, features.len()); } for feature in features.iter() { - if feature.0 == atom!("scroll-position") { - self.gecko.mWillChangeBitField |= NS_STYLE_WILL_CHANGE_SCROLL as u8; - } else if feature.0 == atom!("opacity") { - self.gecko.mWillChangeBitField |= NS_STYLE_WILL_CHANGE_OPACITY as u8; - } else if feature.0 == atom!("transform") { - self.gecko.mWillChangeBitField |= NS_STYLE_WILL_CHANGE_TRANSFORM as u8; - } - unsafe { - Gecko_AppendWillChange(&mut self.gecko, feature.0.as_ptr()); - } - - if let Ok(prop_id) = PropertyId::parse(&feature.0.to_string()) { - match prop_id.as_shorthand() { - Ok(shorthand) => { - for longhand in shorthand.longhands() { - self.gecko.mWillChangeBitField |= - will_change_bitfield_from_prop_flags(longhand); - } - }, - Err(longhand_or_custom) => { - if let PropertyDeclarationId::Longhand(longhand) - = longhand_or_custom { - self.gecko.mWillChangeBitField |= - will_change_bitfield_from_prop_flags(longhand); - } - }, - } + Gecko_AppendWillChange(&mut self.gecko, feature.0.as_ptr()) } } + + self.gecko.mWillChangeBitField = bits.bits(); }, T::Auto => { unsafe { Gecko_ClearWillChange(&mut self.gecko, 0); } + self.gecko.mWillChangeBitField = 0; }, }; } @@ -3607,6 +3557,7 @@ fn static_assert() { use properties::longhands::will_change::computed_value::T; use gecko_bindings::structs::nsAtom; use values::CustomIdent; + use values::specified::box_::WillChangeBits; if self.gecko.mWillChange.len() == 0 { return T::Auto @@ -3618,7 +3569,10 @@ fn static_assert() { } }).collect(); - T::AnimateableFeatures(custom_idents.into_boxed_slice()) + T::AnimateableFeatures { + features: custom_idents.into_boxed_slice(), + bits: WillChangeBits::from_bits_truncate(self.gecko.mWillChangeBitField), + } } <% impl_shape_source("shape_outside", "mShapeOutside") %> diff --git a/servo/components/style/properties/properties.mako.rs b/servo/components/style/properties/properties.mako.rs index 81e9a09cbe8b..7696f15c8b87 100644 --- a/servo/components/style/properties/properties.mako.rs +++ b/servo/components/style/properties/properties.mako.rs @@ -1715,7 +1715,8 @@ impl PropertyId { } } - fn non_custom_id(&self) -> Option { + /// Returns the non-custom property id for this property. + pub fn non_custom_id(&self) -> Option { Some(match *self { PropertyId::Custom(_) => return None, PropertyId::Shorthand(shorthand_id) => shorthand_id.into(), @@ -1750,7 +1751,8 @@ impl PropertyId { id.enabled_for_all_content() } - fn allowed_in(&self, context: &ParserContext) -> bool { + /// Returns whether the property is allowed in a given context. + pub fn allowed_in(&self, context: &ParserContext) -> bool { let id = match self.non_custom_id() { // Custom properties are allowed everywhere None => return true, diff --git a/servo/components/style/values/specified/box.rs b/servo/components/style/values/specified/box.rs index f418b39ef7dc..ed118acc74be 100644 --- a/servo/components/style/values/specified/box.rs +++ b/servo/components/style/values/specified/box.rs @@ -7,6 +7,7 @@ use Atom; use cssparser::Parser; use parser::{Parse, ParserContext}; +use properties::{LonghandId, PropertyId, PropertyFlags, PropertyDeclarationId}; use selectors::parser::SelectorParseErrorKind; use std::fmt::{self, Write}; use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss}; @@ -382,7 +383,15 @@ pub enum WillChange { Auto, /// #[css(comma)] - AnimateableFeatures(#[css(iterable)] Box<[CustomIdent]>), + AnimateableFeatures { + /// The features that are supposed to change. + #[css(iterable)] + features: Box<[CustomIdent]>, + /// A bitfield with the kind of change that the value will create, based + /// on the above field. + #[css(skip)] + bits: WillChangeBits, + }, } impl WillChange { @@ -393,10 +402,72 @@ impl WillChange { } } +bitflags! { + /// The change bits that we care about. + /// + /// These need to be in sync with NS_STYLE_WILL_CHANGE_*. + #[derive(MallocSizeOf, SpecifiedValueInfo, ToComputedValue)] + pub struct WillChangeBits: u8 { + /// Whether the stacking context will change. + const STACKING_CONTEXT = 1 << 0; + /// Whether `transform` will change. + const TRANSFORM = 1 << 1; + /// Whether `scroll-position` will change. + const SCROLL = 1 << 2; + /// Whether `opacity` will change. + const OPACITY = 1 << 3; + /// Fixed pos containing block. + const FIXPOS_CB = 1 << 4; + /// Abs pos containing block. + const ABSPOS_CB = 1 << 5; + } +} + +fn change_bits_for_longhand(longhand: LonghandId) -> WillChangeBits { + let mut flags = match longhand { + LonghandId::Opacity => WillChangeBits::OPACITY, + LonghandId::Transform => WillChangeBits::TRANSFORM, + _ => WillChangeBits::empty(), + }; + + let property_flags = longhand.flags(); + if property_flags.contains(PropertyFlags::CREATES_STACKING_CONTEXT) { + flags |= WillChangeBits::STACKING_CONTEXT; + } + if property_flags.contains(PropertyFlags::FIXPOS_CB) { + flags |= WillChangeBits::FIXPOS_CB; + } + if property_flags.contains(PropertyFlags::ABSPOS_CB) { + flags |= WillChangeBits::ABSPOS_CB; + } + flags +} + +fn change_bits_for_maybe_property(ident: &str, context: &ParserContext) -> WillChangeBits { + let id = match PropertyId::parse(ident) { + Ok(id) => id, + Err(..) => return WillChangeBits::empty(), + }; + + if !id.allowed_in(context) { + return WillChangeBits::empty(); + } + + match id.as_shorthand() { + Ok(shorthand) => { + shorthand.longhands().fold(WillChangeBits::empty(), |flags, p| { + flags | change_bits_for_longhand(p) + }) + } + Err(PropertyDeclarationId::Longhand(longhand)) => change_bits_for_longhand(longhand), + Err(PropertyDeclarationId::Custom(..)) => WillChangeBits::empty(), + } +} + impl Parse for WillChange { /// auto | # fn parse<'i, 't>( - _context: &ParserContext, + context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { if input @@ -406,18 +477,28 @@ impl Parse for WillChange { return Ok(WillChange::Auto); } + let mut bits = WillChangeBits::empty(); let custom_idents = input.parse_comma_separated(|i| { let location = i.current_source_location(); - CustomIdent::from_ident( + let parser_ident = i.expect_ident()?; + let ident = CustomIdent::from_ident( location, - i.expect_ident()?, + parser_ident, &["will-change", "none", "all", "auto"], - ) + )?; + + if ident.0 == atom!("scroll-position") { + bits |= WillChangeBits::SCROLL; + } else { + bits |= change_bits_for_maybe_property(&parser_ident, context); + } + Ok(ident) })?; - Ok(WillChange::AnimateableFeatures( - custom_idents.into_boxed_slice(), - )) + Ok(WillChange::AnimateableFeatures { + features: custom_idents.into_boxed_slice(), + bits, + }) } }