Diff
Modified: trunk/LayoutTests/ChangeLog (208313 => 208314)
--- trunk/LayoutTests/ChangeLog 2016-11-03 00:05:12 UTC (rev 208313)
+++ trunk/LayoutTests/ChangeLog 2016-11-03 00:19:54 UTC (rev 208314)
@@ -1,3 +1,16 @@
+2016-11-02 Simon Fraser <[email protected]>
+
+ REGRESSION (r208025) GraphicsContext state stack assertions loading webkit.org
+ https://bugs.webkit.org/show_bug.cgi?id=164350
+ rdar://problem/29053414
+
+ Reviewed by Dean Jackson.
+
+ Test was reduced from webkit.org.
+
+ * animations/stacking-during-opacity-animation-expected.txt: Added.
+ * animations/stacking-during-opacity-animation.html: Added.
+
2016-11-02 Myles C. Maxfield <[email protected]>
[iOS] [WebGL] Multisample resolve step may operate on stale data
Added: trunk/LayoutTests/animations/stacking-during-opacity-animation-expected.txt (0 => 208314)
--- trunk/LayoutTests/animations/stacking-during-opacity-animation-expected.txt (rev 0)
+++ trunk/LayoutTests/animations/stacking-during-opacity-animation-expected.txt 2016-11-03 00:19:54 UTC (rev 208314)
@@ -0,0 +1 @@
+This test should not crash or assert.
Added: trunk/LayoutTests/animations/stacking-during-opacity-animation.html (0 => 208314)
--- trunk/LayoutTests/animations/stacking-during-opacity-animation.html (rev 0)
+++ trunk/LayoutTests/animations/stacking-during-opacity-animation.html 2016-11-03 00:19:54 UTC (rev 208314)
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+ .background-image {
+ position: relative;
+ margin-top: -1px;
+ height: 0;
+ padding-bottom: 80%;
+ opacity: 0.1;
+ }
+
+ .tile {
+ animation: fade-in 0.4s;
+ }
+
+ @keyframes fade-in {
+ from { opacity: 0; }
+ to { opacity: 1; }
+ }
+
+</style>
+<script>
+ if (window.testRunner)
+ testRunner.dumpAsText();
+</script>
+</head>
+<body>
+ <div class="tile">
+ <div class="background-image"></div>
+ <p>This test should not crash or assert.</p>
+ </div>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (208313 => 208314)
--- trunk/Source/WebCore/ChangeLog 2016-11-03 00:05:12 UTC (rev 208313)
+++ trunk/Source/WebCore/ChangeLog 2016-11-03 00:19:54 UTC (rev 208314)
@@ -1,3 +1,44 @@
+2016-11-02 Simon Fraser <[email protected]>
+
+ REGRESSION (r208025) GraphicsContext state stack assertions loading webkit.org
+ https://bugs.webkit.org/show_bug.cgi?id=164350
+ rdar://problem/29053414
+
+ Reviewed by Dean Jackson.
+
+ After r208025 it as possible for KeyframeAnimation::animate() to produce a RenderStyle
+ with a non-1 opacity, but without the explicit z-index that triggers stacking context.
+ This confused the RenderLayer paintWithTransparency code, triggering mismsatched GraphicsContext
+ save/restores.
+
+ This occurred when the runningOrFillingForwards state was mis-computed. keyframeAnim->animate()
+ can spit out a new style when in the StartWaitTimer sometimes, so "!keyframeAnim->waitingToStart() && !keyframeAnim->postActive()"
+ gave the wrong answser.
+
+ Rather than depend on the super-confusing animation state, use a bool out param from animate() to say
+ when it actually produced a new style, and when true, do the setZIndex(0).
+
+ Test: animations/stacking-during-opacity-animation.html
+
+ * page/animation/AnimationBase.h:
+ * page/animation/CSSPropertyAnimation.cpp:
+ (WebCore::CSSPropertyAnimation::blendProperties): Log after blending so the log shows the blended style.
+ * page/animation/CompositeAnimation.cpp:
+ (WebCore::CompositeAnimation::animate):
+ * page/animation/ImplicitAnimation.cpp:
+ (WebCore::ImplicitAnimation::animate):
+ * page/animation/ImplicitAnimation.h:
+ * page/animation/KeyframeAnimation.cpp:
+ (WebCore::KeyframeAnimation::animate):
+ * page/animation/KeyframeAnimation.h:
+ * platform/graphics/GraphicsContext.cpp:
+ (WebCore::GraphicsContext::restore):
+ * platform/graphics/ca/cocoa/PlatformCALayerCocoa.mm:
+ (PlatformCALayer::drawLayerContents): No functional change, but created scope for the
+ GraphicsContext so that it didn't outlive the CGContextRestoreGState(context).
+ * rendering/RenderLayer.cpp:
+ (WebCore::RenderLayer::beginTransparencyLayers): New assertion that catches the problem earlier.
+
2016-11-02 Myles C. Maxfield <[email protected]>
[iOS] [WebGL] Multisample resolve step may operate on stale data
Modified: trunk/Source/WebCore/page/animation/AnimationBase.h (208313 => 208314)
--- trunk/Source/WebCore/page/animation/AnimationBase.h 2016-11-03 00:05:12 UTC (rev 208313)
+++ trunk/Source/WebCore/page/animation/AnimationBase.h 2016-11-03 00:19:54 UTC (rev 208314)
@@ -136,7 +136,7 @@
double progress(double scale = 1, double offset = 0, const TimingFunction* = nullptr) const;
// Returns true if the animation state changed.
- virtual bool animate(CompositeAnimation*, RenderElement*, const RenderStyle* /*currentStyle*/, const RenderStyle* /*targetStyle*/, std::unique_ptr<RenderStyle>& /*animatedStyle*/) = 0;
+ virtual bool animate(CompositeAnimation*, RenderElement*, const RenderStyle* /*currentStyle*/, const RenderStyle* /*targetStyle*/, std::unique_ptr<RenderStyle>& /*animatedStyle*/, bool& didBlendStyle) = 0;
virtual void getAnimatedStyle(std::unique_ptr<RenderStyle>& /*animatedStyle*/) = 0;
virtual bool computeExtentOfTransformAnimation(LayoutRect&) const = 0;
Modified: trunk/Source/WebCore/page/animation/CSSPropertyAnimation.cpp (208313 => 208314)
--- trunk/Source/WebCore/page/animation/CSSPropertyAnimation.cpp 2016-11-03 00:05:12 UTC (rev 208313)
+++ trunk/Source/WebCore/page/animation/CSSPropertyAnimation.cpp 2016-11-03 00:19:54 UTC (rev 208314)
@@ -1576,10 +1576,10 @@
AnimationPropertyWrapperBase* wrapper = CSSPropertyAnimationWrapperMap::singleton().wrapperForProperty(prop);
if (wrapper) {
+ wrapper->blend(anim, dst, a, b, progress);
#if !LOG_DISABLED
wrapper->logBlend(a, b, dst, progress);
#endif
- wrapper->blend(anim, dst, a, b, progress);
return !wrapper->animationIsAccelerated() || !anim->isAccelerated();
}
return false;
Modified: trunk/Source/WebCore/page/animation/CompositeAnimation.cpp (208313 => 208314)
--- trunk/Source/WebCore/page/animation/CompositeAnimation.cpp 2016-11-03 00:05:12 UTC (rev 208313)
+++ trunk/Source/WebCore/page/animation/CompositeAnimation.cpp 2016-11-03 00:19:54 UTC (rev 208314)
@@ -299,10 +299,12 @@
// to fill in a RenderStyle*& only if needed.
bool checkForStackingContext = false;
for (auto& transition : m_transitions.values()) {
- if (transition->animate(this, &renderer, currentStyle, &targetStyle, blendedStyle))
+ bool didBlendStyle = false;
+ if (transition->animate(this, &renderer, currentStyle, &targetStyle, blendedStyle, didBlendStyle))
animationStateChanged = true;
- checkForStackingContext |= WillChangeData::propertyCreatesStackingContext(transition->animatingProperty());
+ if (didBlendStyle)
+ checkForStackingContext |= WillChangeData::propertyCreatesStackingContext(transition->animatingProperty());
}
if (blendedStyle && checkForStackingContext) {
@@ -327,11 +329,11 @@
for (auto& name : m_keyframeAnimationOrderMap) {
RefPtr<KeyframeAnimation> keyframeAnim = m_keyframeAnimations.get(name);
if (keyframeAnim) {
- if (keyframeAnim->animate(this, &renderer, currentStyle, &targetStyle, blendedStyle))
+ bool didBlendStyle = false;
+ if (keyframeAnim->animate(this, &renderer, currentStyle, &targetStyle, blendedStyle, didBlendStyle))
animationStateChanged = true;
- bool runningOrFillingForwards = !keyframeAnim->waitingToStart() && !keyframeAnim->postActive();
- forceStackingContext |= runningOrFillingForwards && keyframeAnim->triggersStackingContext();
+ forceStackingContext |= didBlendStyle && keyframeAnim->triggersStackingContext();
}
}
Modified: trunk/Source/WebCore/page/animation/ImplicitAnimation.cpp (208313 => 208314)
--- trunk/Source/WebCore/page/animation/ImplicitAnimation.cpp 2016-11-03 00:05:12 UTC (rev 208313)
+++ trunk/Source/WebCore/page/animation/ImplicitAnimation.cpp 2016-11-03 00:19:54 UTC (rev 208314)
@@ -61,7 +61,7 @@
return m_object->document().hasListenerType(inListenerType);
}
-bool ImplicitAnimation::animate(CompositeAnimation*, RenderElement*, const RenderStyle*, const RenderStyle* targetStyle, std::unique_ptr<RenderStyle>& animatedStyle)
+bool ImplicitAnimation::animate(CompositeAnimation*, RenderElement*, const RenderStyle*, const RenderStyle* targetStyle, std::unique_ptr<RenderStyle>& animatedStyle, bool& didBlendStyle)
{
// If we get this far and the animation is done, it means we are cleaning up a just finished animation.
// So just return. Everything is already all cleaned up.
@@ -85,6 +85,8 @@
// Fire the start timeout if needed
fireAnimationEventsIfNeeded();
+
+ didBlendStyle = true;
return state() != oldState;
}
Modified: trunk/Source/WebCore/page/animation/ImplicitAnimation.h (208313 => 208314)
--- trunk/Source/WebCore/page/animation/ImplicitAnimation.h 2016-11-03 00:05:12 UTC (rev 208313)
+++ trunk/Source/WebCore/page/animation/ImplicitAnimation.h 2016-11-03 00:19:54 UTC (rev 208314)
@@ -54,7 +54,7 @@
void pauseAnimation(double timeOffset) override;
void endAnimation() override;
- bool animate(CompositeAnimation*, RenderElement*, const RenderStyle* currentStyle, const RenderStyle* targetStyle, std::unique_ptr<RenderStyle>& animatedStyle) override;
+ bool animate(CompositeAnimation*, RenderElement*, const RenderStyle* currentStyle, const RenderStyle* targetStyle, std::unique_ptr<RenderStyle>& animatedStyle, bool& didBlendStyle) override;
void getAnimatedStyle(std::unique_ptr<RenderStyle>& animatedStyle) override;
void reset(const RenderStyle* to);
Modified: trunk/Source/WebCore/page/animation/KeyframeAnimation.cpp (208313 => 208314)
--- trunk/Source/WebCore/page/animation/KeyframeAnimation.cpp 2016-11-03 00:05:12 UTC (rev 208313)
+++ trunk/Source/WebCore/page/animation/KeyframeAnimation.cpp 2016-11-03 00:19:54 UTC (rev 208314)
@@ -124,7 +124,7 @@
prog = progress(scale, offset, prevKeyframe.timingFunction(name()));
}
-bool KeyframeAnimation::animate(CompositeAnimation* compositeAnimation, RenderElement*, const RenderStyle*, const RenderStyle* targetStyle, std::unique_ptr<RenderStyle>& animatedStyle)
+bool KeyframeAnimation::animate(CompositeAnimation* compositeAnimation, RenderElement*, const RenderStyle*, const RenderStyle* targetStyle, std::unique_ptr<RenderStyle>& animatedStyle, bool& didBlendStyle)
{
// Fire the start timeout if needed
fireAnimationEventsIfNeeded();
@@ -179,6 +179,7 @@
CSSPropertyAnimation::blendProperties(this, propertyID, animatedStyle.get(), fromStyle, toStyle, progress);
}
+ didBlendStyle = true;
return state() != oldState;
}
Modified: trunk/Source/WebCore/page/animation/KeyframeAnimation.h (208313 => 208314)
--- trunk/Source/WebCore/page/animation/KeyframeAnimation.h 2016-11-03 00:05:12 UTC (rev 208313)
+++ trunk/Source/WebCore/page/animation/KeyframeAnimation.h 2016-11-03 00:19:54 UTC (rev 208314)
@@ -45,7 +45,7 @@
return adoptRef(*new KeyframeAnimation(animation, renderer, compositeAnimation, unanimatedStyle));
}
- bool animate(CompositeAnimation*, RenderElement*, const RenderStyle* currentStyle, const RenderStyle* targetStyle, std::unique_ptr<RenderStyle>& animatedStyle) override;
+ bool animate(CompositeAnimation*, RenderElement*, const RenderStyle* currentStyle, const RenderStyle* targetStyle, std::unique_ptr<RenderStyle>& animatedStyle, bool& didBlendStyle) override;
void getAnimatedStyle(std::unique_ptr<RenderStyle>&) override;
bool computeExtentOfTransformAnimation(LayoutRect&) const override;
Modified: trunk/Source/WebCore/platform/graphics/GraphicsContext.cpp (208313 => 208314)
--- trunk/Source/WebCore/platform/graphics/GraphicsContext.cpp 2016-11-03 00:05:12 UTC (rev 208313)
+++ trunk/Source/WebCore/platform/graphics/GraphicsContext.cpp 2016-11-03 00:19:54 UTC (rev 208314)
@@ -367,6 +367,7 @@
LOG_ERROR("ERROR void GraphicsContext::restore() stack is empty");
return;
}
+
m_state = m_stack.last();
m_stack.removeLast();
Modified: trunk/Source/WebCore/platform/graphics/ca/cocoa/PlatformCALayerCocoa.mm (208313 => 208314)
--- trunk/Source/WebCore/platform/graphics/ca/cocoa/PlatformCALayerCocoa.mm 2016-11-03 00:05:12 UTC (rev 208313)
+++ trunk/Source/WebCore/platform/graphics/ca/cocoa/PlatformCALayerCocoa.mm 2016-11-03 00:19:54 UTC (rev 208314)
@@ -1067,46 +1067,48 @@
[NSGraphicsContext setCurrentContext:layerContext];
#endif
- GraphicsContext graphicsContext(context);
- graphicsContext.setIsCALayerContext(true);
- graphicsContext.setIsAcceleratedContext(platformCALayer->acceleratesDrawing());
-
- if (!layerContents->platformCALayerContentsOpaque()) {
- // Turn off font smoothing to improve the appearance of text rendered onto a transparent background.
- graphicsContext.setShouldSmoothFonts(false);
- }
-
+ {
+ GraphicsContext graphicsContext(context);
+ graphicsContext.setIsCALayerContext(true);
+ graphicsContext.setIsAcceleratedContext(platformCALayer->acceleratesDrawing());
+
+ if (!layerContents->platformCALayerContentsOpaque()) {
+ // Turn off font smoothing to improve the appearance of text rendered onto a transparent background.
+ graphicsContext.setShouldSmoothFonts(false);
+ }
+
#if PLATFORM(MAC)
- // It's important to get the clip from the context, because it may be significantly
- // smaller than the layer bounds (e.g. tiled layers)
- ThemeMac::setFocusRingClipRect(CGContextGetClipBoundingBox(context));
+ // It's important to get the clip from the context, because it may be significantly
+ // smaller than the layer bounds (e.g. tiled layers)
+ ThemeMac::setFocusRingClipRect(CGContextGetClipBoundingBox(context));
#endif
-
- for (const auto& rect : dirtyRects) {
- GraphicsContextStateSaver stateSaver(graphicsContext);
- graphicsContext.clip(rect);
- layerContents->platformCALayerPaintContents(platformCALayer, graphicsContext, rect);
- }
-
+ for (const auto& rect : dirtyRects) {
+ GraphicsContextStateSaver stateSaver(graphicsContext);
+ graphicsContext.clip(rect);
+
+ layerContents->platformCALayerPaintContents(platformCALayer, graphicsContext, rect);
+ }
+
#if PLATFORM(IOS)
- fontAntialiasingState.restore();
+ fontAntialiasingState.restore();
#else
- ThemeMac::setFocusRingClipRect(FloatRect());
-
- [NSGraphicsContext restoreGraphicsState];
+ ThemeMac::setFocusRingClipRect(FloatRect());
+
+ [NSGraphicsContext restoreGraphicsState];
#endif
-
+ }
+
+ CGContextRestoreGState(context);
+
// Re-fetch the layer owner, since <rdar://problem/9125151> indicates that it might have been destroyed during painting.
layerContents = platformCALayer->owner();
ASSERT(layerContents);
- CGContextRestoreGState(context);
-
// Always update the repaint count so that it's accurate even if the count itself is not shown. This will be useful
// for the Web Inspector feeding this information through the LayerTreeAgent.
int repaintCount = layerContents->platformCALayerIncrementRepaintCount(platformCALayer);
-
+
if (!platformCALayer->usesTiledBackingLayer() && layerContents && layerContents->platformCALayerShowRepaintCounter(platformCALayer))
drawRepaintIndicator(context, platformCALayer, repaintCount, nullptr);
}
Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (208313 => 208314)
--- trunk/Source/WebCore/rendering/RenderLayer.cpp 2016-11-03 00:05:12 UTC (rev 208313)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp 2016-11-03 00:19:54 UTC (rev 208314)
@@ -1809,6 +1809,7 @@
ancestor->beginTransparencyLayers(context, paintingInfo, dirtyRect);
if (paintsWithTransparency(paintingInfo.paintBehavior)) {
+ ASSERT(isStackingContext());
m_usedTransparency = true;
context.save();
LayoutRect adjustedClipRect = paintingExtent(*this, paintingInfo.rootLayer, dirtyRect, paintingInfo.paintBehavior);
