Diff
Modified: trunk/LayoutTests/ChangeLog (222103 => 222104)
--- trunk/LayoutTests/ChangeLog 2017-09-15 19:53:01 UTC (rev 222103)
+++ trunk/LayoutTests/ChangeLog 2017-09-15 19:54:01 UTC (rev 222104)
@@ -1,3 +1,16 @@
+2017-09-15 Ryan Haddad <ryanhad...@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 <mattba...@apple.com>
Web Inspector: Canvas: recording parameters that include colors should show an InlineSwatch (2D canvas)
Deleted: trunk/LayoutTests/transitions/transition-display-property-2-expected.html (222103 => 222104)
--- trunk/LayoutTests/transitions/transition-display-property-2-expected.html 2017-09-15 19:53:01 UTC (rev 222103)
+++ trunk/LayoutTests/transitions/transition-display-property-2-expected.html 2017-09-15 19:54:01 UTC (rev 222104)
@@ -1,4 +0,0 @@
-<style>
-test { display:inline-block; height:55px; width:10px; border: 2px solid green; }
-</style>
-<test></test>
Deleted: trunk/LayoutTests/transitions/transition-display-property-2.html (222103 => 222104)
--- trunk/LayoutTests/transitions/transition-display-property-2.html 2017-09-15 19:53:01 UTC (rev 222103)
+++ trunk/LayoutTests/transitions/transition-display-property-2.html 2017-09-15 19:54:01 UTC (rev 222104)
@@ -1,22 +0,0 @@
-<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>
-if (window.testRunner) {
- testRunner.waitUntilDone();
- requestAnimationFrame(() => {
- testRunner.notifyDone();
- });
-}
-const test = document.querySelector("test");
-test.offsetWidth;
-test.className = "animate";
-</script>
Modified: trunk/LayoutTests/transitions/transition-display-property.html (222103 => 222104)
--- trunk/LayoutTests/transitions/transition-display-property.html 2017-09-15 19:53:01 UTC (rev 222103)
+++ trunk/LayoutTests/transitions/transition-display-property.html 2017-09-15 19:54:01 UTC (rev 222104)
@@ -5,10 +5,18 @@
<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();
- test._ontransitionend_ = () => testRunner.notifyDone();
+ testUntilComplete();
}
test.offsetWidth;
test.className = "animate";
Modified: trunk/Source/WebCore/ChangeLog (222103 => 222104)
--- trunk/Source/WebCore/ChangeLog 2017-09-15 19:53:01 UTC (rev 222103)
+++ trunk/Source/WebCore/ChangeLog 2017-09-15 19:54:01 UTC (rev 222104)
@@ -1,3 +1,16 @@
+2017-09-15 Ryan Haddad <ryanhad...@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_hor...@apple.com>
Fix the macOS CMake build
Modified: trunk/Source/WebCore/page/animation/CSSAnimationController.cpp (222103 => 222104)
--- trunk/Source/WebCore/page/animation/CSSAnimationController.cpp 2017-09-15 19:53:01 UTC (rev 222103)
+++ trunk/Source/WebCore/page/animation/CSSAnimationController.cpp 2017-09-15 19:54:01 UTC (rev 222104)
@@ -637,18 +637,19 @@
element.invalidateStyleAndLayerComposition();
}
-AnimationUpdate CSSAnimationController::updateAnimations(Element& element, const RenderStyle& newStyle, const RenderStyle* oldStyle)
+bool CSSAnimationController::updateAnimations(Element& element, const RenderStyle& newStyle, std::unique_ptr<RenderStyle>& animatedStyle)
{
- bool hasOrHadAnimations = (oldStyle && oldStyle->hasAnimationsOrTransitions()) || newStyle.hasAnimationsOrTransitions();
- if (!hasOrHadAnimations)
- return { };
+ 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
// against the animations in the style and make sure we're in sync. If destination values
@@ -656,9 +657,8 @@
// a new style.
CompositeAnimation& compositeAnimation = m_data->ensureCompositeAnimation(element);
- auto update = compositeAnimation.animate(element, oldStyle, newStyle);
+ bool animationStateChanged = compositeAnimation.animate(element, oldStyle, newStyle, animatedStyle);
- 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 update;
+ return animationStateChanged;
}
std::unique_ptr<RenderStyle> CSSAnimationController::animatedStyleForRenderer(RenderElement& renderer)
Modified: trunk/Source/WebCore/page/animation/CSSAnimationController.h (222103 => 222104)
--- trunk/Source/WebCore/page/animation/CSSAnimationController.h 2017-09-15 19:53:01 UTC (rev 222103)
+++ trunk/Source/WebCore/page/animation/CSSAnimationController.h 2017-09-15 19:54:01 UTC (rev 222104)
@@ -30,7 +30,6 @@
#include "AnimationBase.h"
#include "CSSPropertyNames.h"
-#include "RenderStyle.h"
#include <wtf/Forward.h>
namespace WebCore {
@@ -41,19 +40,8 @@
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:
@@ -61,7 +49,7 @@
~CSSAnimationController();
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&);
// If possible, compute the visual extent of any transform animation on the given renderer
Modified: trunk/Source/WebCore/page/animation/CompositeAnimation.cpp (222103 => 222104)
--- trunk/Source/WebCore/page/animation/CompositeAnimation.cpp 2017-09-15 19:53:01 UTC (rev 222103)
+++ trunk/Source/WebCore/page/animation/CompositeAnimation.cpp 2017-09-15 19:54:01 UTC (rev 222104)
@@ -287,7 +287,7 @@
std::swap(newAnimations, m_keyframeAnimations);
}
-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).
updateTransitions(element, currentStyle, targetStyle);
@@ -297,8 +297,6 @@
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.
@@ -305,7 +303,7 @@
bool checkForStackingContext = false;
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;
if (didBlendStyle)
@@ -312,17 +310,17 @@
checkForStackingContext |= WillChangeData::propertyCreatesStackingContext(transition->animatingProperty());
}
- 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
)
forceStackingContext = true;
@@ -335,7 +333,7 @@
RefPtr<KeyframeAnimation> keyframeAnim = m_keyframeAnimations.get(name);
if (keyframeAnim) {
bool didBlendStyle = false;
- if (keyframeAnim->animate(*this, targetStyle, animatedStyle, didBlendStyle))
+ if (keyframeAnim->animate(*this, targetStyle, blendedStyle, didBlendStyle))
animationStateChanged = true;
forceStackingContext |= didBlendStyle && keyframeAnim->triggersStackingContext();
@@ -347,12 +345,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 && animatedStyle) {
- if (animatedStyle->hasAutoZIndex())
- animatedStyle->setZIndex(0);
+ if (forceStackingContext && blendedStyle) {
+ if (blendedStyle->hasAutoZIndex())
+ blendedStyle->setZIndex(0);
}
- return { WTFMove(animatedStyle), animationStateChanged };
+ return animationStateChanged;
}
std::unique_ptr<RenderStyle> CompositeAnimation::getAnimatedStyle() const
Modified: trunk/Source/WebCore/page/animation/CompositeAnimation.h (222103 => 222104)
--- trunk/Source/WebCore/page/animation/CompositeAnimation.h 2017-09-15 19:53:01 UTC (rev 222103)
+++ trunk/Source/WebCore/page/animation/CompositeAnimation.h 2017-09-15 19:54:01 UTC (rev 222104)
@@ -28,7 +28,6 @@
#pragma once
-#include "CSSAnimationController.h"
#include "ImplicitAnimation.h"
#include "KeyframeAnimation.h"
#include <wtf/HashMap.h>
@@ -55,7 +54,7 @@
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;
Modified: trunk/Source/WebCore/rendering/RenderElement.cpp (222103 => 222104)
--- trunk/Source/WebCore/rendering/RenderElement.cpp 2017-09-15 19:53:01 UTC (rev 222103)
+++ trunk/Source/WebCore/rendering/RenderElement.cpp 2017-09-15 19:54:01 UTC (rev 222104)
@@ -26,6 +26,7 @@
#include "RenderElement.h"
#include "AXObjectCache.h"
+#include "CSSAnimationController.h"
#include "ContentData.h"
#include "CursorList.h"
#include "ElementChildIterator.h"
@@ -101,6 +102,7 @@
, m_baseTypeFlags(baseTypeFlags)
, m_ancestorLineBoxDirty(false)
, m_hasInitializedStyle(false)
+ , m_hasInitialAnimatedStyle(false)
, m_renderInlineAlwaysCreatesLineBoxes(false)
, m_renderBoxNeedsLazyRepaint(false)
, m_hasPausedImageAnimations(false)
@@ -1102,6 +1104,9 @@
if (m_style.hasFixedBackgroundImage() && !settings().fixedBackgroundsPaintRelativeToDocument())
view().frameView().removeSlowRepaintObject(this);
+ if (element())
+ animation().cancelAnimations(*element());
+
destroyLeftoverChildren();
unregisterForVisibleInViewportCallback();
Modified: trunk/Source/WebCore/rendering/RenderElement.h (222103 => 222104)
--- trunk/Source/WebCore/rendering/RenderElement.h 2017-09-15 19:53:01 UTC (rev 222103)
+++ trunk/Source/WebCore/rendering/RenderElement.h 2017-09-15 19:54:01 UTC (rev 222104)
@@ -131,6 +131,9 @@
// 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);
@@ -326,6 +329,7 @@
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 (222103 => 222104)
--- trunk/Source/WebCore/style/RenderTreeUpdater.cpp 2017-09-15 19:53:01 UTC (rev 222103)
+++ trunk/Source/WebCore/style/RenderTreeUpdater.cpp 2017-09-15 19:54:01 UTC (rev 222104)
@@ -307,10 +307,7 @@
// We may be tearing down a descendant renderer cached in renderTreePosition.
renderTreePosition().invalidateNextSibling();
}
-
- // display:none cancels animations.
- auto teardownType = update.style->display() == NONE ? TeardownType::RendererUpdateCancelingAnimations : TeardownType::RendererUpdate;
- tearDownRenderers(element, teardownType);
+ tearDownRenderers(element, TeardownType::KeepHoverAndActive);
}
bool hasDisplayContents = update.style->display() == CONTENTS;
@@ -399,6 +396,14 @@
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)
@@ -520,11 +525,6 @@
}
}
-void RenderTreeUpdater::tearDownRenderers(Element& root)
-{
- tearDownRenderers(root, TeardownType::Full);
-}
-
void RenderTreeUpdater::tearDownRenderers(Element& root, TeardownType teardownType)
{
WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
@@ -537,18 +537,12 @@
teardownStack.append(&element);
};
- 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();
if (auto* renderer = element.renderer()) {
Modified: trunk/Source/WebCore/style/RenderTreeUpdater.h (222103 => 222104)
--- trunk/Source/WebCore/style/RenderTreeUpdater.h 2017-09-15 19:53:01 UTC (rev 222103)
+++ trunk/Source/WebCore/style/RenderTreeUpdater.h 2017-09-15 19:54:01 UTC (rev 222104)
@@ -47,7 +47,8 @@
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&);
class FirstLetter;
@@ -82,9 +83,6 @@
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 (222103 => 222104)
--- trunk/Source/WebCore/style/StyleTreeResolver.cpp 2017-09-15 19:53:01 UTC (rev 222103)
+++ trunk/Source/WebCore/style/StyleTreeResolver.cpp 2017-09-15 19:54:01 UTC (rev 222104)
@@ -241,25 +241,52 @@
ElementUpdate TreeResolver::createAnimatedElementUpdate(std::unique_ptr<RenderStyle> newStyle, Element& element, Change parentChange)
{
- auto& animationController = element.document().frame()->animation();
+ auto validity = element.styleValidity();
+ bool recompositeLayer = element.styleResolutionShouldRecompositeLayer();
- auto* oldStyle = renderOrDisplayContentsStyle(element);
- auto animationUpdate = animationController.updateAnimations(element, *newStyle, oldStyle);
+ 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 };
+ };
- if (animationUpdate.style)
- newStyle = WTFMove(animationUpdate.style);
+ auto* renderer = element.renderer();
- auto change = oldStyle ? determineChange(*oldStyle, *newStyle) : Detach;
+ bool shouldReconstruct = validity >= Validity::SubtreeAndRenderersInvalid || parentChange == Detach;
+ if (shouldReconstruct)
+ return makeUpdate(WTFMove(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);
+ if (!renderer) {
+ auto change = Detach;
+ if (auto* oldStyle = renderOrDisplayContentsStyle(element))
+ change = determineChange(*oldStyle, *newStyle);
+ return makeUpdate(WTFMove(newStyle), change);
+ }
- bool shouldRecompositeLayer = element.styleResolutionShouldRecompositeLayer() || animationUpdate.stateChanged;
+ std::unique_ptr<RenderStyle> animatedStyle;
+ if (element.document().frame()->animation().updateAnimations(element, *newStyle, animatedStyle))
+ recompositeLayer = true;
- return { WTFMove(newStyle), change, shouldRecompositeLayer };
+ 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);
}
void TreeResolver::pushParent(Element& element, const RenderStyle& style, Change change)
