[Webkit-unassigned] [Bug 203353] [WinCairo] GIFImageReader::m_data is ref/deref in different threads

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=203353

--- Comment #8 from Said Abou-Hallawa <sabouhallawa at apple.com> ---
Comment on attachment 382289
  --> https://bugs.webkit.org/attachment.cgi?id=382289
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=382289&action=review

> Source/WebCore/platform/image-decoders/ScalableImageDecoder.h:75
> +        return *m_data.get();

m_data can be null and this is why it isn't a Ref<SharedBuffer>.

> Source/WebCore/platform/image-decoders/bmp/BMPImageReader.cpp:128
> +    if ((m_decodedOffset > m_parent->data().size()) || ((m_parent->data().size() - m_decodedOffset) < 4))

The decoder here is called m_parent while it is called m_client in GIFImageReader. Can't we use the same name for both? Or can't we add a base class for both of them named "ImageReader"? In ImageReader, we can define the relationship between the reader and the decoder.

> Source/WebCore/platform/image-decoders/bmp/BMPImageReader.cpp:160
> +    if ((m_decodedOffset > m_parent->data().size()) || ((m_parent->data().size() - m_decodedOffset) < m_infoHeader.biSize) || !readInfoHeader())

Since m_parent->data() is repeated many times in this file, can't we add a wrapper: SharedBuffer* data() { returns m_parent->data(); }?

> Source/WebCore/platform/image-decoders/bmp/BMPImageReader.h:45
> +    static inline uint16_t readUint16(const SharedBuffer& data, int offset)

This function is a good candidate to be moved to the new base class ImageReader.

> Source/WebCore/platform/image-decoders/bmp/BMPImageReader.h:55
> +    static inline uint32_t readUint32(const SharedBuffer& data, int offset)

Ditto.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20191030/39e79384/attachment.htm>