[webkit-reviews] review granted: [Bug 171211] Image decoders must have private constructors to avoid refcount misuse: ASSERTION FAILED: m_deletionHasBegun when destroying ImageDecoder : [Attachment 307975] Patch

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has granted Miguel Gomez
<magomez at igalia.com>'s request for review:
Bug 171211: Image decoders must have private constructors to avoid refcount
misuse: ASSERTION FAILED: m_deletionHasBegun when destroying ImageDecoder
https://bugs.webkit.org/show_bug.cgi?id=171211

Attachment 307975: Patch

https://bugs.webkit.org/attachment.cgi?id=307975&action=review




--- Comment #6 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 307975
  --> https://bugs.webkit.org/attachment.cgi?id=307975
Patch

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

To prevent this from happening again, I would make all constructors private to
force everybody to use the create() functions.

> Source/WebCore/platform/graphics/ImageTypes.h:82
> +enum class ImageFormat {
> +    ImageFormatGIF,
> +    ImageFormatPNG,
> +    ImageFormatICO,
> +    ImageFormatCUR,
> +    ImageFormatJPEG,
> +    ImageFormatWEBP,
> +    ImageFormatBMP

When using enum class, including the class name in the elements is redundant,
ImageFormat::GIF, ImageFormat::PNG looks better, IMO.

> Source/WebCore/platform/image-decoders/ImageDecoder.cpp:149
> +    default:
> +	   return nullptr;

This should never happen, I think we should return the default and make this
return a Ref<>