Bug 1331704 - Part 2: Resolve base styles during UpdateProprties. r=birtles, a=gchang
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Sat, 11 Feb 2017 19:11:45 +0900
changeset 376135 a0897e0e0d0fc1c1a3dc76682c81424e87fefe59
parent 376134 40ad75d6e9a519ddd7535c0e776c9c6253455b82
child 376136 08243c8a96514bb8de3e655170c023790af54549
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles, gchang
bugs1331704
milestone53.0a2
Bug 1331704 - Part 2: Resolve base styles during UpdateProprties. r=birtles, a=gchang EnsureBaseStyle() requires an already resolved nsStyleContext and resolves the base style by ResolveStyleByRemovingAnimation(). MozReview-Commit-ID: BHqJiBJspQY
dom/animation/KeyframeEffectReadOnly.cpp
dom/animation/KeyframeEffectReadOnly.h
--- a/dom/animation/KeyframeEffectReadOnly.cpp
+++ b/dom/animation/KeyframeEffectReadOnly.cpp
@@ -280,16 +280,20 @@ KeyframeEffectReadOnly::UpdateProperties
   MOZ_DIAGNOSTIC_ASSERT(!mIsComposingStyle,
                         "Should not be called while processing ComposeStyle()");
   if (mIsComposingStyle) {
     return;
   }
 
   nsTArray<AnimationProperty> properties = BuildProperties(aStyleContext);
 
