Bug 1550554 - Use OwnedSlice for will-change. r=heycam

We could use ArcSlice if wanted I guess, your call. Though will change is not
supposed to be used very frequently.

Differential Revision: https://phabricator.services.mozilla.com/D30548

--HG--
extra : moz-landing-system : lando

layout/base/nsLayoutDebugger.cpp

@@ -169,13 +169,13 @@ static void PrintDisplayItemTo(nsDisplayListBuilder* aBuilder,
   }
 
   const auto& willChange = aItem->Frame()->StyleDisplay()->mWillChange;
-  if (!willChange.IsEmpty()) {
+  if (!willChange.features.IsEmpty()) {
     aStream << " (will-change=";
-    for (size_t i = 0; i < willChange.Length(); i++) {
+    for (size_t i = 0; i < willChange.features.Length(); i++) {
       if (i > 0) {
         aStream << ",";
       }
-      nsDependentAtomString buffer(willChange[i]);
+      nsDependentAtomString buffer(willChange.features.AsSpan()[i].AsAtom());
       aStream << NS_LossyConvertUTF16toASCII(buffer).get();
     }
     aStream << ")";

layout/generic/nsFrame.cpp

@@ -1640,7 +1640,7 @@ bool nsIFrame::HasOpacityInternal(float aThreshold,
                                   EffectSet* aEffectSet) const {
   MOZ_ASSERT(0.0 <= aThreshold && aThreshold <= 1.0, "Invalid argument");
   if (aStyleEffects->mOpacity < aThreshold ||
-      (aStyleDisplay->mWillChangeBitField & StyleWillChangeBits_OPACITY)) {
+      (aStyleDisplay->mWillChange.bits & StyleWillChangeBits_OPACITY)) {
     return true;
   }
 
@@ -2870,7 +2870,7 @@ void nsIFrame::BuildDisplayListForStackingContext(
                              NS_STYLE_POINTER_EVENTS_NONE;
   bool opacityItemForEventsAndPluginsOnly = false;
   if (effects->mOpacity == 0.0 && aBuilder->IsForPainting() &&
-      !(disp->mWillChangeBitField & StyleWillChangeBits_OPACITY) &&
+      !(disp->mWillChange.bits & StyleWillChangeBits_OPACITY) &&
       !nsLayoutUtils::HasAnimationOfPropertySet(
           this, nsCSSPropertyIDSet::OpacityProperties(), effectSetForOpacity)) {
     if (needHitTestInfo || aBuilder->WillComputePluginGeometry()) {
@@ -2880,7 +2880,7 @@ void nsIFrame::BuildDisplayListForStackingContext(
     }
   }
 
-  if (disp->mWillChangeBitField) {
+  if (disp->mWillChange.bits) {
     aBuilder->AddToWillChangeBudget(this, GetSize());
   }
 
@@ -10604,7 +10604,7 @@ bool nsIFrame::IsStackingContext(const nsStyleDisplay* aStyleDisplay,
          nsSVGIntegrationUtils::UsingEffectsForFrame(this) ||
          (aIsPositioned && (aStyleDisplay->IsPositionForcingStackingContext() ||
                             aStylePosition->mZIndex.IsInteger())) ||
-         (aStyleDisplay->mWillChangeBitField &
+         (aStyleDisplay->mWillChange.bits &
           StyleWillChangeBits_STACKING_CONTEXT) ||
          aStyleDisplay->mIsolation != NS_STYLE_ISOLATION_AUTO;
 }

layout/generic/nsGfxScrollFrame.cpp

@@ -3455,7 +3455,7 @@ void ScrollFrameHelper::BuildDisplayList(nsDisplayListBuilder* aBuilder,
   AppendScrollPartsTo(aBuilder, aLists, createLayersForScrollbars, false);
 
   const nsStyleDisplay* disp = mOuter->StyleDisplay();
-  if (disp->mWillChangeBitField & StyleWillChangeBits_SCROLL) {
+  if (disp->mWillChange.bits & StyleWillChangeBits_SCROLL) {
     aBuilder->AddToWillChangeBudget(mOuter, GetVisualViewportSize());
   }
 
@@ -5417,7 +5417,7 @@ bool ScrollFrameHelper::IsScrollbarOnRight() const {
 
 bool ScrollFrameHelper::IsMaybeScrollingActive() const {
   const nsStyleDisplay* disp = mOuter->StyleDisplay();
-  if (disp->mWillChangeBitField & StyleWillChangeBits_SCROLL) {
+  if (disp->mWillChange.bits & StyleWillChangeBits_SCROLL) {
     return true;
   }
 
@@ -5430,7 +5430,7 @@ bool ScrollFrameHelper::IsMaybeScrollingActive() const {
 bool ScrollFrameHelper::IsScrollingActive(
     nsDisplayListBuilder* aBuilder) const {
   const nsStyleDisplay* disp = mOuter->StyleDisplay();
-  if (disp->mWillChangeBitField & StyleWillChangeBits_SCROLL &&
+  if (disp->mWillChange.bits & StyleWillChangeBits_SCROLL &&
       aBuilder->IsInWillChangeBudget(mOuter, GetVisualViewportSize())) {
     return true;
   }

layout/painting/ActiveLayerTracker.cpp

@@ -483,14 +483,14 @@ bool ActiveLayerTracker::IsStyleAnimated(
   const nsIFrame* styleFrame = nsLayoutUtils::GetStyleFrame(aFrame);
   const nsCSSPropertyIDSet transformSet =
       nsCSSPropertyIDSet::TransformLikeProperties();
-  if ((styleFrame && (styleFrame->StyleDisplay()->mWillChangeBitField &
+  if ((styleFrame && (styleFrame->StyleDisplay()->mWillChange.bits &
                       StyleWillChangeBits_TRANSFORM)) &&
       aPropertySet.Intersects(transformSet) &&
       (!aBuilder ||
        aBuilder->IsInWillChangeBudget(aFrame, aFrame->GetSize()))) {
     return true;
   }
-  if ((aFrame->StyleDisplay()->mWillChangeBitField &
+  if ((aFrame->StyleDisplay()->mWillChange.bits &
        StyleWillChangeBits_OPACITY) &&
       aPropertySet.Intersects(nsCSSPropertyIDSet::OpacityProperties()) &&
       (!aBuilder ||

layout/style/GeckoBindings.cpp

@@ -1427,21 +1427,6 @@ void Gecko_EnsureStyleTransitionArrayLength(void* aArray, size_t aLen) {
   EnsureStyleAutoArrayLength(base, aLen);
 }
 
-void Gecko_ClearWillChange(nsStyleDisplay* aDisplay, size_t aLength) {
-  aDisplay->mWillChange.Clear();
-  aDisplay->mWillChange.SetCapacity(aLength);
-}
-
-void Gecko_AppendWillChange(nsStyleDisplay* aDisplay, nsAtom* aAtom) {
-  aDisplay->mWillChange.AppendElement(aAtom);
-}
-
-void Gecko_CopyWillChangeFrom(nsStyleDisplay* aDest,
-                              const nsStyleDisplay* aSrc) {
-  aDest->mWillChange.Clear();
-  aDest->mWillChange.AppendElements(aSrc->mWillChange);
-}
-
 enum class KeyframeSearchDirection {
   Forwards,
   Backwards,

layout/style/GeckoBindings.h

@@ -469,9 +469,6 @@ void Gecko_EnsureImageLayersLength(nsStyleImageLayers* layers, size_t len,
 
 void Gecko_EnsureStyleAnimationArrayLength(void* array, size_t len);
 void Gecko_EnsureStyleTransitionArrayLength(void* array, size_t len);
-void Gecko_ClearWillChange(nsStyleDisplay* display, size_t length);
-void Gecko_AppendWillChange(nsStyleDisplay* display, nsAtom* atom);
-void Gecko_CopyWillChangeFrom(nsStyleDisplay* dest, const nsStyleDisplay* src);
 
 // Searches from the beginning of |keyframes| for a Keyframe object with the
 // specified offset and timing function. If none is found, a new Keyframe object

layout/style/ServoBindings.toml

@@ -460,7 +460,7 @@ cbindgen-types = [
     { gecko = "StyleContain", servo = "values::computed::Contain" },
     { gecko = "StyleRestyleHint", servo = "invalidation::element::restyle_hints::RestyleHint" },
     { gecko = "StyleTouchAction", servo = "values::computed::TouchAction" },
-    { gecko = "StyleWillChangeBits", servo = "values::specified::box_::WillChangeBits" },
+    { gecko = "StyleWillChange", servo = "values::specified::box_::WillChange" },
     { gecko = "StyleTextDecorationLine", servo = "values::computed::TextDecorationLine" },
     { gecko = "StyleTextTransform", servo = "values::computed::TextTransform" },
     { gecko = "StyleMozListReversed", servo = "values::computed::MozListReversed" },

layout/style/ServoStyleConstsInlines.h

@@ -133,9 +133,16 @@ inline StyleArcSlice<T>::~StyleArcSlice() {
 
 inline bool StyleAtom::IsStatic() const { return !!(_0 & 1); }
 
+inline nsAtom* StyleAtom::AsAtom() const {
+  if (IsStatic()) {
+    return const_cast<nsStaticAtom*>(&detail::gGkAtoms.mAtoms[(_0 & ~1) >> 1]);
+  }
+  return reinterpret_cast<nsAtom*>(_0);
+}
+
 inline StyleAtom::~StyleAtom() {
   if (!IsStatic()) {
-    reinterpret_cast<nsAtom*>(_0)->Release();
+    AsAtom()->Release();
   }
 }
 
@@ -145,6 +152,8 @@ inline StyleAtom::StyleAtom(const StyleAtom& aOther) : _0(aOther._0) {
   }
 }
 
+inline nsAtom* StyleCustomIdent::AsAtom() const { return _0.AsAtom(); }
+
 inline nsDependentCSubstring StyleOwnedStr::AsString() const {
   Span<const uint8_t> s = _0.AsSpan();
   return nsDependentCSubstring(reinterpret_cast<const char*>(s.Elements()),

layout/style/nsStyleStruct.cpp

@@ -2916,7 +2916,7 @@ nsStyleDisplay::nsStyleDisplay(const Document& aDocument)
       mOrient(StyleOrient::Inline),
       mIsolation(NS_STYLE_ISOLATION_AUTO),
       mTopLayer(NS_STYLE_TOP_LAYER_NONE),
-      mWillChangeBitField({0}),
+      mWillChange{{}, {0}},
       mTouchAction(StyleTouchAction_AUTO),
       mScrollBehavior(NS_STYLE_SCROLL_BEHAVIOR_AUTO),
       mOverscrollBehaviorX(StyleOverscrollBehavior::Auto),
@@ -2983,7 +2983,6 @@ nsStyleDisplay::nsStyleDisplay(const nsStyleDisplay& aSource)
       mOrient(aSource.mOrient),
       mIsolation(aSource.mIsolation),
       mTopLayer(aSource.mTopLayer),
-      mWillChangeBitField(aSource.mWillChangeBitField),
       mWillChange(aSource.mWillChange),
       mTouchAction(aSource.mTouchAction),
       mScrollBehavior(aSource.mScrollBehavior),
@@ -3290,7 +3289,7 @@ nsChangeHint nsStyleDisplay::CalcDifference(
   // TODO(emilio): Should add xor to the generated cbindgen type.
   auto willChangeBitsChanged =
       StyleWillChangeBits{static_cast<decltype(StyleWillChangeBits::bits)>(
-          mWillChangeBitField.bits ^ aNewData.mWillChangeBitField.bits)};
+          mWillChange.bits.bits ^ aNewData.mWillChange.bits.bits)};
 
   if (willChangeBitsChanged &
       (StyleWillChangeBits_STACKING_CONTEXT | StyleWillChangeBits_SCROLL |

layout/style/nsStyleStruct.h

@@ -1730,11 +1730,7 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleDisplay {
   mozilla::StyleOrient mOrient;
   uint8_t mIsolation;  // NS_STYLE_ISOLATION_*
   uint8_t mTopLayer;   // NS_STYLE_TOP_LAYER_*
-  // Stores a bitfield representation of the properties that are frequently
-  // queried. This should match mWillChange. Also tracks if any of the
-  // properties in the will-change list require a stacking context.
-  mozilla::StyleWillChangeBits mWillChangeBitField;
-  nsTArray<RefPtr<nsAtom>> mWillChange;
+  mozilla::StyleWillChange mWillChange;
 
   mozilla::StyleTouchAction mTouchAction;
   uint8_t mScrollBehavior;  // NS_STYLE_SCROLL_BEHAVIOR_*
@@ -2009,7 +2005,7 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleDisplay {
     return mSpecifiedTransform || mSpecifiedRotate || mSpecifiedTranslate ||
            mSpecifiedScale ||
            mTransformStyle == NS_STYLE_TRANSFORM_STYLE_PRESERVE_3D ||
-           (mWillChangeBitField & mozilla::StyleWillChangeBits_TRANSFORM) ||
+           (mWillChange.bits & mozilla::StyleWillChangeBits_TRANSFORM) ||
            (mMotion && mMotion->HasPath());
   }
 

layout/style/nsStyleStructInlines.h

@@ -131,7 +131,7 @@ bool nsStyleDisplay::IsFixedPosContainingBlockForNonSVGTextFrames(
   // should have the FIXPOS_CB flag set on them.
   NS_ASSERTION(aStyle.StyleDisplay() == this, "unexpected aStyle");
 
-  if (mWillChangeBitField & mozilla::StyleWillChangeBits_FIXPOS_CB) {
+  if (mWillChange.bits & mozilla::StyleWillChangeBits_FIXPOS_CB) {
     return true;
   }
 
@@ -176,7 +176,7 @@ bool nsStyleDisplay::IsAbsPosContainingBlockForNonSVGTextFrames() const {
   // NOTE: Any CSS properties that influence the output of this function
   // should have the ABSPOS_CB set on them.
   return IsAbsolutelyPositionedStyle() || IsRelativelyPositionedStyle() ||
-         (mWillChangeBitField & mozilla::StyleWillChangeBits_ABSPOS_CB);
+         (mWillChange.bits & mozilla::StyleWillChangeBits_ABSPOS_CB);
 }
 
 bool nsStyleDisplay::IsAbsPosContainingBlock(

servo/components/style/properties/gecko.mako.rs

@@ -2513,7 +2513,7 @@ fn static_assert() {
                           transition-timing-function transition-property
                           transform-style
                           rotate scroll-snap-points-x scroll-snap-points-y
-                          scroll-snap-coordinate -moz-binding will-change
+                          scroll-snap-coordinate -moz-binding
                           offset-path shape-outside
                           translate scale -webkit-line-clamp""" %>
 <%self:impl_trait style_struct_name="Box" skip_longhands="${skip_box_longhands}">
@@ -2829,66 +2829,6 @@ fn static_assert() {
     ${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 crate::gecko_bindings::bindings::{Gecko_AppendWillChange, Gecko_ClearWillChange};
-        use crate::values::specified::box_::{WillChangeBits, WillChange};
-
-        match v {
-            WillChange::AnimateableFeatures { features, bits } => {
-                unsafe {
-                    Gecko_ClearWillChange(&mut *self.gecko, features.len());
-                }
-
-                for feature in features.iter() {
-                    unsafe {
-                        Gecko_AppendWillChange(&mut *self.gecko, feature.0.as_ptr())
-                    }
-                }
-
-                self.gecko.mWillChangeBitField = bits;
-            },
-            WillChange::Auto => {
-                unsafe {
-                    Gecko_ClearWillChange(&mut *self.gecko, 0);
-                }
-                self.gecko.mWillChangeBitField = WillChangeBits::empty();
-            },
-        };
-    }
-
-    pub fn copy_will_change_from(&mut self, other: &Self) {
-        use crate::gecko_bindings::bindings::Gecko_CopyWillChangeFrom;
-
-        self.gecko.mWillChangeBitField = other.gecko.mWillChangeBitField;
-        unsafe {
-            Gecko_CopyWillChangeFrom(&mut *self.gecko, &*other.gecko);
-        }
-    }
-
-    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 crate::values::CustomIdent;
-        use crate::values::specified::box_::WillChange;
-
-        if self.gecko.mWillChange.len() == 0 {
-            return WillChange::Auto
-        }
-
-        let custom_idents: Vec<CustomIdent> = self.gecko.mWillChange.iter().map(|gecko_atom| {
-            unsafe {
-                CustomIdent(Atom::from_raw(gecko_atom.mRawPtr))
-            }
-        }).collect();
-
-        WillChange::AnimateableFeatures {
-            features: custom_idents.into_boxed_slice(),
-            bits: self.gecko.mWillChangeBitField,
-        }
-    }
-
     <% impl_shape_source("shape_outside", "mShapeOutside") %>
 
     pub fn set_offset_path(&mut self, v: longhands::offset_path::computed_value::T) {

servo/components/style/values/specified/box.rs

@@ -642,6 +642,7 @@ pub enum OverflowClipBox {
 #[derive(
     Clone,
     Debug,
+    Default,
     MallocSizeOf,
     PartialEq,
     SpecifiedValueInfo,
@@ -650,38 +651,38 @@ pub enum OverflowClipBox {
     ToResolvedValue,
     ToShmem,
 )]
-/// Provides a rendering hint to the user agent,
-/// stating what kinds of changes the author expects
-/// to perform on the element
+#[css(comma)]
+#[repr(C)]
+/// Provides a rendering hint to the user agent, stating what kinds of changes
+/// the author expects to perform on the element.
+///
+/// `auto` is represented by an empty `features` list.
 ///
 /// <https://drafts.csswg.org/css-will-change/#will-change>
-pub enum WillChange {
-    /// Expresses no particular intent
-    Auto,
-    /// <custom-ident>
-    #[css(comma)]
-    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,
-    },
+pub struct WillChange {
+    /// The features that are supposed to change.
+    ///
+    /// TODO(emilio): Consider using ArcSlice since we just clone them from the
+    /// specified value? That'd save an allocation, which could be worth it.
+    #[css(iterable, if_empty = "auto")]
+    features: crate::OwnedSlice<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
+    pub fn auto() -> Self {
+        Self::default()
     }
 }
 
 bitflags! {
     /// The change bits that we care about.
-    #[derive(MallocSizeOf, SpecifiedValueInfo, ToComputedValue, ToResolvedValue, ToShmem)]
+    #[derive(Default, MallocSizeOf, SpecifiedValueInfo, ToComputedValue, ToResolvedValue, ToShmem)]
     #[repr(C)]
     pub struct WillChangeBits: u8 {
         /// Whether the stacking context will change.
@@ -746,7 +747,7 @@ impl Parse for WillChange {
             .try(|input| input.expect_ident_matching("auto"))
             .is_ok()
         {
-            return Ok(WillChange::Auto);
+            return Ok(Self::default());
         }
 
         let mut bits = WillChangeBits::empty();
@@ -767,8 +768,8 @@ impl Parse for WillChange {
             Ok(ident)
         })?;
 
-        Ok(WillChange::AnimateableFeatures {
-            features: custom_idents.into_boxed_slice(),
+        Ok(Self {
+            features: custom_idents.into(),
             bits,
         })
     }

servo/ports/geckolib/cbindgen.toml

@@ -110,7 +110,7 @@ include = [
   "Origin",
   "RestyleHint",
   "TouchAction",
-  "WillChangeBits",
+  "WillChange",
   "TextDecorationLine",
   "TextTransform",
   "MozListReversed",
@@ -385,12 +385,17 @@ renaming_overrides_prefixing = true
   inline bool operator!=(const StyleArcSlice& other) const;
 """
 
+"CustomIdent" = """
+  inline nsAtom* AsAtom() const;
+"""
+
 "Atom" = """
   StyleAtom(size_t) = delete;
   StyleAtom() = delete;
 
   // NOTE(emilio): For now we don't need to expose anything else, but it'd be trivial if we wanted to.
   inline bool IsStatic() const;
+  inline nsAtom* AsAtom() const;
 
   inline StyleAtom(const StyleAtom& aOther);
   inline ~StyleAtom();