It is now often crashing https://build.webkit.org/results/Apple%20Yosemite%20Debug%20WK2%20(Tests)/r207754%20(15818)/svg/as-image/svg-image-with-data-uri-use-data-uri-crash-log.txt e.g.
Created attachment 292601 [details] add a test expectation
Comment on attachment 292601 [details] add a test expectation Clearing flags on attachment: 292601 Committed r207756: <http://trac.webkit.org/changeset/207756>
All reviewed patches have been landed. Closing bug.
This test should be fixed properly
<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?
(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.
Created attachment 296895 [details] Patch
Created attachment 296914 [details] Patch
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.
Created attachment 297305 [details] Patch for landing
Comment on attachment 297305 [details] Patch for landing Clearing flags on attachment: 297305 Committed r209914: <http://trac.webkit.org/changeset/209914>
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?
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);