- Revision
- 225872
- Author
- [email protected]
- Date
- 2017-12-13 14:13:32 -0800 (Wed, 13 Dec 2017)
Log Message
RenderImage can be destroyed even before setting the style on it.
https://bugs.webkit.org/show_bug.cgi?id=180767
<rdar://problem/33965995>
Reviewed by Simon Fraser.
Source/WebCore:
In certain cases, when the newly constructed renderer can't be inserted into the tree (parent can only have specific type of children etc),
RenderTreeUpdater destroys it right away. While destroying a RenderImage, the associated image resource assumes
that the image renderer has been initialized through RenderElement::initializeStyle(). This is an incorrect
assumption.
This patch also makes RenderImageResource's m_renderer a weak pointer.
Test: fast/images/crash-when-image-renderer-is-destroyed-before-calling-initializeStyle.html
* rendering/RenderImageResource.cpp:
(WebCore::RenderImageResource::initialize):
(WebCore::RenderImageResource::setCachedImage):
(WebCore::RenderImageResource::resetAnimation):
(WebCore::RenderImageResource::image const):
(WebCore::RenderImageResource::setContainerContext):
(WebCore::RenderImageResource::imageSize const):
* rendering/RenderImageResource.h:
(WebCore::RenderImageResource::renderer const):
* rendering/RenderImageResourceStyleImage.cpp:
(WebCore::RenderImageResourceStyleImage::shutdown):
LayoutTests:
* fast/images/crash-when-image-renderer-is-destroyed-before-calling-initializeStyle-expected.txt: Added.
* fast/images/crash-when-image-renderer-is-destroyed-before-calling-initializeStyle.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (225871 => 225872)
--- trunk/LayoutTests/ChangeLog 2017-12-13 21:58:19 UTC (rev 225871)
+++ trunk/LayoutTests/ChangeLog 2017-12-13 22:13:32 UTC (rev 225872)
@@ -1,3 +1,14 @@
+2017-12-13 Zalan Bujtas <[email protected]>
+
+ RenderImage can be destroyed even before setting the style on it.
+ https://bugs.webkit.org/show_bug.cgi?id=180767
+ <rdar://problem/33965995>
+
+ Reviewed by Simon Fraser.
+
+ * fast/images/crash-when-image-renderer-is-destroyed-before-calling-initializeStyle-expected.txt: Added.
+ * fast/images/crash-when-image-renderer-is-destroyed-before-calling-initializeStyle.html: Added.
+
2017-12-13 Matt Lewis <[email protected]>
Marked accessibility/mac/html5-input-number.html as flaky on macOS.
Added: trunk/LayoutTests/fast/images/crash-when-image-renderer-is-destroyed-before-calling-initializeStyle-expected.txt (0 => 225872)
--- trunk/LayoutTests/fast/images/crash-when-image-renderer-is-destroyed-before-calling-initializeStyle-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/images/crash-when-image-renderer-is-destroyed-before-calling-initializeStyle-expected.txt 2017-12-13 22:13:32 UTC (rev 225872)
@@ -0,0 +1,2 @@
+PASS if no crash.
+
Added: trunk/LayoutTests/fast/images/crash-when-image-renderer-is-destroyed-before-calling-initializeStyle.html (0 => 225872)
--- trunk/LayoutTests/fast/images/crash-when-image-renderer-is-destroyed-before-calling-initializeStyle.html (rev 0)
+++ trunk/LayoutTests/fast/images/crash-when-image-renderer-is-destroyed-before-calling-initializeStyle.html 2017-12-13 22:13:32 UTC (rev 225872)
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+col { content: url(data:image/gif;base64,foobar); }
+</style>
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+</script>
+</head>
+<body>
+PASS if no crash.
+<table>
+<col>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (225871 => 225872)
--- trunk/Source/WebCore/ChangeLog 2017-12-13 21:58:19 UTC (rev 225871)
+++ trunk/Source/WebCore/ChangeLog 2017-12-13 22:13:32 UTC (rev 225872)
@@ -1,3 +1,31 @@
+2017-12-13 Zalan Bujtas <[email protected]>
+
+ RenderImage can be destroyed even before setting the style on it.
+ https://bugs.webkit.org/show_bug.cgi?id=180767
+ <rdar://problem/33965995>
+
+ Reviewed by Simon Fraser.
+
+ In certain cases, when the newly constructed renderer can't be inserted into the tree (parent can only have specific type of children etc),
+ RenderTreeUpdater destroys it right away. While destroying a RenderImage, the associated image resource assumes
+ that the image renderer has been initialized through RenderElement::initializeStyle(). This is an incorrect
+ assumption.
+ This patch also makes RenderImageResource's m_renderer a weak pointer.
+
+ Test: fast/images/crash-when-image-renderer-is-destroyed-before-calling-initializeStyle.html
+
+ * rendering/RenderImageResource.cpp:
+ (WebCore::RenderImageResource::initialize):
+ (WebCore::RenderImageResource::setCachedImage):
+ (WebCore::RenderImageResource::resetAnimation):
+ (WebCore::RenderImageResource::image const):
+ (WebCore::RenderImageResource::setContainerContext):
+ (WebCore::RenderImageResource::imageSize const):
+ * rendering/RenderImageResource.h:
+ (WebCore::RenderImageResource::renderer const):
+ * rendering/RenderImageResourceStyleImage.cpp:
+ (WebCore::RenderImageResourceStyleImage::shutdown):
+
2017-12-13 Ryosuke Niwa <[email protected]>
Update the SVG use element's shadow trees explicitly before the style recall
Modified: trunk/Source/WebCore/rendering/RenderImageResource.cpp (225871 => 225872)
--- trunk/Source/WebCore/rendering/RenderImageResource.cpp 2017-12-13 21:58:19 UTC (rev 225871)
+++ trunk/Source/WebCore/rendering/RenderImageResource.cpp 2017-12-13 22:13:32 UTC (rev 225872)
@@ -47,7 +47,7 @@
{
ASSERT(!m_renderer);
ASSERT(!m_cachedImage);
- m_renderer = &renderer;
+ m_renderer = makeWeakPtr(renderer);
m_cachedImage = styleCachedImage;
m_cachedImageRemoveClientIsNeeded = !styleCachedImage;
}
@@ -65,15 +65,15 @@
ASSERT(m_renderer);
if (m_cachedImage && m_cachedImageRemoveClientIsNeeded)
- m_cachedImage->removeClient(*m_renderer);
+ m_cachedImage->removeClient(*renderer());
m_cachedImage = newImage;
m_cachedImageRemoveClientIsNeeded = true;
if (!m_cachedImage)
return;
- m_cachedImage->addClient(*m_renderer);
+ m_cachedImage->addClient(*renderer());
if (m_cachedImage->errorOccurred())
- m_renderer->imageChanged(m_cachedImage.get());
+ renderer()->imageChanged(m_cachedImage.get());
}
void RenderImageResource::resetAnimation()
@@ -84,8 +84,8 @@
ASSERT(m_renderer);
image()->resetAnimation();
- if (!m_renderer->needsLayout())
- m_renderer->repaint();
+ if (!renderer()->needsLayout())
+ renderer()->repaint();
}
RefPtr<Image> RenderImageResource::image(const IntSize&) const
@@ -92,7 +92,7 @@
{
if (!m_cachedImage)
return &Image::nullImage();
- if (auto image = m_cachedImage->imageForRenderer(m_renderer))
+ if (auto image = m_cachedImage->imageForRenderer(renderer()))
return image;
return &Image::nullImage();
}
@@ -102,7 +102,7 @@
if (!m_cachedImage)
return;
ASSERT(m_renderer);
- m_cachedImage->setContainerContextForClient(*m_renderer, imageContainerSize, m_renderer->style().effectiveZoom(), imageURL);
+ m_cachedImage->setContainerContextForClient(*renderer(), imageContainerSize, renderer()->style().effectiveZoom(), imageURL);
}
LayoutSize RenderImageResource::imageSize(float multiplier, CachedImage::SizeType type) const
@@ -109,9 +109,9 @@
{
if (!m_cachedImage)
return LayoutSize();
- LayoutSize size = m_cachedImage->imageSizeForRenderer(m_renderer, multiplier, type);
- if (is<RenderImage>(m_renderer))
- size.scale(downcast<RenderImage>(*m_renderer).imageDevicePixelRatio());
+ LayoutSize size = m_cachedImage->imageSizeForRenderer(renderer(), multiplier, type);
+ if (is<RenderImage>(renderer()))
+ size.scale(downcast<RenderImage>(*renderer()).imageDevicePixelRatio());
return size;
}
Modified: trunk/Source/WebCore/rendering/RenderImageResource.h (225871 => 225872)
--- trunk/Source/WebCore/rendering/RenderImageResource.h 2017-12-13 21:58:19 UTC (rev 225871)
+++ trunk/Source/WebCore/rendering/RenderImageResource.h 2017-12-13 22:13:32 UTC (rev 225872)
@@ -29,6 +29,7 @@
#include "CachedResourceHandle.h"
#include "StyleImage.h"
#include <wtf/IsoMalloc.h>
+#include <wtf/WeakPtr.h>
namespace WebCore {
@@ -63,13 +64,13 @@
virtual WrappedImagePtr imagePtr() const { return m_cachedImage.get(); }
protected:
- RenderElement* renderer() const { return m_renderer; }
+ RenderElement* renderer() const { return m_renderer.get(); }
void initialize(RenderElement&, CachedImage*);
private:
virtual LayoutSize imageSize(float multiplier, CachedImage::SizeType) const;
- RenderElement* m_renderer { nullptr };
+ WeakPtr<RenderElement> m_renderer;
CachedResourceHandle<CachedImage> m_cachedImage;
bool m_cachedImageRemoveClientIsNeeded { true };
};
Modified: trunk/Source/WebCore/rendering/RenderImageResourceStyleImage.cpp (225871 => 225872)
--- trunk/Source/WebCore/rendering/RenderImageResourceStyleImage.cpp 2017-12-13 21:58:19 UTC (rev 225871)
+++ trunk/Source/WebCore/rendering/RenderImageResourceStyleImage.cpp 2017-12-13 22:13:32 UTC (rev 225872)
@@ -50,9 +50,9 @@
void RenderImageResourceStyleImage::shutdown()
{
- ASSERT(renderer());
RenderImageResource::shutdown();
- m_styleImage->removeClient(renderer());
+ if (renderer())
+ m_styleImage->removeClient(renderer());
}
RefPtr<Image> RenderImageResourceStyleImage::image(const IntSize& size) const
