Image should clear its ImageObserver* when CachedImage releases the last reference...
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 9 Jun 2017 18:38:39 +0000 (18:38 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 9 Jun 2017 18:38:39 +0000 (18:38 +0000)
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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@218003 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/loader/cache/CachedImage.cpp
Source/WebCore/loader/cache/CachedImage.h

index 79972b7..3d8d2f9 100644 (file)
@@ -1,3 +1,21 @@
+2017-06-09  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        Image should clear its ImageObserver* when CachedImage releases the last reference to its RefCounted<ImageObserver>
+        https://bugs.webkit.org/show_bug.cgi?id=173077
+
+        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:
+
 2017-06-09  Frederic Wang  <fwang@igalia.com>
 
         Add flag allow-popups-to-escape-sandbox to iframe sandbox
index e860d14..398de2b 100644 (file)
@@ -101,7 +101,7 @@ void CachedImage::setBodyDataFrom(const CachedResource& resource)
     m_image = image.m_image;
     m_imageObserver = image.m_imageObserver;
     if (m_imageObserver)
-        m_imageObserver->add(*this);
+        m_imageObserver->cachedImages().add(this);
 
     if (m_image && is<SVGImage>(*m_image))
         m_svgImageCache = std::make_unique<SVGImageCache>(&downcast<SVGImage>(*m_image));
@@ -326,8 +326,7 @@ inline void CachedImage::createImage()
 
 CachedImage::CachedImageObserver::CachedImageObserver(CachedImage& image)
 {
-    m_cachedImages.reserveInitialCapacity(1);
-    m_cachedImages.append(&image);
+    m_cachedImages.add(&image);
 }
 
 void CachedImage::CachedImageObserver::decodedSizeChanged(const Image& image, long long delta)
@@ -367,10 +366,21 @@ void CachedImage::CachedImageObserver::changedInRect(const Image& image, const I
 
 inline void CachedImage::clearImage()
 {
+    if (!m_image)
+        return;
+
     if (m_imageObserver) {
-        m_imageObserver->remove(*this);
+        m_imageObserver->cachedImages().remove(this);
+
+        if (m_imageObserver->cachedImages().isEmpty()) {
+            ASSERT(m_image->hasOneRef());
+            ASSERT(m_imageObserver->hasOneRef());
+            m_image->setImageObserver(nullptr);
+        }
+
         m_imageObserver = nullptr;
     }
+
     m_image = nullptr;
 }
 
index e0c6f60..336f16e 100644 (file)
@@ -120,14 +120,14 @@ private:
     class CachedImageObserver final : public RefCounted<CachedImageObserver>, public ImageObserver {
     public:
         static Ref<CachedImageObserver> create(CachedImage& image) { return adoptRef(*new CachedImageObserver(image)); }
-        void add(CachedImage& image) { m_cachedImages.append(&image); }
-        void remove(CachedImage& image) { m_cachedImages.removeFirst(&image); }
+        HashSet<CachedImage*>& cachedImages() { return m_cachedImages; }
+        const HashSet<CachedImage*>& cachedImages() const { return m_cachedImages; }
 
     private:
         explicit CachedImageObserver(CachedImage&);
 
         // ImageObserver API
-        URL sourceUrl() const override { return !m_cachedImages.isEmpty() ? m_cachedImages[0]->url() : URL(); }
+        URL sourceUrl() const override { return !m_cachedImages.isEmpty() ? (*m_cachedImages.begin())->url() : URL(); }
         void decodedSizeChanged(const Image&, long long delta) final;
         void didDraw(const Image&) final;
 
@@ -135,7 +135,7 @@ private:
         void imageFrameAvailable(const Image&, ImageAnimatingState, const IntRect* changeRect = nullptr) final;
         void changedInRect(const Image&, const IntRect*) final;
 
-        Vector<CachedImage*> m_cachedImages;
+        HashSet<CachedImage*> m_cachedImages;
     };
 
     void decodedSizeChanged(const Image&, long long delta);