Changeset 254582 in webkit


Ignore:
Timestamp:
Jan 15, 2020 11:14:26 AM (4 years ago)
Author:
Alan Coon
Message:

Cherry-pick r254054. rdar://problem/58549108

REGRESSION (r252724): Unable to tap on play button on google video 'See the top search trends of 2019'
​https://bugs.webkit.org/show_bug.cgi?id=205694
<rdar://problem/58062987>

Reviewed by Zalan Bujtas.

Source/WebCore:

After r252724, which separated 'used' from 'specified' z-index in style, we need to copy
the specified to the used z-index in animated styles, while preserving the existing 'forceStackingContext'
behavior which set the used z-index to 0.

Do so by creating Adjuster::adjustAnimatedStyle(), which is called from TreeResolver::createAnimatedElementUpdate()
if any animations could have affected the style. We need to pass back information about whether the animation should
force stacking context.

Test: animations/z-index-in-keyframe.html

  • animation/KeyframeEffect.cpp: (WebCore::KeyframeEffect::apply):
  • animation/KeyframeEffect.h: (WebCore::KeyframeEffect::triggersStackingContext const):
  • dom/Element.cpp: (WebCore::Element::applyKeyframeEffects):
  • dom/Element.h:
  • page/animation/CSSAnimationController.h: (): Deleted.
  • page/animation/CompositeAnimation.cpp: (WebCore::CompositeAnimation::animate):
  • style/StyleAdjuster.cpp: (WebCore::Style::Adjuster::adjustAnimatedStyle):
  • style/StyleAdjuster.h:
  • style/StyleTreeResolver.cpp: (WebCore::Style::TreeResolver::createAnimatedElementUpdate):

LayoutTests:

  • animations/z-index-in-keyframe-expected.html: Added.
  • animations/z-index-in-keyframe.html: Added.

git-svn-id: ​https://svn.webkit.org/repository/webkit/trunk@254054 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Location:
branches/safari-609-branch
Files:
2 added
12 edited

Legend:

