Bug 1334036 - Part 8: Need to update cascade result after change the style. draft
authorBoris Chiou <boris.chiou@gmail.com>
Sat, 18 Feb 2017 20:31:25 +0800
changeset 486981 e5c057002cca946db349e4e8d430bbb11f57d294
parent 486980 60db9b299c06923183b4f4ec8e3b1d1dc66af4f3
child 486982 0016ffab4e24d4fa59b65dd5e989b2818a67aafa
push id46109
push userbmo:boris.chiou@gmail.com
push dateMon, 20 Feb 2017 11:01:51 +0000
bugs1334036, 1285407, 1260983
Bug 1334036 - Part 8: Need to update cascade result after change the style. This patch is to fix the problem mentioned in Bug 1285407 (Animations which can be run on the compositor don't start when "!important" style is removed while it's in effect.). This is still not perfect because we may have many redundant calls of EffectSet::MarkCascadeNeedsUpdate(), which causes many redundant calls of EffectCompositor::UpdateCascadeResults(). Besides, we don't update effect properties after change the style, so still cannot pass many test cases. (e.g. Bug 1260983 - Update animation properties in response to style changes) MozReview-Commit-ID: IYTyqdgVfuX
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -246,16 +246,22 @@ ServoRestyleManager::RecreateStyleContex
     // XXX This could not always work as expected: there are kinds of content
     // with the first split and the last sharing style, but others not. We
     // should handle those properly.
     for (nsIFrame* f = primaryFrame; f;
          f = GetNextContinuationWithSameStyle(f, oldStyleContext)) {
+    // Style context change might need to update CSS cascade level, so we might
+    // need to update cascade result for animations.
+    EffectCompositor::MaybeUpdateCascadeResults(aElement,
+                                                CSSPseudoElementType::NotPseudo,
+                                                newContext);
     // Some changes to animations don't affect the computed style and yet still
     // require the layer to be updated. For example, pausing an animation via
     // the Web Animations API won't affect an element's style but still
     // requires us to pull the animation off the layer.
     // Although we only expect this code path to be called when computed style
     // is not changing, we can sometimes reach this at the end of a transition
     // when the animated style is being removed.
@@ -293,16 +299,20 @@ ServoRestyleManager::RecreateStyleContex
           if (n->IsNodeOfType(nsINode::eTEXT)) {
             RefPtr<nsStyleContext> childContext =
               aStyleSet->ResolveStyleForText(n, pseudoContext);
                        "How? This node is created at FC time!");
+        EffectCompositor::MaybeUpdateCascadeResults(aElement, pseudoType,
+                                                    pseudoContext);
         AddLayerChangesForAnimation(pseudoFrame, aElement,
   bool traverseElementChildren = aElement->HasDirtyDescendantsForServo();
   bool traverseTextChildren = recreateContext;
--- a/layout/style/ServoStyleSet.cpp
+++ b/layout/style/ServoStyleSet.cpp
@@ -2,16 +2,17 @@
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 #include "mozilla/ServoStyleSet.h"
 #include "mozilla/DocumentStyleRootIterator.h"
+#include "mozilla/EffectSet.h"
 #include "mozilla/ServoRestyleManager.h"
 #include "mozilla/dom/ChildIterator.h"
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/ElementInlines.h"
 #include "nsAnimationManager.h"
 #include "nsCSSAnonBoxes.h"
 #include "nsCSSPseudoElements.h"
 #include "nsHTMLStyleSheet.h"
@@ -169,16 +170,34 @@ ServoStyleSet::GetContext(already_AddRef
     // changed.
     // FIXME: This isn't right place to update CSS animations. We should do it
     // , presumably, in cascade_node() in servo side and process the initial
     // restyle there too.
     // To do that we need to make updating CSS animations process independent
     // from nsStyleContext. Also we need to make the process thread safe.
+    // Mark cascade needs update, so we can update the !important immediately
+    // after the style is changed.
+    // FIXME: Calling MarkCascadeNeedsUpdate is not enough. Just like what we do
+    // in Gecko, we should call UpdateEffectProperties directly, which also
+    // updates the animation properties if there is any style changed (i.e.
+    // non-animation change).
+    // However, GetContext() is called not only by style changed, but also by
+    // restyling of animation tick, and we don't want to update properties for
+    // animation tick. (Actually, neither updating properties nor marking
+    // CascadeNeedsUpdated is needed in this case.) We need something like
+    // eDoAnimation in nsStyleSet to know this GetContext needs to update
+    // animation properties.
+    EffectSet* effectSet =
+      EffectSet::GetEffectSet(aElementForAnimation, aPseudoType);
+    if (effectSet) {
+      effectSet->MarkCascadeNeedsUpdate();
+    }
   return result.forget();