Bug 216104 – [CG] Cache the last status of the image encoded data

• Status: RESOLVED
• Resolution: FIXED
• Component: Images

[Said Abou-Hallawa]

In one of the WebKit traces, opening a Wikipedia page shows that CGImageSourceCopyPropertiesAtIndex() was called over 500 times. ImageDecoderCG::encodedDataStatus() keeps calling this function as long as the encoded data is not complete. This may happen if the network is slow. This case can be optimized by caching the last status of the image encoded data.

[Said Abou-Hallawa]

Created attachment 407830 [details]
Patch

[Peng Liu]

Comment on attachment 407830 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        can be made with calling system functions.

s/with/without/
Sounds like?

[Said Abou-Hallawa]

Created attachment 407838 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/68601184>

[Tim Horton]

Comment on attachment 407838 [details]
Patch

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

> Source/WebCore/platform/graphics/cg/ImageDecoderCG.cpp:-239
> -        // Ragnaros yells: TOO SOON! You have awakened me TOO SOON, Executus!

This critical historical documentation must remain!

[Said Abou-Hallawa]

Created attachment 408470 [details]
Patch

[Said Abou-Hallawa]

Comment on attachment 407838 [details]
Patch

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

>> Source/WebCore/platform/graphics/cg/ImageDecoderCG.cpp:-239
>> -        // Ragnaros yells: TOO SOON! You have awakened me TOO SOON, Executus!
> 
> This critical historical documentation must remain!

I put it back :)

[youenn fablet]

Comment on attachment 408470 [details]
Patch

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

> Source/WebCore/platform/graphics/cg/ImageDecoderCG.cpp:253
> +            m_encodedDataStatus = EncodedDataStatus::Error;

Could be a one liner:
m_encodedDataStatus = !m_isAllDataReceived ? EncodedDataStatus::Unknown : EncodedDataStatus::Error;

> Source/WebCore/platform/graphics/cg/ImageDecoderCG.cpp:260
>          RetainPtr<CFDictionaryRef> image0Properties = adoptCF(CGImageSourceCopyPropertiesAtIndex(m_nativeDecoder.get(), 0, imageSourceOptions().get()));

auto

> Source/WebCore/platform/graphics/cg/ImageDecoderCG.cpp:266
> +        if (!CFDictionaryContainsKey(image0Properties.get(), kCGImagePropertyPixelWidth) || !CFDictionaryContainsKey(image0Properties.get(), kCGImagePropertyPixelHeight)) {

You could group both this if and previous if in a single statement.

[Said Abou-Hallawa]

Created attachment 408694 [details]
Patch

[EWS]

Committed r267016: <https://trac.webkit.org/changeset/267016>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 408694 [details].