[Webkit-unassigned] [Bug 164864] REGRESSION(r208511): ImageDecoders: Crash decoding GIF images since r208511

[bugzilla-daemon at webkit.org]

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

--- Comment #10 from Said Abou-Hallawa <sabouhallawa at apple.com> ---
(In reply to comment #8)
> (In reply to comment #6)
> > Comment on attachment 295048 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=295048&action=review
> > 
> > Would it be better to rename the existing virtual frameBufferAtIndex()
> > functions to be internalFrameBufferAtIndex() and create a non virtual
> > function called frameBufferAtIndex() in imageDecoder class which does the
> > locking and call internalFrameBufferAtIndex()?
> 
> I thought about that but I also want to make sure the returned ImageFrame is
> not modified by another thread, that's why don't release the lock until the
> end of the function.
> 

But the only function that changes the ImageFrame or the ImageDecoder:: m_frameBufferCache is ImageDecoder::frameBufferAtIndex(). It does that via calling ImageDecoder::decode() which calls ImageReader::decode(). The ImageReader::decode() is the one that is responsible for caching the ImageFrames in ImageDecoder:: m_frameBufferCache.

There is another calls to ImageDecoder::decode() from ImageDecoder::isSizeAvailable() but this calls decodes the size of the image frame only. It does not cache any ImageFrame.

So I think it's cleaner to lock m_frameBufferCache for writing in the one place that changes it instead of locking every place that makes a call to the same function.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20161118/08742128/attachment.html>