⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 259830 in webkit


Ignore:
Timestamp:
Apr 9, 2020, 2:45:57 PM (6 years ago)
Author:
commit-queue@webkit.org
Message:

REGRESSION: CSS animations inside an embedded SVG image do not animate
​https://bugs.webkit.org/show_bug.cgi?id=209370

Patch by Said Abou-Hallawa <​sabouhallawa@apple.com> on 2020-04-09
Reviewed by Simon Fraser.

Source/WebCore:

If WebAnimation is enabled and the SVGImage includes CSS animations, the
DocumentTimeline is added to the SVGDocument of the embedded SVGImage.
Because the SVGImage has its own Page the RenderingUpdate is scheduled
and the updateRendering steps run in this Page.

The Page of the SVGImage is inactive such that scheduling RenderingUpdate
fails; therefore the updateRendering steps never run and the CSS animation
never advances.

The fix is:

1) Scheduling the RenderingUpdate: This has to happen in the Page which

contains the renderer of the SVGImage. Because DocumentTimeline is
added to SVGDocument, this scheduling will go through these hubs:

  • DocumentTimeline
  • Page
  • ChromeClient -> SVGImageChromeClient
  • SVGImage
  • ImageObserver -> CachedImageObserver
  • CachedImage
  • CachedImageClient -> RenderElement
  • Page

2) Running the updateRendering steps: Each document in the Page will

enumerate its cached SVGImages. The updateRendering of the Page of
each SVGImage will be called.

To make enumerating the cached SVGImages of a Document faster, the URL
of the cached SVGImage will be added to the cachedSVGImagesURLs of
CachedResourceLoader when notifyFinished() is called for associated
CachedImage.

Tests: svg/animations/css-animation-background-svg.html

svg/animations/css-animation-embedded-svg.html
svg/animations/css-animation-hover-svg.html

  • animation/DocumentTimeline.cpp:

(WebCore::DocumentTimeline::scheduleAnimationResolution):
(WebCore::DocumentTimeline::updateAnimationsAndSendEvents):

  • html/ImageBitmap.cpp:
  • loader/cache/CachedImage.cpp:

(WebCore::CachedImage::hasSVGImage const):
(WebCore::CachedImage::CachedImageObserver::scheduleTimedRenderingUpdate):
(WebCore::CachedImage::scheduleTimedRenderingUpdate):

  • loader/cache/CachedImage.h:
  • loader/cache/CachedImageClient.h:

(WebCore::CachedImageClient::scheduleTimedRenderingUpdate):

  • loader/cache/CachedResourceLoader.cpp:

(WebCore::isSVGImageCachedResource):
(WebCore::cachedResourceSVGImage):
(WebCore::CachedResourceLoader::notifyFinished):
(WebCore:: const):

  • loader/cache/CachedResourceLoader.h:
  • page/ChromeClient.h:

(WebCore::ChromeClient::scheduleTimedRenderingUpdate):

  • page/Page.cpp:

(WebCore::Page::scheduleTimedRenderingUpdate):
(WebCore::Page::updateRendering):

  • page/Page.h:
  • platform/graphics/ImageObserver.h:
  • rendering/RenderElement.cpp:

(WebCore::RenderElement::notifyFinished):
(WebCore::RenderElement::scheduleTimedRenderingUpdate):

  • rendering/RenderElement.h:
  • rendering/RenderImage.cpp:

(WebCore::RenderImage::notifyFinished):

  • svg/graphics/SVGImage.h:
  • svg/graphics/SVGImageClients.h:

LayoutTests:

  • svg/animations/css-animation-background-svg-expected.html: Added.
  • svg/animations/css-animation-background-svg.html: Added.
  • svg/animations/css-animation-embedded-svg-expected.html: Added.
  • svg/animations/css-animation-embedded-svg.html: Added.
  • svg/animations/css-animation-hover-svg-expected.html: Added.
  • svg/animations/css-animation-hover-svg.html: Added.
