diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index 75cbbf26d8d7..0425e7341322 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,60 @@ +2017-08-18 Said Abou-Hallawa + + 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 Factor render tree mutation code from RenderListItem to RenderTreeUpdater diff --git a/Source/WebCore/rendering/RenderImage.cpp b/Source/WebCore/rendering/RenderImage.cpp index 5b236d3f7e86..f69904e068ab 100644 --- 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 index d8de5f9a1deb..3051495c8a21 100644 --- 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 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 index a8874d6acef4..a27327dfb0d4 100644 --- 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(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 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 index 52f6286938a4..4db1357d75e2 100644 --- a/Source/WebCore/rendering/RenderImageResourceStyleImage.cpp +++ b/Source/WebCore/rendering/RenderImageResourceStyleImage.cpp @@ -39,28 +39,17 @@ 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 RenderImageResourceStyleImage::image(const IntSize& size) const @@ -68,15 +57,15 @@ RefPtr 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 index 97721f22e46e..059b8dfbdc36 100644 --- 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(const IntSize& = { }) const override; - bool errorOccurred() const override { return m_styleImage->errorOccurred(); } + RefPtr 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 m_styleImage; }; diff --git a/Source/WebCore/rendering/RenderSnapshottedPlugIn.cpp b/Source/WebCore/rendering/RenderSnapshottedPlugIn.cpp index 2afc0da0fd7b..29bc23e0a5f4 100644 --- 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()) { - 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 index ce9bd4ac351f..e6e9bc491eb6 100644 --- 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()) { - imageResource().initialize(this); + imageResource().initialize(*this); } RenderSVGImage::~RenderSVGImage()