Unmodified
Added
Removed

  • branches/safari-609-branch/LayoutTests/ChangeLog

    r254475 r254582  
     12020-01-14  Alan Coon  <alancoon@apple.com>
     2
     3        Cherry-pick r254054. rdar://problem/58549108
     4
     5    REGRESSION (r252724): Unable to tap on play button on google video 'See the top search trends of 2019'
     6    https://bugs.webkit.org/show_bug.cgi?id=205694
     7    <rdar://problem/58062987>
     8   
     9    Reviewed by Zalan Bujtas.
     10   
     11    Source/WebCore:
     12   
     13    After r252724, which separated 'used' from 'specified' z-index in style, we need to copy
     14    the specified to the used z-index in animated styles, while preserving the existing 'forceStackingContext'
     15    behavior which set the used z-index to 0.
     16   
     17    Do so by creating Adjuster::adjustAnimatedStyle(), which is called from TreeResolver::createAnimatedElementUpdate()
     18    if any animations could have affected the style. We need to pass back information about whether the animation should
     19    force stacking context.
     20   
     21    Test: animations/z-index-in-keyframe.html
     22   
     23    * animation/KeyframeEffect.cpp:
     24    (WebCore::KeyframeEffect::apply):
     25    * animation/KeyframeEffect.h:
     26    (WebCore::KeyframeEffect::triggersStackingContext const):
     27    * dom/Element.cpp:
     28    (WebCore::Element::applyKeyframeEffects):
     29    * dom/Element.h:
     30    * page/animation/CSSAnimationController.h:
     31    (): Deleted.
     32    * page/animation/CompositeAnimation.cpp:
     33    (WebCore::CompositeAnimation::animate):
     34    * style/StyleAdjuster.cpp:
     35    (WebCore::Style::Adjuster::adjustAnimatedStyle):
     36    * style/StyleAdjuster.h:
     37    * style/StyleTreeResolver.cpp:
     38    (WebCore::Style::TreeResolver::createAnimatedElementUpdate):
     39   
     40    LayoutTests:
     41   
     42    * animations/z-index-in-keyframe-expected.html: Added.
     43    * animations/z-index-in-keyframe.html: Added.
     44   
     45    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@254054 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     46
     47    2020-01-05  Simon Fraser  <simon.fraser@apple.com>
     48
     49            REGRESSION (r252724): Unable to tap on play button on google video 'See the top search trends of 2019'
     50            https://bugs.webkit.org/show_bug.cgi?id=205694
     51            <rdar://problem/58062987>
     52
     53            Reviewed by Zalan Bujtas.
     54
     55            * animations/z-index-in-keyframe-expected.html: Added.
     56            * animations/z-index-in-keyframe.html: Added.
     57
    1582020-01-13  Alan Coon  <alancoon@apple.com>
    259

  • branches/safari-609-branch/Source/WebCore/ChangeLog

    r254475 r254582  
     12020-01-14  Alan Coon  <alancoon@apple.com>
     2
     3        Cherry-pick r254054. rdar://problem/58549108
     4
     5    REGRESSION (r252724): Unable to tap on play button on google video 'See the top search trends of 2019'
     6    https://bugs.webkit.org/show_bug.cgi?id=205694
     7    <rdar://problem/58062987>
     8   
     9    Reviewed by Zalan Bujtas.
     10   
     11    Source/WebCore:
     12   
     13    After r252724, which separated 'used' from 'specified' z-index in style, we need to copy
     14    the specified to the used z-index in animated styles, while preserving the existing 'forceStackingContext'
     15    behavior which set the used z-index to 0.
     16   
     17    Do so by creating Adjuster::adjustAnimatedStyle(), which is called from TreeResolver::createAnimatedElementUpdate()
     18    if any animations could have affected the style. We need to pass back information about whether the animation should
     19    force stacking context.
     20   
     21    Test: animations/z-index-in-keyframe.html
     22   
     23    * animation/KeyframeEffect.cpp:
     24    (WebCore::KeyframeEffect::apply):
     25    * animation/KeyframeEffect.h:
     26    (WebCore::KeyframeEffect::triggersStackingContext const):
     27    * dom/Element.cpp:
     28    (WebCore::Element::applyKeyframeEffects):
     29    * dom/Element.h:
     30    * page/animation/CSSAnimationController.h:
     31    (): Deleted.
     32    * page/animation/CompositeAnimation.cpp:
     33    (WebCore::CompositeAnimation::animate):
     34    * style/StyleAdjuster.cpp:
     35    (WebCore::Style::Adjuster::adjustAnimatedStyle):
     36    * style/StyleAdjuster.h:
     37    * style/StyleTreeResolver.cpp:
     38    (WebCore::Style::TreeResolver::createAnimatedElementUpdate):
     39   
     40    LayoutTests:
     41   
     42    * animations/z-index-in-keyframe-expected.html: Added.
     43    * animations/z-index-in-keyframe.html: Added.
     44   
     45    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@254054 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     46
     47    2020-01-05  Simon Fraser  <simon.fraser@apple.com>
     48
     49            REGRESSION (r252724): Unable to tap on play button on google video 'See the top search trends of 2019'
     50            https://bugs.webkit.org/show_bug.cgi?id=205694
     51            <rdar://problem/58062987>
     52
     53            Reviewed by Zalan Bujtas.
     54
     55            After r252724, which separated 'used' from 'specified' z-index in style, we need to copy
     56            the specified to the used z-index in animated styles, while preserving the existing 'forceStackingContext'
     57            behavior which set the used z-index to 0.
     58
     59            Do so by creating Adjuster::adjustAnimatedStyle(), which is called from TreeResolver::createAnimatedElementUpdate()
     60            if any animations could have affected the style. We need to pass back information about whether the animation should
     61            force stacking context.
     62
     63            Test: animations/z-index-in-keyframe.html
     64
     65            * animation/KeyframeEffect.cpp:
     66            (WebCore::KeyframeEffect::apply):
     67            * animation/KeyframeEffect.h:
     68            (WebCore::KeyframeEffect::triggersStackingContext const):
     69            * dom/Element.cpp:
     70            (WebCore::Element::applyKeyframeEffects):
     71            * dom/Element.h:
     72            * page/animation/CSSAnimationController.h:
     73            (): Deleted.
     74            * page/animation/CompositeAnimation.cpp:
     75            (WebCore::CompositeAnimation::animate):
     76            * style/StyleAdjuster.cpp:
     77            (WebCore::Style::Adjuster::adjustAnimatedStyle):
     78            * style/StyleAdjuster.h:
     79            * style/StyleTreeResolver.cpp:
     80            (WebCore::Style::TreeResolver::createAnimatedElementUpdate):
     81
    1822020-01-13  Alan Coon  <alancoon@apple.com>
    283

  • branches/safari-609-branch/Source/WebCore/animation/KeyframeEffect.cpp

    r253728 r254582  
    5151#include "RenderStyle.h"
    5252#include "RuntimeEnabledFeatures.h"
     53#include "StyleAdjuster.h"
    5354#include "StylePendingResources.h"
    5455#include "StyleResolver.h"
    … …  
    10911092
    10921093    setAnimatedPropertiesInStyle(targetStyle, computedTiming.progress.value());
    1093 
    1094     // https://w3c.github.io/web-animations/#side-effects-section
    1095     // For every property targeted by at least one animation effect that is current or in effect, the user agent
    1096     // must act as if the will-change property ([css-will-change-1]) on the target element includes the property.
    1097     if (m_triggersStackingContext && targetStyle.hasAutoUsedZIndex())
    1098         targetStyle.setUsedZIndex(0);
    10991094}
    11001095

  • branches/safari-609-branch/Source/WebCore/animation/KeyframeEffect.h

    r253728 r254582  
    127127    const RenderStyle& currentStyle() const override;
    128128    bool isAccelerated() const { return m_shouldRunAccelerated; }
     129    bool triggersStackingContext() const { return m_triggersStackingContext; }
    129130    bool filterFunctionListsMatch() const override { return m_filterFunctionListsMatch; }
    130131    bool transformFunctionListsMatch() const override { return m_transformFunctionListsMatch; }

  • branches/safari-609-branch/Source/WebCore/dom/Element.cpp

    r253923 r254582  
    37253725}
    37263726
    3727 bool Element::applyKeyframeEffects(RenderStyle& targetStyle)
    3728 {
    3729     bool hasNonAcceleratedAnimationProperty = false;
     3727OptionSet<AnimationImpact> Element::applyKeyframeEffects(RenderStyle& targetStyle)
     3728{
     3729    OptionSet<AnimationImpact> impact;
    37303730
    37313731    for (const auto& effect : ensureKeyframeEffectStack().sortedEffects()) {
    … …  
    37333733        effect->animation()->resolve(targetStyle);
    37343734
    3735         if (!hasNonAcceleratedAnimationProperty && !effect->isAccelerated())
    3736             hasNonAcceleratedAnimationProperty = true;
    3737     }
    3738 
    3739     return !hasNonAcceleratedAnimationProperty;
     3735        if (effect->isAccelerated())
     3736            impact.add(AnimationImpact::RequiresRecomposite);
     3737
     3738        if (effect->triggersStackingContext())
     3739            impact.add(AnimationImpact::ForcesStackingContext);
     3740    }
     3741
     3742    return impact;
    37403743}
    37413744

  • branches/safari-609-branch/Source/WebCore/dom/Element.h

    r253923 r254582  
    7474}
    7575
     76enum class AnimationImpact;
     77
    7678class Element : public ContainerNode {
    7779    WTF_MAKE_ISO_ALLOCATED(Element);
    … …  
    481483    KeyframeEffectStack& ensureKeyframeEffectStack();
    482484    bool hasKeyframeEffects() const;
    483     bool applyKeyframeEffects(RenderStyle&);
     485    OptionSet<AnimationImpact> applyKeyframeEffects(RenderStyle&);
    484486
    485487#if ENABLE(FULLSCREEN_API)

  • branches/safari-609-branch/Source/WebCore/page/animation/AnimationBase.h

    r252554 r254582  
    5050};
    5151
     52enum class AnimationImpact {
     53    RequiresRecomposite     = 1 << 0,
     54    ForcesStackingContext   = 1 << 1
     55};
     56
    5257class AnimationBase : public RefCounted<AnimationBase>
    5358    , public CSSPropertyBlendingClient {

  • branches/safari-609-branch/Source/WebCore/page/animation/CSSAnimationController.h

    r246490 r254582  
    3131#include "AnimationBase.h"
    3232#include "CSSPropertyNames.h"
     33#include "Element.h"
    3334#include "RenderStyle.h"
    3435#include <wtf/Forward.h>
    … …  
    3839class CSSAnimationControllerPrivate;
    3940class Document;
    40 class Element;
    4141class Frame;
    4242class LayoutRect;
    … …  
    4545struct AnimationUpdate {
    4646    std::unique_ptr<RenderStyle> style;
    47     bool animationChangeRequiresRecomposite { false };
     47    OptionSet<AnimationImpact> impact;
     48
    4849};
    4950

  • branches/safari-609-branch/Source/WebCore/page/animation/CompositeAnimation.cpp

    r252724 r254582  
    3838#include "RenderElement.h"
    3939#include "RenderStyle.h"
     40#include "StyleAdjuster.h"
    4041#include <wtf/NeverDestroyed.h>
    4142#include <wtf/text/CString.h>
    … …  
    287288    m_keyframeAnimations.checkConsistency();
    288289
    289     bool animationChangeRequiresRecomposite = false;
    290     bool forceStackingContext = false;
     290    OptionSet<AnimationImpact> imapct;
    291291
    292292    std::unique_ptr<RenderStyle> animatedStyle;
    … …  
    301301                checkForStackingContext |= WillChangeData::propertyCreatesStackingContext(transition->animatingProperty());
    302302
    303             animationChangeRequiresRecomposite = changes.contains(AnimateChange::RunningStateChange) && transition->affectsAcceleratedProperty();
     303            if (changes.contains(AnimateChange::RunningStateChange) && transition->affectsAcceleratedProperty())
     304                imapct.add(AnimationImpact::RequiresRecomposite);
    304305        }
    305306
    … …  
    317318#endif
    318319                )
    319             forceStackingContext = true;
     320            imapct.add(AnimationImpact::ForcesStackingContext);
    320321        }
    321322    }
    … …  
    327328        if (keyframeAnim) {
    328329            auto changes = keyframeAnim->animate(*this, targetStyle, animatedStyle);
    329             animationChangeRequiresRecomposite = changes.contains(AnimateChange::RunningStateChange) && keyframeAnim->affectsAcceleratedProperty();
    330             forceStackingContext |= changes.contains(AnimateChange::StyleBlended) && keyframeAnim->triggersStackingContext();
     330            if (changes.contains(AnimateChange::RunningStateChange) && keyframeAnim->affectsAcceleratedProperty())
     331                imapct.add(AnimationImpact::RequiresRecomposite);
     332
     333            if (changes.contains(AnimateChange::StyleBlended) && keyframeAnim->triggersStackingContext())
     334                imapct.add(AnimationImpact::ForcesStackingContext);
     335
    331336            m_hasAnimationThatDependsOnLayout |= keyframeAnim->dependsOnLayout();
    332337        }
    333338    }
    334339
    335     // https://drafts.csswg.org/css-animations-1/
    336     // While an animation is applied but has not finished, or has finished but has an animation-fill-mode of forwards or both,
    337     // the user agent must act as if the will-change property ([css-will-change-1]) on the element additionally
    338     // includes all the properties animated by the animation.
    339     if (forceStackingContext && animatedStyle) {
    340         if (animatedStyle->hasAutoUsedZIndex())
    341             animatedStyle->setUsedZIndex(0);
    342     }
    343 
    344     return { WTFMove(animatedStyle), animationChangeRequiresRecomposite };
     340    return { WTFMove(animatedStyle), imapct };
    345341}
    346342

  • branches/safari-609-branch/Source/WebCore/style/StyleAdjuster.cpp

    r253917 r254582  
    3131#include "StyleAdjuster.h"
    3232
     33#include "AnimationBase.h"
    3334#include "CSSFontSelector.h"
    3435#include "Element.h"
    … …  
    4344#include "Page.h"
    4445#include "Quirks.h"
     46#include "RenderBox.h"
    4547#include "RenderStyle.h"
    4648#include "RenderTheme.h"
    … …  
    525527}
    526528
     529void Adjuster::adjustAnimatedStyle(RenderStyle& style, const RenderStyle* parentBoxStyle, OptionSet<AnimationImpact> impact)
     530{
     531    // Set an explicit used z-index in two cases:
     532    // 1. When the element respects z-index, and the style has an explicit z-index set (for example, the animation
     533    //    itself may animate z-index).
     534    // 2. When we want the stacking context side-effets of explicit z-index, via forceStackingContext.
     535    // It's important to not clobber an existing used z-index, since an earlier animation may have set it, but we
     536    // may still need to update the used z-index value from the specified value.
     537    bool elementRespectsZIndex = style.position() != PositionType::Static || (parentBoxStyle && parentBoxStyle->isDisplayFlexibleOrGridBox());
     538
     539    if (elementRespectsZIndex && !style.hasAutoSpecifiedZIndex())
     540        style.setUsedZIndex(style.specifiedZIndex());
     541    else if (impact.contains(AnimationImpact::ForcesStackingContext))
     542        style.setUsedZIndex(0);
     543}
     544
    527545void Adjuster::adjustForSiteSpecificQuirks(RenderStyle& style) const
    528546{

  • branches/safari-609-branch/Source/WebCore/style/StyleAdjuster.h

    r252599 r254582  
    4343
    4444    static void adjustSVGElementStyle(RenderStyle&, const SVGElement&);
     45    static void adjustAnimatedStyle(RenderStyle&, const RenderStyle* parentBoxStyle, OptionSet<AnimationImpact>);
     46
    4547#if ENABLE(TEXT_AUTOSIZING)
    4648    static bool adjustForTextAutosizing(RenderStyle&, const Element&);

  • branches/safari-609-branch/Source/WebCore/style/StyleTreeResolver.cpp

    r253987 r254582  
    5050#include "Settings.h"
    5151#include "ShadowRoot.h"
     52#include "StyleAdjuster.h"
    5253#include "StyleFontSizeFunctions.h"
    5354#include "StyleResolver.h"
    … …  
    303304{
    304305    auto* oldStyle = element.renderOrDisplayContentsStyle();
    305 
    306     bool shouldRecompositeLayer = false;
     306   
     307    OptionSet<AnimationImpact> animationImpact;
    307308
    308309    // New code path for CSS Animations and CSS Transitions.
    … …  
    324325    if (element.hasKeyframeEffects()) {
    325326        auto animatedStyle = RenderStyle::clonePtr(*newStyle);
    326         shouldRecompositeLayer = element.applyKeyframeEffects(*animatedStyle);
     327        animationImpact = element.applyKeyframeEffects(*animatedStyle);
    327328        newStyle = WTFMove(animatedStyle);
    328329    }
    … …  
    333334
    334335        auto animationUpdate = animationController.updateAnimations(element, *newStyle, oldStyle);
    335         shouldRecompositeLayer = animationUpdate.animationChangeRequiresRecomposite;
     336        animationImpact.add(animationUpdate.impact);
    336337
    337338        if (animationUpdate.style)
    338339            newStyle = WTFMove(animationUpdate.style);
    339340    }
     341
     342    if (animationImpact)
     343        Adjuster::adjustAnimatedStyle(*newStyle, parentBoxStyle(), animationImpact);
    340344
    341345    auto change = oldStyle ? determineChange(*oldStyle, *newStyle) : Detach;
    … …  
    345349        change = Detach;
    346350
    347     shouldRecompositeLayer |= element.styleResolutionShouldRecompositeLayer();
    348 
     351    bool shouldRecompositeLayer = animationImpact.contains(AnimationImpact::RequiresRecomposite) || element.styleResolutionShouldRecompositeLayer();
    349352    return { WTFMove(newStyle), change, shouldRecompositeLayer };
    350353}
Note: See TracChangeset for help on using the changeset viewer.