+  // We need to update base styles even if any properties are not changed at all
+  // since base styles might have been changed due to parent style changes, etc.
+  EnsureBaseStyles(aStyleContext, properties);
+
   if (mProperties == properties) {
     return;
   }
 
   // Preserve the state of the mIsRunningOnCompositor flag.
   nsCSSPropertyIDSet runningOnCompositorProperties;
 
   for (const AnimationProperty& property : mProperties) {
@@ -387,19 +391,17 @@ KeyframeEffectReadOnly::GetUnderlyingSty
     // If we have already composed style for the property, we use the style
     // as the underlying style.
     DebugOnly<bool> success = aAnimationRule->GetValue(aProperty, result);
     MOZ_ASSERT(success, "AnimValuesStyleRule::GetValue should not fail");
   } else {
     // If we are composing with composite operation that is not 'replace'
     // and we have not composed style for the property yet, we have to get
     // the base style for the property.
-    RefPtr<nsStyleContext> styleContext =
-      GetTargetStyleContextWithoutAnimation();
-    result = ResolveBaseStyle(aProperty, styleContext);
+    result = BaseStyle(aProperty);
   }
 
   return result;
 }
 
 StyleAnimationValue
 KeyframeEffectReadOnly::CompositeValue(
   nsCSSPropertyID aProperty,
@@ -431,48 +433,34 @@ KeyframeEffectReadOnly::CompositeValue(
 
   return CompositeValue(aProperty,
                         aValueToComposite,
                         result,
                         aCompositeOperation);
 }
 
 void
-KeyframeEffectReadOnly::EnsureBaseStylesForCompositor(
-  const nsCSSPropertyIDSet& aPropertiesToSkip)
+KeyframeEffectReadOnly::EnsureBaseStyles(
+  nsStyleContext* aStyleContext,
+  const nsTArray<AnimationProperty>& aProperties)
 {
   if (!mTarget) {
     return;
   }
 
-  RefPtr<nsStyleContext> styleContext;
+  mBaseStyleValues.Clear();
 
-  for (const AnimationProperty& property : mProperties) {
-    if (!nsCSSProps::PropHasFlags(property.mProperty,
-                                  CSS_PROPERTY_CAN_ANIMATE_ON_COMPOSITOR)) {
-      continue;
-    }
-
-    if (aPropertiesToSkip.HasProperty(property.mProperty)) {
-      continue;
-    }
-
+  for (const AnimationProperty& property : aProperties) {
     for (const AnimationPropertySegment& segment : property.mSegments) {
       if (segment.mFromComposite == dom::CompositeOperation::Replace &&
           segment.mToComposite == dom::CompositeOperation::Replace) {
         continue;
       }
 
-      if (!styleContext) {
-        styleContext = GetTargetStyleContextWithoutAnimation();
-      }
-      MOZ_RELEASE_ASSERT(styleContext);
-
-      StyleAnimationValue result =
-        ResolveBaseStyle(property.mProperty, styleContext);
+      Unused << ResolveBaseStyle(property.mProperty, aStyleContext);
       break;
     }
   }
 }
 
 void
 KeyframeEffectReadOnly::ComposeStyle(
   RefPtr<AnimValuesStyleRule>& aStyleRule,
@@ -489,26 +477,16 @@ KeyframeEffectReadOnly::ComposeStyle(
 
   ComputedTiming computedTiming = GetComputedTiming();
   mProgressOnLastCompose = computedTiming.mProgress;
   mCurrentIterationOnLastCompose = computedTiming.mCurrentIteration;
 
   // If the progress is null, we don't have fill data for the current
   // time so we shouldn't animate.
   if (computedTiming.mProgress.IsNull()) {
-    // If we are not in-effect, this effect might still be sent to the
-    // compositor and later become in-effect (e.g. if it is in the delay phase,
-    // or, if it is in the end delay phase but with a negative playback rate).
-    // In that case, we might need the base style in order to perform
-    // additive/accumulative animation on the compositor.
-
-    // Note, however, that we don't actually send animations with a negative
-    // playback rate in their end delay phase to the compositor at this stage
-    // (bug 1330498).
-    EnsureBaseStylesForCompositor(aPropertiesToSkip);
     return;
   }
 
   for (size_t propIdx = 0, propEnd = mProperties.Length();
        propIdx != propEnd; ++propIdx)
   {
     const AnimationProperty& prop = mProperties[propIdx];
 
@@ -606,24 +584,16 @@ KeyframeEffectReadOnly::ComposeStyle(
                                          valuePosition, val)) {
       aStyleRule->AddValue(prop.mProperty, Move(val));
     } else if (valuePosition < 0.5) {
       aStyleRule->AddValue(prop.mProperty, Move(fromValue));
     } else {
       aStyleRule->AddValue(prop.mProperty, Move(toValue));
     }
   }
-
-  // For properties that can be run on the compositor, we may need to prepare
-  // base styles to send to the compositor even if the current processing
-  // segment for properties does not have either an additive or accumulative
-  // composite mode, and even if the animation is not in-effect. That's because
-  // the animation may later progress to a segment which has an additive or
-  // accumulative composite on the compositor mode.
-  EnsureBaseStylesForCompositor(aPropertiesToSkip);
 }
 
 bool
 KeyframeEffectReadOnly::IsRunningOnCompositor() const
 {
   // We consider animation is running on compositor if there is at least
   // one property running on compositor.
   // Animation.IsRunningOnCompotitor will return more fine grained
--- a/dom/animation/KeyframeEffectReadOnly.h
+++ b/dom/animation/KeyframeEffectReadOnly.h
@@ -445,20 +445,19 @@ protected:
     const StyleAnimationValue& aValueToComposite,
     CompositeOperation aCompositeOperation);
 
   // Returns underlying style animation value for |aProperty|.
   StyleAnimationValue GetUnderlyingStyle(
     nsCSSPropertyID aProperty,
     const RefPtr<AnimValuesStyleRule>& aAnimationRule);
 
-  // Ensure the base styles is available for any properties that can be run on
-  // the compositor and which are not includes in |aPropertiesToSkip|.
-  void EnsureBaseStylesForCompositor(
-    const nsCSSPropertyIDSet& aPropertiesToSkip);
+  // Ensure the base styles is available for any properties in |aProperties|.
+  void EnsureBaseStyles(nsStyleContext* aStyleContext,
+                        const nsTArray<AnimationProperty>& aProperties);
 
   // Returns the base style resolved by |aStyleContext| for |aProperty|.
   StyleAnimationValue ResolveBaseStyle(nsCSSPropertyID aProperty,
                                        nsStyleContext* aStyleContext);
 
   Maybe<OwningAnimationTarget> mTarget;
 
   KeyframeEffectParams mEffectOptions;