Merge r220934 - Followup (r220289): RenderImageResourceStyleImage cod…

…e clean up https://bugs.webkit.org/show_bug.cgi?id=175444 Patch by Said Abou-Hallawa <sabouhallawa@apple.com> on 2017-08-18 Reviewed by Darin Adler. RenderImageResourceStyleImage may be created with a StyleImage of type StyleGeneratedImage. It may also be associated with a CachedImage which is loaded through a source URL. In this case, adding and removing m_renderer as a client of the CachedImage will be done through RenderImageResource::setCachedImage(). RenderImageResource::setCachedImage() is already called from ImageLoader::updateRenderer() when the CachedImage finishes loading. This call adds m_renderer to the clients of the CachedImage. RenderImageResource::setCachedImage() will also be called from RenderImageResourceStyleImage::shutdown() via RenderImageResource::shutdown() to remove m_renderer from the clients of CachedImage by passing a null pointer. * rendering/RenderImage.cpp: (WebCore::RenderImage::styleWillChange): * rendering/RenderImageResource.cpp: (WebCore::RenderImageResource::initialize): (WebCore::RenderImageResource::shutdown): (WebCore::RenderImageResource::setCachedImage): (WebCore::RenderImageResource::resetAnimation): (WebCore::RenderImageResource::image const): (WebCore::RenderImageResource::setContainerSizeForRenderer): (WebCore::RenderImageResource::imageSize const): (WebCore::RenderImageResource::~RenderImageResource): Deleted. (WebCore::RenderImageResource::errorOccurred const): Deleted. (WebCore::RenderImageResource::imageHasRelativeWidth const): Deleted. (WebCore::RenderImageResource::imageHasRelativeHeight const): Deleted. (WebCore::RenderImageResource::intrinsicSize const): Deleted. (WebCore::RenderImageResource::getImageSize const): Deleted. * rendering/RenderImageResource.h: (WebCore::RenderImageResource::initialize): (WebCore::RenderImageResource::renderer const): (WebCore::RenderImageResource::errorOccurred const): (WebCore::RenderImageResource::imageHasRelativeWidth const): (WebCore::RenderImageResource::imageHasRelativeHeight const): (WebCore::RenderImageResource::imageSize const): (WebCore::RenderImageResource::intrinsicSize const): (WebCore::RenderImageResource::imagePtr const): * rendering/RenderImageResourceStyleImage.cpp: (WebCore::RenderImageResourceStyleImage::initialize): (WebCore::RenderImageResourceStyleImage::shutdown): (WebCore::RenderImageResourceStyleImage::image const): (WebCore::RenderImageResourceStyleImage::setContainerSizeForRenderer): (WebCore::RenderImageResourceStyleImage::~RenderImageResourceStyleImage): Deleted. * rendering/RenderImageResourceStyleImage.h: * rendering/RenderSnapshottedPlugIn.cpp: (WebCore::RenderSnapshottedPlugIn::RenderSnapshottedPlugIn): * rendering/svg/RenderSVGImage.cpp: (WebCore::RenderSVGImage::RenderSVGImage):
WebKit · Aug 28, 2017 · 57ed198 · 57ed198
1 parent f7891c2
commit 57ed198
Show file tree

Hide file tree

