Changeset 222104 in webkit

Timestamp:

Sep 15, 2017 12:54:01 PM (7 years ago)

Author:

Ryan Haddad

Message:

Unreviewed, rolling out r222040.

The LayoutTest added with this change is a flaky image failure
on mac-wk1 debug bots.

Reverted changeset:

"Computing animated style should not require renderers"
​https://bugs.webkit.org/show_bug.cgi?id=171926
​http://trac.webkit.org/changeset/222040

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/transitions/transition-display-property-2-expected.html (deleted)
• LayoutTests/transitions/transition-display-property-2.html (deleted)
• LayoutTests/transitions/transition-display-property.html (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/page/animation/CSSAnimationController.cpp (modified) (3 diffs)
• Source/WebCore/page/animation/CSSAnimationController.h (modified) (3 diffs)
• Source/WebCore/page/animation/CompositeAnimation.cpp (modified) (6 diffs)
• Source/WebCore/page/animation/CompositeAnimation.h (modified) (2 diffs)
• Source/WebCore/rendering/RenderElement.cpp (modified) (3 diffs)
• Source/WebCore/rendering/RenderElement.h (modified) (2 diffs)
• Source/WebCore/style/RenderTreeUpdater.cpp (modified) (4 diffs)
• Source/WebCore/style/RenderTreeUpdater.h (modified) (2 diffs)
• Source/WebCore/style/StyleTreeResolver.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2017-09-15  Ryan Haddad  <ryanhaddad@apple.com>
+
+        Unreviewed, rolling out r222040.
+
+        The LayoutTest added with this change is a flaky image failure
+        on mac-wk1 debug bots.
+
+        Reverted changeset:
+
+        "Computing animated style should not require renderers"
+        https://bugs.webkit.org/show_bug.cgi?id=171926
+        http://trac.webkit.org/changeset/222040
+
 2017-09-15  Matt Baker  <mattbaker@apple.com>
 

trunk/LayoutTests/transitions/transition-display-property.html

@@ -6 +6 @@
 <script>
 var test = document.querySelector("test");
+var count = 0;
+function testUntilComplete()
+{
+    if (test.offsetWidth == 10 || ++count == 10)
+        testRunner.notifyDone();
+    else
+        setTimeout(testUntilComplete, 0.1);
+}
 
 if (window.testRunner) {
     testRunner.waitUntilDone();
-    test.ontransitionend = () => testRunner.notifyDone();
+    testUntilComplete();
 }
 test.offsetWidth;

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-09-15  Ryan Haddad  <ryanhaddad@apple.com>
+
+        Unreviewed, rolling out r222040.
+
+        The LayoutTest added with this change is a flaky image failure
+        on mac-wk1 debug bots.
+
+        Reverted changeset:
+
+        "Computing animated style should not require renderers"
+        https://bugs.webkit.org/show_bug.cgi?id=171926
+        http://trac.webkit.org/changeset/222040
+
 2017-09-15  Tim Horton  <timothy_horton@apple.com>
 

trunk/Source/WebCore/page/animation/CSSAnimationController.cpp

@@ -638 +638 @@
 }
 
