Diff
Modified: trunk/LayoutTests/ChangeLog (222128 => 222129)
--- trunk/LayoutTests/ChangeLog 2017-09-16 09:27:22 UTC (rev 222128)
+++ trunk/LayoutTests/ChangeLog 2017-09-16 12:33:04 UTC (rev 222129)
@@ -1,3 +1,15 @@
+2017-09-16 Antti Koivisto <[email protected]>
+
+ Computing animated style should not require renderers
+ https://bugs.webkit.org/show_bug.cgi?id=171926
+ <rdar://problem/34428035>
+
+ Reviewed by Sam Weinig.
+
+ * transitions/transition-display-property-2-expected.html: Added.
+ * transitions/transition-display-property-2.html: Added.
+ * transitions/transition-display-property.html:
+
2017-09-16 Carlos Garcia Campos <[email protected]>
Unreviewed GTK+ gardening. Rebaseline tests after r222090.
Added: trunk/LayoutTests/transitions/transition-display-property-2-expected.html (0 => 222129)
--- trunk/LayoutTests/transitions/transition-display-property-2-expected.html (rev 0)
+++ trunk/LayoutTests/transitions/transition-display-property-2-expected.html 2017-09-16 12:33:04 UTC (rev 222129)
@@ -0,0 +1,4 @@
+<style>
+test { display:inline-block; height:55px; width:10px; border: 2px solid green; }
+</style>
+<test></test>
Added: trunk/LayoutTests/transitions/transition-display-property-2.html (0 => 222129)
--- trunk/LayoutTests/transitions/transition-display-property-2.html (rev 0)
+++ trunk/LayoutTests/transitions/transition-display-property-2.html 2017-09-16 12:33:04 UTC (rev 222129)
@@ -0,0 +1,23 @@
+<style>
+test {
+ transition: 5s;
+ display: block;
+ height: 100px;
+ border: 2px solid green;
+ transition-timing-function: steps(2, start);
+}
+.animate { display:inline-block; height:10px; width:10px; }
+</style>
+<test></test>
+<script>
+const test = document.querySelector("test");
+test.offsetWidth;
+
+if (window.testRunner) {
+ testRunner.waitUntilDone();
+ requestAnimationFrame(() => {
+ testRunner.notifyDone();
+ });
+}
+test.className = "animate";
+</script>
Modified: trunk/LayoutTests/transitions/transition-display-property.html (222128 => 222129)
--- trunk/LayoutTests/transitions/transition-display-property.html 2017-09-16 09:27:22 UTC (rev 222128)
+++ trunk/LayoutTests/transitions/transition-display-property.html 2017-09-16 12:33:04 UTC (rev 222129)
@@ -5,18 +5,10 @@
<test></test>
<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();
- testUntilComplete();
+ test._ontransitionend_ = () => testRunner.notifyDone();
}
test.offsetWidth;
test.className = "animate";
Modified: trunk/Source/WebCore/ChangeLog (222128 => 222129)
--- trunk/Source/WebCore/ChangeLog 2017-09-16 09:27:22 UTC (rev 222128)
+++ trunk/Source/WebCore/ChangeLog 2017-09-16 12:33:04 UTC (rev 222129)
@@ -1,3 +1,56 @@
+2017-09-16 Antti Koivisto <[email protected]>
+
+ Computing animated style should not require renderers
+ https://bugs.webkit.org/show_bug.cgi?id=171926
+ <rdar://problem/34428035>
+
+ Reviewed by Sam Weinig.
+
+ CSS animation system is now element rather than renderer based. This allows cleaning up
+ style resolution and render tree update code.
+
+ This also fixes bug animation doesn't run if display property is animated from one rendered type
+ to another. Added a test case for this.
+
+ Test: transitions/transition-display-property-2.html
+
+ * page/animation/CSSAnimationController.cpp:
+ (WebCore::CSSAnimationController::updateAnimations):
+
+ Pass in the old style instead of getting it from the renderer.
+ Factor to return the animated style as a return value.
+
+ * page/animation/CSSAnimationController.h:
+ * rendering/RenderElement.cpp:
+ (WebCore::RenderElement::RenderElement):
+ (WebCore::RenderElement::willBeDestroyed):
+
+ Animation are now canceled by RenderTreeUpdater::tearDownRenderers.
+
+ * rendering/RenderElement.h:
+ (WebCore::RenderElement::hasInitialAnimatedStyle const): Deleted.
+ (WebCore::RenderElement::setHasInitialAnimatedStyle): Deleted.
+
+ We no longer need to this concept.
+
+ * style/RenderTreeUpdater.cpp:
+ (WebCore::RenderTreeUpdater::updateElementRenderer):
+ (WebCore::RenderTreeUpdater::createRenderer):
+
+ We now get correct animated style from style resolution in all cases so we don't need to compute
+ it separately for new renderers.
+
+ (WebCore::RenderTreeUpdater::tearDownRenderers):
+
+ Cancel animations when render tree is fully torn down. Keep them when updating style.
+
+ * style/RenderTreeUpdater.h:
+ * style/StyleTreeResolver.cpp:
+ (WebCore::Style::TreeResolver::createAnimatedElementUpdate):
+
+ We can now compute animated style without renderer. Special cases dealing with rendererless case
+ can be removed.
+
2017-09-15 Carlos Garcia Campos <[email protected]>
[Harbuzz] Test fast/text/international/harfbuzz-runs-with-no-glyph.html is crashing
Modified: trunk/Source/WebCore/page/animation/CSSAnimationController.cpp (222128 => 222129)
--- trunk/Source/WebCore/page/animation/CSSAnimationController.cpp 2017-09-16 09:27:22 UTC (rev 222128)
+++ trunk/Source/WebCore/page/animation/CSSAnimationController.cpp 2017-09-16 12:33:04 UTC (rev 222129)
@@ -637,19 +637,18 @@
element.invalidateStyleAndLayerComposition();
}
-bool CSSAnimationController::updateAnimations(Element& element, const RenderStyle& newStyle, std::unique_ptr<RenderStyle>& animatedStyle)
+AnimationUpdate CSSAnimationController::updateAnimations(Element& element, const RenderStyle& newStyle, const RenderStyle* oldStyle)
{
- auto* renderer = element.renderer();
- auto* oldStyle = (renderer && renderer->hasInitializedStyle()) ? &renderer->style() : nullptr;
- if ((!oldStyle || (!oldStyle->animations() && !oldStyle->transitions())) && (!newStyle.animations() && !newStyle.transitions()))
- return false;
+ bool hasOrHadAnimations = (oldStyle && oldStyle->hasAnimationsOrTransitions()) || newStyle.hasAnimationsOrTransitions();
+ if (!hasOrHadAnimations)
+ return { };
if (element.document().pageCacheState() != Document::NotInPageCache)
- return false;
+ return { };
// Don't run transitions when printing.
if (element.document().renderView()->printing())
- return false;
+ return { };
// Fetch our current set of implicit animations from a hashtable. We then compare them
// against the animations in the style and make sure we're in sync. If destination values
@@ -657,8 +656,9 @@
// a new style.
CompositeAnimation& compositeAnimation = m_data->ensureCompositeAnimation(element);
- bool animationStateChanged = compositeAnimation.animate(element, oldStyle, newStyle, animatedStyle);
+ auto update = compositeAnimation.animate(element, oldStyle, newStyle);
+ auto* renderer = element.renderer();
if ((renderer && renderer->parent()) || newStyle.animations() || (oldStyle && oldStyle->animations())) {
auto& frameView = *element.document().view();
if (compositeAnimation.hasAnimationThatDependsOnLayout())
@@ -667,7 +667,7 @@
frameView.scheduleAnimation();
}
- return animationStateChanged;
+ return update;
}
std::unique_ptr<RenderStyle> CSSAnimationController::animatedStyleForRenderer(RenderElement& renderer)
Modified: trunk/Source/WebCore/page/animation/CSSAnimationController.h (222128 => 222129)
--- trunk/Source/WebCore/page/animation/CSSAnimationController.h 2017-09-16 09:27:22 UTC (rev 222128)
+++ trunk/Source/WebCore/page/animation/CSSAnimationController.h 2017-09-16 12:33:04 UTC (rev 222129)
@@ -30,6 +30,7 @@
#include "AnimationBase.h"
#include "CSSPropertyNames.h"
+#include "RenderStyle.h"
#include <wtf/Forward.h>
namespace WebCore {
@@ -40,8 +41,19 @@
class Frame;
class LayoutRect;
class RenderElement;
-class RenderStyle;
+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 CSSAnimationController {
WTF_MAKE_FAST_ALLOCATED;
public:
@@ -49,7 +61,7 @@
~CSSAnimationController();
void cancelAnimations(Element&);
- bool updateAnimations(Element&, const RenderStyle& newStyle, std::unique_ptr<RenderStyle>& animatedStyle);
+ AnimationUpdate updateAnimations(Element&, const RenderStyle& newStyle, const RenderStyle* oldStyle);
std::unique_ptr<RenderStyle> animatedStyleForRenderer(RenderElement&);
// If possible, compute the visual extent of any transform animation on the given renderer
Modified: trunk/Source/WebCore/page/animation/CompositeAnimation.cpp (222128 => 222129)
--- trunk/Source/WebCore/page/animation/CompositeAnimation.cpp 2017-09-16 09:27:22 UTC (rev 222128)
+++ trunk/Source/WebCore/page/animation/CompositeAnimation.cpp 2017-09-16 12:33:04 UTC (rev 222129)
@@ -287,7 +287,7 @@
std::swap(newAnimations, m_keyframeAnimations);
}
-bool CompositeAnimation::animate(Element& element, const RenderStyle* currentStyle, const RenderStyle& targetStyle, std::unique_ptr<RenderStyle>& blendedStyle)
+AnimationUpdate CompositeAnimation::animate(Element& element, const RenderStyle* currentStyle, const RenderStyle& targetStyle)
{
// We don't do any transitions if we don't have a currentStyle (on startup).
updateTransitions(element, currentStyle, targetStyle);
@@ -297,6 +297,8 @@
bool animationStateChanged = false;
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
// to fill in a RenderStyle*& only if needed.
@@ -303,7 +305,7 @@
bool checkForStackingContext = false;
for (auto& transition : m_transitions.values()) {
bool didBlendStyle = false;
- if (transition->animate(*this, targetStyle, blendedStyle, didBlendStyle))
+ if (transition->animate(*this, targetStyle, animatedStyle, didBlendStyle))
animationStateChanged = true;
if (didBlendStyle)
@@ -310,17 +312,17 @@
checkForStackingContext |= WillChangeData::propertyCreatesStackingContext(transition->animatingProperty());
}
- if (blendedStyle && checkForStackingContext) {
+ if (animatedStyle && checkForStackingContext) {
// Note that this is similar to code in StyleResolver::adjustRenderStyle() but only needs to consult
// animatable properties that can trigger stacking context.
- if (blendedStyle->opacity() < 1.0f
- || blendedStyle->hasTransformRelatedProperty()
- || blendedStyle->hasMask()
- || blendedStyle->clipPath()
- || blendedStyle->boxReflect()
- || blendedStyle->hasFilter()
+ if (animatedStyle->opacity() < 1.0f
+ || animatedStyle->hasTransformRelatedProperty()
+ || animatedStyle->hasMask()
+ || animatedStyle->clipPath()
+ || animatedStyle->boxReflect()
+ || animatedStyle->hasFilter()
#if ENABLE(FILTERS_LEVEL_2)
- || blendedStyle->hasBackdropFilter()
+ || animatedStyle->hasBackdropFilter()
#endif
)
forceStackingContext = true;
@@ -333,7 +335,7 @@
RefPtr<KeyframeAnimation> keyframeAnim = m_keyframeAnimations.get(name);
if (keyframeAnim) {
bool didBlendStyle = false;
- if (keyframeAnim->animate(*this, targetStyle, blendedStyle, didBlendStyle))
+ if (keyframeAnim->animate(*this, targetStyle, animatedStyle, didBlendStyle))
animationStateChanged = true;
forceStackingContext |= didBlendStyle && keyframeAnim->triggersStackingContext();
@@ -345,12 +347,12 @@
// While an animation is applied but has not finished, or has finished but has an animation-fill-mode of forwards or both,
// 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 && blendedStyle) {
- if (blendedStyle->hasAutoZIndex())
- blendedStyle->setZIndex(0);
+ if (forceStackingContext && animatedStyle) {
+ if (animatedStyle->hasAutoZIndex())
+ animatedStyle->setZIndex(0);
}
- return animationStateChanged;
+ return { WTFMove(animatedStyle), animationStateChanged };
}
std::unique_ptr<RenderStyle> CompositeAnimation::getAnimatedStyle() const
Modified: trunk/Source/WebCore/page/animation/CompositeAnimation.h (222128 => 222129)
--- trunk/Source/WebCore/page/animation/CompositeAnimation.h 2017-09-16 09:27:22 UTC (rev 222128)
+++ trunk/Source/WebCore/page/animation/CompositeAnimation.h 2017-09-16 12:33:04 UTC (rev 222129)
@@ -28,6 +28,7 @@
#pragma once
+#include "CSSAnimationController.h"
#include "ImplicitAnimation.h"
#include "KeyframeAnimation.h"
#include <wtf/HashMap.h>
@@ -54,7 +55,7 @@
void clearElement();
- bool animate(Element&, const RenderStyle* currentStyle, const RenderStyle& targetStyle, std::unique_ptr<RenderStyle>& blendedStyle);
+ AnimationUpdate animate(Element&, const RenderStyle* currentStyle, const RenderStyle& targetStyle);
std::unique_ptr<RenderStyle> getAnimatedStyle() const;
bool computeExtentOfTransformAnimation(LayoutRect&) const;
Modified: trunk/Source/WebCore/rendering/RenderElement.cpp (222128 => 222129)
--- trunk/Source/WebCore/rendering/RenderElement.cpp 2017-09-16 09:27:22 UTC (rev 222128)
+++ trunk/Source/WebCore/rendering/RenderElement.cpp 2017-09-16 12:33:04 UTC (rev 222129)
@@ -26,7 +26,6 @@
#include "RenderElement.h"
#include "AXObjectCache.h"
-#include "CSSAnimationController.h"
#include "ContentData.h"
#include "CursorList.h"
#include "ElementChildIterator.h"
@@ -102,7 +101,6 @@
, m_baseTypeFlags(baseTypeFlags)
, m_ancestorLineBoxDirty(false)
, m_hasInitializedStyle(false)
- , m_hasInitialAnimatedStyle(false)
, m_renderInlineAlwaysCreatesLineBoxes(false)
, m_renderBoxNeedsLazyRepaint(false)
, m_hasPausedImageAnimations(false)
@@ -1104,9 +1102,6 @@
if (m_style.hasFixedBackgroundImage() && !settings().fixedBackgroundsPaintRelativeToDocument())
view().frameView().removeSlowRepaintObject(this);
- if (element())
- animation().cancelAnimations(*element());
-
destroyLeftoverChildren();
unregisterForVisibleInViewportCallback();
Modified: trunk/Source/WebCore/rendering/RenderElement.h (222128 => 222129)
--- trunk/Source/WebCore/rendering/RenderElement.h 2017-09-16 09:27:22 UTC (rev 222128)
+++ trunk/Source/WebCore/rendering/RenderElement.h 2017-09-16 12:33:04 UTC (rev 222129)
@@ -131,9 +131,6 @@
// and so only should be called when the style is known not to have changed (or from setStyle).
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);
@@ -329,7 +326,6 @@
unsigned m_baseTypeFlags : 6;
unsigned m_ancestorLineBoxDirty : 1;
unsigned m_hasInitializedStyle : 1;
- unsigned m_hasInitialAnimatedStyle : 1;
unsigned m_renderInlineAlwaysCreatesLineBoxes : 1;
unsigned m_renderBoxNeedsLazyRepaint : 1;
Modified: trunk/Source/WebCore/style/RenderTreeUpdater.cpp (222128 => 222129)
--- trunk/Source/WebCore/style/RenderTreeUpdater.cpp 2017-09-16 09:27:22 UTC (rev 222128)
+++ trunk/Source/WebCore/style/RenderTreeUpdater.cpp 2017-09-16 12:33:04 UTC (rev 222129)
@@ -307,7 +307,10 @@
// We may be tearing down a descendant renderer cached in renderTreePosition.
renderTreePosition().invalidateNextSibling();
}
- tearDownRenderers(element, TeardownType::KeepHoverAndActive);
+
+ // display:none cancels animations.
+ auto teardownType = update.style->display() == NONE ? TeardownType::RendererUpdateCancelingAnimations : TeardownType::RendererUpdate;
+ tearDownRenderers(element, teardownType);
}
bool hasDisplayContents = update.style->display() == CONTENTS;
@@ -396,14 +399,6 @@
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();
#if ENABLE(FULLSCREEN_API)
@@ -525,6 +520,11 @@
}
}
+void RenderTreeUpdater::tearDownRenderers(Element& root)
+{
+ tearDownRenderers(root, TeardownType::Full);
+}
+
void RenderTreeUpdater::tearDownRenderers(Element& root, TeardownType teardownType)
{
WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
@@ -537,12 +537,18 @@
teardownStack.append(&element);
};
+ auto& animationController = root.document().frame()->animation();
+
auto pop = [&] (unsigned depth) {
while (teardownStack.size() > depth) {
auto& element = *teardownStack.takeLast();
- if (teardownType != TeardownType::KeepHoverAndActive)
+ if (teardownType == TeardownType::Full || teardownType == TeardownType::RendererUpdateCancelingAnimations)
+ animationController.cancelAnimations(element);
+
+ if (teardownType == TeardownType::Full)
element.clearHoverAndActiveStatusBeforeDetachingRenderer();
+
element.clearStyleDerivedDataBeforeDetachingRenderer();
if (auto* renderer = element.renderer()) {
Modified: trunk/Source/WebCore/style/RenderTreeUpdater.h (222128 => 222129)
--- trunk/Source/WebCore/style/RenderTreeUpdater.h 2017-09-16 09:27:22 UTC (rev 222128)
+++ trunk/Source/WebCore/style/RenderTreeUpdater.h 2017-09-16 12:33:04 UTC (rev 222129)
@@ -47,8 +47,7 @@
void commit(std::unique_ptr<const Style::Update>);
- enum class TeardownType { Normal, KeepHoverAndActive };
- static void tearDownRenderers(Element&, TeardownType = TeardownType::Normal);
+ static void tearDownRenderers(Element&);
static void tearDownRenderer(Text&);
class FirstLetter;
@@ -83,6 +82,9 @@
void popParent();
void popParentsToDepth(unsigned depth);
+ enum class TeardownType { Full, RendererUpdate, RendererUpdateCancelingAnimations };
+ static void tearDownRenderers(Element&, TeardownType);
+
RenderView& renderView();
Document& m_document;
Modified: trunk/Source/WebCore/style/StyleTreeResolver.cpp (222128 => 222129)
--- trunk/Source/WebCore/style/StyleTreeResolver.cpp 2017-09-16 09:27:22 UTC (rev 222128)
+++ trunk/Source/WebCore/style/StyleTreeResolver.cpp 2017-09-16 12:33:04 UTC (rev 222129)
@@ -241,52 +241,25 @@
ElementUpdate TreeResolver::createAnimatedElementUpdate(std::unique_ptr<RenderStyle> newStyle, Element& element, Change parentChange)
{
- auto validity = element.styleValidity();
- bool recompositeLayer = element.styleResolutionShouldRecompositeLayer();
+ auto& animationController = element.document().frame()->animation();
- 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* oldStyle = renderOrDisplayContentsStyle(element);
+ auto animationUpdate = animationController.updateAnimations(element, *newStyle, oldStyle);
- auto* renderer = element.renderer();
+ if (animationUpdate.style)
+ newStyle = WTFMove(animationUpdate.style);
- bool shouldReconstruct = validity >= Validity::SubtreeAndRenderersInvalid || parentChange == Detach;
- if (shouldReconstruct)
- return makeUpdate(WTFMove(newStyle), Detach);
+ auto change = oldStyle ? determineChange(*oldStyle, *newStyle) : Detach;
- if (!renderer) {
- auto change = Detach;
- if (auto* oldStyle = renderOrDisplayContentsStyle(element))
- change = determineChange(*oldStyle, *newStyle);
- return makeUpdate(WTFMove(newStyle), change);
- }
+ 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);
- std::unique_ptr<RenderStyle> animatedStyle;
- if (element.document().frame()->animation().updateAnimations(element, *newStyle, animatedStyle))
- recompositeLayer = true;
+ bool shouldRecompositeLayer = element.styleResolutionShouldRecompositeLayer() || animationUpdate.stateChanged;
- 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);
+ return { WTFMove(newStyle), change, shouldRecompositeLayer };
}
void TreeResolver::pushParent(Element& element, const RenderStyle& style, Change change)