Showing 8 changed files with 118 additions and 95 deletions.
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,60 @@
+2017-08-18  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        Followup (r220289): RenderImageResourceStyleImage code clean up
+        https://bugs.webkit.org/show_bug.cgi?id=175444
+
+        Reviewed by Darin Adler.
+
+        RenderImageResourceStyleImage may be created with a StyleImage of type
+        StyleGeneratedImage. It may also be associated with a CachedImage which 
+        is loaded through a source URL. In this case, adding and removing m_renderer
+        as a client of the CachedImage will be done through 
+        RenderImageResource::setCachedImage().
+
+        RenderImageResource::setCachedImage() is already called from 
+        ImageLoader::updateRenderer() when the CachedImage finishes loading. This
+        call adds m_renderer to the clients of the CachedImage. 
+        RenderImageResource::setCachedImage() will also be called from 
+        RenderImageResourceStyleImage::shutdown() via RenderImageResource::shutdown()
+        to remove m_renderer from the clients of CachedImage by passing a null pointer.
+
+        * rendering/RenderImage.cpp:
+        (WebCore::RenderImage::styleWillChange):
+        * rendering/RenderImageResource.cpp:
+        (WebCore::RenderImageResource::initialize):
+        (WebCore::RenderImageResource::shutdown):
+        (WebCore::RenderImageResource::setCachedImage):
+        (WebCore::RenderImageResource::resetAnimation):
+        (WebCore::RenderImageResource::image const):
+        (WebCore::RenderImageResource::setContainerSizeForRenderer):
+        (WebCore::RenderImageResource::imageSize const):
+        (WebCore::RenderImageResource::~RenderImageResource): Deleted.
+        (WebCore::RenderImageResource::errorOccurred const): Deleted.
+        (WebCore::RenderImageResource::imageHasRelativeWidth const): Deleted.
+        (WebCore::RenderImageResource::imageHasRelativeHeight const): Deleted.
+        (WebCore::RenderImageResource::intrinsicSize const): Deleted.
+        (WebCore::RenderImageResource::getImageSize const): Deleted.
+        * rendering/RenderImageResource.h:
+        (WebCore::RenderImageResource::initialize):
+        (WebCore::RenderImageResource::renderer const):
+        (WebCore::RenderImageResource::errorOccurred const):
+        (WebCore::RenderImageResource::imageHasRelativeWidth const):
+        (WebCore::RenderImageResource::imageHasRelativeHeight const):
+        (WebCore::RenderImageResource::imageSize const):
+        (WebCore::RenderImageResource::intrinsicSize const):
+        (WebCore::RenderImageResource::imagePtr const):
+        * rendering/RenderImageResourceStyleImage.cpp:
+        (WebCore::RenderImageResourceStyleImage::initialize):
+        (WebCore::RenderImageResourceStyleImage::shutdown):
+        (WebCore::RenderImageResourceStyleImage::image const):
+        (WebCore::RenderImageResourceStyleImage::setContainerSizeForRenderer):
+        (WebCore::RenderImageResourceStyleImage::~RenderImageResourceStyleImage): Deleted.
+        * rendering/RenderImageResourceStyleImage.h:
+        * rendering/RenderSnapshottedPlugIn.cpp:
+        (WebCore::RenderSnapshottedPlugIn::RenderSnapshottedPlugIn):
+        * rendering/svg/RenderSVGImage.cpp:
+        (WebCore::RenderSVGImage::RenderSVGImage):
+
 2017-08-18  Antti Koivisto  <antti@apple.com>
 
         Factor render tree mutation code from RenderListItem to RenderTreeUpdater

