RESOLVED FIXED 163887
svg/as-image/svg-image-with-data-uri-use-data-uri.svg is flaky after r207754
https://bugs.webkit.org/show_bug.cgi?id=163887
Summary svg/as-image/svg-image-with-data-uri-use-data-uri.svg is flaky after r207754
Attachments
add a test expectation (1.32 KB, patch)
2016-10-24 04:17 PDT, youenn fablet
no flags
Patch (26.36 KB, patch)
2016-12-12 00:02 PST, youenn fablet
no flags
Patch (27.08 KB, patch)
2016-12-12 06:26 PST, youenn fablet
no flags
Patch for landing (26.98 KB, patch)
2016-12-16 00:07 PST, youenn fablet
no flags
youenn fablet
Comment 1 2016-10-24 04:17:26 PDT
Created attachment 292601 [details] add a test expectation
WebKit Commit Bot
Comment 2 2016-10-24 04:53:17 PDT
Comment on attachment 292601 [details] add a test expectation Clearing flags on attachment: 292601 Committed r207756: <http://trac.webkit.org/changeset/207756>
WebKit Commit Bot
Comment 3 2016-10-24 04:53:20 PDT
All reviewed patches have been landed. Closing bug.
youenn fablet
Comment 4 2016-10-24 04:54:07 PDT
This test should be fixed properly
Alexey Proskuryakov
Comment 5 2016-11-15 14:54:34 PST
<rdar://problem/29266436> Keeping a test as a flaky crash for so long is undesirable, as crashing impacts stability of other tests. Youenn, is this something that you can look into soon?
youenn fablet
Comment 6 2016-11-16 07:56:25 PST
(In reply to comment #5) > <rdar://problem/29266436> > > Keeping a test as a flaky crash for so long is undesirable, as crashing > impacts stability of other tests. Youenn, is this something that you can > look into soon? The issue might be due to the fact that a SVGImage/Image can be owned by several CachedImages. But the observer system which assumes a 1-to-1 relationship. The straightforward solution is to make the 1-to-1 relationship back again by cloning the SVGImage/Image when needed. I'll try to fix it next week.
youenn fablet
Comment 7 2016-12-12 00:02:17 PST
youenn fablet
Comment 8 2016-12-12 06:26:05 PST
Alex Christensen
Comment 9 2016-12-12 23:41:29 PST
Comment on attachment 296914 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=296914&action=review Seems good, but I'm not awake enough right now to give this an r+ > LayoutTests/http/tests/security/cross-origin-cached-images-with-memory-pressure.html:99 > + if (window.internals) This can be merged with the previous statement.
youenn fablet
Comment 10 2016-12-16 00:07:15 PST
Created attachment 297305 [details] Patch for landing
WebKit Commit Bot
Comment 11 2016-12-16 00:43:06 PST
Comment on attachment 297305 [details] Patch for landing Clearing flags on attachment: 297305 Committed r209914: <http://trac.webkit.org/changeset/209914>
WebKit Commit Bot
Comment 12 2016-12-16 00:43:12 PST
All reviewed patches have been landed. Closing bug.
Said Abou-Hallawa
Comment 13 2017-02-17 12:30:42 PST
Comment on attachment 296914 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=296914&action=review > Source/WebCore/loader/cache/CachedImage.cpp:361 > + for (auto cachedImage : m_cachedImages) > + cachedImage->decodedSizeChanged(image, delta); This loop is confusing. All the m_cachedImages must have references to the same Image object. When BitmapImage decodes a frame it calls this function which will call decodedSizeChanged() of all m_cachedImages. CachedImages:: decodedSizeChanged() will call CachedResource::setDecodedSize() to update its m_decodedSize. If I have an image whose ImageFrame size is say 2MB and which is referenced by 10 CachedImages, will the memoryCache see a 2MB or 20MB allocation?
Said Abou-Hallawa
Comment 14 2017-02-17 12:33:17 PST
Comment on attachment 296914 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=296914&action=review >> Source/WebCore/loader/cache/CachedImage.cpp:361 >> + cachedImage->decodedSizeChanged(image, delta); > > This loop is confusing. All the m_cachedImages must have references to the same Image object. When BitmapImage decodes a frame it calls this function which will call decodedSizeChanged() of all m_cachedImages. CachedImages:: decodedSizeChanged() will call CachedResource::setDecodedSize() to update its m_decodedSize. If I have an image whose ImageFrame size is say 2MB and which is referenced by 10 CachedImages, will the memoryCache see a 2MB or 20MB allocation? I think this function should be written like this: m_cachedImages[0]->decodedSizeChanged(image, delta);
Fujii Hironori
Comment 15 2024-09-09 21:50:45 PDT
EWS
Comment 16 2024-09-09 21:55:20 PDT
Test gardening commit 283392@main (92a5820525a6): <https://commits.webkit.org/283392@main> Reviewed commits have been landed. Closing PR #33372 and removing active labels.
Note You need to log in before you can comment on or make changes to this bug.