[webkit-reviews] review denied: [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]

Michael Catanzaro <mcatanzaro at igalia.com> has denied  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 #7 from Michael Catanzaro <mcatanzaro 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

Thanks Miguel!

Yeah, private constructors please.

>> Source/WebCore/platform/image-decoders/ImageDecoder.cpp:149
>> +	    return nullptr;
> 
> This should never happen, I think we should return the default and make this
return a Ref<>

What do you mean by "return the default"... is there a default image decoder? I
would do RELEASE_ASSERT_NOT_REACHED().

> Source/WebCore/platform/image-decoders/ico/ICOImageDecoder.h:142
> +	   typedef Vector<RefPtr<ImageDecoder>> PNGDecoders;

So if you take Carlos's suggestion, then this should use Ref rather than
RefPtr.