Bug 1288586 - Don't calculate style difference if there are no properties change. r=birtles
authorHiroyuki Ikezoe <hiikezoe@mozilla-japan.org>
Fri, 22 Jul 2016 15:17:37 +0900
changeset 346259 6cd25af34d5cacb3fec462f09f5a24abcb2ddd18
parent 346258 fb7ba7f4456c88251958b18dfca4465bc6f7689d
child 346260 666ed766fc728df065573de3efb46721eec1edc4
push id6389
push userraliiev@mozilla.com
push dateMon, 19 Sep 2016 13:38:22 +0000
treeherdermozilla-beta@01d67bfe6c81 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles
bugs1288586
milestone50.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 1288586 - Don't calculate style difference if there are no properties change. r=birtles We don't actually need to re-calculate if the updated properties are the same as the old one. This change avoids problematic nested calls of nsStyleSet::GetContext() in particular cases. MozReview-Commit-ID: JksiTGX57Fy
dom/animation/KeyframeEffect.cpp
dom/animation/KeyframeEffect.h
dom/animation/KeyframeUtils.cpp
--- a/dom/animation/KeyframeEffect.cpp
+++ b/dom/animation/KeyframeEffect.cpp
@@ -604,17 +604,17 @@ KeyframeEffectReadOnly::UpdateProperties
 
   for (AnimationProperty& property : mProperties) {
     property.mWinsInCascade =
       winningInCascadeProperties.HasProperty(property.mProperty);
     property.mIsRunningOnCompositor =
       runningOnCompositorProperties.HasProperty(property.mProperty);
   }
 
-  CalculateCumulativeChangeHint();
+  CalculateCumulativeChangeHint(aStyleContext);
 
   if (mTarget) {
     EffectSet* effectSet = EffectSet::GetEffectSet(mTarget->mElement,
                                                    mTarget->mPseudoType);
     if (effectSet) {
       effectSet->MarkCascadeNeedsUpdate();
     }
 
@@ -1466,24 +1466,71 @@ KeyframeEffectReadOnly::SetPerformanceWa
         nsAutoCString logMessage = NS_ConvertUTF16toUTF8(localizedString);
         AnimationUtils::LogAsyncAnimationFailure(logMessage, mTarget->mElement);
       }
       return;
     }
   }
 }
 
+static already_AddRefed<nsStyleContext>
+CreateStyleContextForAnimationValue(nsCSSProperty aProperty,
+                                    StyleAnimationValue aValue,
+                                    nsStyleContext* aBaseStyleContext)
+{
+  MOZ_ASSERT(aBaseStyleContext,
+             "CreateStyleContextForAnimationValue needs to be called "
+             "with a valid nsStyleContext");
+
+  RefPtr<AnimValuesStyleRule> styleRule = new AnimValuesStyleRule();
+  styleRule->AddValue(aProperty, aValue);
+
+  nsCOMArray<nsIStyleRule> rules;
+  rules.AppendObject(styleRule);
+
+  MOZ_ASSERT(aBaseStyleContext->PresContext()->StyleSet()->IsGecko(),
+             "ServoStyleSet should not use StyleAnimationValue for animations");
+  nsStyleSet* styleSet =
+    aBaseStyleContext->PresContext()->StyleSet()->AsGecko();
+
+  RefPtr<nsStyleContext> styleContext =
+    styleSet->ResolveStyleByAddingRules(aBaseStyleContext, rules);
+
+  // We need to call StyleData to generate cached data for the style context.
+  // Otherwise CalcStyleDifference returns no meaningful result.
+  styleContext->StyleData(nsCSSProps::kSIDTable[aProperty]);
+
+  return styleContext.forget();
+}
+
 void
