Changeset 277112 in webkit


Ignore:
Timestamp:
May 6, 2021 1:11:19 PM (15 months ago)
Author:
graouts@webkit.org
Message:

CSS custom properties on pseudo elements background gradients causes infinite layout and high CPU load
​https://bugs.webkit.org/show_bug.cgi?id=194332
<rdar://problem/47873895>

Reviewed by Simon Fraser.

Source/WebCore:

When a background-image uses a CSS custom property the resulting StyleGeneratedImage may not be the same
object as during prior style updates. This caused transitions to be triggered for all style updates for
such background-image values. To fix this, we modify the == operator for StyleGeneratedImage to use
arePointingToEqualData() with the CSSImageGeneratorValue member and added an == operator for the
CSSImageGeneratorValue class to call into the existing equals() methods. These equals() methods
are now overrides of the virtual CSSImageGeneratorValue method.

This change in behavior required a change in RenderElement::updateFillImages() to not only check whether
the images were identical, but to also check whether the renderer was registered as a client on the new
images. To do this, we add a new virtual hasClient() method on StyleImage.

Test: webanimations/css-transition-element-with-gradient-background-image-and-css-custom-property.html

  • css/CSSImageGeneratorValue.cpp:

(WebCore::CSSImageGeneratorValue::operator== const):

  • css/CSSImageGeneratorValue.h:
  • rendering/RenderElement.cpp:

(WebCore::RenderElement::updateFillImages):

  • rendering/style/FillLayer.cpp:

(WebCore::FillLayer::imagesIdentical): Deleted.

  • rendering/style/FillLayer.h:
  • rendering/style/StyleCachedImage.cpp:

(WebCore::StyleCachedImage::hasClient const):

  • rendering/style/StyleCachedImage.h:
  • rendering/style/StyleGeneratedImage.cpp:

(WebCore::StyleGeneratedImage::operator== const):
(WebCore::StyleGeneratedImage::hasClient const):

  • rendering/style/StyleGeneratedImage.h:
  • rendering/style/StyleImage.h:
  • rendering/style/StyleMultiImage.cpp:

(WebCore::StyleMultiImage::hasClient const):

  • rendering/style/StyleMultiImage.h:

LayoutTests:

Add a test where an element with a background-image set to a CSS gradient using a custom property as a color
stop changes another property targeted by a transition to check that there is no background-image transition
generated.

  • webanimations/css-transition-element-with-gradient-background-image-and-css-custom-property-expected.txt: Added.
  • webanimations/css-transition-element-with-gradient-background-image-and-css-custom-property.html: Added.
