- Revision
- 220048
- Author
- [email protected]
- Date
- 2017-07-30 00:38:31 -0700 (Sun, 30 Jul 2017)
Log Message
RenderImageResourceStyleImage::image() should return the nullImage() if the image is not available
https://bugs.webkit.org/show_bug.cgi?id=174874
<rdar://problem/33530130>
Patch by Said Abou-Hallawa <[email protected]> on 2017-07-30
Reviewed by Darin Adler.
Source/WebCore:
If an <img> element has image content data for a none cached image, e.g.
-webkit-named-image, RenderImageResourceStyleImage will be created and
attached to the RenderImage. RenderImageResourceStyleImage::m_cachedImage
will be set to null because the m_styleImage->isCachedImage() is false in
this case. When ImageLoader finishes loading the url of the src attribute,
RenderImageResource::setCachedImage() will be called to set m_cachedImage.
A crash will happen when the RenderImage is destroyed. Destroying the
RenderImage calls RenderImageResourceStyleImage::shutdown() which checks
m_cachedImage and finds it not null, so it calls RenderImageResourceStyleImage::image()
which ends up calling CSSNamedImageValue::image() which returns a null pointer
because the size is empty. RenderImageResourceStyleImage::shutdown() calls
image()->stopAnimation() without checking the return value of image().
Like the base class virtual method RenderImageResource::image(),
RenderImageResourceStyleImage::image() should return the nullImage() if
the image is not available.
Test: fast/images/image-element-image-content-data.html
* css/CSSCrossfadeValue.cpp:
* css/CSSFilterImageValue.cpp:
* page/EventHandler.cpp:
* page/PageSerializer.cpp:
* rendering/RenderElement.cpp:
* rendering/RenderImageResource.cpp:
* rendering/RenderImageResourceStyleImage.cpp:
(WebCore::RenderImageResourceStyleImage::initialize):
(WebCore::RenderImageResourceStyleImage::shutdown): Revert back the changes
of r208511 in this function. Add a call to image()->stopAnimation() without
checking the return of image() since it will return the nullImage() if
the image not available. There is no need to check m_cachedImage before
calling image() because image() does not check or access m_cachedImage.
(WebCore::RenderImageResourceStyleImage::image): The base class method
RenderImageResource::image() returns the nullImage() if the image not
available. This is because CachedImage::imageForRenderer() returns
the nullImage() if the image is not available; see CachedImage.h. We should
do the same for the derived class for consistency.
* rendering/style/ContentData.cpp:
* rendering/style/StyleCachedImage.cpp:
* style/StylePendingResources.cpp:
LayoutTests:
* fast/images/image-element-image-content-data-expected.txt: Added.
* fast/images/image-element-image-content-data.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (220047 => 220048)
--- trunk/LayoutTests/ChangeLog 2017-07-30 03:01:12 UTC (rev 220047)
+++ trunk/LayoutTests/ChangeLog 2017-07-30 07:38:31 UTC (rev 220048)
@@ -1,3 +1,14 @@
+2017-07-30 Said Abou-Hallawa <[email protected]>
+
+ RenderImageResourceStyleImage::image() should return the nullImage() if the image is not available
+ https://bugs.webkit.org/show_bug.cgi?id=174874
+ <rdar://problem/33530130>
+
+ Reviewed by Darin Adler.
+
+ * fast/images/image-element-image-content-data-expected.txt: Added.
+ * fast/images/image-element-image-content-data.html: Added.
+
2017-07-29 Nan Wang <[email protected]>
AX: findMatchingObjects doesn't work when the startObject is ignored
Added: trunk/LayoutTests/fast/images/image-element-image-content-data-expected.txt (0 => 220048)
--- trunk/LayoutTests/fast/images/image-element-image-content-data-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/images/image-element-image-content-data-expected.txt 2017-07-30 07:38:31 UTC (rev 220048)
@@ -0,0 +1,3 @@
+PASS if no crash happens.
+
+
Added: trunk/LayoutTests/fast/images/image-element-image-content-data.html (0 => 220048)
--- trunk/LayoutTests/fast/images/image-element-image-content-data.html (rev 0)
+++ trunk/LayoutTests/fast/images/image-element-image-content-data.html 2017-07-30 07:38:31 UTC (rev 220048)
@@ -0,0 +1,20 @@
+<style>
+ img {
+ width: 100px;
+ height: 100px;
+ border: 2px solid black;
+ content: -webkit-named-image(apple-pay-logo-white);
+ }
+</style>
+<body>
+ <p>PASS if no crash happens.</p>
+ <img src=''>
+ <script>
+ if (window.testRunner)
+ testRunner.dumpAsText(true);
+ setTimeout(function() {
+ var image = document.querySelector('img');
+ image.remove();
+ }, 0);
+ </script>
+</body>
Modified: trunk/Source/WebCore/ChangeLog (220047 => 220048)
--- trunk/Source/WebCore/ChangeLog 2017-07-30 03:01:12 UTC (rev 220047)
+++ trunk/Source/WebCore/ChangeLog 2017-07-30 07:38:31 UTC (rev 220048)
@@ -1,3 +1,56 @@
+2017-07-30 Said Abou-Hallawa <[email protected]>
+
+ RenderImageResourceStyleImage::image() should return the nullImage() if the image is not available
+ https://bugs.webkit.org/show_bug.cgi?id=174874
+ <rdar://problem/33530130>
+
+ Reviewed by Darin Adler.
+
+ If an <img> element has image content data for a none cached image, e.g.
+ -webkit-named-image, RenderImageResourceStyleImage will be created and
+ attached to the RenderImage. RenderImageResourceStyleImage::m_cachedImage
+ will be set to null because the m_styleImage->isCachedImage() is false in
+ this case. When ImageLoader finishes loading the url of the src attribute,
+ RenderImageResource::setCachedImage() will be called to set m_cachedImage.
+
+ A crash will happen when the RenderImage is destroyed. Destroying the
+ RenderImage calls RenderImageResourceStyleImage::shutdown() which checks
+ m_cachedImage and finds it not null, so it calls RenderImageResourceStyleImage::image()
+ which ends up calling CSSNamedImageValue::image() which returns a null pointer
+ because the size is empty. RenderImageResourceStyleImage::shutdown() calls
+ image()->stopAnimation() without checking the return value of image().
+
+ Like the base class virtual method RenderImageResource::image(),
+ RenderImageResourceStyleImage::image() should return the nullImage() if
+ the image is not available.
+
+ Test: fast/images/image-element-image-content-data.html
+
+ * css/CSSCrossfadeValue.cpp:
+ * css/CSSFilterImageValue.cpp:
+ * page/EventHandler.cpp:
+ * page/PageSerializer.cpp:
+ * rendering/RenderElement.cpp:
+ * rendering/RenderImageResource.cpp:
+ * rendering/RenderImageResourceStyleImage.cpp:
+ (WebCore::RenderImageResourceStyleImage::initialize):
+
+ (WebCore::RenderImageResourceStyleImage::shutdown): Revert back the changes
+ of r208511 in this function. Add a call to image()->stopAnimation() without
+ checking the return of image() since it will return the nullImage() if
+ the image not available. There is no need to check m_cachedImage before
+ calling image() because image() does not check or access m_cachedImage.
+
+ (WebCore::RenderImageResourceStyleImage::image): The base class method
+ RenderImageResource::image() returns the nullImage() if the image not
+ available. This is because CachedImage::imageForRenderer() returns
+ the nullImage() if the image is not available; see CachedImage.h. We should
+ do the same for the derived class for consistency.
+
+ * rendering/style/ContentData.cpp:
+ * rendering/style/StyleCachedImage.cpp:
+ * style/StylePendingResources.cpp:
+
2017-07-29 Filip Pizlo <[email protected]>
Unreviewed, rollout r220044 because it set the bots on fire.
Modified: trunk/Source/WebCore/css/CSSCrossfadeValue.cpp (220047 => 220048)
--- trunk/Source/WebCore/css/CSSCrossfadeValue.cpp 2017-07-30 03:01:12 UTC (rev 220047)
+++ trunk/Source/WebCore/css/CSSCrossfadeValue.cpp 2017-07-30 07:38:31 UTC (rev 220048)
@@ -33,7 +33,6 @@
#include "CachedResourceLoader.h"
#include "CrossfadeGeneratedImage.h"
#include "RenderElement.h"
-#include "StyleCachedImage.h"
#include <wtf/text/StringBuilder.h>
namespace WebCore {
Modified: trunk/Source/WebCore/css/CSSFilterImageValue.cpp (220047 => 220048)
--- trunk/Source/WebCore/css/CSSFilterImageValue.cpp 2017-07-30 03:01:12 UTC (rev 220047)
+++ trunk/Source/WebCore/css/CSSFilterImageValue.cpp 2017-07-30 07:38:31 UTC (rev 220048)
@@ -33,7 +33,6 @@
#include "GraphicsContext.h"
#include "ImageBuffer.h"
#include "RenderElement.h"
-#include "StyleCachedImage.h"
#include "StyleResolver.h"
#include <wtf/text/StringBuilder.h>
Modified: trunk/Source/WebCore/page/EventHandler.cpp (220047 => 220048)
--- trunk/Source/WebCore/page/EventHandler.cpp 2017-07-30 03:01:12 UTC (rev 220047)
+++ trunk/Source/WebCore/page/EventHandler.cpp 2017-07-30 07:38:31 UTC (rev 220048)
@@ -86,7 +86,6 @@
#include "Settings.h"
#include "ShadowRoot.h"
#include "SpatialNavigation.h"
-#include "StyleCachedImage.h"
#include "TextEvent.h"
#include "TextIterator.h"
#include "UserGestureIndicator.h"
Modified: trunk/Source/WebCore/page/PageSerializer.cpp (220047 => 220048)
--- trunk/Source/WebCore/page/PageSerializer.cpp 2017-07-30 03:01:12 UTC (rev 220047)
+++ trunk/Source/WebCore/page/PageSerializer.cpp 2017-07-30 07:38:31 UTC (rev 220048)
@@ -52,7 +52,6 @@
#include "MarkupAccumulator.h"
#include "Page.h"
#include "RenderElement.h"
-#include "StyleCachedImage.h"
#include "StyleImage.h"
#include "StyleProperties.h"
#include "StyleRule.h"
Modified: trunk/Source/WebCore/rendering/RenderElement.cpp (220047 => 220048)
--- trunk/Source/WebCore/rendering/RenderElement.cpp 2017-07-30 03:01:12 UTC (rev 220047)
+++ trunk/Source/WebCore/rendering/RenderElement.cpp 2017-07-30 07:38:31 UTC (rev 220048)
@@ -50,7 +50,6 @@
#include "RenderDescendantIterator.h"
#include "RenderFlexibleBox.h"
#include "RenderImage.h"
-#include "RenderImageResourceStyleImage.h"
#include "RenderInline.h"
#include "RenderIterator.h"
#include "RenderLayer.h"
Modified: trunk/Source/WebCore/rendering/RenderImageResource.cpp (220047 => 220048)
--- trunk/Source/WebCore/rendering/RenderImageResource.cpp 2017-07-30 03:01:12 UTC (rev 220047)
+++ trunk/Source/WebCore/rendering/RenderImageResource.cpp 2017-07-30 07:38:31 UTC (rev 220048)
@@ -32,7 +32,6 @@
#include "Image.h"
#include "RenderElement.h"
#include "RenderImage.h"
-#include "RenderImageResourceStyleImage.h"
namespace WebCore {
Modified: trunk/Source/WebCore/rendering/RenderImageResourceStyleImage.cpp (220047 => 220048)
--- trunk/Source/WebCore/rendering/RenderImageResourceStyleImage.cpp 2017-07-30 03:01:12 UTC (rev 220047)
+++ trunk/Source/WebCore/rendering/RenderImageResourceStyleImage.cpp 2017-07-30 07:38:31 UTC (rev 220048)
@@ -30,7 +30,6 @@
#include "CachedImage.h"
#include "RenderElement.h"
-#include "StyleCachedImage.h"
namespace WebCore {
@@ -48,7 +47,7 @@
RenderImageResource::initialize(renderer);
if (m_styleImage->isCachedImage())
- m_cachedImage = m_styleImage.get().cachedImage();
+ m_cachedImage = m_styleImage->cachedImage();
m_styleImage->addClient(m_renderer);
}
@@ -56,17 +55,19 @@
void RenderImageResourceStyleImage::shutdown()
{
ASSERT(m_renderer);
+ image()->stopAnimation();
m_styleImage->removeClient(m_renderer);
- if (m_cachedImage) {
- image()->stopAnimation();
- m_cachedImage = nullptr;
- }
+ m_cachedImage = nullptr;
}
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.
- return !m_styleImage->isPending() ? m_styleImage->image(m_renderer, size) : &Image::nullImage();
+ if (m_styleImage->isPending())
+ return &Image::nullImage();
+ if (auto image = m_styleImage->image(m_renderer, size))
+ return image;
+ return &Image::nullImage();
}
void RenderImageResourceStyleImage::setContainerSizeForRenderer(const IntSize& size)
Modified: trunk/Source/WebCore/rendering/style/ContentData.cpp (220047 => 220048)
--- trunk/Source/WebCore/rendering/style/ContentData.cpp 2017-07-30 03:01:12 UTC (rev 220047)
+++ trunk/Source/WebCore/rendering/style/ContentData.cpp 2017-07-30 07:38:31 UTC (rev 220048)
@@ -25,7 +25,6 @@
#include "RenderCounter.h"
#include "RenderImage.h"
#include "RenderImageResource.h"
-#include "RenderImageResourceStyleImage.h"
#include "RenderQuote.h"
#include "RenderStyle.h"
#include "RenderTextFragment.h"
Modified: trunk/Source/WebCore/rendering/style/StyleCachedImage.cpp (220047 => 220048)
--- trunk/Source/WebCore/rendering/style/StyleCachedImage.cpp 2017-07-30 03:01:12 UTC (rev 220047)
+++ trunk/Source/WebCore/rendering/style/StyleCachedImage.cpp 2017-07-30 07:38:31 UTC (rev 220048)
@@ -29,7 +29,6 @@
#include "CSSImageValue.h"
#include "CachedImage.h"
#include "RenderElement.h"
-#include "RenderView.h"
namespace WebCore {
Modified: trunk/Source/WebCore/style/StylePendingResources.cpp (220047 => 220048)
--- trunk/Source/WebCore/style/StylePendingResources.cpp 2017-07-30 03:01:12 UTC (rev 220047)
+++ trunk/Source/WebCore/style/StylePendingResources.cpp 2017-07-30 07:38:31 UTC (rev 220048)
@@ -34,7 +34,6 @@
#include "Document.h"
#include "RenderStyle.h"
#include "SVGURIReference.h"
-#include "StyleCachedImage.h"
#include "StyleGeneratedImage.h"
#include "TransformFunctions.h"
