RESOLVED FIXED 86668
Animated GIFs in page cache get updated
https://bugs.webkit.org/show_bug.cgi?id=86668
Summary Animated GIFs in page cache get updated
Jon Lee
Reported 2012-05-16 13:22:32 PDT
A live page referencing an animated GIF that is also in a cached page gets updated, which violates the assumption that cached pages remain untouched. <rdar://problem/11395549>
Attachments
Patch (1.73 KB, patch)
2012-05-16 13:50 PDT, Jon Lee
no flags
Patch (8.13 KB, patch)
2012-05-16 16:45 PDT, Jon Lee
beidson: review+
Jon Lee
Comment 1 2012-05-16 13:50:20 PDT
Brady Eidson
Comment 2 2012-05-16 14:27:13 PDT
Comment on attachment 142339 [details] Patch This change looks great. I've asked Jon on IRC to explore a Layout test which *might* be possible, but might also be impossible or unreasonably difficult. But I'm fine with the change as-is
Jon Lee
Comment 3 2012-05-16 16:45:24 PDT
Jon Lee
Comment 4 2012-05-16 16:46:12 PDT
Added test.
Brady Eidson
Comment 5 2012-05-16 16:48:45 PDT
Comment on attachment 142370 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=142370&action=review Awesome! > LayoutTests/fast/loader/image-in-page-cache-expected.txt:17 > + > + > +PASS successfullyParsed is true > + > +TEST COMPLETE > +Opening animated-image.html in new window > +PASS Opened animated-image.html > +Opening animated-image2.html in otherWindow > +PASS Opened animated-image2.html > +Opening animated-image3.html in otherWindow > +PASS Opened animated-image3.html > +Closing otherWindow > +PASS Closed otherWindow > + It's weird to me that TEST COMPLETE shows up so early in the output, but I won't stress about it.
Brady Eidson
Comment 6 2012-05-16 16:51:42 PDT
Comment on attachment 142370 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=142370&action=review > LayoutTests/fast/loader/image-in-page-cache.html:14 > +function runTest() { > + if (window.layoutTestController) { > + layoutTestController.dumpAsText(); > + layoutTestController.waitUntilDone(); > + layoutTestController.setCanOpenWindows(); > + layoutTestController.overridePreference("WebKitUsesPageCachePreferenceKey", 1); > + } Actually, please fix this indenting
Brady Eidson
Comment 7 2012-05-16 16:52:15 PDT
Comment on attachment 142370 [details] Patch r+, after you give the indentation a once-over
Jon Lee
Comment 8 2012-05-16 16:58:02 PDT
(In reply to comment #5) > (From update of attachment 142370 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=142370&action=review > > Awesome! > > > LayoutTests/fast/loader/image-in-page-cache-expected.txt:17 > > + > > + > > +PASS successfullyParsed is true > > + > > +TEST COMPLETE > > +Opening animated-image.html in new window > > +PASS Opened animated-image.html > > +Opening animated-image2.html in otherWindow > > +PASS Opened animated-image2.html > > +Opening animated-image3.html in otherWindow > > +PASS Opened animated-image3.html > > +Closing otherWindow > > +PASS Closed otherWindow > > + > > It's weird to me that TEST COMPLETE shows up so early in the output, but I won't stress about it. It's a timing issue that I realize can be fixed with a window.jsTestIsAsync = true. I will update the test and results with this.
Jon Lee
Comment 9 2012-05-16 17:08:19 PDT
Note You need to log in before you can comment on or make changes to this bug.