Location:
trunk
Files:
2 added
14 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r277098 r277112  
     12021-05-06  Antoine Quint  <graouts@webkit.org>
     2
     3        CSS custom properties on pseudo elements background gradients causes infinite layout and high CPU load
     4        https://bugs.webkit.org/show_bug.cgi?id=194332
     5        <rdar://problem/47873895>
     6
     7        Reviewed by Simon Fraser.
     8
     9        Add a test where an element with a background-image set to a CSS gradient using a custom property as a color
     10        stop changes another property targeted by a transition to check that there is no background-image transition
     11        generated.
     12
     13        * webanimations/css-transition-element-with-gradient-background-image-and-css-custom-property-expected.txt: Added.
     14        * webanimations/css-transition-element-with-gradient-background-image-and-css-custom-property.html: Added.
     15
    1162021-05-06  Diego Pino Garcia  <dpino@igalia.com>
    217

  • trunk/Source/WebCore/ChangeLog

    r277100 r277112  
     12021-05-06  Antoine Quint  <graouts@webkit.org>
     2
     3        CSS custom properties on pseudo elements background gradients causes infinite layout and high CPU load
     4        https://bugs.webkit.org/show_bug.cgi?id=194332
     5        <rdar://problem/47873895>
     6
     7        Reviewed by Simon Fraser.
     8
     9        When a background-image uses a CSS custom property the resulting StyleGeneratedImage may not be the same
     10        object as during prior style updates. This caused transitions to be triggered for all style updates for
     11        such background-image values. To fix this, we modify the == operator for StyleGeneratedImage to use
     12        arePointingToEqualData() with the CSSImageGeneratorValue member and added an == operator for the
     13        CSSImageGeneratorValue class to call into the existing equals() methods. These equals() methods
     14        are now overrides of the virtual CSSImageGeneratorValue method.
     15
     16        This change in behavior required a change in RenderElement::updateFillImages() to not only check whether
     17        the images were identical, but to also check whether the renderer was registered as a client on the new
     18        images. To do this, we add a new virtual hasClient() method on StyleImage.
     19
     20        Test: webanimations/css-transition-element-with-gradient-background-image-and-css-custom-property.html
     21
     22        * css/CSSImageGeneratorValue.cpp:
     23        (WebCore::CSSImageGeneratorValue::operator== const):
     24        * css/CSSImageGeneratorValue.h:
     25        * rendering/RenderElement.cpp:
     26        (WebCore::RenderElement::updateFillImages):
     27        * rendering/style/FillLayer.cpp:
     28        (WebCore::FillLayer::imagesIdentical): Deleted.
     29        * rendering/style/FillLayer.h:
     30        * rendering/style/StyleCachedImage.cpp:
     31        (WebCore::StyleCachedImage::hasClient const):
     32        * rendering/style/StyleCachedImage.h:
     33        * rendering/style/StyleGeneratedImage.cpp:
     34        (WebCore::StyleGeneratedImage::operator== const):
     35        (WebCore::StyleGeneratedImage::hasClient const):
     36        * rendering/style/StyleGeneratedImage.h:
     37        * rendering/style/StyleImage.h:
     38        * rendering/style/StyleMultiImage.cpp:
     39        (WebCore::StyleMultiImage::hasClient const):
     40        * rendering/style/StyleMultiImage.h:
     41
    1422021-05-06  Philippe Normand  <pnormand@igalia.com>
    243

  • trunk/Source/WebCore/css/CSSImageGeneratorValue.cpp

    r251493 r277112  
    299299}
    300300
     301bool CSSImageGeneratorValue::operator==(const CSSImageGeneratorValue& other) const
     302{
     303    if (classType() != other.classType())
     304        return false;
     305
     306    switch (classType()) {
     307    case CrossfadeClass:
     308        return downcast<CSSCrossfadeValue>(*this).equals(downcast<CSSCrossfadeValue>(other));
     309    case CanvasClass:
     310        return downcast<CSSCanvasValue>(*this).equals(downcast<CSSCanvasValue>(other));
     311    case FilterImageClass:
     312        return downcast<CSSFilterImageValue>(*this).equals(downcast<CSSFilterImageValue>(other));
     313    case LinearGradientClass:
     314        return downcast<CSSLinearGradientValue>(*this).equals(downcast<CSSLinearGradientValue>(other));
     315    case RadialGradientClass:
     316        return downcast<CSSRadialGradientValue>(*this).equals(downcast<CSSRadialGradientValue>(other));
     317    case ConicGradientClass:
     318        return downcast<CSSConicGradientValue>(*this).equals(downcast<CSSConicGradientValue>(other));
     319#if ENABLE(CSS_PAINTING_API)
     320    case PaintImageClass:
     321        return downcast<CSSPaintImageValue>(*this).equals(downcast<CSSPaintImageValue>(other));
     322#endif
     323    default:
     324        ASSERT_NOT_REACHED();
     325    }
     326
     327    return false;
     328}
     329
    301330bool CSSImageGeneratorValue::subimageIsPending(const CSSValue& value)
    302331{

  • trunk/Source/WebCore/css/CSSImageGeneratorValue.h

    r219268 r277112  
    5858    void loadSubimages(CachedResourceLoader&, const ResourceLoaderOptions&);
    5959
     60    bool operator==(const CSSImageGeneratorValue& other) const;
     61
    6062protected:
    6163    CSSImageGeneratorValue(ClassType);

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

    r276236 r277112  
    354354void RenderElement::updateFillImages(const FillLayer* oldLayers, const FillLayer& newLayers)
    355355{
    356     // Optimize the common case.
    357     if (FillLayer::imagesIdentical(oldLayers, &newLayers))
    358         return;
    359    
     356    auto fillImagesAreIdentical = [](const FillLayer* layer1, const FillLayer* layer2) -> bool {
     357        if (layer1 == layer2)
     358            return true;
     359
     360        for (; layer1 && layer2; layer1 = layer1->next(), layer2 = layer2->next()) {
     361            if (!arePointingToEqualData(layer1->image(), layer2->image()))
     362                return false;
     363        }
     364
     365        return !layer1 && !layer2;
     366    };
     367
     368    auto isRegisteredWithNewFillImages = [&]() -> bool {
     369        for (auto* layer = &newLayers; layer; layer = layer->next()) {
     370            if (layer->image() && !layer->image()->hasClient(*this))
     371                return false;
     372        }
     373        return true;
     374    };
     375
     376    // If images have the same characteristics and this element is already registered as a
     377    // client to the new images, there is nothing to do.
     378    if (fillImagesAreIdentical(oldLayers, &newLayers) && isRegisteredWithNewFillImages())
     379        return;
     380
    360381    // Add before removing, to avoid removing all clients of an image that is in both sets.
    361382    for (auto* layer = &newLayers; layer; layer = layer->next()) {

  • trunk/Source/WebCore/rendering/style/FillLayer.cpp

    r274560 r277112  
    397397}
    398398
    399 bool FillLayer::imagesIdentical(const FillLayer* layer1, const FillLayer* layer2)
    400 {
    401     if (layer1 == layer2)
    402         return true;
    403 
    404     for (; layer1 && layer2; layer1 = layer1->next(), layer2 = layer2->next()) {
    405         if (!arePointingToEqualData(layer1->image(), layer2->image()))
    406             return false;
    407     }
    408 
    409     return !layer1 && !layer2;
    410 }
    411 
    412399TextStream& operator<<(TextStream& ts, FillSize fillSize)
    413400{

  • trunk/Source/WebCore/rendering/style/FillLayer.h

    r272805 r277112  
    160160    void cullEmptyLayers();
    161161
    162     static bool imagesIdentical(const FillLayer*, const FillLayer*);
    163 
    164162    static FillAttachment initialFillAttachment(FillLayerType) { return FillAttachment::ScrollBackground; }
    165163    static FillBox initialFillClip(FillLayerType) { return FillBox::Border; }

  • trunk/Source/WebCore/rendering/style/StyleCachedImage.cpp

    r272421 r277112  
    173173}
    174174
     175bool StyleCachedImage::hasClient(RenderElement& renderer) const
     176{
     177    ASSERT(!m_isPending);
     178    if (!m_cachedImage)
     179        return false;
     180    return m_cachedImage->hasClient(renderer);
     181}
     182
    175183RefPtr<Image> StyleCachedImage::image(RenderElement* renderer, const FloatSize&) const
    176184{

  • trunk/Source/WebCore/rendering/style/StyleCachedImage.h

    r272421 r277112  
    6161    void addClient(RenderElement&) final;
    6262    void removeClient(RenderElement&) final;
     63    bool hasClient(RenderElement&) const final;
    6364    RefPtr<Image> image(RenderElement*, const FloatSize&) const final;
    6465    float imageScaleFactor() const final;

  • trunk/Source/WebCore/rendering/style/StyleGeneratedImage.cpp

    r272805 r277112  
    3636{
    3737    m_isGeneratedImage = true;
     38}
     39
     40bool StyleGeneratedImage::operator==(const StyleImage& other) const
     41{
     42    if (is<StyleGeneratedImage>(other))
     43        return arePointingToEqualData(m_imageGeneratorValue.ptr(), downcast<StyleGeneratedImage>(other).m_imageGeneratorValue.ptr());
     44    return false;
    3845}
    3946
    … …  
    97104}
    98105
     106bool StyleGeneratedImage::hasClient(RenderElement& renderer) const
     107{
     108    return m_imageGeneratorValue->clients().contains(&renderer);
     109}
     110
    99111RefPtr<Image> StyleGeneratedImage::image(RenderElement* renderer, const FloatSize& size) const
    100112{

  • trunk/Source/WebCore/rendering/style/StyleGeneratedImage.h

    r272421 r277112  
    4141
    4242private:
    43     bool operator==(const StyleImage& other) const final { return data() == other.data(); }
     43    bool operator==(const StyleImage& other) const final;
    4444
    4545    WrappedImagePtr data() const final { return m_imageGeneratorValue.ptr(); }
    … …  
    5757    void addClient(RenderElement&) final;
    5858    void removeClient(RenderElement&) final;
     59    bool hasClient(RenderElement&) const final;
    5960    RefPtr<Image> image(RenderElement*, const FloatSize&) const final;
    6061    bool knownToBeOpaque(const RenderElement&) const final;

  • trunk/Source/WebCore/rendering/style/StyleImage.h

    r272421 r277112  
    6363    virtual void addClient(RenderElement&) = 0;
    6464    virtual void removeClient(RenderElement&) = 0;
     65    virtual bool hasClient(RenderElement&) const = 0;
    6566    virtual RefPtr<Image> image(RenderElement*, const FloatSize&) const = 0;
    6667    virtual WrappedImagePtr data() const = 0;

  • trunk/Source/WebCore/rendering/style/StyleMultiImage.cpp

    r272421 r277112  
    149149}
    150150
     151bool StyleMultiImage::hasClient(RenderElement& renderer) const
     152{
     153    if (!m_selectedImage)
     154        return false;
     155    return m_selectedImage->hasClient(renderer);
     156}
     157
    151158RefPtr<Image> StyleMultiImage::image(RenderElement* renderer, const FloatSize& size) const
    152159{

  • trunk/Source/WebCore/rendering/style/StyleMultiImage.h

    r272421 r277112  
    6060    void addClient(RenderElement&) final;
    6161    void removeClient(RenderElement&) final;
     62    bool hasClient(RenderElement&) const final;
    6263    RefPtr<Image> image(RenderElement*, const FloatSize&) const final;
    6364    float imageScaleFactor() const final;
Note: See TracChangeset for help on using the changeset viewer.