WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
youenn fablet
Reported
2016-10-24 04:13:32 PDT
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.
Attachments
add a test expectation
(1.32 KB, patch)
2016-10-24 04:17 PDT
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Patch
(26.36 KB, patch)
2016-12-12 00:02 PST
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Patch
(27.08 KB, patch)
2016-12-12 06:26 PST
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Patch for landing
(26.98 KB, patch)
2016-12-16 00:07 PST
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
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
Created
attachment 296895
[details]
Patch
youenn fablet
Comment 8
2016-12-12 06:26:05 PST
Created
attachment 296914
[details]
Patch
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
Re-opening for pull request
https://github.com/WebKit/WebKit/pull/33372
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.
Top of Page
Format For Printing
XML
Clone This Bug