[Webkit-unassigned] [Bug 172122] Make ImageDecoder be a member of ImageFrameCache only

[bugzilla-daemon at webkit.org]

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

--- Comment #3 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 310147
  --> https://bugs.webkit.org/attachment.cgi?id=310147
Patch

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

> Source/WebCore/platform/graphics/ImageFrameCache.cpp:244
> +    frame.m_isSubsamplingAllowed = m_decoder->frameAllowSubsamplingAtIndex(index);

Why is this different from ImageFrameCache::isSubsamplingAllowed()?

> Source/WebCore/platform/graphics/ImageFrameCache.cpp:594
> +    if (isDecoderAvailable())
> +        m_decoder->setTargetContext(targetContext);

I think we should keep the "set a GraphicsContext* on the decoder" concept specific to Windows, with #ifdefs.

> Source/WebCore/platform/graphics/ImageFrameCache.h:63
> +    void clear(SharedBuffer*);

Why does clear() have a parameter that passes in data?

> Source/WebCore/platform/graphics/ImageFrameCache.h:151
>      RefPtr<ImageDecoder> m_decoder;

If the decoder is a RefPtr, what's the guarantee that a reference is not handed out to some other class? a unique_ptr would be better.

> Source/WebCore/platform/image-decoders/ImageDecoder.h:200
> +    void setTargetContext(const GraphicsContext*) { }

It's a shame to make this windows-only ugliness used for all platforms.

-- 
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/20170515/aeaec3f9/attachment.html>