Skip to content

Commit d209322

Browse files
Said Abou-Hallawawebkit-commit-queue
Said Abou-Hallawa
authored and
webkit-commit-queue
committed
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 <sabouhallawa@apple.com> 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. Canonical link: https://commits.webkit.org/191764@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@220048 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent ea550fa commit d209322

14 files changed

Lines changed: 95 additions & 16 deletions

‎LayoutTests/ChangeLog‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,14 @@
1+
2017-07-30 Said Abou-Hallawa <sabouhallawa@apple.com>
2+
3+
RenderImageResourceStyleImage::image() should return the nullImage() if the image is not available
4+
https://bugs.webkit.org/show_bug.cgi?id=174874
5+
<rdar://problem/33530130>
6+
7+
Reviewed by Darin Adler.
8+
9+
* fast/images/image-element-image-content-data-expected.txt: Added.
10+
* fast/images/image-element-image-content-data.html: Added.
11+
112
2017-07-29 Nan Wang <n_wang@apple.com>
213

314
AX: findMatchingObjects doesn't work when the startObject is ignored

‎LayoutTests/fast/images/image-element-image-content-data-expected.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
PASS if no crash happens.
2+
3+

‎LayoutTests/fast/images/image-element-image-content-data.html‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<style>
2+
img {
3+
width: 100px;
4+
height: 100px;
5+
border: 2px solid black;
6+
content: -webkit-named-image(apple-pay-logo-white);
7+
}
8+
</style>
9+
<body>
10+
<p>PASS if no crash happens.</p>
11+
<img src='resources/green-400x400.png'>
12+
<script>
13+
if (window.testRunner)
14+
testRunner.dumpAsText(true);
15+
setTimeout(function() {
16+
var image = document.querySelector('img');
17+
image.remove();
18+
}, 0);
19+
</script>
20+
</body>

‎Source/WebCore/ChangeLog‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,56 @@
1+
2017-07-30 Said Abou-Hallawa <sabouhallawa@apple.com>
2+
3+
RenderImageResourceStyleImage::image() should return the nullImage() if the image is not available
4+
https://bugs.webkit.org/show_bug.cgi?id=174874
5+
<rdar://problem/33530130>
6+
7+
Reviewed by Darin Adler.
8+
9+
If an <img> element has image content data for a none cached image, e.g.
10+
-webkit-named-image, RenderImageResourceStyleImage will be created and
11+
attached to the RenderImage. RenderImageResourceStyleImage::m_cachedImage
12+
will be set to null because the m_styleImage->isCachedImage() is false in
13+
this case. When ImageLoader finishes loading the url of the src attribute,
14+
RenderImageResource::setCachedImage() will be called to set m_cachedImage.
15+
16+
A crash will happen when the RenderImage is destroyed. Destroying the
17+
RenderImage calls RenderImageResourceStyleImage::shutdown() which checks
18+
m_cachedImage and finds it not null, so it calls RenderImageResourceStyleImage::image()
19+
which ends up calling CSSNamedImageValue::image() which returns a null pointer
20+
because the size is empty. RenderImageResourceStyleImage::shutdown() calls
21+
image()->stopAnimation() without checking the return value of image().
22+
23+
Like the base class virtual method RenderImageResource::image(),
24+
RenderImageResourceStyleImage::image() should return the nullImage() if
25+
the image is not available.
26+
27+
Test: fast/images/image-element-image-content-data.html
28+
29+
* css/CSSCrossfadeValue.cpp:
30+
* css/CSSFilterImageValue.cpp:
31+
* page/EventHandler.cpp:
32+
* page/PageSerializer.cpp:
33+
* rendering/RenderElement.cpp:
34+
* rendering/RenderImageResource.cpp:
35+
* rendering/RenderImageResourceStyleImage.cpp:
36+
(WebCore::RenderImageResourceStyleImage::initialize):
37+
38+
(WebCore::RenderImageResourceStyleImage::shutdown): Revert back the changes
39+
of r208511 in this function. Add a call to image()->stopAnimation() without
40+
checking the return of image() since it will return the nullImage() if
41+
the image not available. There is no need to check m_cachedImage before
42+
calling image() because image() does not check or access m_cachedImage.
43+
44+
(WebCore::RenderImageResourceStyleImage::image): The base class method
45+
RenderImageResource::image() returns the nullImage() if the image not
46+
available. This is because CachedImage::imageForRenderer() returns
47+
the nullImage() if the image is not available; see CachedImage.h. We should
48+
do the same for the derived class for consistency.
49+
50+
* rendering/style/ContentData.cpp:
51+
* rendering/style/StyleCachedImage.cpp:
52+
* style/StylePendingResources.cpp:
53+
154
2017-07-29 Filip Pizlo <fpizlo@apple.com>
255

356
Unreviewed, rollout r220044 because it set the bots on fire.

‎Source/WebCore/css/CSSCrossfadeValue.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
#include "CachedResourceLoader.h"
3434
#include "CrossfadeGeneratedImage.h"
3535
#include "RenderElement.h"
36-
#include "StyleCachedImage.h"
3736
#include <wtf/text/StringBuilder.h>
3837

3938
namespace WebCore {

‎Source/WebCore/css/CSSFilterImageValue.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
#include "GraphicsContext.h"
3434
#include "ImageBuffer.h"
3535
#include "RenderElement.h"
36-
#include "StyleCachedImage.h"
3736
#include "StyleResolver.h"
3837
#include <wtf/text/StringBuilder.h>
3938

‎Source/WebCore/page/EventHandler.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,6 @@
8686
#include "Settings.h"
8787
#include "ShadowRoot.h"
8888
#include "SpatialNavigation.h"
89-
#include "StyleCachedImage.h"
9089
#include "TextEvent.h"
9190
#include "TextIterator.h"
9291
#include "UserGestureIndicator.h"

‎Source/WebCore/page/PageSerializer.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@
5252
#include "MarkupAccumulator.h"
5353
#include "Page.h"
5454
#include "RenderElement.h"
55-
#include "StyleCachedImage.h"
5655
#include "StyleImage.h"
5756
#include "StyleProperties.h"
5857
#include "StyleRule.h"

‎Source/WebCore/rendering/RenderElement.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@
5050
#include "RenderDescendantIterator.h"
5151
#include "RenderFlexibleBox.h"
5252
#include "RenderImage.h"
53-
#include "RenderImageResourceStyleImage.h"
5453
#include "RenderInline.h"
5554
#include "RenderIterator.h"
5655
#include "RenderLayer.h"

‎Source/WebCore/rendering/RenderImageResource.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
#include "Image.h"
3333
#include "RenderElement.h"
3434
#include "RenderImage.h"
35-
#include "RenderImageResourceStyleImage.h"
3635

3736
namespace WebCore {
3837

0 commit comments

Comments
 (0)