Changeset 218253 in webkit

Timestamp:

Jun 14, 2017 4:23:25 AM (7 years ago)

Author:

magomez@igalia.com

Message:

REGRESSION(r216901): ImageDecoders: rendering of large images is broken since r216901
​https://bugs.webkit.org/show_bug.cgi?id=172502

Reviewed by Carlos Garcia Campos.

When using GTK and WPE image decoders, the decoded frames are stored inside a Vector of
ImageFrames inside the decoders. These ImageFrames have and ImageBackingStore with the
pixels. When a NativeImagePtr is requested, a cairo surface is created from the data
in those ImageBackingStores, but the data keeps being owned by the backing stores. Due
to this, if the decoder that created the image gets destroyed, the backing stores for
the decoded frames get destroyed as well, causing the cairo surfaces that were using
that data to contain garbage (and potentially cause a crash).

To fix this, we change ImageBackingStore so the pixels are stored in a SharedBuffer. The
buffer will be reffed everytime a cairo surface is created with it, and the cairo surfaces
will unref the buffer when they are destroyed. This way, the pixel data won't be freed
while there are cairo surfaces using it.

No new tests, no behaviour change.

• platform/graphics/ImageBackingStore.h:

(WebCore::ImageBackingStore::setSize):
(WebCore::ImageBackingStore::ImageBackingStore):

• platform/image-decoders/cairo/ImageBackingStoreCairo.cpp:

(WebCore::ImageBackingStore::image):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/ImageBackingStore.h (modified) (4 diffs)
• platform/image-decoders/cairo/ImageBackingStoreCairo.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-06-14  Miguel Gomez  <magomez@igalia.com>
+
+        REGRESSION(r216901): ImageDecoders: rendering of large images is broken since r216901
+        https://bugs.webkit.org/show_bug.cgi?id=172502
+
+        Reviewed by Carlos Garcia Campos.
+
+        When using GTK and WPE image decoders, the decoded frames are stored inside a Vector of
+        ImageFrames inside the decoders. These ImageFrames have and ImageBackingStore with the
+        pixels. When a NativeImagePtr is requested, a cairo surface is created from the data
+        in those ImageBackingStores, but the data keeps being owned by the backing stores. Due
+        to this, if the decoder that created the image gets destroyed, the backing stores for
+        the decoded frames get destroyed as well, causing the cairo surfaces that were using
+        that data to contain garbage (and potentially cause a crash).
+
+        To fix this, we change ImageBackingStore so the pixels are stored in a SharedBuffer. The
+        buffer will be reffed everytime a cairo surface is created with it, and the cairo surfaces
+        will unref the buffer when they are destroyed. This way, the pixel data won't be freed
+        while there are cairo surfaces using it.
+
+        No new tests, no behaviour change.
+
+        * platform/graphics/ImageBackingStore.h:
+        (WebCore::ImageBackingStore::setSize):
+        (WebCore::ImageBackingStore::ImageBackingStore):
+        * platform/image-decoders/cairo/ImageBackingStoreCairo.cpp:
+        (WebCore::ImageBackingStore::image):
+
 2017-06-14  Zan Dobersek  <zdobersek@igalia.com>
 

trunk/Source/WebCore/platform/graphics/ImageBackingStore.h

@@ -30 +30 @@
 #include "IntSize.h"
 #include "NativeImage.h"
-
-#include <wtf/Vector.h>
+#include "SharedBuffer.h"
 
 namespace WebCore {
@@ -55 +54 @@
             return false;
 
-        unsigned area = size.area().unsafeGet();
-        if (!m_pixels.tryReserveCapacity(area))
+        Vector<char> buffer;
+        size_t bufferSize = size.area().unsafeGet() * sizeof(RGBA32);
+
+        if (!buffer.tryReserveCapacity(bufferSize))
             return false;
 
-        m_pixels.resize(area);
-        m_pixelsPtr = m_pixels.data();
+        buffer.resize(bufferSize);
+        m_pixels = SharedBuffer::create(WTFMove(buffer));
+        m_pixelsPtr = reinterpret_cast<RGBA32*>(const_cast<char*>(m_pixels->data()));
         m_size = size;
         m_frameRect = IntRect(IntPoint(), m_size);
@@ -184 +186 @@
 
     ImageBackingStore(const ImageBackingStore& other)
-        : m_pixels(other.m_pixels)
-        , m_size(other.m_size)
+        : m_size(other.m_size)
         , m_premultiplyAlpha(other.m_premultiplyAlpha)
     {
         ASSERT(!m_size.isEmpty() && !isOverSize(m_size));
-        m_pixelsPtr = m_pixels.data();
+        m_pixels = other.m_pixels->copy();
+        m_pixelsPtr = reinterpret_cast<RGBA32*>(const_cast<char*>(m_pixels->data()));
     }
 
@@ -213 +215 @@
     }
 
-    Vector<RGBA32> m_pixels;
+    RefPtr<SharedBuffer> m_pixels;
     RGBA32* m_pixelsPtr { nullptr };
     IntSize m_size;

trunk/Source/WebCore/platform/image-decoders/cairo/ImageBackingStoreCairo.cpp

@@ -33 +33 @@
 NativeImagePtr ImageBackingStore::image() const
 {
-    return adoptRef(cairo_image_surface_create_for_data(
+    m_pixels->ref();
+    RefPtr<cairo_surface_t> surface = adoptRef(cairo_image_surface_create_for_data(
         reinterpret_cast<unsigned char*>(const_cast<RGBA32*>(m_pixelsPtr)),
         CAIRO_FORMAT_ARGB32, size().width(), size().height(), size().width() * sizeof(RGBA32)));
+    static cairo_user_data_key_t s_surfaceDataKey;
+    cairo_surface_set_user_data(surface.get(), &s_surfaceDataKey, m_pixels.get(), [](void* data) { static_cast<SharedBuffer*>(data)->deref(); });
+
+    return surface;
 }