diff --git a/Source/WebCore/rendering/RenderImage.cpp b/Source/WebCore/rendering/RenderImage.cpp
@@ -208,7 +208,7 @@ ImageSizeChangeType RenderImage::setImageSizeForAltText(CachedImage* newImage /*
 void RenderImage::styleWillChange(StyleDifference diff, const RenderStyle& newStyle)
 {
     if (!hasInitializedStyle())
-        imageResource().initialize(this);
+        imageResource().initialize(*this);
     RenderReplaced::styleWillChange(diff, newStyle);
 }
 

diff --git a/Source/WebCore/rendering/RenderImageResource.cpp b/Source/WebCore/rendering/RenderImageResource.cpp
@@ -40,51 +40,45 @@ RenderImageResource::RenderImageResource()
 {
 }
 
-RenderImageResource::~RenderImageResource()
-{
-}
-
-void RenderImageResource::initialize(RenderElement* renderer)
+void RenderImageResource::initialize(RenderElement& renderer, CachedImage* styleCachedImage)
 {
     ASSERT(!m_renderer);
-    ASSERT(renderer);
-    m_renderer = renderer;
+    ASSERT(!m_cachedImage);
+    m_renderer = &renderer;
+    m_cachedImage = styleCachedImage;
+    m_cachedImageRemoveClientIsNeeded = !styleCachedImage;
 }
 
 void RenderImageResource::shutdown()
 {
-    if (!m_cachedImage)
-        return;
-
-    ASSERT(m_renderer);
     image()->stopAnimation();
-    m_cachedImage->removeClient(*m_renderer);
+    setCachedImage(nullptr);
 }
 
 void RenderImageResource::setCachedImage(CachedImage* newImage)
 {
-    ASSERT(m_renderer);
-
     if (m_cachedImage == newImage)
         return;
 
-    if (m_cachedImage)
+    ASSERT(m_renderer);
+    if (m_cachedImage && m_cachedImageRemoveClientIsNeeded)
         m_cachedImage->removeClient(*m_renderer);
     m_cachedImage = newImage;
-    if (m_cachedImage) {
-        m_cachedImage->addClient(*m_renderer);
-        if (m_cachedImage->errorOccurred())
-            m_renderer->imageChanged(m_cachedImage.get());
-    }
+    m_cachedImageRemoveClientIsNeeded = true;
+    if (!m_cachedImage)
+        return;
+
+    m_cachedImage->addClient(*m_renderer);
+    if (m_cachedImage->errorOccurred())
+        m_renderer->imageChanged(m_cachedImage.get());
 }
 
 void RenderImageResource::resetAnimation()
 {
-    ASSERT(m_renderer);
-
     if (!m_cachedImage)
         return;
 
+    ASSERT(m_renderer);
     image()->resetAnimation();
 
     if (!m_renderer->needsLayout())
@@ -93,42 +87,22 @@ void RenderImageResource::resetAnimation()
 
 RefPtr<Image> RenderImageResource::image(const IntSize&) const
 {
-    return m_cachedImage ? m_cachedImage->imageForRenderer(m_renderer) : &Image::nullImage();
-}
-
-bool RenderImageResource::errorOccurred() const
-{
-    return m_cachedImage && m_cachedImage->errorOccurred();
+    if (!m_cachedImage)
+        return &Image::nullImage();
+    if (auto image = m_cachedImage->imageForRenderer(m_renderer))
+        return image;
+    return &Image::nullImage();
 }
 
 void RenderImageResource::setContainerSizeForRenderer(const IntSize& imageContainerSize)
 {
+    if (!m_cachedImage)
+        return;
     ASSERT(m_renderer);
-    if (m_cachedImage)
-        m_cachedImage->setContainerSizeForRenderer(m_renderer, imageContainerSize, m_renderer->style().effectiveZoom());
-}
-
-bool RenderImageResource::imageHasRelativeWidth() const
-{
-    return m_cachedImage ? m_cachedImage->imageHasRelativeWidth() : false;
-}
-
-bool RenderImageResource::imageHasRelativeHeight() const
-{
-    return m_cachedImage ? m_cachedImage->imageHasRelativeHeight() : false;
-}
-
-LayoutSize RenderImageResource::imageSize(float multiplier) const
-{
-    return getImageSize(multiplier, CachedImage::UsedSize);
-}
-
-LayoutSize RenderImageResource::intrinsicSize(float multiplier) const
-{
-    return getImageSize(multiplier, CachedImage::IntrinsicSize);
+    m_cachedImage->setContainerSizeForRenderer(m_renderer, imageContainerSize, m_renderer->style().effectiveZoom());
 }
 
-LayoutSize RenderImageResource::getImageSize(float multiplier, CachedImage::SizeType type) const
+LayoutSize RenderImageResource::imageSize(float multiplier, CachedImage::SizeType type) const
 {
     if (!m_cachedImage)
         return LayoutSize();

diff --git a/Source/WebCore/rendering/RenderImageResource.h b/Source/WebCore/rendering/RenderImageResource.h
@@ -38,9 +38,9 @@ class RenderImageResource {
     WTF_MAKE_NONCOPYABLE(RenderImageResource); WTF_MAKE_FAST_ALLOCATED;
 public:
     RenderImageResource();
-    virtual ~RenderImageResource();
+    virtual ~RenderImageResource() = default;
 
-    virtual void initialize(RenderElement*);
+    virtual void initialize(RenderElement& renderer) { initialize(renderer, nullptr); }
     virtual void shutdown();
 
     void setCachedImage(CachedImage*);
@@ -49,23 +49,28 @@ class RenderImageResource {
     void resetAnimation();
 
     virtual RefPtr<Image> image(const IntSize& size = { }) const;
-    virtual bool errorOccurred() const;
+    virtual bool errorOccurred() const { return m_cachedImage && m_cachedImage->errorOccurred(); }
 
     virtual void setContainerSizeForRenderer(const IntSize&);
-    virtual bool imageHasRelativeWidth() const;
-    virtual bool imageHasRelativeHeight() const;
 
-    virtual LayoutSize imageSize(float multiplier) const;
-    virtual LayoutSize intrinsicSize(float multiplier) const;
+    virtual bool imageHasRelativeWidth() const { return m_cachedImage && m_cachedImage->imageHasRelativeWidth(); }
+    virtual bool imageHasRelativeHeight() const { return m_cachedImage && m_cachedImage->imageHasRelativeHeight(); }
+
+    inline LayoutSize imageSize(float multiplier) const { return imageSize(multiplier, CachedImage::UsedSize); }
+    inline LayoutSize intrinsicSize(float multiplier) const { return imageSize(multiplier, CachedImage::IntrinsicSize); }
 
     virtual WrappedImagePtr imagePtr() const { return m_cachedImage.get(); }
 
 protected:
+    RenderElement* renderer() const { return m_renderer; }
+    void initialize(RenderElement&, CachedImage*);
+
+private:
+    virtual LayoutSize imageSize(float multiplier, CachedImage::SizeType) const;
+
     RenderElement* m_renderer { nullptr };
     CachedResourceHandle<CachedImage> m_cachedImage;
-
-private:
-    LayoutSize getImageSize(float multiplier, CachedImage::SizeType) const;
+    bool m_cachedImageRemoveClientIsNeeded { true };
 };
 
 } // namespace WebCore
diff --git a/Source/WebCore/rendering/RenderImageResourceStyleImage.cpp b/Source/WebCore/rendering/RenderImageResourceStyleImage.cpp
@@ -39,44 +39,33 @@ RenderImageResourceStyleImage::RenderImageResourceStyleImage(StyleImage& styleIm
 {
 }
 
-RenderImageResourceStyleImage::~RenderImageResourceStyleImage()
+void RenderImageResourceStyleImage::initialize(RenderElement& renderer)
 {
-}
-
-void RenderImageResourceStyleImage::initialize(RenderElement* renderer)
-{
-    RenderImageResource::initialize(renderer);
-
-    if (m_styleImage->isCachedImage())
-        m_cachedImage = m_styleImage.get().cachedImage();
-
-    m_styleImage->addClient(m_renderer);
+    RenderImageResource::initialize(renderer, m_styleImage->isCachedImage() ? m_styleImage.get().cachedImage() : nullptr);
+    m_styleImage->addClient(this->renderer());
 }
 
 void RenderImageResourceStyleImage::shutdown()
 {
-    ASSERT(m_renderer);
-    image()->stopAnimation();
-    m_styleImage->removeClient(m_renderer);
-    if (!m_styleImage->isCachedImage() && m_cachedImage)
-        m_cachedImage->removeClient(*m_renderer);
-    m_cachedImage = nullptr;
+    ASSERT(renderer());
+    RenderImageResource::shutdown();
+    m_styleImage->removeClient(renderer());
 }
 
 RefPtr<Image> RenderImageResourceStyleImage::image(const IntSize& size) const
 {
     // Generated content may trigger calls to image() while we're still pending, don't assert but gracefully exit.
     if (m_styleImage->isPending())
         return &Image::nullImage();
-    if (auto image = m_styleImage->image(m_renderer, size))
+    if (auto image = m_styleImage->image(renderer(), size))
         return image;
     return &Image::nullImage();
 }
 
 void RenderImageResourceStyleImage::setContainerSizeForRenderer(const IntSize& size)
 {
-    ASSERT(m_renderer);
-    m_styleImage->setContainerSizeForRenderer(m_renderer, size, m_renderer->style().effectiveZoom());
+    ASSERT(renderer());
+    m_styleImage->setContainerSizeForRenderer(renderer(), size, renderer()->style().effectiveZoom());
 }
 
 } // namespace WebCore
diff --git a/Source/WebCore/rendering/RenderImageResourceStyleImage.h b/Source/WebCore/rendering/RenderImageResourceStyleImage.h
@@ -36,23 +36,21 @@ class RenderElement;
 class RenderImageResourceStyleImage final : public RenderImageResource {
 public:
     explicit RenderImageResourceStyleImage(StyleImage&);
-    virtual ~RenderImageResourceStyleImage();
 
 private:
-    void initialize(RenderElement*) override;
-    void shutdown() override;
+    void initialize(RenderElement&) final;
+    void shutdown() final;
 
-    RefPtr<Image> image(const IntSize& = { }) const override;
-    bool errorOccurred() const override { return m_styleImage->errorOccurred(); }
+    RefPtr<Image> image(const IntSize& = { }) const final;
+    bool errorOccurred() const final { return m_styleImage->errorOccurred(); }
 
-    void setContainerSizeForRenderer(const IntSize&) override;
-    bool imageHasRelativeWidth() const override { return m_styleImage->imageHasRelativeWidth(); }
-    bool imageHasRelativeHeight() const override { return m_styleImage->imageHasRelativeHeight(); }
+    void setContainerSizeForRenderer(const IntSize&) final;
 
-    LayoutSize imageSize(float multiplier) const override { return LayoutSize(m_styleImage->imageSize(m_renderer, multiplier)); }
-    LayoutSize intrinsicSize(float multiplier) const override { return LayoutSize(m_styleImage->imageSize(m_renderer, multiplier)); }
+    bool imageHasRelativeWidth() const final { return m_styleImage->imageHasRelativeWidth(); }
+    bool imageHasRelativeHeight() const final { return m_styleImage->imageHasRelativeHeight(); }
 
-    WrappedImagePtr imagePtr() const override { return m_styleImage->data(); }
+    WrappedImagePtr imagePtr() const final { return m_styleImage->data(); }
+    LayoutSize imageSize(float multiplier, CachedImage::SizeType) const final { return LayoutSize(m_styleImage->imageSize(renderer(), multiplier)); }
 
     Ref<StyleImage> m_styleImage;
 };

diff --git a/Source/WebCore/rendering/RenderSnapshottedPlugIn.cpp b/Source/WebCore/rendering/RenderSnapshottedPlugIn.cpp
@@ -52,7 +52,7 @@ RenderSnapshottedPlugIn::RenderSnapshottedPlugIn(HTMLPlugInImageElement& element
     : RenderEmbeddedObject(element, WTFMove(style))
     , m_snapshotResource(std::make_unique<RenderImageResource>())
 {
-    m_snapshotResource->initialize(this);
+    m_snapshotResource->initialize(*this);
 }
 
 RenderSnapshottedPlugIn::~RenderSnapshottedPlugIn()

diff --git a/Source/WebCore/rendering/svg/RenderSVGImage.cpp b/Source/WebCore/rendering/svg/RenderSVGImage.cpp
@@ -49,7 +49,7 @@ RenderSVGImage::RenderSVGImage(SVGImageElement& element, RenderStyle&& style)
     , m_needsTransformUpdate(true)
     , m_imageResource(std::make_unique<RenderImageResource>())
 {
-    imageResource().initialize(this);
+    imageResource().initialize(*this);
 }
 
 RenderSVGImage::~RenderSVGImage()