Bug 1268385 - Clear isRunningOnCompositor for script animations if associated nsIFrame is destroyed. r=birtles
authorHiroyuki Ikezoe <hiikezoe@mozilla-japan.org>
Mon, 16 May 2016 16:25:46 +0900
changeset 297509 0041bc955a8d3f68c99c0de24e91f93d004022d4
parent 297508 53919da3aec7d7e215fa58bbae5899d3855bead7
child 297510 01f780f3be075944a158d55b7fa1cb3b414859a2
push id76768
push userhiikezoe@mozilla-japan.org
push dateMon, 16 May 2016 07:37:47 +0000
treeherdermozilla-inbound@0041bc955a8d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles
bugs1268385
milestone49.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 1268385 - Clear isRunningOnCompositor for script animations if associated nsIFrame is destroyed. r=birtles MozReview-Commit-ID: 3tTkDtxkHcT
dom/animation/KeyframeEffect.cpp
dom/animation/KeyframeEffect.h
dom/animation/test/chrome/test_restyles.html
layout/base/RestyleManager.cpp
layout/generic/nsFrame.cpp
--- a/dom/animation/KeyframeEffect.cpp
+++ b/dom/animation/KeyframeEffect.cpp
@@ -685,16 +685,24 @@ KeyframeEffectReadOnly::SetIsRunningOnCo
       if (aIsRunning) {
         property.mPerformanceWarning.reset();
       }
       return;
     }
   }
 }
 
+void
+KeyframeEffectReadOnly::ResetIsRunningOnCompositor()
+{
+  for (AnimationProperty& property : mProperties) {
+    property.mIsRunningOnCompositor = false;
+  }
+}
+
 KeyframeEffectReadOnly::~KeyframeEffectReadOnly()
 {
 }
 
 static Maybe<OwningAnimationTarget>
 ConvertTarget(const Nullable<ElementOrCSSPseudoElement>& aTarget)
 {
   // Return value optimization.
@@ -746,24 +754,16 @@ KeyframeEffectReadOnly::ConstructKeyfram
   if (aRv.Failed()) {
     return nullptr;
   }
 
   return effect.forget();
 }
 
 void
-KeyframeEffectReadOnly::ResetIsRunningOnCompositor()
-{
-  for (AnimationProperty& property : mProperties) {
-    property.mIsRunningOnCompositor = false;
-  }
-}
-
-void
 KeyframeEffectReadOnly::ResetWinsInCascade()
 {
   for (AnimationProperty& property : mProperties) {
     property.mWinsInCascade = false;
   }
 }
 
 void
--- a/dom/animation/KeyframeEffect.h
+++ b/dom/animation/KeyframeEffect.h
@@ -307,16 +307,17 @@ public:
   // AnimationEffect for the current time except any properties already
   // contained in |aSetProperties|.
   // Any updated properties are added to |aSetProperties|.
   void ComposeStyle(RefPtr<AnimValuesStyleRule>& aStyleRule,
                     nsCSSPropertySet& aSetProperties);
   // Returns true if at least one property is being animated on compositor.
   bool IsRunningOnCompositor() const;
   void SetIsRunningOnCompositor(nsCSSProperty aProperty, bool aIsRunning);
+  void ResetIsRunningOnCompositor();
 
   // Returns true if this effect, applied to |aFrame|, contains properties
   // that mean we shouldn't run transform compositor animations on this element.
   //
   // For example, if we have an animation of geometric properties like 'left'
   // and 'top' on an element, we force all 'transform' animations running at
   // the same time on the same element to run on the main thread.
   //
@@ -347,17 +348,16 @@ protected:
   template<class KeyframeEffectType, class OptionsType>
   static already_AddRefed<KeyframeEffectType>
   ConstructKeyframeEffect(const GlobalObject& aGlobal,
                           const Nullable<ElementOrCSSPseudoElement>& aTarget,
                           JS::Handle<JSObject*> aKeyframes,
                           const OptionsType& aOptions,
                           ErrorResult& aRv);
 