Location:
trunk
Files:
6 added
18 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r259829 r259830  
     12020-04-09  Said Abou-Hallawa  <sabouhallawa@apple.com>
     2
     3        REGRESSION: CSS animations inside an embedded SVG image do not animate
     4        https://bugs.webkit.org/show_bug.cgi?id=209370
     5
     6        Reviewed by Simon Fraser.
     7
     8        * svg/animations/css-animation-background-svg-expected.html: Added.
     9        * svg/animations/css-animation-background-svg.html: Added.
     10        * svg/animations/css-animation-embedded-svg-expected.html: Added.
     11        * svg/animations/css-animation-embedded-svg.html: Added.
     12        * svg/animations/css-animation-hover-svg-expected.html: Added.
     13        * svg/animations/css-animation-hover-svg.html: Added.
     14
    1152020-04-09  Keith Miller  <keith_miller@apple.com>
    216

  • trunk/Source/WebCore/ChangeLog

    r259829 r259830  
     12020-04-09  Said Abou-Hallawa  <sabouhallawa@apple.com>
     2
     3        REGRESSION: CSS animations inside an embedded SVG image do not animate
     4        https://bugs.webkit.org/show_bug.cgi?id=209370
     5
     6        Reviewed by Simon Fraser.
     7
     8        If WebAnimation is enabled and the SVGImage includes CSS animations, the
     9        DocumentTimeline is added to the SVGDocument of the embedded SVGImage.
     10        Because the SVGImage has its own Page the RenderingUpdate is scheduled
     11        and the updateRendering steps run in this Page.
     12
     13        The Page of the SVGImage is inactive such that scheduling RenderingUpdate
     14        fails; therefore the updateRendering steps never run and the CSS animation
     15        never advances.
     16
     17        The fix is:
     18
     19        1) Scheduling the RenderingUpdate: This has to happen in the Page which
     20           contains the renderer of the SVGImage. Because DocumentTimeline is
     21           added to SVGDocument, this scheduling will go through these hubs:
     22                - DocumentTimeline
     23                - Page
     24                - ChromeClient -> SVGImageChromeClient
     25                - SVGImage
     26                - ImageObserver -> CachedImageObserver
     27                - CachedImage
     28                - CachedImageClient -> RenderElement
     29                - Page
     30
     31        2) Running the updateRendering steps: Each document in the Page will
     32           enumerate its cached SVGImages. The updateRendering of the Page of
     33           each SVGImage will be called.
     34
     35        To make enumerating the cached SVGImages of a Document faster, the URL
     36        of the cached SVGImage will be added to the cachedSVGImagesURLs of
     37        CachedResourceLoader when notifyFinished() is called for associated
     38        CachedImage.
     39
     40        Tests: svg/animations/css-animation-background-svg.html
     41               svg/animations/css-animation-embedded-svg.html
     42               svg/animations/css-animation-hover-svg.html
     43
     44        * animation/DocumentTimeline.cpp:
     45        (WebCore::DocumentTimeline::scheduleAnimationResolution):
     46        (WebCore::DocumentTimeline::updateAnimationsAndSendEvents):
     47        * html/ImageBitmap.cpp:
     48        * loader/cache/CachedImage.cpp:
     49        (WebCore::CachedImage::hasSVGImage const):
     50        (WebCore::CachedImage::CachedImageObserver::scheduleTimedRenderingUpdate):
     51        (WebCore::CachedImage::scheduleTimedRenderingUpdate):
     52        * loader/cache/CachedImage.h:
     53        * loader/cache/CachedImageClient.h:
     54        (WebCore::CachedImageClient::scheduleTimedRenderingUpdate):
     55        * loader/cache/CachedResourceLoader.cpp:
     56        (WebCore::isSVGImageCachedResource):
     57        (WebCore::cachedResourceSVGImage):
     58        (WebCore::CachedResourceLoader::notifyFinished):
     59        (WebCore:: const):
     60        * loader/cache/CachedResourceLoader.h:
     61        * page/ChromeClient.h:
     62        (WebCore::ChromeClient::scheduleTimedRenderingUpdate):
     63        * page/Page.cpp:
     64        (WebCore::Page::scheduleTimedRenderingUpdate):
     65        (WebCore::Page::updateRendering):
     66        * page/Page.h:
     67        * platform/graphics/ImageObserver.h:
     68        * rendering/RenderElement.cpp:
     69        (WebCore::RenderElement::notifyFinished):
     70        (WebCore::RenderElement::scheduleTimedRenderingUpdate):
     71        * rendering/RenderElement.h:
     72        * rendering/RenderImage.cpp:
     73        (WebCore::RenderImage::notifyFinished):
     74        * svg/graphics/SVGImage.h:
     75        * svg/graphics/SVGImageClients.h:
     76
    1772020-04-09  Keith Miller  <keith_miller@apple.com>
    278

  • trunk/Source/WebCore/animation/DocumentTimeline.cpp

    r258834 r259830  
    340340        return;
    341341
    342     m_document->page()->renderingUpdateScheduler().scheduleTimedRenderingUpdate();
     342    m_document->page()->scheduleTimedRenderingUpdate();
    343343    m_animationResolutionScheduled = true;
    344344}
    … …  
    365365void DocumentTimeline::updateAnimationsAndSendEvents()
    366366{
    367 
    368367    // Updating animations and sending events may invalidate the timing of some animations, so we must set the m_animationResolutionScheduled
    369368    // flag to false prior to running that procedure to allow animation with timing model updates to schedule updates.

  • trunk/Source/WebCore/html/ImageBitmap.cpp

    r254893 r259830  
    529529    void imageFrameAvailable(const Image&, ImageAnimatingState, const IntRect* = nullptr, DecodingStatus = DecodingStatus::Invalid) override { }
    530530    void changedInRect(const Image&, const IntRect* = nullptr) override { }
     531    void scheduleTimedRenderingUpdate(const Image&) override { }
    531532
    532533private:

  • trunk/Source/WebCore/loader/cache/CachedImage.cpp

    r256482 r259830  
    269269}
    270270
     271bool CachedImage::hasSVGImage() const
     272{
     273    return m_image && m_image->isSVGImage();
     274}
     275
    271276void CachedImage::setContainerContextForClient(const CachedImageClient& client, const LayoutSize& containerSize, float containerZoom, const URL& imageURL)
    272277{
    … …  
    433438    for (auto cachedImage : m_cachedImages)
    434439        cachedImage->changedInRect(image, rect);
     440}
     441
     442void CachedImage::CachedImageObserver::scheduleTimedRenderingUpdate(const Image& image)
     443{
     444    for (auto cachedImage : m_cachedImages)
     445        cachedImage->scheduleTimedRenderingUpdate(image);
    435446}
    436447
    … …  
    685696}
    686697
     698void CachedImage::scheduleTimedRenderingUpdate(const Image& image)
     699{
     700    if (&image != m_image)
     701        return;
     702
     703    CachedResourceClientWalker<CachedImageClient> walker(m_clients);
     704    while (auto* client = walker.next())
     705        client->scheduleTimedRenderingUpdate();
     706}
     707
    687708bool CachedImage::currentFrameKnownToBeOpaque(const RenderElement* renderer)
    688709{

  • trunk/Source/WebCore/loader/cache/CachedImage.h

    r256786 r259830  
    5757    WEBCORE_EXPORT Image* imageForRenderer(const RenderObject*); // Returns the nullImage() if the image is not available yet.
    5858    bool hasImage() const { return m_image.get(); }
     59    bool hasSVGImage() const;
    5960    bool currentFrameKnownToBeOpaque(const RenderElement*);
    6061
    … …  
    153154        void imageFrameAvailable(const Image&, ImageAnimatingState, const IntRect* changeRect = nullptr, DecodingStatus = DecodingStatus::Invalid) final;
    154155        void changedInRect(const Image&, const IntRect*) final;
     156        void scheduleTimedRenderingUpdate(const Image&) final;
    155157
    156158        HashSet<CachedImage*> m_cachedImages;
    … …  
    163165    void imageFrameAvailable(const Image&, ImageAnimatingState, const IntRect* changeRect = nullptr, DecodingStatus = DecodingStatus::Invalid);
    164166    void changedInRect(const Image&, const IntRect*);
     167    void scheduleTimedRenderingUpdate(const Image&);
    165168
    166169    void updateBufferInternal(SharedBuffer&);

  • trunk/Source/WebCore/loader/cache/CachedImageClient.h

    r223728 r259830  
    4949
    5050    virtual void didRemoveCachedImageClient(CachedImage&) { }
     51
     52    virtual void scheduleTimedRenderingUpdate() { }
    5153};
    5254

  • trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp

    r259308 r259830  
    7070#include "RuntimeApplicationChecks.h"
    7171#include "RuntimeEnabledFeatures.h"
     72#include "SVGImage.h"
    7273#include "ScriptController.h"
    7374#include "SecurityOrigin.h"
    … …  
    787788    ASSERT_NOT_REACHED();
    788789    return FetchOptions::Destination::EmptyString;
     790}
     791
     792static inline bool isSVGImageCachedResource(const CachedResource* resource)
     793{
     794    if (!resource || !is<CachedImage>(*resource))
     795        return false;
     796    return downcast<CachedImage>(*resource).hasSVGImage();
     797}
     798
     799static inline SVGImage* cachedResourceSVGImage(CachedResource* resource)
     800{
     801    if (!isSVGImageCachedResource(resource))
     802        return nullptr;
     803    return downcast<SVGImage>(downcast<CachedImage>(*resource).image());
    789804}
    790805
    … …  
    14021417}
    14031418
     1419void CachedResourceLoader::notifyFinished(const CachedResource& resource)
     1420{
     1421    if (isSVGImageCachedResource(&resource))
     1422        m_cachedSVGImagesURLs.add(resource.url());
     1423}
     1424
     1425Vector<Ref<SVGImage>> CachedResourceLoader::allCachedSVGImages() const
     1426{
     1427    Vector<Ref<SVGImage>> allCachedSVGImages;
     1428
     1429    for (auto& cachedSVGImageURL : m_cachedSVGImagesURLs) {
     1430        auto* resource = cachedResource(cachedSVGImageURL);
     1431        if (auto* image = cachedResourceSVGImage(resource))
     1432            allCachedSVGImages.append(*image);
     1433    }
     1434       
     1435    return allCachedSVGImages;
     1436}
     1437
    14041438ResourceErrorOr<CachedResourceHandle<CachedResource>> CachedResourceLoader::preload(CachedResource::Type type, CachedResourceRequest&& request)
    14051439{

  • trunk/Source/WebCore/loader/cache/CachedResourceLoader.h

    r259116 r259830  
    5858class ImageLoader;
    5959class Page;
     60class SVGImage;
    6061class Settings;
    6162
    … …  
    117118    const DocumentResourceMap& allCachedResources() const { return m_documentResources; }
    118119
     120    void notifyFinished(const CachedResource&);
     121    Vector<Ref<SVGImage>> allCachedSVGImages() const;
     122
    119123    bool autoLoadImages() const { return m_autoLoadImages; }
    120124    void setAutoLoadImages(bool);
    … …  
    194198
    195199    HashSet<String> m_validatedURLs;
     200    HashSet<String> m_cachedSVGImagesURLs;
    196201    mutable DocumentResourceMap m_documentResources;
    197202    WeakPtr<Document> m_document;

  • trunk/Source/WebCore/page/ChromeClient.h

    r259330 r259830  
    317317    // to do an eager layout before the drawing.
    318318    virtual void scheduleRenderingUpdate() = 0;
     319    virtual bool scheduleTimedRenderingUpdate() { return false; }
    319320    virtual bool needsImmediateRenderingUpdate() const { return false; }
    320321    // Returns whether or not the client can render the composited layer,

  • trunk/Source/WebCore/page/Page.cpp

    r259820 r259830  
    3030#include "CSSAnimationController.h"
    3131#include "CacheStorageProvider.h"
     32#include "CachedResourceLoader.h"
    3233#include "Chrome.h"
    3334#include "ChromeClient.h"
    … …  
    105106#include "RuntimeEnabledFeatures.h"
    106107#include "SVGDocumentExtensions.h"
     108#include "SVGImage.h"
    107109#include "ScriptController.h"
    108110#include "ScriptDisallowedScope.h"
    … …  
    12991301}
    13001302
     1303void Page::scheduleTimedRenderingUpdate()
     1304{
     1305    if (chrome().client().scheduleTimedRenderingUpdate())
     1306        return;
     1307    renderingUpdateScheduler().scheduleTimedRenderingUpdate();
     1308}
     1309
    13011310// https://html.spec.whatwg.org/multipage/webappapis.html#update-the-rendering
    13021311void Page::updateRendering()
    … …  
    13501359    });
    13511360#endif
     1361
     1362    forEachDocument([] (Document& document) {
     1363        for (auto& image : document.cachedResourceLoader().allCachedSVGImages()) {
     1364            if (auto* page = image->internalPage())
     1365                page->updateRendering();
     1366        }
     1367    });
    13521368
    13531369    layoutIfNeeded();

  • trunk/Source/WebCore/page/Page.h

    r259820 r259830  
    479479   
    480480    WEBCORE_EXPORT void scheduleRenderingUpdate();
     481    void scheduleTimedRenderingUpdate();
    481482
    482483    WEBCORE_EXPORT void suspendScriptedAnimations();

  • trunk/Source/WebCore/platform/graphics/ImageObserver.h

    r239636 r259830  
    5151    virtual void imageFrameAvailable(const Image&, ImageAnimatingState, const IntRect* changeRect = nullptr, DecodingStatus = DecodingStatus::Invalid) = 0;
    5252    virtual void changedInRect(const Image&, const IntRect* changeRect = nullptr) = 0;
     53    virtual void scheduleTimedRenderingUpdate(const Image&) = 0;
    5354};
    5455

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

    r259762 r259830  
    2727
    2828#include "AXObjectCache.h"
     29#include "CachedResourceLoader.h"
    2930#if PLATFORM(IOS_FAMILY)
    3031#include "ContentChangeObserver.h"
    … …  
    13251326}
    13261327
     1328void RenderElement::notifyFinished(CachedResource& resource)
     1329{
     1330    document().cachedResourceLoader().notifyFinished(resource);
     1331}
     1332
    13271333void RenderElement::didRemoveCachedImageClient(CachedImage& cachedImage)
    13281334{
    13291335    if (hasPausedImageAnimations())
    13301336        view().removeRendererWithPausedImageAnimations(*this, cachedImage);
     1337}
     1338
     1339void RenderElement::scheduleTimedRenderingUpdate()
     1340{
     1341    if (auto* page = document().page())
     1342        page->scheduleTimedRenderingUpdate();
    13311343}
    13321344

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

    r259703 r259830  
    264264    void willBeRemovedFromTree() override;
    265265    void willBeDestroyed() override;
     266    void notifyFinished(CachedResource&) override;
    266267
    267268    void setRenderInlineAlwaysCreatesLineBoxes(bool b) { m_renderInlineAlwaysCreatesLineBoxes = b; }
    … …  
    321322    VisibleInViewportState imageFrameAvailable(CachedImage&, ImageAnimatingState, const IntRect* changeRect) final;
    322323    void didRemoveCachedImageClient(CachedImage&) final;
     324    void scheduleTimedRenderingUpdate() final;
    323325
    324326    bool getLeadingCorner(FloatPoint& output, bool& insideFixed) const;

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

    r257917 r259830  
    419419    if (is<HTMLImageElement>(element()))
    420420        page().didFinishLoadingImageForElement(downcast<HTMLImageElement>(*element()));
     421
     422    RenderReplaced::notifyFinished(newImage);
    421423}
    422424

  • trunk/Source/WebCore/svg/graphics/SVGImage.h

    r249364 r259830  
    6969    NativeImagePtr nativeImage(const GraphicsContext* = nullptr) final;
    7070#endif
     71   
     72    Page* internalPage() { return m_page.get(); }
    7173
    7274private:

  • trunk/Source/WebCore/svg/graphics/SVGImageClients.h

    r216901 r259830  
    6363        imageObserver->imageFrameAvailable(*m_image, m_image->isAnimating() ? ImageAnimatingState::Yes : ImageAnimatingState::No, &r);
    6464    }
    65    
     65
     66    bool scheduleTimedRenderingUpdate() final
     67    {
     68        if (m_image && m_image->imageObserver())
     69            m_image->imageObserver()->scheduleTimedRenderingUpdate(*m_image);
     70        return true;
     71    }
     72
    6673    SVGImage* m_image;
    6774};
Note: See TracChangeset for help on using the changeset viewer.