Bug 1368889 - Post animation restyle hint againt pseudo element instead of its parent. r?birtles draft
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Thu, 08 Jun 2017 10:22:25 +0900
changeset 590732 8df138eb38f05c2090f567d26762c56f4ba32107
parent 590317 a49112c7a5765802096b3fc298069b9495436107
child 632289 6925d72c1df8a224f1c54dbc973c602b93f41d4d
push id62810
push userhikezoe@mozilla.com
push dateThu, 08 Jun 2017 01:22:53 +0000
reviewersbirtles
bugs1368889
milestone55.0a1
Bug 1368889 - Post animation restyle hint againt pseudo element instead of its parent. r?birtles To traverse pseudo elements in animation-only restyle, the pseudo element itself needs the animation-only dirty bit. MozReview-Commit-ID: 11RfVqnPXfJ
dom/animation/EffectCompositor.cpp
dom/animation/EffectCompositor.h
dom/smil/nsSMILAnimationController.cpp
layout/base/ServoRestyleManager.cpp
layout/base/ServoRestyleManager.h
layout/style/test/stylo-failures.md
--- a/dom/animation/EffectCompositor.cpp
+++ b/dom/animation/EffectCompositor.cpp
@@ -1037,16 +1037,17 @@ EffectCompositor::PreTraverseInSubtree(E
       }
 
       // We need to post restyle hints even if the target is not in EffectSet to
       // ensure the final restyling for removed animations.
       // We can't call PostRestyleEvent directly here since we are still in the
       // middle of the servo traversal.
       mPresContext->RestyleManager()->AsServo()->
         PostRestyleEventForAnimations(target.mElement,
+                                      target.mPseudoType,
                                       cascadeLevel == CascadeLevel::Transitions
                                         ? eRestyle_CSSTransitions
                                         : eRestyle_CSSAnimations);
 
       foundElementsNeedingRestyle = true;
 
       EffectSet* effects = EffectSet::GetEffectSet(target.mElement,
                                                    target.mPseudoType);
