- Revision
- 230703
- Author
- grao...@webkit.org
- Date
- 2018-04-17 00:33:57 -0700 (Tue, 17 Apr 2018)
Log Message
Layout Test animations/needs-layout.html is a flaky Image Failure.
https://bugs.webkit.org/show_bug.cgi?id=172397
Reviewed by Dean Jackson.
Source/WebCore:
Animations that animate a transform and uses a relative value for either the x or y components
require a layout before starting, which CSSAnimationController would perform in the call to
CSSAnimationControllerPrivate::animationTimerFired() made immediately after a CSS animation was
created.
We now perform a similar task where upon setting new blending keyframes we compute a flag indicating
if the keyframe effect is animating a transform with relative x or y components. Then, when we perform
the first invalidation task, which runs in the next run loop after a change to the timing model has
been made, such as a call to play() on a CSSAnimation made in the TreeResolver::createAnimatedElementUpdate()
where the CSSAnimation was created, we call forceLayout() on this element's FrameView. We also ensure
we commit animations on the compositor immediately after that too, instead of waiting until the next
DisplayRefreshMonitor callback.
* animation/DocumentTimeline.cpp:
(WebCore::DocumentTimeline::performInvalidationTask):
(WebCore::DocumentTimeline::updateAnimations):
* animation/KeyframeEffectReadOnly.cpp:
(WebCore::KeyframeEffectReadOnly::forceLayoutIfNeeded):
(WebCore::KeyframeEffectReadOnly::setBlendingKeyframes):
(WebCore::KeyframeEffectReadOnly::computedNeedsForcedLayout):
(WebCore::KeyframeEffectReadOnly::applyPendingAcceleratedActions):
* animation/KeyframeEffectReadOnly.h:
LayoutTests:
No longer mark this test as flaky.
* platform/ios-wk2/TestExpectations:
* platform/mac-wk1/TestExpectations:
* platform/mac-wk2/TestExpectations:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (230702 => 230703)
--- trunk/LayoutTests/ChangeLog 2018-04-17 06:54:26 UTC (rev 230702)
+++ trunk/LayoutTests/ChangeLog 2018-04-17 07:33:57 UTC (rev 230703)
@@ -1,3 +1,16 @@
+2018-04-16 Antoine Quint <grao...@apple.com>
+
+ Layout Test animations/needs-layout.html is a flaky Image Failure.
+ https://bugs.webkit.org/show_bug.cgi?id=172397
+
+ Reviewed by Dean Jackson.
+
+ No longer mark this test as flaky.
+
+ * platform/ios-wk2/TestExpectations:
+ * platform/mac-wk1/TestExpectations:
+ * platform/mac-wk2/TestExpectations:
+
2018-04-16 Keith Rollin <krol...@apple.com>
REGRESSION: [mac-wk2 release] LayoutTest http/tests/security/contentSecurityPolicy/script-src-blocked-error-event.html is flaky
Modified: trunk/LayoutTests/platform/ios-wk2/TestExpectations (230702 => 230703)
--- trunk/LayoutTests/platform/ios-wk2/TestExpectations 2018-04-17 06:54:26 UTC (rev 230702)
+++ trunk/LayoutTests/platform/ios-wk2/TestExpectations 2018-04-17 07:33:57 UTC (rev 230703)
@@ -1319,7 +1319,6 @@
webkit.org/b/177501 webrtc/video-mute.html [ Pass Timeout ]
-webkit.org/b/172397 [ Debug ] animations/needs-layout.html [ Pass ImageOnlyFailure ]
webkit.org/b/172397 [ Debug ] legacy-animation-engine/animations/needs-layout.html [ Pass ImageOnlyFailure ]
webkit.org/b/179176 [ Release ] svg/wicd/test-rightsizing-a.xhtml [ Pass Failure ]
Modified: trunk/LayoutTests/platform/mac-wk1/TestExpectations (230702 => 230703)
--- trunk/LayoutTests/platform/mac-wk1/TestExpectations 2018-04-17 06:54:26 UTC (rev 230702)
+++ trunk/LayoutTests/platform/mac-wk1/TestExpectations 2018-04-17 07:33:57 UTC (rev 230703)
@@ -492,7 +492,6 @@
[ HighSierra+ ] fast/forms/textarea-maxlength.html [ Crash ]
[ HighSierra+ ] fast/text/system-font-fallback-emoji.html [ Crash ]
-webkit.org/b/172397 [ Debug ] animations/needs-layout.html [ Pass ImageOnlyFailure ]
webkit.org/b/172397 [ Debug ] legacy-animation-engine/animations/needs-layout.html [ Pass ImageOnlyFailure ]
webkit.org/b/179775 imported/w3c/web-platform-tests/XMLHttpRequest/firing-events-http-no-content-length.html [ Pass Failure ]
Modified: trunk/LayoutTests/platform/mac-wk2/TestExpectations (230702 => 230703)
--- trunk/LayoutTests/platform/mac-wk2/TestExpectations 2018-04-17 06:54:26 UTC (rev 230702)
+++ trunk/LayoutTests/platform/mac-wk2/TestExpectations 2018-04-17 07:33:57 UTC (rev 230703)
@@ -730,7 +730,6 @@
webkit.org/b/167757 workers/bomb.html [ Pass Timeout ]
-webkit.org/b/172397 [ Debug ] animations/needs-layout.html [ Pass ImageOnlyFailure ]
webkit.org/b/172397 [ Debug ] legacy-animation-engine/animations/needs-layout.html [ Pass ImageOnlyFailure ]
# Touch events are not available on open source bots, thus only tested on Mac.
Modified: trunk/Source/WebCore/ChangeLog (230702 => 230703)
--- trunk/Source/WebCore/ChangeLog 2018-04-17 06:54:26 UTC (rev 230702)
+++ trunk/Source/WebCore/ChangeLog 2018-04-17 07:33:57 UTC (rev 230703)
@@ -1,3 +1,33 @@
+2018-04-16 Antoine Quint <grao...@apple.com>
+
+ Layout Test animations/needs-layout.html is a flaky Image Failure.
+ https://bugs.webkit.org/show_bug.cgi?id=172397
+
+ Reviewed by Dean Jackson.
+
+ Animations that animate a transform and uses a relative value for either the x or y components
+ require a layout before starting, which CSSAnimationController would perform in the call to
+ CSSAnimationControllerPrivate::animationTimerFired() made immediately after a CSS animation was
+ created.
+
+ We now perform a similar task where upon setting new blending keyframes we compute a flag indicating
+ if the keyframe effect is animating a transform with relative x or y components. Then, when we perform
+ the first invalidation task, which runs in the next run loop after a change to the timing model has
+ been made, such as a call to play() on a CSSAnimation made in the TreeResolver::createAnimatedElementUpdate()
+ where the CSSAnimation was created, we call forceLayout() on this element's FrameView. We also ensure
+ we commit animations on the compositor immediately after that too, instead of waiting until the next
+ DisplayRefreshMonitor callback.
+
+ * animation/DocumentTimeline.cpp:
+ (WebCore::DocumentTimeline::performInvalidationTask):
+ (WebCore::DocumentTimeline::updateAnimations):
+ * animation/KeyframeEffectReadOnly.cpp:
+ (WebCore::KeyframeEffectReadOnly::forceLayoutIfNeeded):
+ (WebCore::KeyframeEffectReadOnly::setBlendingKeyframes):
+ (WebCore::KeyframeEffectReadOnly::computedNeedsForcedLayout):
+ (WebCore::KeyframeEffectReadOnly::applyPendingAcceleratedActions):
+ * animation/KeyframeEffectReadOnly.h:
+
2018-04-16 Pablo Saavedra <psaave...@igalia.com>
Inconsistent EGL defines in ImageBufferCairo
Modified: trunk/Source/WebCore/animation/DocumentTimeline.cpp (230702 => 230703)
--- trunk/Source/WebCore/animation/DocumentTimeline.cpp 2018-04-17 06:54:26 UTC (rev 230702)
+++ trunk/Source/WebCore/animation/DocumentTimeline.cpp 2018-04-17 07:33:57 UTC (rev 230703)
@@ -176,6 +176,8 @@
downcast<DeclarativeAnimation>(*animation).invalidateDOMEvents();
}
+ applyPendingAcceleratedAnimations();
+
updateAnimationSchedule();
m_cachedCurrentTime = std::nullopt;
}
@@ -244,8 +246,6 @@
m_document->updateStyleIfNeeded();
}
- applyPendingAcceleratedAnimations();
-
// Time has advanced, the timing model requires invalidation now.
timingModelDidChange();
}
@@ -332,8 +332,16 @@
void DocumentTimeline::applyPendingAcceleratedAnimations()
{
- for (auto& animation : m_acceleratedAnimationsPendingRunningStateChange)
+ bool hasForcedLayout = false;
+ for (auto& animation : m_acceleratedAnimationsPendingRunningStateChange) {
+ if (!hasForcedLayout) {
+ auto* effect = animation->effect();
+ if (is<KeyframeEffectReadOnly>(effect))
+ hasForcedLayout |= downcast<KeyframeEffectReadOnly>(effect)->forceLayoutIfNeeded();
+ }
animation->applyPendingAcceleratedActions();
+ }
+
m_acceleratedAnimationsPendingRunningStateChange.clear();
}
Modified: trunk/Source/WebCore/animation/KeyframeEffectReadOnly.cpp (230702 => 230703)
--- trunk/Source/WebCore/animation/KeyframeEffectReadOnly.cpp 2018-04-17 06:54:26 UTC (rev 230702)
+++ trunk/Source/WebCore/animation/KeyframeEffectReadOnly.cpp 2018-04-17 07:33:57 UTC (rev 230703)
@@ -46,6 +46,7 @@
#include "StylePendingResources.h"
#include "StyleResolver.h"
#include "TimingFunction.h"
+#include "TranslateTransformOperation.h"
#include "WillChangeData.h"
#include <wtf/UUID.h>
@@ -693,10 +694,28 @@
setBlendingKeyframes(keyframeList);
}
+bool KeyframeEffectReadOnly::forceLayoutIfNeeded()
+{
+ if (!m_needsForcedLayout || !m_target)
+ return false;
+
+ auto* renderer = m_target->renderer();
+ if (!renderer || !renderer->parent())
+ return false;
+
+ auto* frameView = m_target->document().view();
+ if (!frameView)
+ return false;
+
+ frameView->forceLayout();
+ return true;
+}
+
void KeyframeEffectReadOnly::setBlendingKeyframes(KeyframeList& blendingKeyframes)
{
m_blendingKeyframes = WTFMove(blendingKeyframes);
+ computedNeedsForcedLayout();
computeStackingContextImpact();
computeShouldRunAccelerated();
@@ -896,6 +915,34 @@
return !CSSPropertyAnimation::propertiesEqual(property, m_blendingKeyframes[1].style(), &newStyle);
}
+void KeyframeEffectReadOnly::computedNeedsForcedLayout()
+{
+ m_needsForcedLayout = false;
+ if (is<CSSTransition>(animation()) || !m_blendingKeyframes.containsProperty(CSSPropertyTransform))
+ return;
+
+ size_t numberOfKeyframes = m_blendingKeyframes.size();
+ for (size_t i = 0; i < numberOfKeyframes; i++) {
+ auto* keyframeStyle = m_blendingKeyframes[i].style();
+ if (!keyframeStyle) {
+ ASSERT_NOT_REACHED();
+ continue;
+ }
+ if (keyframeStyle->hasTransform()) {
+ auto& transformOperations = keyframeStyle->transform();
+ for (auto operation : transformOperations.operations()) {
+ if (operation->isTranslateTransformOperationType()) {
+ auto translation = downcast<TranslateTransformOperation>(operation.get());
+ if (translation->x().isPercent() || translation->y().isPercent()) {
+ m_needsForcedLayout = true;
+ return;
+ }
+ }
+ }
+ }
+ }
+}
+
void KeyframeEffectReadOnly::computeStackingContextImpact()
{
m_triggersStackingContext = false;
@@ -1180,6 +1227,11 @@
void KeyframeEffectReadOnly::applyPendingAcceleratedActions()
{
+ // Once an accelerated animation has been committed, we no longer want to force a layout.
+ // This should have been performed by a call to forceLayoutIfNeeded() prior to applying
+ // pending accelerated actions.
+ m_needsForcedLayout = false;
+
auto* renderer = this->renderer();
if (!renderer || !renderer->isComposited())
return;
Modified: trunk/Source/WebCore/animation/KeyframeEffectReadOnly.h (230702 => 230703)
--- trunk/Source/WebCore/animation/KeyframeEffectReadOnly.h 2018-04-17 06:54:26 UTC (rev 230702)
+++ trunk/Source/WebCore/animation/KeyframeEffectReadOnly.h 2018-04-17 07:33:57 UTC (rev 230703)
@@ -120,6 +120,7 @@
bool computeExtentOfTransformAnimation(LayoutRect&) const;
bool computeTransformedExtentViaTransformList(const FloatRect&, const RenderStyle&, LayoutRect&) const;
bool computeTransformedExtentViaMatrix(const FloatRect&, const RenderStyle&, LayoutRect&) const;
+ bool forceLayoutIfNeeded();
protected:
void copyPropertiesFromSource(Ref<KeyframeEffectReadOnly>&&);
@@ -136,6 +137,7 @@
void setAnimatedPropertiesInStyle(RenderStyle&, double);
TimingFunction* timingFunctionForKeyframeAtIndex(size_t);
Ref<const Animation> backingAnimationForCompositedRenderer() const;
+ void computedNeedsForcedLayout();
void computeStackingContextImpact();
void updateBlendingKeyframes();
void computeCSSAnimationBlendingKeyframes();
@@ -149,6 +151,7 @@
#endif
bool m_shouldRunAccelerated { false };
+ bool m_needsForcedLayout { false };
bool m_triggersStackingContext { false };
bool m_started { false };
bool m_startedAccelerated { false };
