[webkit-reviews] review granted: [Bug 171900] REGRESSION (216471): Infinite repaint-drawing loop when asynchronously decoding incomplete image frames : [Attachment 309914] Patch

[bugzilla-daemon at webkit.org]

Tim Horton <thorton at apple.com> has granted Said Abou-Hallawa
<sabouhallawa at apple.com>'s request for review:
Bug 171900: REGRESSION (216471): Infinite repaint-drawing loop when
asynchronously decoding incomplete image frames
https://bugs.webkit.org/show_bug.cgi?id=171900

Attachment 309914: Patch

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




--- Comment #18 from Tim Horton <thorton at apple.com> ---
Comment on attachment 309914
  --> https://bugs.webkit.org/attachment.cgi?id=309914
Patch

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

> Source/WebCore/ChangeLog:15
> +	   'Decoding'. This new value will not never be cached in the
ImageFrame::
> +	   m_decodingStatus. Add a member m_currentFrameDecodingStatus to
BitmapImage.

not never? Can we de-double-negativeify this?

> Source/WebCore/ChangeLog:34
> +	   which is: does the ImageFrame of the nextFrame has a native image
with 
> +	   decoded with the native size or not?

Grammar here goes downhill as the sentence goes on.

> Source/WebCore/ChangeLog:66
> +	   decides whether another request for the same image frame is allowed
or not.

s/decides/decide/

> Source/WebCore/platform/graphics/ImageFrameCache.h:137
> +    void cacheAsyncFrameNativeImageAtIndex(NativeImagePtr&&, size_t,
SubsamplingLevel, const DecodingOptions&, ImageFrame::DecodingStatus);

I think the Async should have come at the end of this method name
(cacheFrameNativeImageAtIndex{Async}), but that's not relevant to this patch.
Also could probably drop the "frame" because this is the "ImageFrameCache" so
it's implied, but w/e.

> Source/WebCore/platform/graphics/ImageSource.cpp:-116
> -    m_frameCache->destroyIncompleteDecodedData();

Is BitmapImage the only way to get here? Or are there other paths to get here
which won't destroyIncompleteDecodedData now?