-  void ResetIsRunningOnCompositor();
   void ResetWinsInCascade();
 
   // This effect is registered with its target element so long as:
   //
   // (a) It has a target element, and
   // (b) It is "relevant" (i.e. yet to finish but not idle, or finished but
   //     filling forwards)
   //
--- a/dom/animation/test/chrome/test_restyles.html
+++ b/dom/animation/test/chrome/test_restyles.html
@@ -416,19 +416,19 @@ waitForAllPaints(function() {
 
     // We need to wait a frame to apply display:none style.
     yield waitForFrame();
 
     is(animation.playState, 'running',
        'Opacity script animations keep running even when the target element ' +
        'has "display: none" style');
 
-    todo(!animation.isRunningOnCompositor,
-         'Opacity script animations on "display:none" element should not ' +
-         'run on the compositor');
+    ok(!animation.isRunningOnCompositor,
+       'Opacity script animations on "display:none" element should not ' +
+       'run on the compositor');
 
     var markers = yield observeStyling(5);
     is(markers.length, 0,
        'Opacity script animations on "display: none" element should not ' +
        'update styles');
 
     div.style.display = '';
 
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -1150,16 +1150,25 @@ RestyleManager::AnimationsWithDestroyedF
   for (nsIContent* content : aArray) {
     if (content->GetPrimaryFrame()) {
       continue;
     }
     dom::Element* element = content->AsElement();
 
     animationManager->StopAnimationsForElement(element, aPseudoType);
     transitionManager->StopTransitionsForElement(element, aPseudoType);
+
+    // All other animations should keep running but not running on the
+    // *compositor* at this point.
+    EffectSet* effectSet = EffectSet::GetEffectSet(element, aPseudoType);
+    if (effectSet) {
+      for (KeyframeEffectReadOnly* effect : *effectSet) {
+        effect->ResetIsRunningOnCompositor();
+      }
+    }
   }
 }
 
 static inline dom::Element*
 ElementForStyleContext(nsIContent* aParentContent,
                        nsIFrame* aFrame,
                        CSSPseudoElementType aPseudoType);
 
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -87,16 +87,17 @@
 #include "StickyScrollContainer.h"
 #include "nsFontInflationData.h"
 #include "gfxASurface.h"
 #include "nsRegion.h"
 #include "nsIFrameInlines.h"
 
 #include "mozilla/AsyncEventDispatcher.h"
 #include "mozilla/EffectCompositor.h"
+#include "mozilla/EffectSet.h"
 #include "mozilla/EventListenerManager.h"
 #include "mozilla/EventStateManager.h"
 #include "mozilla/EventStates.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/LookAndFeel.h"
 #include "mozilla/MouseEvents.h"
 #include "mozilla/css/ImageLoader.h"
 #include "mozilla/gfx/Tools.h"
@@ -695,17 +696,17 @@ nsFrame::DestroyFrom(nsIFrame* aDestruct
       RestyleManager::ReframingStyleContexts* rsc =
         presContext->RestyleManager()->AsGecko()->GetReframingStyleContexts();
       if (rsc) {
         rsc->Put(mContent, mStyleContext);
       }
     }
   }
 
-  if (HasCSSAnimations() || HasCSSTransitions()) {
+  if (EffectSet::GetEffectSet(this)) {
     // If no new frame for this element is created by the end of the
     // restyling process, stop animations and transitions for this frame
     if (presContext->RestyleManager()->IsGecko()) {
       RestyleManager::AnimationsWithDestroyedFrame* adf =
         presContext->RestyleManager()->AsGecko()->GetAnimationsWithDestroyedFrame();
       // AnimationsWithDestroyedFrame only lives during the restyling process.
       if (adf) {
         adf->Put(mContent, mStyleContext);