Bug 1447828 part 1 - Remove StyleBackendType uses from dom/animation. r?hiro draft
authorXidorn Quan <me@upsuper.org>
Wed, 28 Mar 2018 15:50:47 +1100
changeset 773741 1bcf22436ced3f5b2afd6a7db4250ff5b79f71c0
parent 773579 990385558c3b8a251d3ecc50b43a10ec97e6c7b8
child 773742 4972e279ef76127e4898874478fcbd75c9631d76
push id104288
push userxquan@mozilla.com
push dateWed, 28 Mar 2018 11:04:19 +0000
reviewershiro
bugs1447828
milestone61.0a1
Bug 1447828 part 1 - Remove StyleBackendType uses from dom/animation. r?hiro MozReview-Commit-ID: 3INWUuocNPm
dom/animation/EffectCompositor.cpp
dom/animation/EffectCompositor.h
dom/animation/KeyframeUtils.cpp
dom/animation/KeyframeUtils.h
layout/style/ServoBindings.cpp
--- a/dom/animation/EffectCompositor.cpp
+++ b/dom/animation/EffectCompositor.cpp
@@ -162,21 +162,18 @@ FindAnimationsForCompositor(const nsIFra
   // from test control after seeking where it might not be the case.
   //
   // Those cases are probably not important but just to be safe, let's make
   // sure the cascade is up to date since if it *is* up to date, this is
   // basically a no-op.
   Maybe<NonOwningAnimationTarget> pseudoElement =
     EffectCompositor::GetAnimationElementAndPseudoForFrame(aFrame);
   if (pseudoElement) {
-    StyleBackendType backend = StyleBackendType::Servo;
-    EffectCompositor::MaybeUpdateCascadeResults(backend,
-                                                pseudoElement->mElement,
-                                                pseudoElement->mPseudoType,
-                                                aFrame->Style());
+    EffectCompositor::MaybeUpdateCascadeResults(pseudoElement->mElement,
+                                                pseudoElement->mPseudoType);
   }
 
   if (!nsLayoutUtils::AreAsyncAnimationsEnabled()) {
     if (nsLayoutUtils::IsAnimationLoggingEnabled()) {
       nsCString message;
       message.AppendLiteral("Performance warning: Async animations are "
                             "disabled");
       AnimationUtils::LogAsyncAnimationFailure(message);
@@ -539,28 +536,25 @@ EffectCompositor::ClearIsRunningOnCompos
   }
 
   for (KeyframeEffectReadOnly* effect : *effects) {
     effect->SetIsRunningOnCompositor(aProperty, false);
   }
 }
 
 /* static */ void
-EffectCompositor::MaybeUpdateCascadeResults(StyleBackendType aBackendType,
-                                            Element* aElement,
-                                            CSSPseudoElementType aPseudoType,
-                                            ComputedStyle* aComputedStyle)
+EffectCompositor::MaybeUpdateCascadeResults(Element* aElement,
+                                            CSSPseudoElementType aPseudoType)
 {
   EffectSet* effects = EffectSet::GetEffectSet(aElement, aPseudoType);
   if (!effects || !effects->CascadeNeedsUpdate()) {
     return;
   }
 
-  UpdateCascadeResults(aBackendType, *effects, aElement, aPseudoType,
-                       aComputedStyle);
+  UpdateCascadeResults(*effects, aElement, aPseudoType);
 
   MOZ_ASSERT(!effects->CascadeNeedsUpdate(), "Failed to update cascade state");
 }
 
 /* static */ Maybe<NonOwningAnimationTarget>
 EffectCompositor::GetAnimationElementAndPseudoForFrame(const nsIFrame* aFrame)
 {
   // Always return the same object to benefit from return-value optimization.
@@ -594,32 +588,26 @@ EffectCompositor::GetAnimationElementAnd
 
   result.emplace(content->AsElement(), pseudoType);
 
   return result;
 }
 
 
 /* static */ nsCSSPropertyIDSet
-EffectCompositor::GetOverriddenProperties(StyleBackendType aBackendType,
-                                          EffectSet& aEffectSet,
+EffectCompositor::GetOverriddenProperties(EffectSet& aEffectSet,
                                           Element* aElement,
-                                          CSSPseudoElementType aPseudoType,
-                                          ComputedStyle* aComputedStyle)
+                                          CSSPseudoElementType aPseudoType)
 {
-  MOZ_ASSERT(aBackendType != StyleBackendType::Servo || aElement,
-             "Should have an element to get style data from if we are using"
-             " the Servo backend");
+  MOZ_ASSERT(aElement, "Should have an element to get style data from");
 
   nsCSSPropertyIDSet result;
 
   Element* elementToRestyle = GetElementToRestyle(aElement, aPseudoType);
-  if (aBackendType == StyleBackendType::Gecko && !aComputedStyle) {
-    MOZ_CRASH("old style system disabled");
-  } else if (aBackendType == StyleBackendType::Servo && !elementToRestyle) {
+  if (!elementToRestyle) {
     return result;
   }
 
   AutoTArray<nsCSSPropertyID, LayerAnimationInfo::kRecords> propertiesToTrack;
   {
     nsCSSPropertyIDSet propertiesToTrackAsSet;
     for (KeyframeEffectReadOnly* effect : aEffectSet) {
       for (const AnimationProperty& property : effect->Properties()) {
@@ -637,39 +625,26 @@ EffectCompositor::GetOverriddenPropertie
       }
     }
   }
 
   if (propertiesToTrack.IsEmpty()) {
     return result;
   }
 
-  switch (aBackendType) {
-    case StyleBackendType::Servo:
-      Servo_GetProperties_Overriding_Animation(elementToRestyle,
-                                               &propertiesToTrack,
-                                               &result);
-      break;
-    case StyleBackendType::Gecko:
-      MOZ_CRASH("old style system disabled");
-      break;
-
-    default:
-      MOZ_ASSERT_UNREACHABLE("Unsupported style backend");
-  }
-
+  Servo_GetProperties_Overriding_Animation(elementToRestyle,
+                                           &propertiesToTrack,
+                                           &result);
   return result;
 }
 
 /* static */ void
-EffectCompositor::UpdateCascadeResults(StyleBackendType aBackendType,
-                                       EffectSet& aEffectSet,
+EffectCompositor::UpdateCascadeResults(EffectSet& aEffectSet,
                                        Element* aElement,
-                                       CSSPseudoElementType aPseudoType,
-                                       ComputedStyle* aComputedStyle)
+                                       CSSPseudoElementType aPseudoType)
 {
   MOZ_ASSERT(EffectSet::GetEffectSet(aElement, aPseudoType) == &aEffectSet,
              "Effect set should correspond to the specified (pseudo-)element");
   if (aEffectSet.IsEmpty()) {
     aEffectSet.MarkCascadeUpdated();
     return;
   }
 
@@ -681,20 +656,17 @@ EffectCompositor::UpdateCascadeResults(S
   sortedEffectList.Sort(EffectCompositeOrderComparator());
 
   // Get properties that override the *animations* level of the cascade.
   //
   // We only do this for properties that we can animate on the compositor
   // since we will apply other properties on the main thread where the usual
   // cascade applies.
   nsCSSPropertyIDSet overriddenProperties =
-    GetOverriddenProperties(aBackendType,
-                            aEffectSet,
-                            aElement, aPseudoType,
-                            aComputedStyle);
+    GetOverriddenProperties(aEffectSet, aElement, aPseudoType);
 
   // Returns a bitset the represents which properties from
   // LayerAnimationInfo::sRecords are present in |aPropertySet|.
   auto compositorPropertiesInSet =
     [](nsCSSPropertyIDSet& aPropertySet) ->
       std::bitset<LayerAnimationInfo::kRecords> {
         std::bitset<LayerAnimationInfo::kRecords> result;
         for (size_t i = 0; i < LayerAnimationInfo::kRecords; i++) {
@@ -893,20 +865,17 @@ EffectCompositor::PreTraverseInSubtree(S
         continue;
       }
 
       elementsWithCascadeUpdates.AppendElement(target);
     }
   }
 
   for (const NonOwningAnimationTarget& target: elementsWithCascadeUpdates) {
-      MaybeUpdateCascadeResults(StyleBackendType::Servo,
-                                target.mElement,
-                                target.mPseudoType,
-                                nullptr);
+      MaybeUpdateCascadeResults(target.mElement, target.mPseudoType);
   }
   elementsWithCascadeUpdates.Clear();
 
   for (size_t i = 0; i < kCascadeLevelCount; ++i) {
     CascadeLevel cascadeLevel = CascadeLevel(i);
     auto& elementSet = mElementsToRestyle[cascadeLevel];
     for (auto iter = elementSet.Iter(); !iter.Done(); iter.Next()) {
       const NonOwningAnimationTarget& target = getNeededRestyleTarget(iter);
@@ -1005,19 +974,17 @@ EffectCompositor::PreTraverse(dom::Eleme
       PostRestyleEventForAnimations(aElement,
                                     aPseudoType,
                                     cascadeLevel == CascadeLevel::Transitions
                                       ? eRestyle_CSSTransitions
                                       : eRestyle_CSSAnimations);
 
     EffectSet* effects = EffectSet::GetEffectSet(aElement, aPseudoType);
     if (effects) {
-      MaybeUpdateCascadeResults(StyleBackendType::Servo,
-                                aElement, aPseudoType,
-                                nullptr);
+      MaybeUpdateCascadeResults(aElement, aPseudoType);
 
       for (KeyframeEffectReadOnly* effect : *effects) {
         effect->GetAnimation()->WillComposeStyle();
       }
     }
 
     elementSet.Remove(key);
     found = true;
--- a/dom/animation/EffectCompositor.h
+++ b/dom/animation/EffectCompositor.h
@@ -148,53 +148,38 @@ public:
   static void ClearIsRunningOnCompositor(const nsIFrame* aFrame,
                                          nsCSSPropertyID aProperty);
 
   // Update animation cascade results for the specified (pseudo-)element
   // but only if we have marked the cascade as needing an update due a
   // the change in the set of effects or a change in one of the effects'
   // "in effect" state.
   //
-  // When |aBackendType| is StyleBackendType::Gecko, |aComputedStyle| is used to
-  // find overridden properties. If it is nullptr, the ComputedStyle of the
-  // primary frame of the specified (pseudo-)element, if available, is used.
-  //
-  // When |aBackendType| is StyleBackendType::Servo, we fetch the rule node
-  // from the |aElement| (i.e. |aComputedStyle| is ignored).
-  //
   // This method does NOT detect if other styles that apply above the
   // animation level of the cascade have changed.
   static void
-  MaybeUpdateCascadeResults(StyleBackendType aBackendType,
-                            dom::Element* aElement,
-                            CSSPseudoElementType aPseudoType,
-                            ComputedStyle* aComputedStyle);
+  MaybeUpdateCascadeResults(dom::Element* aElement,
+                            CSSPseudoElementType aPseudoType);
 
   // Update the mPropertiesWithImportantRules and
   // mPropertiesForAnimationsLevel members of the given EffectSet, and also
   // request any restyles required by changes to the cascade result.
   //
   // NOTE: This can be expensive so we should only call it if styles that apply
   // above the animation level of the cascade might have changed. For all
   // other cases we should call MaybeUpdateCascadeResults.
   //
   // This is typically reserved for internal callers but is public here since
   // when we detect changes to the cascade on the Servo side we can't call
   // MarkCascadeNeedsUpdate during the traversal so instead we call this as part
   // of a follow-up sequential task.
-  //
-  // As with MaybeUpdateCascadeResults, |aComputedStyle| is only used
-  // when |aBackendType| is StyleBackendType::Gecko. When |aBackendType| is
-  // StyleBackendType::Servo, it is ignored.
   static void
-  UpdateCascadeResults(StyleBackendType aBackendType,
-                       EffectSet& aEffectSet,
+  UpdateCascadeResults(EffectSet& aEffectSet,
                        dom::Element* aElement,
-                       CSSPseudoElementType aPseudoType,
-                       ComputedStyle* aComputedStyle);
+                       CSSPseudoElementType aPseudoType);
 
   // Helper to fetch the corresponding element and pseudo-type from a frame.
   //
   // For frames corresponding to pseudo-elements, the returned element is the
   // element on which we store the animations (i.e. the EffectSet and/or
   // AnimationCollection), *not* the generated content.
   //
   // Returns an empty result when a suitable element cannot be found including
@@ -235,31 +220,20 @@ public:
 
 private:
   ~EffectCompositor() = default;
 
 
   // Get the properties in |aEffectSet| that we are able to animate on the
   // compositor but which are also specified at a higher level in the cascade
   // than the animations level.
-  //
-  // When |aBackendType| is StyleBackendType::Gecko, we determine which
-  // properties are specified using the provided |aComputedStyle| and
-  // |aElement| and |aPseudoType| are ignored. If |aComputedStyle| is nullptr,
-  // we automatically look up the ComputedStyle of primary frame of the
-  // (pseudo-)element.
-  //
-  // When |aBackendType| is StyleBackendType::Servo, we use the |StrongRuleNode|
-  // stored on the (pseudo-)element indicated by |aElement| and |aPseudoType|.
   static nsCSSPropertyIDSet
-  GetOverriddenProperties(StyleBackendType aBackendType,
-                          EffectSet& aEffectSet,
+  GetOverriddenProperties(EffectSet& aEffectSet,
                           dom::Element* aElement,
-                          CSSPseudoElementType aPseudoType,
-                          ComputedStyle* aComputedStyle);
+                          CSSPseudoElementType aPseudoType);
 
   static nsPresContext* GetPresContext(dom::Element* aElement);
 
   nsPresContext* mPresContext;
 
   // Elements with a pending animation restyle. The associated bool value is
   // true if a pending animation restyle has also been dispatched. For
   // animations that can be throttled, we will add an entry to the hashtable to
--- a/dom/animation/KeyframeUtils.cpp
+++ b/dom/animation/KeyframeUtils.cpp
@@ -334,17 +334,16 @@ ConvertKeyframeSequence(JSContext* aCx,
                         nsIDocument* aDocument,
                         JS::ForOfIterator& aIterator,
                         nsTArray<Keyframe>& aResult);
 
 static bool
 GetPropertyValuesPairs(JSContext* aCx,
                        JS::Handle<JSObject*> aObject,
                        ListAllowance aAllowLists,
-                       StyleBackendType aBackend,
                        nsTArray<PropertyValuesPair>& aResult);
 
 static bool
 AppendStringOrStringSequenceToArray(JSContext* aCx,
                                     JS::Handle<JS::Value> aValue,
                                     ListAllowance aAllowLists,
                                     nsTArray<nsString>& aValues);
 
@@ -518,46 +517,25 @@ KeyframeUtils::GetAnimationPropertiesFro
     }
   }
 
   BuildSegmentsFromValueEntries(entries, result);
   return result;
 }
 
 /* static */ bool
-KeyframeUtils::IsAnimatableProperty(nsCSSPropertyID aProperty,
-                                    StyleBackendType aBackend)
+KeyframeUtils::IsAnimatableProperty(nsCSSPropertyID aProperty)
 {
   // Regardless of the backend type, treat the 'display' property as not
-  // animatable. (The Servo backend will report it as being animatable, since
-  // it is in fact animatable by SMIL.)
+  // animatable. (Servo will report it as being animatable, since it is
+  // in fact animatable by SMIL.)
   if (aProperty == eCSSProperty_display) {
     return false;
   }
-
-  if (aBackend == StyleBackendType::Servo) {
-    return Servo_Property_IsAnimatable(aProperty);
-  }
-
-  if (aProperty == eCSSProperty_UNKNOWN) {
-    return false;
-  }
-
-  if (!nsCSSProps::IsShorthand(aProperty)) {
-    return nsCSSProps::kAnimTypeTable[aProperty] != eStyleAnimType_None;
-  }
-
-  CSSPROPS_FOR_SHORTHAND_SUBPROPERTIES(subprop, aProperty,
-                                       CSSEnabledState::eForAllContent) {
-    if (nsCSSProps::kAnimTypeTable[*subprop] != eStyleAnimType_None) {
-      return true;
-    }
-  }
-
-  return false;
+  return Servo_Property_IsAnimatable(aProperty);
 }
 
 // ------------------------------------------------------------------
 //
 // Internal helpers
 //
 // ------------------------------------------------------------------
 
@@ -657,17 +635,16 @@ ConvertKeyframeSequence(JSContext* aCx,
     }
 
     // Look for additional property-values pairs on the object.
     nsTArray<PropertyValuesPair> propertyValuePairs;
     if (value.isObject()) {
       JS::Rooted<JSObject*> object(aCx, &value.toObject());
       if (!GetPropertyValuesPairs(aCx, object,
                                   ListAllowance::eDisallow,
-                                  aDocument->GetStyleBackendType(),
                                   propertyValuePairs)) {
         return false;
       }
     }
 
     if (!parseEasingResult.Failed()) {
       keyframe->mTimingFunction =
         TimingParams::ParseEasing(keyframeDict.mEasing,
@@ -716,28 +693,25 @@ ConvertKeyframeSequence(JSContext* aCx,
 
 /**
  * Reads the property-values pairs from the specified JS object.
  *
  * @param aObject The JS object to look at.
  * @param aAllowLists If eAllow, values will be converted to
  *   (DOMString or sequence<DOMString); if eDisallow, values
  *   will be converted to DOMString.
- * @param aBackend The style backend in use. Used to determine which properties
- *   are animatable since only animatable properties are read.
  * @param aResult The array into which the enumerated property-values
  *   pairs will be stored.
  * @return false on failure or JS exception thrown while interacting
  *   with aObject; true otherwise.
  */
 static bool
 GetPropertyValuesPairs(JSContext* aCx,
                        JS::Handle<JSObject*> aObject,
                        ListAllowance aAllowLists,
-                       StyleBackendType aBackend,
                        nsTArray<PropertyValuesPair>& aResult)
 {
   nsTArray<AdditionalProperty> properties;
 
   // Iterate over all the properties on aObject and append an
   // entry to properties for them.
   //
   // We don't compare the jsids that we encounter with those for
@@ -750,17 +724,17 @@ GetPropertyValuesPairs(JSContext* aCx,
   for (size_t i = 0, n = ids.length(); i < n; i++) {
     nsAutoJSString propName;
     if (!propName.init(aCx, ids[i])) {
       return false;
     }
     nsCSSPropertyID property =
       nsCSSProps::LookupPropertyByIDLName(propName,
                                           CSSEnabledState::eForAllContent);
-    if (KeyframeUtils::IsAnimatableProperty(property, aBackend)) {
+    if (KeyframeUtils::IsAnimatableProperty(property)) {
       AdditionalProperty* p = properties.AppendElement();
       p->mProperty = property;
       p->mJsidIndex = i;
     }
   }
 
   // Sort the entries by IDL name and then get each value and
   // convert it either to a DOMString or to a
@@ -1242,17 +1216,16 @@ GetKeyframeListFromPropertyIndexedKeyfra
     aRv.Throw(NS_ERROR_FAILURE);
     return;
   }
 
   // Get all the property--value-list pairs off the object.
   JS::Rooted<JSObject*> object(aCx, &aValue.toObject());
   nsTArray<PropertyValuesPair> propertyValuesPairs;
   if (!GetPropertyValuesPairs(aCx, object, ListAllowance::eAllow,
-                              aDocument->GetStyleBackendType(),
                               propertyValuesPairs)) {
     aRv.Throw(NS_ERROR_FAILURE);
     return;
   }
 
   // Create a set of keyframes for each property.
   nsCSSParser parser(aDocument->CSSLoader());
   nsClassHashtable<nsFloatHashKey, Keyframe> processedKeyframes;
--- a/dom/animation/KeyframeUtils.h
+++ b/dom/animation/KeyframeUtils.h
@@ -100,15 +100,14 @@ public:
    * Check if the property or, for shorthands, one or more of
    * its subproperties, is animatable.
    *
    * @param aProperty The property to check.
    * @param aBackend  The style backend, Servo or Gecko, that should determine
    *                  if the property is animatable or not.
    * @return true if |aProperty| is animatable.
    */
-  static bool IsAnimatableProperty(nsCSSPropertyID aProperty,
-                                   StyleBackendType aBackend);
+  static bool IsAnimatableProperty(nsCSSPropertyID aProperty);
 };
 
 } // namespace mozilla
 
 #endif // mozilla_KeyframeUtils_h
--- a/layout/style/ServoBindings.cpp
+++ b/layout/style/ServoBindings.cpp
@@ -670,21 +670,19 @@ Gecko_UpdateAnimations(RawGeckoElementBo
     // animations to update.
     if (effectSet) {
       // We call UpdateCascadeResults directly (intead of
       // MaybeUpdateCascadeResults) since we know for sure that the cascade has
       // changed, but we were unable to call MarkCascadeUpdated when we noticed
       // it since we avoid mutating state as part of the Servo parallel
       // traversal.
       presContext->EffectCompositor()
-                 ->UpdateCascadeResults(StyleBackendType::Servo,
-                                        *effectSet,
+                 ->UpdateCascadeResults(*effectSet,
                                         const_cast<Element*>(aElement),
-                                        pseudoType,
-                                        nullptr);
+                                        pseudoType);
     }
   }
 
   if (aTasks & UpdateAnimationsTasks::DisplayChangedFromNone) {
     presContext->EffectCompositor()
                ->RequestRestyle(const_cast<Element*>(aElement),
                                 pseudoType,
                                 EffectCompositor::RestyleType::Standard,