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 486711 930c66eba3847bde797d15d1516c37674e1b65a8
parent 486707 8afd73c66fc8ee5578c38aaece14d12ae1210147
child 486712 8ac822475ce9a9c7a1241ef125130368e0b90575
push id46041
push userbmo:boris.chiou@gmail.com
push dateSun, 19 Feb 2017 11:14:23 +0000
bugs1334036, 1285407, 1260983
milestone54.0a1
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 perfectly correct 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
layout/base/ServoRestyleManager.cpp
layout/style/ServoStyleSet.cpp
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -279,16 +279,21 @@ ServoRestyleManager::RecreateStyleContex
               aStyleSet->ResolveStyleForText(n, pseudoContext);
             MOZ_ASSERT(n->GetPrimaryFrame(),
                        "How? This node is created at FC time!");
             n->GetPrimaryFrame()->SetStyleContext(childContext);
           }
         }
       }
     }
+    // Style context change might need to update CSS cascade level, so we might
+    // need to update cascade result for animations.
+    EffectCompositor::MaybeUpdateCascadeResults(aElement,
+                                                newContext->GetPseudoType(),
+                                                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
--- 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,33 @@ 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.
     mPresContext->AnimationManager()->UpdateAnimations(result,
                                                        aElementForAnimation);
+
+    // 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.
+    // However, GetContext() is called not only by style changed, but also by
+    // restyling of animations, and we don't want to update properties if the
+    // restyling is caused only by animations. (Actually, neither updating
+    // properties nor marking CascadeNeedsUpdated is needed in this case.)
+    // We need a more elegant way, e.g. adding a flag, to know this
+    // GetContext needs to update animation properties.
+    EffectSet* effectSet =
+      EffectSet::GetEffectSet(aElementForAnimation, aPseudoType);
+    if (effectSet) {
+      effectSet->MarkCascadeNeedsUpdate();
+    }
   }
 
   return result.forget();
 }
 
 void
 ServoStyleSet::ResolveMappedAttrDeclarationBlocks()
 {