-AnimationUpdate CSSAnimationController::updateAnimations(Element& element, const RenderStyle& newStyle, const RenderStyle* oldStyle)
-{
-    bool hasOrHadAnimations = (oldStyle && oldStyle->hasAnimationsOrTransitions()) || newStyle.hasAnimationsOrTransitions();
-    if (!hasOrHadAnimations)
-        return { };
+bool CSSAnimationController::updateAnimations(Element& element, const RenderStyle& newStyle, std::unique_ptr<RenderStyle>& animatedStyle)
+{
+    auto* renderer = element.renderer();
+    auto* oldStyle = (renderer && renderer->hasInitializedStyle()) ? &renderer->style() : nullptr;
+    if ((!oldStyle || (!oldStyle->animations() && !oldStyle->transitions())) && (!newStyle.animations() && !newStyle.transitions()))
+        return false;
 
     if (element.document().pageCacheState() != Document::NotInPageCache)
-        return { };
+        return false;
 
     // Don't run transitions when printing.
     if (element.document().renderView()->printing())
-        return { };
+        return false;
 
     // Fetch our current set of implicit animations from a hashtable. We then compare them
@@ -657 +658 @@
 
     CompositeAnimation& compositeAnimation = m_data->ensureCompositeAnimation(element);
-    auto update = compositeAnimation.animate(element, oldStyle, newStyle);
-
-    auto* renderer = element.renderer();
+    bool animationStateChanged = compositeAnimation.animate(element, oldStyle, newStyle, animatedStyle);
+
     if ((renderer && renderer->parent()) || newStyle.animations() || (oldStyle && oldStyle->animations())) {
         auto& frameView = *element.document().view();
@@ -668 +668 @@
     }
 
-    return update;
+    return animationStateChanged;
 }
 

trunk/Source/WebCore/page/animation/CSSAnimationController.h

@@ -31 +31 @@
 #include "AnimationBase.h"
 #include "CSSPropertyNames.h"
-#include "RenderStyle.h"
 #include <wtf/Forward.h>
 
@@ -42 +41 @@
 class LayoutRect;
 class RenderElement;
-
-struct AnimationUpdate {
-#if !COMPILER_SUPPORTS(NSDMI_FOR_AGGREGATES)
-    AnimationUpdate() = default;
-    AnimationUpdate(std::unique_ptr<RenderStyle> style, bool stateChanged)
-        : style(WTFMove(style))
-        , stateChanged(stateChanged)
-    { }
-#endif
-    std::unique_ptr<RenderStyle> style;
-    bool stateChanged { false };
-};
+class RenderStyle;
 
 class CSSAnimationController {
@@ -62 +50 @@
 
     void cancelAnimations(Element&);
-    AnimationUpdate updateAnimations(Element&, const RenderStyle& newStyle, const RenderStyle* oldStyle);
+    bool updateAnimations(Element&, const RenderStyle& newStyle, std::unique_ptr<RenderStyle>& animatedStyle);
     std::unique_ptr<RenderStyle> animatedStyleForRenderer(RenderElement&);
 

trunk/Source/WebCore/page/animation/CompositeAnimation.cpp

@@ -288 +288 @@
 }
 
-AnimationUpdate CompositeAnimation::animate(Element& element, const RenderStyle* currentStyle, const RenderStyle& targetStyle)
+bool CompositeAnimation::animate(Element& element, const RenderStyle* currentStyle, const RenderStyle& targetStyle, std::unique_ptr<RenderStyle>& blendedStyle)
 {
     // We don't do any transitions if we don't have a currentStyle (on startup).
@@ -298 +298 @@
     bool forceStackingContext = false;
 
-    std::unique_ptr<RenderStyle> animatedStyle;
-
     if (currentStyle) {
         // Now that we have transition objects ready, let them know about the new goal state.  We want them
@@ -306 +304 @@
         for (auto& transition : m_transitions.values()) {
             bool didBlendStyle = false;
-            if (transition->animate(*this, targetStyle, animatedStyle, didBlendStyle))
+            if (transition->animate(*this, targetStyle, blendedStyle, didBlendStyle))
                 animationStateChanged = true;
 
@@ -313 +311 @@
         }
 
-        if (animatedStyle && checkForStackingContext) {
+        if (blendedStyle && checkForStackingContext) {
             // Note that this is similar to code in StyleResolver::adjustRenderStyle() but only needs to consult
             // animatable properties that can trigger stacking context.
-            if (animatedStyle->opacity() < 1.0f
-                || animatedStyle->hasTransformRelatedProperty()
-                || animatedStyle->hasMask()
-                || animatedStyle->clipPath()
-                || animatedStyle->boxReflect()
-                || animatedStyle->hasFilter()
+            if (blendedStyle->opacity() < 1.0f
+                || blendedStyle->hasTransformRelatedProperty()
+                || blendedStyle->hasMask()
+                || blendedStyle->clipPath()
+                || blendedStyle->boxReflect()
+                || blendedStyle->hasFilter()
 #if ENABLE(FILTERS_LEVEL_2)
-                || animatedStyle->hasBackdropFilter()
+                || blendedStyle->hasBackdropFilter()
 #endif
                 )
@@ -336 +334 @@
         if (keyframeAnim) {
             bool didBlendStyle = false;
-            if (keyframeAnim->animate(*this, targetStyle, animatedStyle, didBlendStyle))
+            if (keyframeAnim->animate(*this, targetStyle, blendedStyle, didBlendStyle))
                 animationStateChanged = true;
 
@@ -348 +346 @@
     // the user agent must act as if the will-change property ([css-will-change-1]) on the element additionally
     // includes all the properties animated by the animation.
-    if (forceStackingContext && animatedStyle) {
-        if (animatedStyle->hasAutoZIndex())
-            animatedStyle->setZIndex(0);
-    }
-
-    return { WTFMove(animatedStyle), animationStateChanged };
+    if (forceStackingContext && blendedStyle) {
+        if (blendedStyle->hasAutoZIndex())
+            blendedStyle->setZIndex(0);
+    }
+
+    return animationStateChanged;
 }
 

trunk/Source/WebCore/page/animation/CompositeAnimation.h

@@ -29 +29 @@
 #pragma once
 
-#include "CSSAnimationController.h"
 #include "ImplicitAnimation.h"
 #include "KeyframeAnimation.h"
@@ -56 +55 @@
     void clearElement();
 
-    AnimationUpdate animate(Element&, const RenderStyle* currentStyle, const RenderStyle& targetStyle);
+    bool animate(Element&, const RenderStyle* currentStyle, const RenderStyle& targetStyle, std::unique_ptr<RenderStyle>& blendedStyle);
     std::unique_ptr<RenderStyle> getAnimatedStyle() const;
     bool computeExtentOfTransformAnimation(LayoutRect&) const;

trunk/Source/WebCore/rendering/RenderElement.cpp

@@ -27 +27 @@
 
 #include "AXObjectCache.h"
+#include "CSSAnimationController.h"
 #include "ContentData.h"
 #include "CursorList.h"
@@ -102 +103 @@
     , m_ancestorLineBoxDirty(false)
     , m_hasInitializedStyle(false)
+    , m_hasInitialAnimatedStyle(false)
     , m_renderInlineAlwaysCreatesLineBoxes(false)
     , m_renderBoxNeedsLazyRepaint(false)
@@ -1103 +1105 @@
         view().frameView().removeSlowRepaintObject(this);
 
+    if (element())
+        animation().cancelAnimations(*element());
+
     destroyLeftoverChildren();
 

trunk/Source/WebCore/rendering/RenderElement.h

@@ -132 +132 @@
     void setStyleInternal(RenderStyle&& style) { m_style = WTFMove(style); }
 
+    bool hasInitialAnimatedStyle() const { return m_hasInitialAnimatedStyle; }
+    void setHasInitialAnimatedStyle(bool b) { m_hasInitialAnimatedStyle = b; }
+
     // Repaint only if our old bounds and new bounds are different. The caller may pass in newBounds and newOutlineBox if they are known.
     bool repaintAfterLayoutIfNeeded(const RenderLayerModelObject* repaintContainer, const LayoutRect& oldBounds, const LayoutRect& oldOutlineBox, const LayoutRect* newBoundsPtr = nullptr, const LayoutRect* newOutlineBoxPtr = nullptr);
@@ -327 +330 @@
     unsigned m_ancestorLineBoxDirty : 1;
     unsigned m_hasInitializedStyle : 1;
+    unsigned m_hasInitialAnimatedStyle : 1;
 
     unsigned m_renderInlineAlwaysCreatesLineBoxes : 1;

trunk/Source/WebCore/style/RenderTreeUpdater.cpp

@@ -308 +308 @@
             renderTreePosition().invalidateNextSibling();
         }
-
-        // display:none cancels animations.
-        auto teardownType = update.style->display() == NONE ? TeardownType::RendererUpdateCancelingAnimations : TeardownType::RendererUpdate;
-        tearDownRenderers(element, teardownType);
+        tearDownRenderers(element, TeardownType::KeepHoverAndActive);
     }
 
@@ -399 +396 @@
 
     element.setRenderer(newRenderer);
+
+    auto& initialStyle = newRenderer->style();
+    std::unique_ptr<RenderStyle> animatedStyle;
+    newRenderer->animation().updateAnimations(element, initialStyle, animatedStyle);
+    if (animatedStyle) {
+        newRenderer->setStyleInternal(WTFMove(*animatedStyle));
+        newRenderer->setHasInitialAnimatedStyle(true);
+    }
 
     newRenderer->initializeStyle();
@@ -521 +526 @@
 }
 
-void RenderTreeUpdater::tearDownRenderers(Element& root)
-{
-    tearDownRenderers(root, TeardownType::Full);
-}
-
 void RenderTreeUpdater::tearDownRenderers(Element& root, TeardownType teardownType)
 {
@@ -538 +538 @@
     };
 
-    auto& animationController = root.document().frame()->animation();
-
     auto pop = [&] (unsigned depth) {
         while (teardownStack.size() > depth) {
             auto& element = *teardownStack.takeLast();
 
-            if (teardownType == TeardownType::Full || teardownType == TeardownType::RendererUpdateCancelingAnimations)
-                animationController.cancelAnimations(element);
-
-            if (teardownType == TeardownType::Full)
+            if (teardownType != TeardownType::KeepHoverAndActive)
                 element.clearHoverAndActiveStatusBeforeDetachingRenderer();
-
             element.clearStyleDerivedDataBeforeDetachingRenderer();
 

trunk/Source/WebCore/style/RenderTreeUpdater.h

@@ -48 +48 @@
     void commit(std::unique_ptr<const Style::Update>);
 
-    static void tearDownRenderers(Element&);
+    enum class TeardownType { Normal, KeepHoverAndActive };
+    static void tearDownRenderers(Element&, TeardownType = TeardownType::Normal);
     static void tearDownRenderer(Text&);
 
@@ -83 +84 @@
     void popParentsToDepth(unsigned depth);
 
-    enum class TeardownType { Full, RendererUpdate, RendererUpdateCancelingAnimations };
-    static void tearDownRenderers(Element&, TeardownType);
-
     RenderView& renderView();
 

trunk/Source/WebCore/style/StyleTreeResolver.cpp

@@ -242 +242 @@
 ElementUpdate TreeResolver::createAnimatedElementUpdate(std::unique_ptr<RenderStyle> newStyle, Element& element, Change parentChange)
 {
-    auto& animationController = element.document().frame()->animation();
-
-    auto* oldStyle = renderOrDisplayContentsStyle(element);
-    auto animationUpdate = animationController.updateAnimations(element, *newStyle, oldStyle);
-
-    if (animationUpdate.style)
-        newStyle = WTFMove(animationUpdate.style);
-
-    auto change = oldStyle ? determineChange(*oldStyle, *newStyle) : Detach;
-
     auto validity = element.styleValidity();
-    if (validity >= Validity::SubtreeInvalid)
-        change = std::max(change, validity == Validity::SubtreeAndRenderersInvalid ? Detach : Force);
-    if (parentChange >= Force)
-        change = std::max(change, parentChange);
-
-    bool shouldRecompositeLayer = element.styleResolutionShouldRecompositeLayer() || animationUpdate.stateChanged;
-
-    return { WTFMove(newStyle), change, shouldRecompositeLayer };
+    bool recompositeLayer = element.styleResolutionShouldRecompositeLayer();
+
+    auto makeUpdate = [&] (std::unique_ptr<RenderStyle> style, Change change) {
+        if (validity >= Validity::SubtreeInvalid)
+            change = std::max(change, validity == Validity::SubtreeAndRenderersInvalid ? Detach : Force);
+        if (parentChange >= Force)
+            change = std::max(change, parentChange);
+        return ElementUpdate { WTFMove(style), change, recompositeLayer };
+    };
+
+    auto* renderer = element.renderer();
+
+    bool shouldReconstruct = validity >= Validity::SubtreeAndRenderersInvalid || parentChange == Detach;
+    if (shouldReconstruct)
+        return makeUpdate(WTFMove(newStyle), Detach);
+
+    if (!renderer) {
+        auto change = Detach;
+        if (auto* oldStyle = renderOrDisplayContentsStyle(element))
+            change = determineChange(*oldStyle, *newStyle);
+        return makeUpdate(WTFMove(newStyle), change);
+    }
+
+    std::unique_ptr<RenderStyle> animatedStyle;
+    if (element.document().frame()->animation().updateAnimations(element, *newStyle, animatedStyle))
+        recompositeLayer = true;
+
+    if (animatedStyle) {
+        auto change = determineChange(renderer->style(), *animatedStyle);
+        if (renderer->hasInitialAnimatedStyle()) {
+            renderer->setHasInitialAnimatedStyle(false);
+            // When we initialize a newly created renderer with initial animated style we don't inherit it to descendants.
+            // The first animation frame needs to correct this.
+            // FIXME: We should compute animated style correctly during initial style resolution when we don't have renderers yet.
+            //        https://bugs.webkit.org/show_bug.cgi?id=171926
+            change = std::max(change, Inherit);
+        }
+        // If animation forces render tree reconstruction pass the original style. The animation will be applied on renderer construction.
+        // FIXME: We should always use the animated style here.
+        auto style = change == Detach ? WTFMove(newStyle) : WTFMove(animatedStyle);
+        return makeUpdate(WTFMove(style), change);
+    }
+
+    auto change = determineChange(renderer->style(), *newStyle);
+    return makeUpdate(WTFMove(newStyle), change);
 }