-KeyframeEffectReadOnly::CalculateCumulativeChangeHint()
+KeyframeEffectReadOnly::CalculateCumulativeChangeHint(
+  nsStyleContext *aStyleContext)
 {
   mCumulativeChangeHint = nsChangeHint(0);
 
   for (const AnimationProperty& property : mProperties) {
     for (const AnimationPropertySegment& segment : property.mSegments) {
-      mCumulativeChangeHint |= segment.mChangeHint;
+      RefPtr<nsStyleContext> fromContext =
+        CreateStyleContextForAnimationValue(property.mProperty,
+                                            segment.mFromValue, aStyleContext);
+
+      RefPtr<nsStyleContext> toContext =
+        CreateStyleContextForAnimationValue(property.mProperty,
+                                            segment.mToValue, aStyleContext);
+
+      uint32_t equalStructs = 0;
+      uint32_t samePointerStructs = 0;
+      nsChangeHint changeHint =
+        fromContext->CalcStyleDifference(toContext,
+                                         nsChangeHint(0),
+                                         &equalStructs,
+                                         &samePointerStructs);
+
+      mCumulativeChangeHint |= changeHint;
     }
   }
 }
 
 bool
 KeyframeEffectReadOnly::CanIgnoreIfNotVisible() const
 {
   if (!AnimationUtils::IsOffscreenThrottlingEnabled()) {
--- a/dom/animation/KeyframeEffect.h
+++ b/dom/animation/KeyframeEffect.h
@@ -115,18 +115,16 @@ struct Keyframe
 };
 
 struct AnimationPropertySegment
 {
   float mFromKey, mToKey;
   StyleAnimationValue mFromValue, mToValue;
   Maybe<ComputedTimingFunction> mTimingFunction;
 
-  nsChangeHint mChangeHint;
-
   bool operator==(const AnimationPropertySegment& aOther) const {
     return mFromKey == aOther.mFromKey &&
            mToKey == aOther.mToKey &&
            mFromValue == aOther.mFromValue &&
            mToValue == aOther.mToValue &&
            mTimingFunction == aOther.mTimingFunction;
   }
   bool operator!=(const AnimationPropertySegment& aOther) const {
@@ -342,17 +340,17 @@ public:
   // compositor. |aParams| and |aParamsLength| are optional parameters which
   // will be used to generate a localized message for devtools.
   void SetPerformanceWarning(
     nsCSSProperty aProperty,
     const AnimationPerformanceWarning& aWarning);
 
   // Cumulative change hint on each segment for each property.
   // This is used for deciding the animation is paint-only.
-  void CalculateCumulativeChangeHint();
+  void CalculateCumulativeChangeHint(nsStyleContext* aStyleContext);
 
   // Returns true if all of animation properties' change hints
   // can ignore painting if the animation is not visible.
   // See nsChangeHint_Hints_CanIgnoreIfNotVisible in nsChangeHint.h
   // in detail which change hint can be ignored.
   bool CanIgnoreIfNotVisible() const;
 
 protected:
--- a/dom/animation/KeyframeUtils.cpp
+++ b/dom/animation/KeyframeUtils.cpp
@@ -1069,46 +1069,16 @@ MarkAsComputeValuesFailureKey(PropertyVa
 static bool
 IsComputeValuesFailureKey(const PropertyValuePair& aPair)
 {
   return nsCSSProps::IsShorthand(aPair.mProperty) &&
          aPair.mValue.GetTokenStreamValue()->mPropertyID ==
            eCSSPropertyExtra_no_properties;
 }
 
-static already_AddRefed<nsStyleContext>
-CreateStyleContextForAnimationValue(nsCSSProperty aProperty,
-                                    StyleAnimationValue aValue,
-                                    nsStyleContext* aBaseStyleContext)
-{
-  MOZ_ASSERT(aBaseStyleContext,
-             "CreateStyleContextForAnimationValue needs to be called "
-             "with a valid nsStyleContext");
-
-  RefPtr<AnimValuesStyleRule> styleRule = new AnimValuesStyleRule();
-  styleRule->AddValue(aProperty, aValue);
-
-  nsCOMArray<nsIStyleRule> rules;
-  rules.AppendObject(styleRule);
-
-  MOZ_ASSERT(aBaseStyleContext->PresContext()->StyleSet()->IsGecko(),
-             "ServoStyleSet should not use StyleAnimationValue for animations");
-  nsStyleSet* styleSet =
-    aBaseStyleContext->PresContext()->StyleSet()->AsGecko();
-
-  RefPtr<nsStyleContext> styleContext =
-    styleSet->ResolveStyleByAddingRules(aBaseStyleContext, rules);
-
-  // We need to call StyleData to generate cached data for the style context.
-  // Otherwise CalcStyleDifference returns no meaningful result.
-  styleContext->StyleData(nsCSSProps::kSIDTable[aProperty]);
-
-  return styleContext.forget();
-}
-
 /**
  * Builds an array of AnimationProperty objects to represent the keyframe
  * animation segments in aEntries.
  */
 static void
 BuildSegmentsFromValueEntries(nsStyleContext* aStyleContext,
                               nsTArray<KeyframeValueEntry>& aEntries,
                               nsTArray<AnimationProperty>& aResult)
@@ -1238,32 +1208,16 @@ BuildSegmentsFromValueEntries(nsStyleCon
     AnimationPropertySegment* segment =
       animationProperty->mSegments.AppendElement();
     segment->mFromKey   = aEntries[i].mOffset;
     segment->mToKey     = aEntries[j].mOffset;
     segment->mFromValue = aEntries[i].mValue;
     segment->mToValue   = aEntries[j].mValue;
     segment->mTimingFunction = aEntries[i].mTimingFunction;
 
-    RefPtr<nsStyleContext> fromContext =
-      CreateStyleContextForAnimationValue(animationProperty->mProperty,
-                                          segment->mFromValue, aStyleContext);
-
-    RefPtr<nsStyleContext> toContext =
-      CreateStyleContextForAnimationValue(animationProperty->mProperty,
-                                          segment->mToValue, aStyleContext);
-
-    uint32_t equalStructs = 0;
-    uint32_t samePointerStructs = 0;
-    segment->mChangeHint =
-        fromContext->CalcStyleDifference(toContext,
-          nsChangeHint(0),
-          &equalStructs,
-          &samePointerStructs);
-
     i = j;
   }
 }
 
 /**
  * Converts a JS object representing a property-indexed keyframe into
  * an array of Keyframe objects.
  *