Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit de568c1

Browse filesBrowse files
Said Abou-Hallawawebkit-commit-queue
authored andcommitted
Image should clear its ImageObserver* when CachedImage releases the last reference to its RefCounted<ImageObserver>
https://bugs.webkit.org/show_bug.cgi?id=173077 Patch by Said Abou-Hallawa <sabouhallawa@apple.com> on 2017-06-09 Reviewed by Simon Fraser. Before dereferencing ImageObserver, CachedImage::clearImage() should check whether it is the only object that holds a reference to this ImageObserver. And if this is true, m_image have to clear its raw pointer to the deleted ImageObserver by calling m_image->setImageObserver(nullptr). * loader/cache/CachedImage.cpp: (WebCore::CachedImage::setBodyDataFrom): (WebCore::CachedImage::CachedImageObserver::CachedImageObserver): (WebCore::CachedImage::clearImage): * loader/cache/CachedImage.h: Canonical link: https://commits.webkit.org/190050@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@218038 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 8e38c07 commit de568c1
Copy full SHA for de568c1

3 files changed

+35-8Lines changed: 35 additions & 8 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎Source/WebCore/ChangeLog‎

Copy file name to clipboardExpand all lines: Source/WebCore/ChangeLog
+18Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,21 @@
1+
2017-06-09 Said Abou-Hallawa <sabouhallawa@apple.com>
2+
3+
Image should clear its ImageObserver* when CachedImage releases the last reference to its RefCounted<ImageObserver>
4+
https://bugs.webkit.org/show_bug.cgi?id=173077
5+
6+
Reviewed by Simon Fraser.
7+
8+
Before dereferencing ImageObserver, CachedImage::clearImage() should check
9+
whether it is the only object that holds a reference to this ImageObserver.
10+
And if this is true, m_image have to clear its raw pointer to the deleted
11+
ImageObserver by calling m_image->setImageObserver(nullptr).
12+
13+
* loader/cache/CachedImage.cpp:
14+
(WebCore::CachedImage::setBodyDataFrom):
15+
(WebCore::CachedImage::CachedImageObserver::CachedImageObserver):
16+
(WebCore::CachedImage::clearImage):
17+
* loader/cache/CachedImage.h:
18+
119
2017-06-09 Daniel Bates <dabates@apple.com>
220

321
Attempt to fix layout test failures following <https://trac.webkit.org/changeset/218028/webkit>
Collapse file

‎Source/WebCore/loader/cache/CachedImage.cpp‎

Copy file name to clipboardExpand all lines: Source/WebCore/loader/cache/CachedImage.cpp
+13-4Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ void CachedImage::setBodyDataFrom(const CachedResource& resource)
101101
m_image = image.m_image;
102102
m_imageObserver = image.m_imageObserver;
103103
if (m_imageObserver)
104-
m_imageObserver->add(*this);
104+
m_imageObserver->cachedImages().add(this);
105105

106106
if (m_image && is<SVGImage>(*m_image))
107107
m_svgImageCache = std::make_unique<SVGImageCache>(&downcast<SVGImage>(*m_image));
@@ -326,8 +326,7 @@ inline void CachedImage::createImage()
326326

327327
CachedImage::CachedImageObserver::CachedImageObserver(CachedImage& image)
328328
{
329-
m_cachedImages.reserveInitialCapacity(1);
330-
m_cachedImages.append(&image);
329+
m_cachedImages.add(&image);
331330
}
332331

333332
void CachedImage::CachedImageObserver::decodedSizeChanged(const Image& image, long long delta)
@@ -367,10 +366,20 @@ void CachedImage::CachedImageObserver::changedInRect(const Image& image, const I
367366

368367
inline void CachedImage::clearImage()
369368
{
369+
if (!m_image)
370+
return;
371+
370372
if (m_imageObserver) {
371-
m_imageObserver->remove(*this);
373+
m_imageObserver->cachedImages().remove(this);
374+
375+
if (m_imageObserver->cachedImages().isEmpty()) {
376+
ASSERT(m_imageObserver->hasOneRef());
377+
m_image->setImageObserver(nullptr);
378+
}
379+
372380
m_imageObserver = nullptr;
373381
}
382+
374383
m_image = nullptr;
375384
}
376385

Collapse file

‎Source/WebCore/loader/cache/CachedImage.h‎

Copy file name to clipboardExpand all lines: Source/WebCore/loader/cache/CachedImage.h
+4-4Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,22 +120,22 @@ class CachedImage final : public CachedResource {
120120
class CachedImageObserver final : public RefCounted<CachedImageObserver>, public ImageObserver {
121121
public:
122122
static Ref<CachedImageObserver> create(CachedImage& image) { return adoptRef(*new CachedImageObserver(image)); }
123-
void add(CachedImage& image) { m_cachedImages.append(&image); }
124-
void remove(CachedImage& image) { m_cachedImages.removeFirst(&image); }
123+
HashSet<CachedImage*>& cachedImages() { return m_cachedImages; }
124+
const HashSet<CachedImage*>& cachedImages() const { return m_cachedImages; }
125125

126126
private:
127127
explicit CachedImageObserver(CachedImage&);
128128

129129
// ImageObserver API
130-
URL sourceUrl() const override { return !m_cachedImages.isEmpty() ? m_cachedImages[0]->url() : URL(); }
130+
URL sourceUrl() const override { return !m_cachedImages.isEmpty() ? (*m_cachedImages.begin())->url() : URL(); }
131131
void decodedSizeChanged(const Image&, long long delta) final;
132132
void didDraw(const Image&) final;
133133

134134
bool canDestroyDecodedData(const Image&) final;
135135
void imageFrameAvailable(const Image&, ImageAnimatingState, const IntRect* changeRect = nullptr) final;
136136
void changedInRect(const Image&, const IntRect*) final;
137137

138-
Vector<CachedImage*> m_cachedImages;
138+
HashSet<CachedImage*> m_cachedImages;
139139
};
140140

141141
void decodedSizeChanged(const Image&, long long delta);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.