Title: [208335] branches/safari-602-branch

Diff

Modified: branches/safari-602-branch/LayoutTests/ChangeLog (208334 => 208335)


--- branches/safari-602-branch/LayoutTests/ChangeLog	2016-11-03 18:04:43 UTC (rev 208334)
+++ branches/safari-602-branch/LayoutTests/ChangeLog	2016-11-03 18:04:48 UTC (rev 208335)
@@ -1,5 +1,22 @@
 2016-11-03  Matthew Hanson  <matthew_han...@apple.com>
 
+        Merge r208314. rdar://problem/29084077
+
+    2016-11-02  Simon Fraser  <simon.fra...@apple.com>
+
+            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-03  Matthew Hanson  <matthew_han...@apple.com>
+
         Merge r208101. rdar://problem/29053206
 
     2016-10-29  Youenn Fablet  <you...@apple.com>

Added: branches/safari-602-branch/LayoutTests/animations/stacking-during-opacity-animation-expected.txt (0 => 208335)


--- branches/safari-602-branch/LayoutTests/animations/stacking-during-opacity-animation-expected.txt	                        (rev 0)
+++ branches/safari-602-branch/LayoutTests/animations/stacking-during-opacity-animation-expected.txt	2016-11-03 18:04:48 UTC (rev 208335)
@@ -0,0 +1 @@
+This test should not crash or assert.

Added: branches/safari-602-branch/LayoutTests/animations/stacking-during-opacity-animation.html (0 => 208335)


--- branches/safari-602-branch/LayoutTests/animations/stacking-during-opacity-animation.html	                        (rev 0)
+++ branches/safari-602-branch/LayoutTests/animations/stacking-during-opacity-animation.html	2016-11-03 18:04:48 UTC (rev 208335)
@@ -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: branches/safari-602-branch/Source/WebCore/ChangeLog (208334 => 208335)


--- branches/safari-602-branch/Source/WebCore/ChangeLog	2016-11-03 18:04:43 UTC (rev 208334)
+++ branches/safari-602-branch/Source/WebCore/ChangeLog	2016-11-03 18:04:48 UTC (rev 208335)
@@ -1,5 +1,50 @@
 2016-11-03  Matthew Hanson  <matthew_han...@apple.com>
 
+        Merge r208314. rdar://problem/29084077
+
+    2016-11-02  Simon Fraser  <simon.fra...@apple.com>
+
+            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-03  Matthew Hanson  <matthew_han...@apple.com>
+
         Merge r208307. rdar://problem/29078457
 
     2016-11-02  David Kilzer  <ddkil...@apple.com>

Modified: branches/safari-602-branch/Source/WebCore/page/animation/AnimationBase.h (208334 => 208335)


--- branches/safari-602-branch/Source/WebCore/page/animation/AnimationBase.h	2016-11-03 18:04:43 UTC (rev 208334)
+++ branches/safari-602-branch/Source/WebCore/page/animation/AnimationBase.h	2016-11-03 18:04:48 UTC (rev 208335)
@@ -138,7 +138,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: branches/safari-602-branch/Source/WebCore/page/animation/CompositeAnimation.cpp (208334 => 208335)


--- branches/safari-602-branch/Source/WebCore/page/animation/CompositeAnimation.cpp	2016-11-03 18:04:43 UTC (rev 208334)
+++ branches/safari-602-branch/Source/WebCore/page/animation/CompositeAnimation.cpp	2016-11-03 18:04:48 UTC (rev 208335)
@@ -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: branches/safari-602-branch/Source/WebCore/page/animation/ImplicitAnimation.cpp (208334 => 208335)


--- branches/safari-602-branch/Source/WebCore/page/animation/ImplicitAnimation.cpp	2016-11-03 18:04:43 UTC (rev 208334)
+++ branches/safari-602-branch/Source/WebCore/page/animation/ImplicitAnimation.cpp	2016-11-03 18:04:48 UTC (rev 208335)
@@ -60,7 +60,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.
@@ -92,6 +92,8 @@
 
     // Fire the start timeout if needed
     fireAnimationEventsIfNeeded();
+    
+    didBlendStyle = true;
     return state() != oldState;
 }
 

Modified: branches/safari-602-branch/Source/WebCore/page/animation/ImplicitAnimation.h (208334 => 208335)


--- branches/safari-602-branch/Source/WebCore/page/animation/ImplicitAnimation.h	2016-11-03 18:04:43 UTC (rev 208334)
+++ branches/safari-602-branch/Source/WebCore/page/animation/ImplicitAnimation.h	2016-11-03 18:04:48 UTC (rev 208335)
@@ -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: branches/safari-602-branch/Source/WebCore/page/animation/KeyframeAnimation.cpp (208334 => 208335)


--- branches/safari-602-branch/Source/WebCore/page/animation/KeyframeAnimation.cpp	2016-11-03 18:04:43 UTC (rev 208334)
+++ branches/safari-602-branch/Source/WebCore/page/animation/KeyframeAnimation.cpp	2016-11-03 18:04:48 UTC (rev 208335)
@@ -125,7 +125,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();
@@ -186,6 +186,7 @@
             animatedStyle->setIsRunningAcceleratedAnimation();
     }
     
+    didBlendStyle = true;
     return state() != oldState;
 }
 

Modified: branches/safari-602-branch/Source/WebCore/page/animation/KeyframeAnimation.h (208334 => 208335)


--- branches/safari-602-branch/Source/WebCore/page/animation/KeyframeAnimation.h	2016-11-03 18:04:43 UTC (rev 208334)
+++ branches/safari-602-branch/Source/WebCore/page/animation/KeyframeAnimation.h	2016-11-03 18:04:48 UTC (rev 208335)
@@ -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: branches/safari-602-branch/Source/WebCore/platform/graphics/GraphicsContext.cpp (208334 => 208335)


--- branches/safari-602-branch/Source/WebCore/platform/graphics/GraphicsContext.cpp	2016-11-03 18:04:43 UTC (rev 208334)
+++ branches/safari-602-branch/Source/WebCore/platform/graphics/GraphicsContext.cpp	2016-11-03 18:04:48 UTC (rev 208335)
@@ -367,6 +367,7 @@
         LOG_ERROR("ERROR void GraphicsContext::restore() stack is empty");
         return;
     }
+
     m_state = m_stack.last();
     m_stack.removeLast();
 

Modified: branches/safari-602-branch/Source/WebCore/platform/graphics/ca/cocoa/PlatformCALayerCocoa.mm (208334 => 208335)


--- branches/safari-602-branch/Source/WebCore/platform/graphics/ca/cocoa/PlatformCALayerCocoa.mm	2016-11-03 18:04:43 UTC (rev 208334)
+++ branches/safari-602-branch/Source/WebCore/platform/graphics/ca/cocoa/PlatformCALayerCocoa.mm	2016-11-03 18:04:48 UTC (rev 208335)
@@ -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: branches/safari-602-branch/Source/WebCore/rendering/RenderLayer.cpp (208334 => 208335)


--- branches/safari-602-branch/Source/WebCore/rendering/RenderLayer.cpp	2016-11-03 18:04:43 UTC (rev 208334)
+++ branches/safari-602-branch/Source/WebCore/rendering/RenderLayer.cpp	2016-11-03 18:04:48 UTC (rev 208335)
@@ -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);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to