Created attachment 407702 [details] Low speed of cache fetching Even if the images are in browser memory cache, Safari weirdly takes too long fetch the images. Whereas in Chrome, once the images are stored in the browser cache, it doesn't even make an extra network call to fetch them. Problem: I am showing different images on hover since it is a hover, the images should switch quickly, this works perfectly in Chrome, but Safari fetches the cached images on every hover which takes about half a second. Is this common with safari or am I missing something? Thanks in advance
Thank you for the report! Could you please provide a reproducible test case?
Note that as I remember it, the values reported by Web Inspector are wrong when the resource comes from the cache. Web Inspector reports actual networking values (when the resource was originally fetched from the network) even the resource is returned by the memory cache. Network metrics are stored in the cache and that's what Web Inspector use. However, I assume you also see visibly slower loading? Or are you submitting this bug report based on bad Web Inspector data?
I see slow loading aswel, I am not sure if this is because of React or the CSS or do I have to load the images on a CDN, but basically if you go on this link: http://ironvillage-esports.surge.sh Click on "All games" and then try to hover on the games circular cards, you see that the images are slow to load up. If I try to the same link on chrome or firefox, things are working fine. How I am doing this CSS effect is that is: I have 3 states of the game cards: "active", "inactive", "hovered" and "disabled", Unfortunately all the states have different images, to avoid lag, I am loading all images together (Don't quote me on the approach :D) and then stacking them together using position absolute. Finally according the "state". I change the z-index of the images. I tried to replicate a similar thing using codepen(https://codepen.io/androiditya/full/GRZmrqR) but this seems to work fine🤷🏽♂️ I hope this helps
Is it because of how I am adding hashing the images in webpack, because I basically tried the same code on a codesandbox and deployed it, which seems to work just fine: Code: https://codesandbox.io/s/reverent-brown-cbsf0?file=/src/App.js Deployed: https://cbsf0.csb.app/
Most probably, the memory cache is not hit and either the HTTP cache is used or revalidation happens. If you have access to your website logs, you might be able to see whether it receives requests for these images.
Looking at the images, Cache-Control is 'Cache-Control: public, max-age=31536000, no-cache' Given there is 'no-cache', Safari is probably revalidating the cached resource.
Testing on Chrome and Firefox, it does not seem there is any revalidation being done.
Reduced test: <html> <body> <img onclick='newImage()' src='http://ironvillage-esports.surge.sh/ca5a87e6413ecf84fb4e9f21aa5d4487.png'></img> <div id='images'></div> <script> function newImage() { images.innerHTML += "<img src='http://ironvillage-esports.surge.sh/ca5a87e6413ecf84fb4e9f21aa5d4487.png'></img>"; } </script> </body> </html> Clicking on the top level image triggers a revalidation load in Safari, not Chrome and Firefox. Also, the past images being loaded disappear until the revalidation load is complete. It seems there are two issues: - Safari may be revalidating resources in cases where it is not needed. - Invalidating a CachedImage in the memory cache seems to trigger some blinking.
<rdar://problem/68260952>
> - Safari may be revalidating resources in cases where it is not needed. HTTP spec seems clear we should revalidate. If I change this to a fetch load, revalidation happens in both Chrome and Firefox. Skipping the revalidation in Chrome and Firefox might be specific to images.
https://source.chromium.org/chromium/chromium/src/+/master:third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc;l=1628;drc=90815a3ab6230a067f0c31935d60838dfccca7a0 seems to indicate Chrome has a specific behavior for images.
Created attachment 407879 [details] Patch
Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/25380
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Maybe related to this https://html.spec.whatwg.org/multipage/images.html#the-list-of-available-images (which we don't currently fully implement)?
(In reply to Antti Koivisto from comment #15) > Maybe related to this > https://html.spec.whatwg.org/multipage/images.html#the-list-of-available- > images (which we don't currently fully implement)? Yes, patch is trying to implement a very simple version of it but it apparently fails some existing test. Yoav also told me that the current implementation (not up to date with the spec) is creating some annoying inconsistencies that would be worth fixing.
Created attachment 407959 [details] Patch
Created attachment 407965 [details] Patch
Bug 195628 is another instance of this.
Comment on attachment 407965 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=407965&action=review > Source/WebCore/loader/cache/CachedResourceLoader.h:77 > -class CachedResourceLoader : public RefCounted<CachedResourceLoader> { > +class CachedResourceLoader : public RefCounted<CachedResourceLoader>, public CanMakeWeakPtr<CachedResourceLoader> { Can’t the CachedImage hang on to a weak pointer to the Document instead of to the CachedResourceLoader? I wouldn't add CanMakeWeakPtr to another class lightly, since it makes the objects bigger and more expensive to create/destroy.
Comment on attachment 407965 [details] 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.
> > Source/WebCore/loader/cache/CachedResourceLoader.h:77 > > -class CachedResourceLoader : public RefCounted<CachedResourceLoader> { > > +class CachedResourceLoader : public RefCounted<CachedResourceLoader>, public CanMakeWeakPtr<CachedResourceLoader> { > > Can’t the CachedImage hang on to a weak pointer to the Document instead of > to the CachedResourceLoader? I wouldn't add CanMakeWeakPtr to another class > lightly, since it makes the objects bigger and more expensive to > create/destroy. OK > 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. OK > > 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? I will add a comment. FWIW, spec is loosely implemented and we should specify a memory cache behavior that HTML would piggy back on for images.
> > 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. Current patch does not handle redirections, I'll update it and it should fix bug 195628.
Created attachment 408173 [details] Patch
*** Bug 195628 has been marked as a duplicate of this bug. ***
Created attachment 408179 [details] Patch
Committed r266699: <https://trac.webkit.org/changeset/266699> All reviewed patches have been landed. Closing bug and clearing flags on attachment 408179 [details].
As seen on EWS before landing, http/wpt/html/dom/elements/images/hover-image-change.html is consistently timing out on iOS bots: https://ews-build.webkit.org/#/builders/24/builds/25132
(In reply to Ryan Haddad from comment #28) > As seen on EWS before landing, > http/wpt/html/dom/elements/images/hover-image-change.html is consistently > timing out on iOS bots: > https://ews-build.webkit.org/#/builders/24/builds/25132 Correction: the result wasn't posted by EWS before this patch was landed.
mouse events are not a thing in iOS. I will fix this
(In reply to youenn fablet from comment #30) > mouse events are not a thing in iOS. > I will fix this Filed https://bugs.webkit.org/show_bug.cgi?id=216265