Changeset 208314 in webkit


Ignore:
Timestamp:
Nov 2, 2016, 5:19:54 PM (9 years ago)
Author:
Simon Fraser
Message:

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.

Source/WebCore:

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.

LayoutTests:

Test was reduced from webkit.org.

  • animations/stacking-during-opacity-animation-expected.txt: Added.
  • animations/stacking-during-opacity-animation.html: Added.
Location:
trunk
Files:
2 added
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r208313 r208314  
     12016-11-02  Simon Fraser  <simon.fraser@apple.com>
     2
     3        REGRESSION (r208025) GraphicsContext state stack assertions loading webkit.org
     4        https://bugs.webkit.org/show_bug.cgi?id=164350
     5        rdar://problem/29053414
     6
     7        Reviewed by Dean Jackson.
     8
     9        Test was reduced from webkit.org.
     10
     11        * animations/stacking-during-opacity-animation-expected.txt: Added.
     12        * animations/stacking-during-opacity-animation.html: Added.
     13
    1142016-11-02  Myles C. Maxfield  <mmaxfield@apple.com>
    215

  • trunk/Source/WebCore/ChangeLog

    r208313 r208314  
     12016-11-02  Simon Fraser  <simon.fraser@apple.com>
     2
     3        REGRESSION (r208025) GraphicsContext state stack assertions loading webkit.org
     4        https://bugs.webkit.org/show_bug.cgi?id=164350
     5        rdar://problem/29053414
     6
     7        Reviewed by Dean Jackson.
     8
     9        After r208025 it as possible for KeyframeAnimation::animate() to produce a RenderStyle
     10        with a non-1 opacity, but without the explicit z-index that triggers stacking context.
     11        This confused the RenderLayer paintWithTransparency code, triggering mismsatched GraphicsContext
     12        save/restores.
     13
     14        This occurred when the runningOrFillingForwards state was mis-computed. keyframeAnim->animate()
     15        can spit out a new style when in the StartWaitTimer sometimes, so "!keyframeAnim->waitingToStart() && !keyframeAnim->postActive()"
     16        gave the wrong answser.
     17
     18        Rather than depend on the super-confusing animation state, use a bool out param from animate() to say
     19        when it actually produced a new style, and when true, do the setZIndex(0).
     20
     21        Test: animations/stacking-during-opacity-animation.html
     22
     23        * page/animation/AnimationBase.h:
     24        * page/animation/CSSPropertyAnimation.cpp:
     25        (WebCore::CSSPropertyAnimation::blendProperties): Log after blending so the log shows the blended style.
     26        * page/animation/CompositeAnimation.cpp:
     27        (WebCore::CompositeAnimation::animate):
     28        * page/animation/ImplicitAnimation.cpp:
     29        (WebCore::ImplicitAnimation::animate):
     30        * page/animation/ImplicitAnimation.h:
     31        * page/animation/KeyframeAnimation.cpp:
     32        (WebCore::KeyframeAnimation::animate):
     33        * page/animation/KeyframeAnimation.h:
     34        * platform/graphics/GraphicsContext.cpp:
     35        (WebCore::GraphicsContext::restore):
     36        * platform/graphics/ca/cocoa/PlatformCALayerCocoa.mm:
     37        (PlatformCALayer::drawLayerContents): No functional change, but created scope for the
     38        GraphicsContext so that it didn't outlive the CGContextRestoreGState(context).
     39        * rendering/RenderLayer.cpp:
     40        (WebCore::RenderLayer::beginTransparencyLayers): New assertion that catches the problem earlier.
     41
    1422016-11-02  Myles C. Maxfield  <mmaxfield@apple.com>
    243

  • trunk/Source/WebCore/page/animation/AnimationBase.h

    r204466 r208314  
    137137
    138138    // Returns true if the animation state changed.
    139     virtual bool animate(CompositeAnimation*, RenderElement*, const RenderStyle* /*currentStyle*/, const RenderStyle* /*targetStyle*/, std::unique_ptr<RenderStyle>& /*animatedStyle*/) = 0;
     139    virtual bool animate(CompositeAnimation*, RenderElement*, const RenderStyle* /*currentStyle*/, const RenderStyle* /*targetStyle*/, std::unique_ptr<RenderStyle>& /*animatedStyle*/, bool& didBlendStyle) = 0;
    140140    virtual void getAnimatedStyle(std::unique_ptr<RenderStyle>& /*animatedStyle*/) = 0;
    141141

  • trunk/Source/WebCore/page/animation/CSSPropertyAnimation.cpp

    r208025 r208314  
    15771577    AnimationPropertyWrapperBase* wrapper = CSSPropertyAnimationWrapperMap::singleton().wrapperForProperty(prop);
    15781578    if (wrapper) {
     1579        wrapper->blend(anim, dst, a, b, progress);
    15791580#if !LOG_DISABLED
    15801581        wrapper->logBlend(a, b, dst, progress);
    15811582#endif
    1582         wrapper->blend(anim, dst, a, b, progress);
    15831583        return !wrapper->animationIsAccelerated() || !anim->isAccelerated();
    15841584    }

  • trunk/Source/WebCore/page/animation/CompositeAnimation.cpp

    r208025 r208314  
    300300        bool checkForStackingContext = false;
    301301        for (auto& transition : m_transitions.values()) {
    302             if (transition->animate(this, &renderer, currentStyle, &targetStyle, blendedStyle))
     302            bool didBlendStyle = false;
     303            if (transition->animate(this, &renderer, currentStyle, &targetStyle, blendedStyle, didBlendStyle))
    303304                animationStateChanged = true;
    304305
    305             checkForStackingContext |= WillChangeData::propertyCreatesStackingContext(transition->animatingProperty());
     306            if (didBlendStyle)
     307                checkForStackingContext |= WillChangeData::propertyCreatesStackingContext(transition->animatingProperty());
    306308        }
    307309
    … …  
    328330        RefPtr<KeyframeAnimation> keyframeAnim = m_keyframeAnimations.get(name);
    329331        if (keyframeAnim) {
    330             if (keyframeAnim->animate(this, &renderer, currentStyle, &targetStyle, blendedStyle))
     332            bool didBlendStyle = false;
     333            if (keyframeAnim->animate(this, &renderer, currentStyle, &targetStyle, blendedStyle, didBlendStyle))
    331334                animationStateChanged = true;
    332335
    333             bool runningOrFillingForwards = !keyframeAnim->waitingToStart() && !keyframeAnim->postActive();
    334             forceStackingContext |= runningOrFillingForwards && keyframeAnim->triggersStackingContext();
     336            forceStackingContext |= didBlendStyle && keyframeAnim->triggersStackingContext();
    335337        }
    336338    }

  • trunk/Source/WebCore/page/animation/ImplicitAnimation.cpp

    r208033 r208314  
    6262}
    6363
    64 bool ImplicitAnimation::animate(CompositeAnimation*, RenderElement*, const RenderStyle*, const RenderStyle* targetStyle, std::unique_ptr<RenderStyle>& animatedStyle)
     64bool ImplicitAnimation::animate(CompositeAnimation*, RenderElement*, const RenderStyle*, const RenderStyle* targetStyle, std::unique_ptr<RenderStyle>& animatedStyle, bool& didBlendStyle)
    6565{
    6666    // If we get this far and the animation is done, it means we are cleaning up a just finished animation.
    … …  
    8686    // Fire the start timeout if needed
    8787    fireAnimationEventsIfNeeded();
     88   
     89    didBlendStyle = true;
    8890    return state() != oldState;
    8991}

  • trunk/Source/WebCore/page/animation/ImplicitAnimation.h

    r200098 r208314  
    5555    void endAnimation() override;
    5656
    57     bool animate(CompositeAnimation*, RenderElement*, const RenderStyle* currentStyle, const RenderStyle* targetStyle, std::unique_ptr<RenderStyle>& animatedStyle) override;
     57    bool animate(CompositeAnimation*, RenderElement*, const RenderStyle* currentStyle, const RenderStyle* targetStyle, std::unique_ptr<RenderStyle>& animatedStyle, bool& didBlendStyle) override;
    5858    void getAnimatedStyle(std::unique_ptr<RenderStyle>& animatedStyle) override;
    5959    void reset(const RenderStyle* to);

  • trunk/Source/WebCore/page/animation/KeyframeAnimation.cpp

    r208033 r208314  
    125125}
    126126
    127 bool KeyframeAnimation::animate(CompositeAnimation* compositeAnimation, RenderElement*, const RenderStyle*, const RenderStyle* targetStyle, std::unique_ptr<RenderStyle>& animatedStyle)
     127bool KeyframeAnimation::animate(CompositeAnimation* compositeAnimation, RenderElement*, const RenderStyle*, const RenderStyle* targetStyle, std::unique_ptr<RenderStyle>& animatedStyle, bool& didBlendStyle)
    128128{
    129129    // Fire the start timeout if needed
    … …  
    180180    }
    181181   
     182    didBlendStyle = true;
    182183    return state() != oldState;
    183184}

  • trunk/Source/WebCore/page/animation/KeyframeAnimation.h

    r208025 r208314  
    4646    }
    4747
    48     bool animate(CompositeAnimation*, RenderElement*, const RenderStyle* currentStyle, const RenderStyle* targetStyle, std::unique_ptr<RenderStyle>& animatedStyle) override;
     48    bool animate(CompositeAnimation*, RenderElement*, const RenderStyle* currentStyle, const RenderStyle* targetStyle, std::unique_ptr<RenderStyle>& animatedStyle, bool& didBlendStyle) override;
    4949    void getAnimatedStyle(std::unique_ptr<RenderStyle>&) override;
    5050

  • trunk/Source/WebCore/platform/graphics/GraphicsContext.cpp

    r207020 r208314  
    368368        return;
    369369    }
     370
    370371    m_state = m_stack.last();
    371372    m_stack.removeLast();

  • trunk/Source/WebCore/platform/graphics/ca/cocoa/PlatformCALayerCocoa.mm

    r202858 r208314  
    10681068#endif
    10691069   
    1070     GraphicsContext graphicsContext(context);
    1071     graphicsContext.setIsCALayerContext(true);
    1072     graphicsContext.setIsAcceleratedContext(platformCALayer->acceleratesDrawing());
    1073    
    1074     if (!layerContents->platformCALayerContentsOpaque()) {
    1075         // Turn off font smoothing to improve the appearance of text rendered onto a transparent background.
    1076         graphicsContext.setShouldSmoothFonts(false);
    1077     }
    1078    
     1070    {
     1071        GraphicsContext graphicsContext(context);
     1072        graphicsContext.setIsCALayerContext(true);
     1073        graphicsContext.setIsAcceleratedContext(platformCALayer->acceleratesDrawing());
     1074       
     1075        if (!layerContents->platformCALayerContentsOpaque()) {
     1076            // Turn off font smoothing to improve the appearance of text rendered onto a transparent background.
     1077            graphicsContext.setShouldSmoothFonts(false);
     1078        }
     1079       
    10791080#if PLATFORM(MAC)
    1080     // It's important to get the clip from the context, because it may be significantly
    1081     // smaller than the layer bounds (e.g. tiled layers)
    1082     ThemeMac::setFocusRingClipRect(CGContextGetClipBoundingBox(context));
    1083 #endif
    1084    
    1085     for (const auto& rect : dirtyRects) {
    1086         GraphicsContextStateSaver stateSaver(graphicsContext);
    1087         graphicsContext.clip(rect);
     1081        // It's important to get the clip from the context, because it may be significantly
     1082        // smaller than the layer bounds (e.g. tiled layers)
     1083        ThemeMac::setFocusRingClipRect(CGContextGetClipBoundingBox(context));
     1084#endif
    10881085       
    1089         layerContents->platformCALayerPaintContents(platformCALayer, graphicsContext, rect);
    1090     }
    1091    
     1086        for (const auto& rect : dirtyRects) {
     1087            GraphicsContextStateSaver stateSaver(graphicsContext);
     1088            graphicsContext.clip(rect);
     1089           
     1090            layerContents->platformCALayerPaintContents(platformCALayer, graphicsContext, rect);
     1091        }
     1092       
    10921093#if PLATFORM(IOS)
    1093     fontAntialiasingState.restore();
     1094        fontAntialiasingState.restore();
    10941095#else
    1095     ThemeMac::setFocusRingClipRect(FloatRect());
    1096    
    1097     [NSGraphicsContext restoreGraphicsState];
    1098 #endif
    1099    
     1096        ThemeMac::setFocusRingClipRect(FloatRect());
     1097       
     1098        [NSGraphicsContext restoreGraphicsState];
     1099#endif
     1100    }
     1101
     1102    CGContextRestoreGState(context);
     1103
    11001104    // Re-fetch the layer owner, since <rdar://problem/9125151> indicates that it might have been destroyed during painting.
    11011105    layerContents = platformCALayer->owner();
    11021106    ASSERT(layerContents);
    11031107   
    1104     CGContextRestoreGState(context);
    1105    
    11061108    // Always update the repaint count so that it's accurate even if the count itself is not shown. This will be useful
    11071109    // for the Web Inspector feeding this information through the LayerTreeAgent.
    11081110    int repaintCount = layerContents->platformCALayerIncrementRepaintCount(platformCALayer);
    1109    
     1111
    11101112    if (!platformCALayer->usesTiledBackingLayer() && layerContents && layerContents->platformCALayerShowRepaintCounter(platformCALayer))
    11111113        drawRepaintIndicator(context, platformCALayer, repaintCount, nullptr);

  • trunk/Source/WebCore/rendering/RenderLayer.cpp

    r208033 r208314  
    18101810   
    18111811    if (paintsWithTransparency(paintingInfo.paintBehavior)) {
     1812        ASSERT(isStackingContext());
    18121813        m_usedTransparency = true;
    18131814        context.save();
Note: See TracChangeset for help on using the changeset viewer.