Bug 86668 - Animated GIFs in page cache get updated
Summary: Animated GIFs in page cache get updated
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jon Lee
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2012-05-16 13:22 PDT by Jon Lee
Modified: 2012-05-16 17:08 PDT (History)
5 users (show)

See Also:


Attachments
Patch (1.73 KB, patch)
2012-05-16 13:50 PDT, Jon Lee
no flags Details | Formatted Diff | Diff
Patch (8.13 KB, patch)
2012-05-16 16:45 PDT, Jon Lee
beidson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jon Lee 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>
Comment 1 Jon Lee 2012-05-16 13:50:20 PDT
Created attachment 142339 [details]
Patch
Comment 2 Brady Eidson 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
Comment 3 Jon Lee 2012-05-16 16:45:24 PDT
Created attachment 142370 [details]
Patch
Comment 4 Jon Lee 2012-05-16 16:46:12 PDT
Added test.
Comment 5 Brady Eidson 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.
Comment 6 Brady Eidson 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
Comment 7 Brady Eidson 2012-05-16 16:52:15 PDT
Comment on attachment 142370 [details]
Patch

r+, after you give the indentation a once-over
Comment 8 Jon Lee 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.
Comment 9 Jon Lee 2012-05-16 17:08:19 PDT
Committed r117366: <http://trac.webkit.org/changeset/117366>