Bug 86668 – Animated GIFs in page cache get updated

• Component: Page Loading
• Status: RESOLVED FIXED

[Jon Lee]

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>

[Jon Lee]

Created attachment 142339 [details]
Patch

[Brady Eidson]

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]

Created attachment 142370 [details]
Patch

[Jon Lee]

Added test.

[Brady Eidson]

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 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 on attachment 142370 [details]
Patch

r+, after you give the indentation a once-over

[Jon Lee]

(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]

Committed r117366: <http://trac.webkit.org/changeset/117366>