Bug 1304922 - Part 7: Drop mWinsInCascade. r?birtles draft
authorHiroyuki Ikezoe <hiikezoe@mozilla-japan.org>
Wed, 05 Oct 2016 14:48:05 +0900
changeset 421032 10a08d2fba99c7f98e1ee00d70ee8f3b14e74d53
parent 421031 572f5213fbc85dcbfc0ee31e808b7c5cc4586b92
child 421033 5a81fd0dd57c6d2165c2cfae6a371d4d924a05a5
push id31361
push userbmo:hiikezoe@mozilla-japan.org
push dateWed, 05 Oct 2016 06:56:42 +0000
reviewersbirtles
bugs1304922
milestone52.0a1
Bug 1304922 - Part 7: Drop mWinsInCascade. r?birtles MozReview-Commit-ID: 1q4glZenZNa
dom/animation/EffectCompositor.cpp
dom/animation/EffectCompositor.h
dom/animation/EffectSet.h
dom/animation/KeyframeEffect.cpp
dom/animation/KeyframeEffectReadOnly.cpp
dom/animation/KeyframeEffectReadOnly.h
--- a/dom/animation/EffectCompositor.cpp
+++ b/dom/animation/EffectCompositor.cpp
@@ -706,65 +706,36 @@ EffectCompositor::UpdateCascadeResults(E
       compositorPropertiesInSet(propertiesWithImportantRules);
   std::bitset<LayerAnimationInfo::kRecords>
     prevCompositorPropertiesForAnimationsLevel =
       compositorPropertiesInSet(propertiesForAnimationsLevel);
 
   propertiesWithImportantRules.Empty();
   propertiesForAnimationsLevel.Empty();
 
-  nsCSSPropertyIDSet animatedProperties;
   bool hasCompositorPropertiesForTransition = false;
 
-  for (KeyframeEffectReadOnly* effect : sortedEffectList) {
+  for (const KeyframeEffectReadOnly* effect : sortedEffectList) {
     MOZ_ASSERT(effect->GetAnimation(),
                "Effects on a target element should have an Animation");
-    bool inEffect = effect->IsInEffect();
     CascadeLevel cascadeLevel = effect->GetAnimation()->CascadeLevel();
 
-    for (AnimationProperty& prop : effect->Properties()) {
-
-      bool winsInCascade = !animatedProperties.HasProperty(prop.mProperty) &&
-                           inEffect;
-
+    for (const AnimationProperty& prop : effect->Properties()) {
       if (overriddenProperties.HasProperty(prop.mProperty)) {
         propertiesWithImportantRules.AddProperty(prop.mProperty);
       }
       if (cascadeLevel == EffectCompositor::CascadeLevel::Animations) {
         propertiesForAnimationsLevel.AddProperty(prop.mProperty);
       }
 
       if (nsCSSProps::PropHasFlags(prop.mProperty,
                                    CSS_PROPERTY_CAN_ANIMATE_ON_COMPOSITOR) &&
           cascadeLevel == EffectCompositor::CascadeLevel::Transitions) {
         hasCompositorPropertiesForTransition = true;
       }
-
-      // If this property wins in the cascade, add it to the set of animated
-      // properties. We need to do this even if the property is overridden
-      // (in which case we set winsInCascade to false below) since we don't
-      // want to fire transitions on these properties.
-      if (winsInCascade) {
-        animatedProperties.AddProperty(prop.mProperty);
-      }
-
-      // For effects that will be applied to the animations level of the
-      // cascade, we need to check that the property isn't being set by
-      // something with higher priority in the cascade.
-      //
-      // We only do this, however, for properties that can be animated on
-      // the compositor. For properties animated on the main thread the usual
-      // cascade ensures these animations will be correctly overridden.
-      if (winsInCascade &&
-          effect->GetAnimation()->CascadeLevel() == CascadeLevel::Animations &&
-          overriddenProperties.HasProperty(prop.mProperty)) {
-        winsInCascade = false;
-      }
-
-      prop.mWinsInCascade = winsInCascade;
     }
   }
 
   aEffectSet.MarkCascadeUpdated();
 
   nsPresContext* presContext = GetPresContext(aElement);
   if (!presContext) {
     return;
--- a/dom/animation/EffectCompositor.h
+++ b/dom/animation/EffectCompositor.h
@@ -180,20 +180,18 @@ public:
   //
   // This method does NOT detect if other styles that apply above the
   // animation level of the cascade have changed.
   static void
   MaybeUpdateCascadeResults(dom::Element* aElement,
                             CSSPseudoElementType aPseudoType,
                             nsStyleContext* aStyleContext);
 
-  // Update the mWinsInCascade member for each property and in effects
-  // targetting the specified (pseudo-)element. Also updates the
-  // mPropertiesWithImportantRules and mPropertiesForAnimationsLevel members
-  // of the corresponding EffectSet.
+  // Update the mPropertiesWithImportantRules and
+  // mPropertiesForAnimationsLevel members of the corresponding EffectSet.
   //
   // 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.
   static void
   UpdateCascadeResults(dom::Element* aElement,
                        CSSPseudoElementType aPseudoType,
                        nsStyleContext* aStyleContext);
--- a/dom/animation/EffectSet.h
+++ b/dom/animation/EffectSet.h
@@ -218,18 +218,19 @@ private:
   // timestamp when the rule was last updated. This is used for certain
   // animations which are updated only periodically (e.g. transform animations
   // running on the compositor that affect the scrollable overflow region).
   EnumeratedArray<EffectCompositor::CascadeLevel,
                   EffectCompositor::CascadeLevel(
                     EffectCompositor::kCascadeLevelCount),
                   TimeStamp> mAnimationRuleRefreshTime;
 
-  // Dirty flag to represent when the mWinsInCascade flag on effects in
-  // this set might need to be updated.
+  // Dirty flag to represent when the mPropertiesWithImportantRules and
+  // mPropertiesForAnimationsLevel on effects in this set might need to be
+  // updated.
   //
   // Set to true any time the set of effects is changed or when
   // one the effects goes in or out of the "in effect" state.
   bool mCascadeNeedsUpdate;
 
   // RestyleManager keeps track of the number of animation restyles.
   // 'mini-flushes' (see nsTransitionManager::UpdateAllThrottledStyles()).
   // mAnimationGeneration is the sequence number of the last flush where a
--- a/dom/animation/KeyframeEffect.cpp
+++ b/dom/animation/KeyframeEffect.cpp
@@ -84,19 +84,16 @@ KeyframeEffect::SetTarget(const Nullable
   if (mTarget == newTarget) {
     // Assign the same target, skip it.
     return;
   }
 
   if (mTarget) {
     UnregisterTarget();
     ResetIsRunningOnCompositor();
-    // We don't need to reset the mWinsInCascade member since it will be updated
-    // when we later associate with a different target (and until that time this
-    // flag is not used).
 
     RequestRestyle(EffectCompositor::RestyleType::Layer);
 
     nsAutoAnimationMutationBatch mb(mTarget->mElement->OwnerDoc());
     if (mAnimation) {
       nsNodeUtils::AnimationRemoved(mAnimation);
     }
   }
--- a/dom/animation/KeyframeEffectReadOnly.cpp
+++ b/dom/animation/KeyframeEffectReadOnly.cpp
@@ -281,34 +281,28 @@ KeyframeEffectReadOnly::UpdateProperties
 
     mKeyframes.SwapElements(keyframesCopy);
   }
 
   if (mProperties == properties) {
     return;
   }
 
-  // Preserve the state of mWinsInCascade and mIsRunningOnCompositor flags.
-  nsCSSPropertyIDSet winningInCascadeProperties;
+  // Preserve the state of the mIsRunningOnCompositor flag.
   nsCSSPropertyIDSet runningOnCompositorProperties;
 
   for (const AnimationProperty& property : mProperties) {
-    if (property.mWinsInCascade) {
-      winningInCascadeProperties.AddProperty(property.mProperty);
-    }
     if (property.mIsRunningOnCompositor) {
       runningOnCompositorProperties.AddProperty(property.mProperty);
     }
   }
 
   mProperties = Move(properties);
 
   for (AnimationProperty& property : mProperties) {
-    property.mWinsInCascade =
-      winningInCascadeProperties.HasProperty(property.mProperty);
     property.mIsRunningOnCompositor =
       runningOnCompositorProperties.HasProperty(property.mProperty);
   }
 
   CalculateCumulativeChangeHint(aStyleContext);
 
   MarkCascadeNeedsUpdate();
 
@@ -1292,18 +1286,16 @@ KeyframeEffectReadOnly::CanIgnoreIfNotVi
 void
 KeyframeEffectReadOnly::MaybeUpdateFrameForCompositor()
 {
   nsIFrame* frame = GetAnimationFrame();
   if (!frame) {
     return;
   }
 
-  // We don't check mWinsInCascade flag here because, at this point,
-  // UpdateCascadeResults has not yet run.
   // FIXME: Bug 1272495: If this effect does not win in the cascade, the
   // NS_FRAME_MAY_BE_TRANSFORMED flag should be removed when the animation
   // will be removed from effect set or the transform keyframes are removed
   // by setKeyframes. The latter case will be hard to solve though.
   for (const AnimationProperty& property : mProperties) {
     if (property.mProperty == eCSSProperty_transform) {
       frame->AddStateBits(NS_FRAME_MAY_BE_TRANSFORMED);
       return;
--- a/dom/animation/KeyframeEffectReadOnly.h
+++ b/dom/animation/KeyframeEffectReadOnly.h
@@ -128,55 +128,37 @@ struct AnimationPropertySegment
     return !(*this == aOther);
   }
 };
 
 struct AnimationProperty
 {
   nsCSSPropertyID mProperty = eCSSProperty_UNKNOWN;
 
-  // Does this property win in the CSS Cascade?
-  //
-  // For CSS transitions, this is true as long as a CSS animation on the
-  // same property and element is not running, in which case we set this
-  // to false so that the animation (lower in the cascade) can win.  We
-  // then use this to decide whether to apply the style both in the CSS
-  // cascade and for OMTA.
-  //
-  // For CSS Animations, which are overridden by !important rules in the
-  // cascade, we actually determine this from the CSS cascade
-  // computations, and then use it for OMTA.
-  //
-  // **NOTE**: This member is not included when comparing AnimationProperty
-  // objects for equality.
-  bool mWinsInCascade = false;
-
   // If true, the propery is currently being animated on the compositor.
   //
   // Note that when the owning Animation requests a non-throttled restyle, in
   // between calling RequestRestyle on its EffectCompositor and when the
   // restyle is performed, this member may temporarily become false even if
   // the animation remains on the layer after the restyle.
   //
   // **NOTE**: This member is not included when comparing AnimationProperty
   // objects for equality.
   bool mIsRunningOnCompositor = false;
 
   Maybe<AnimationPerformanceWarning> mPerformanceWarning;
 
   InfallibleTArray<AnimationPropertySegment> mSegments;
 
-  // NOTE: This operator does *not* compare the mWinsInCascade member *or* the
-  // mIsRunningOnCompositor member.
+  // NOTE: This operator does *not* compare the mIsRunningOnCompositor member.
   // This is because AnimationProperty objects are compared when recreating
   // CSS animations to determine if mutation observer change records need to
   // be created or not. However, at the point when these objects are compared
-  // neither the mWinsInCascade nor the mIsRunningOnCompositor will have been
-  // set on the new objects so we ignore these members to avoid generating
-  // spurious change records.
+  // the mIsRunningOnCompositor will not have been set on the new objects so
+  // we ignore this member to avoid generating spurious change records.
   bool operator==(const AnimationProperty& aOther) const {
     return mProperty == aOther.mProperty &&
            mSegments == aOther.mSegments;
   }
   bool operator!=(const AnimationProperty& aOther) const {
     return !(*this == aOther);
   }
 };