@@ -1103,16 +1104,17 @@ EffectCompositor::PreTraverse(dom::Eleme
     bool hasUnthrottledRestyle = false;
     if (!elementSet.Get(key, &hasUnthrottledRestyle) ||
         (!flushThrottledRestyles && !hasUnthrottledRestyle)) {
       continue;
     }
 
     mPresContext->RestyleManager()->AsServo()->
       PostRestyleEventForAnimations(aElement,
+                                    aPseudoType,
                                     cascadeLevel == CascadeLevel::Transitions
                                       ? eRestyle_CSSTransitions
                                       : eRestyle_CSSAnimations);
 
     EffectSet* effects = EffectSet::GetEffectSet(aElement, aPseudoType);
     if (effects) {
       MaybeUpdateCascadeResults(StyleBackendType::Servo,
                                 aElement, aPseudoType,
--- a/dom/animation/EffectCompositor.h
+++ b/dom/animation/EffectCompositor.h
@@ -244,29 +244,34 @@ public:
   // Similar to the above but only for the (pseudo-)element.
   bool PreTraverse(dom::Element* aElement, CSSPseudoElementType aPseudoType);
 
   // Similar to the above but for all elements in the subtree rooted
   // at aElement.
   bool PreTraverseInSubtree(dom::Element* aElement,
                             AnimationRestyleType aRestyleType);
 
+  // Returns the target element for restyling.
+  //
+  // If |aPseudoType| is ::after or ::before, returns the generated content
+  // element of which |aElement| is the parent. If |aPseudoType| is any other
+  // pseudo type (other thant CSSPseudoElementType::NotPseudo) returns nullptr.
+  // Otherwise, returns |aElement|.
+  static dom::Element* GetElementToRestyle(dom::Element* aElement,
+                                           CSSPseudoElementType aPseudoType);
+
 private:
   ~EffectCompositor() = default;
 
   // Rebuilds the animation rule corresponding to |aCascadeLevel| on the
   // EffectSet associated with the specified (pseudo-)element.
   static void ComposeAnimationRule(dom::Element* aElement,
                                    CSSPseudoElementType aPseudoType,
                                    CascadeLevel aCascadeLevel);
 
-  static dom::Element* GetElementToRestyle(dom::Element* aElement,
-                                           CSSPseudoElementType
-                                             aPseudoType);
-
   // Get the properties in |aEffectSet| that we are able to animate on the
   // compositor but which are also specified at a higher level in the cascade
   // than the animations level.
   //
   // When |aBackendType| is StyleBackendType::Gecko, we determine which
   // properties are specified using the provided |aStyleContext| and
   // |aElement| and |aPseudoType| are ignored. If |aStyleContext| is nullptr,
   // we automatically look up the style context of primary frame of the
--- a/dom/smil/nsSMILAnimationController.cpp
+++ b/dom/smil/nsSMILAnimationController.cpp
@@ -762,16 +762,17 @@ nsSMILAnimationController::PreTraverseIn
     if (aRoot &&
         !nsContentUtils::ContentIsFlattenedTreeDescendantOf(key.mElement,
                                                             aRoot)) {
       continue;
     }
 
     context->RestyleManager()->AsServo()->
       PostRestyleEventForAnimations(key.mElement,
+                                    CSSPseudoElementType::NotPseudo,
                                     eRestyle_StyleAttribute_Animations);
 
     foundElementsNeedingRestyle = true;
   }
 
   // Only clear the mMightHavePendingStyleUpdates flag if we definitely posted
   // all restyles.
   if (!aRoot) {
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -69,20 +69,32 @@ ServoRestyleManager::PostRestyleEvent(El
 void
 ServoRestyleManager::PostRestyleEventForCSSRuleChanges()
 {
   mRestyleForCSSRuleChanges = true;
   mPresContext->PresShell()->EnsureStyleFlush();
 }
 
 /* static */ void
-ServoRestyleManager::PostRestyleEventForAnimations(Element* aElement,
-                                                   nsRestyleHint aRestyleHint)
+ServoRestyleManager::PostRestyleEventForAnimations(
+  Element* aElement,
+  CSSPseudoElementType aPseudoType,
+  nsRestyleHint aRestyleHint)
 {
-  Servo_NoteExplicitHints(aElement, aRestyleHint, nsChangeHint(0));
+  Element* elementToRestyle =
+    EffectCompositor::GetElementToRestyle(aElement, aPseudoType);
+
+  if (!elementToRestyle) {
+    // FIXME: Bug 1371107: When reframing happens,
+    // EffectCompositor::mElementsToRestyle still has unbinded old pseudo
+    // element. We should drop it.
+    return;
+  }
+
+  Servo_NoteExplicitHints(elementToRestyle, aRestyleHint, nsChangeHint(0));
 }
 
 void
 ServoRestyleManager::RebuildAllStyleData(nsChangeHint aExtraHint,
                                          nsRestyleHint aRestyleHint)
 {
    // NOTE(emilio): GeckoRestlyeManager does a sync style flush, which seems not
    // to be needed in my testing.
--- a/layout/base/ServoRestyleManager.h
+++ b/layout/base/ServoRestyleManager.h
@@ -102,16 +102,17 @@ public:
    * This is only called for the second traversal for CSS animations during
    * updating CSS animations in a SequentialTask.
    * This function does neither register a refresh observer nor flag that a
    * style flush is needed since this function is supposed to be called during
    * restyling process and this restyle event will be processed in the second
    * traversal of the same restyling process.
    */
   static void PostRestyleEventForAnimations(dom::Element* aElement,
+                                            CSSPseudoElementType aPseudoType,
                                             nsRestyleHint aRestyleHint);
 protected:
   ~ServoRestyleManager() override
   {
     MOZ_ASSERT(!mReentrantChanges);
   }
 
 private:
--- a/layout/style/test/stylo-failures.md
+++ b/layout/style/test/stylo-failures.md
@@ -37,17 +37,16 @@ to mochitest command.
   * test_media_queries_dynamic_xbl.html: xbl support bug 1290276 [2]
 * Animation support:
   * SMIL Animation
     * test_restyles_in_smil_animation.html [2]
 * console support bug 1352669
   * test_bug413958.html `monitorConsole` [3]
   * test_parser_diagnostics_unprintables.html [550]
 * Transition support:
-  * test_transitions.html: pseudo elements [4]
   * test_transitions_and_reframes.html `pseudo-element`: bug 1366422 [4]
   * Events:
     * test_animations_event_order.html [2]
 * Unimplemented \@font-face descriptors:
   * test_font_face_parser.html `font-language-override`: bug 1355364 [8]
 * keyword values should be preserved in \@font-face bug 1355368
   * test_font_face_parser.html `font-weight` [4]
   * test_font_loading_api.html `weight` [1]