[webkit-reviews] review granted: [Bug 216048] Safari takes too long to fetch images from memory cache : [Attachment 407965] Patch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted youenn fablet
<youennf at gmail.com>'s request for review:
Bug 216048: Safari takes too long to fetch images from memory cache
https://bugs.webkit.org/show_bug.cgi?id=216048

Attachment 407965: Patch

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




--- Comment #21 from Antti Koivisto <koivisto at iki.fi> ---
Comment on attachment 407965
  --> https://bugs.webkit.org/attachment.cgi?id=407965
Patch

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

Can you check that https://bugs.webkit.org/show_bug.cgi?id=195628 is also
fixed? There is a test there that could be added.

> Source/WebCore/loader/cache/CachedImage.cpp:736
> +    // Skip revalidation as per
https://html.spec.whatwg.org/#ignore-higher-layer-caching.
> +    if (loader && m_cachedResourceLoader && loader->document() ==
m_cachedResourceLoader->document())
> +	   return RevalidationDecision::No;

It would be good to mention this doesn't really implement the algorithm as
described (we don't maintain a per-Document list of available images). Maybe we
should at some point?

> Source/WebCore/loader/cache/CachedImage.h:190
> +    WeakPtr<CachedResourceLoader> m_cachedResourceLoader;

Like Darin said, this could be Document. It might also be helpful to add
comment why it is here since CachedResources are generally not associated with
any particular Document.