Bug 1466008: Make will-change honor prefs properly, and clean it up while at it. r=xidorn
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 01 Jun 2018 11:35:57 +0200
changeset 420854 ff4fa69beec83326b08a45b8abc0277e2972b4f4
parent 420853 4f9eec6361279d8657ffc4e6ef5c84e8f5d08c56
child 420855 23e1f479bfd60730b949bad388ee238599d555ee
push id103900
push userecoal95@gmail.com
push dateFri, 01 Jun 2018 12:50:13 +0000
treeherdermozilla-inbound@ff4fa69beec8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersxidorn
bugs1466008
milestone62.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
layout/reftests/bugs/1466008.html
layout/reftests/bugs/reftest.list
layout/style/nsStyleConsts.h
servo/components/style/properties/gecko.mako.rs
servo/components/style/properties/properties.mako.rs
servo/components/style/values/specified/box.rs
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/1466008-ref.html
@@ -0,0 +1,2 @@
+<!doctype html>
+<div style="position: absolute; left: 0; top: 0; width: 100px; height: 100px; background: green;"></div>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/1466008.html
@@ -0,0 +1,5 @@
+<!doctype html>
+<div style="height: 100px"></div>
+<div style="will-change: contain;">
+  <div style="position: absolute; top: 0; left: 0; width: 100px; height: 100px; background: green"></div>
+</div>
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -2071,8 +2071,9 @@ test-pref(font.size.systemFontScale,200)
 fuzzy(74,2234) random-if(webrender) == 1425243-1.html 1425243-1-ref.html
 fuzzy-if(Android,66,574) fuzzy-if(d2d,89,777) fuzzy-if(!Android&&!d2d,1,31219) == 1425243-2.html 1425243-2-ref.html
 == 1430869.html 1430869-ref.html
 == 1432541.html 1432541-ref.html
 pref(layout.css.moz-document.url-prefix-hack.enabled,true) == 1446470.html 1035091-ref.html
 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&&gtkWidget),1-2,17500-17500) == 1412375.html 1412375-ref.html
+test-pref(layout.css.contain.enabled,false) == 1466008.html 1466008-ref.html
--- a/layout/style/nsStyleConsts.h
+++ b/layout/style/nsStyleConsts.h
@@ -227,16 +227,18 @@ enum class StyleWindowDragging : uint8_t
 enum class StyleOrient : uint8_t {
   Inline,
   Block,
   Horizontal,
   Vertical,
 };
 
 // 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)
 #define NS_STYLE_WILL_CHANGE_OPACITY            (1<<3)
 #define NS_STYLE_WILL_CHANGE_FIXPOS_CB          (1<<4)
 #define NS_STYLE_WILL_CHANGE_ABSPOS_CB          (1<<5)
 
 // See AnimationEffect.webidl
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -3510,87 +3510,37 @@ fn static_assert() {
     }
 
     ${impl_individual_transform('rotate', 'Rotate', 'mSpecifiedRotate')}
     ${impl_individual_transform('translate', 'Translate', 'mSpecifiedTranslate')}
     ${impl_individual_transform('scale', 'Scale', 'mSpecifiedScale')}
 
     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;
             },
         };
     }
 
     pub fn copy_will_change_from(&mut self, other: &Self) {
         use gecko_bindings::bindings::Gecko_CopyWillChangeFrom;
 
         self.gecko.mWillChangeBitField = other.gecko.mWillChangeBitField;
@@ -3602,28 +3552,32 @@ fn static_assert() {
     pub fn reset_will_change(&mut self, other: &Self) {
         self.copy_will_change_from(other)
     }
 
     pub fn clone_will_change(&self) -> longhands::will_change::computed_value::T {
         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
         }
 
         let custom_idents: Vec<CustomIdent> = self.gecko.mWillChange.iter().map(|gecko_atom| {
             unsafe {
                 CustomIdent(Atom::from_raw(gecko_atom.mRawPtr as *mut nsAtom))
             }
         }).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") %>
 
     pub fn set_contain(&mut self, v: longhands::contain::computed_value::T) {
         use gecko_bindings::structs::NS_STYLE_CONTAIN_NONE;
         use gecko_bindings::structs::NS_STYLE_CONTAIN_STRICT;
         use gecko_bindings::structs::NS_STYLE_CONTAIN_CONTENT;
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -1710,17 +1710,18 @@ impl PropertyId {
             PropertyId::Custom(ref name) => {
                 let mut s = String::new();
                 write!(&mut s, "--{}", name).unwrap();
                 s.into()
             }
         }
     }
 
-    fn non_custom_id(&self) -> Option<NonCustomPropertyId> {
+    /// Returns the non-custom property id for this property.
+    pub fn non_custom_id(&self) -> Option<NonCustomPropertyId> {
         Some(match *self {
             PropertyId::Custom(_) => return None,
             PropertyId::Shorthand(shorthand_id) => shorthand_id.into(),
             PropertyId::Longhand(longhand_id) => longhand_id.into(),
             PropertyId::ShorthandAlias(_, alias_id) => alias_id.into(),
             PropertyId::LonghandAlias(_, alias_id) => alias_id.into(),
         })
     }
@@ -1745,17 +1746,18 @@ impl PropertyId {
             // Custom properties are allowed everywhere
             None => return true,
             Some(id) => id,
         };
 
         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,
             Some(id) => id,
         };
         id.allowed_in(context)
     }
 
--- a/servo/components/style/values/specified/box.rs
+++ b/servo/components/style/values/specified/box.rs
@@ -2,16 +2,17 @@
  * 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/. */
 
 //! Specified types for box properties.
 
 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};
 use values::{CustomIdent, KeyframesName};
 use values::generics::box_::AnimationIterationCount as GenericAnimationIterationCount;
 use values::generics::box_::Perspective as GenericPerspective;
 use values::generics::box_::VerticalAlign as GenericVerticalAlign;
 use values::specified::{AllowQuirks, Number};
@@ -377,52 +378,132 @@ pub enum OverflowClipBox {
 /// to perform on the element
 ///
 /// <https://drafts.csswg.org/css-will-change/#will-change>
 pub enum WillChange {
     /// Expresses no particular intent
     Auto,
     /// <custom-ident>
     #[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 {
     #[inline]
     /// Get default value of `will-change` as `auto`
     pub fn auto() -> WillChange {
         WillChange::Auto
     }
 }
 
+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 | <animateable-feature>#
     fn parse<'i, 't>(
-        _context: &ParserContext,
+        context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<WillChange, ParseError<'i>> {
         if input
             .try(|input| input.expect_ident_matching("auto"))
             .is_ok()
         {
             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,
+        })
     }
 }
 
 bitflags! {
     #[cfg_attr(feature = "gecko", derive(MallocSizeOf))]
     #[derive(SpecifiedValueInfo, ToComputedValue)]
     /// These constants match Gecko's `NS_STYLE_TOUCH_ACTION_*` constants.
     #[value_info(other_values = "auto,none,manipulation,pan-x,pan-y")]