Bug 60598

Summary: HTML5 Application Cache Update Failures on Page Refresh
Product: WebKit Reporter: Andy Riedel <ariedel>
Component: WebCore Misc.Assignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Major CC: ap, commit-queue, joepeck, michaeln, mrobinson, rniwa, xan.lopez
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Mac (Intel)   
OS: OS X 10.6   
URL: http://ogw.watercooler-inc.com
Attachments:
Description Flags
proposed fix
darin: review+, ap: commit-queue-
patch for landing none

Andy Riedel
Reported 2011-05-10 16:58:01 PDT
Summary: We have been able to reproduce various different HTML5 application/offline cache update bugs when a page is refreshed during a cache update. If a cache update is interrupted by refreshing the page midstream, expected cache events are not fired making it impossible to tell if files are actually being cached and making the use of event monitoring for things like cache update progress unreliable. -------------------------------------------------- Expected Result: If a cache update is interrupted midstream, a complete new cache update sequence should occur: CHECKING DOWNLOAD PROGRESS ......... CACHED -------------------------------------------------- Observed Results: Different results occur on Firefox desktop, Safari desktop, and Safari mobile. Tests were performed at http://ogw.watercooler-inc.com using Firefox 4.0 (desktop) and Safari 5.0.5 (6533.21.1) (desktop). Our index page includes a standard manifest attribute to enable offline caching: <html manifest="/cache.manifest"> This initiates an offline cache update and so long as the update is not interrupted by refreshing the browser page, we receive JavaScript events from the application cache in the expected order: CHECKING, DOWNLOAD, PROGRESS, ......, PROGRESS, CACHED. Firefox (Desktop) ----------------------- On Firefox, there is a continuation of PROGRESS events for each file that had not been cached yet. For example, if 20 files were specified in the manifest and the page is refreshed after the 8th PROGRESS event is fired, 12 more PROGRESS events will be fired after the refresh. But, there is no initial CHECKING event after the refresh and there is no completing CACHED or UPDATEREADY event after the final PROGRESS event. If another page refresh is performed after this final PROGRESS event, the expected CHECKING and NOUPDATE events are fired. Here is a view of the sequence of events: [INITIAL PAGE LOAD] CHECKING DOWNLOADING PROGRESS (8 times) [REFRESH PAGE] PROGRESS (12 times) [END OF EVENT ACTIVITY] [REFRESH PAGE AGAIN] CHECKING NOUPDATE [END OF EVENT ACTIVITY] Safari (Desktop) ---------------------- On Safari, things are worse. If the page is refreshed during the cache update, only a CHECKING event and a DOWNLOAD event are fired and then no additional activity occurs. Additional reloads of the page will yield the same result of only a CHECKING and DOWNLOAD event and nothing else. The only way to actually update the cache a this point seems to be to restart Safari. Upon restart, the complete sequence of regular expected events will occur: CHECKING, DOWNLOAD, PROGRESS, ......, PROGRESS, CACHED. Here is a view of the sequence of events: [INITIAL PAGE LOAD] CHECKING DOWNLOADING PROGRESS (8 times) [REFRESH PAGE] CHECKING DOWNLOADING [END OF EVENT ACTIVITY] [REFRESH PAGE AGAIN] CHECKING DOWNLOADING [END OF EVENT ACTIVITY] [RESTART SAFARI] CHECKING DOWNLOADING PROGRESS ...... CACHED -- Andy Riedel Vice President, Mobile Kabam, Inc. (650) 784-9575 ariedel@kabam.com
Attachments
proposed fix (8.14 KB, patch)
2011-05-13 16:47 PDT, Alexey Proskuryakov 		darin: review+
ap: commit-queue-
DetailsFormatted DiffDiff
patch for landing (8.10 KB, patch)
2011-05-13 17:38 PDT, Alexey Proskuryakov 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Alexey Proskuryakov
Comment 1 2011-05-10 21:18:38 PDT
<rdar://problem/9418639>
Alexey Proskuryakov
Comment 2 2011-05-11 00:20:25 PDT
Both Safari and Firefox behaviors seem buggy indeed. > The only way to actually update the cache a this point seems to be to restart Safari Closing the current window and opening a new one seems to work, too.
Alexey Proskuryakov
Comment 3 2011-05-13 16:47:57 PDT
Created attachment 93530 [details] proposed fix
Michael Nordman
Comment 4 2011-05-13 17:19:39 PDT
Comment on attachment 93530 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=93530&action=review > LayoutTests/http/tests/appcache/interrupted-update.html:15 > +if (applicationCache.status) { This check doesn't look necessary and it will cause this test to fail erroneously for the chromium port. Since the page contains a manifest attribute, the status value will return CHECKING when sampled at this time. > LayoutTests/http/tests/appcache/interrupted-update.html:49 > + log("\nPASS"); This test can't PASS the test when a cache is already present. Maybe add the alert() about a cache already being present here instead (or just include the message in the logging output)
Alexey Proskuryakov
Comment 5 2011-05-13 17:31:21 PDT
Comment on attachment 93530 [details] proposed fix OK, I'll change the test - but isn't it a bug in Chromium? Having status of CHECKING before a checking event is dispatched seems wrong.
Alexey Proskuryakov
Comment 6 2011-05-13 17:38:14 PDT
Created attachment 93540 [details] patch for landing
Michael Nordman
Comment 7 2011-05-13 18:14:44 PDT
(In reply to comment #5) > OK, I'll change the test - but isn't it a bug in Chromium? Having status of CHECKING before a checking event is dispatched seems wrong. Thank you. As for the behavior, here's what the spec says... "The status attribute, on getting, must return the current state of the application cache that the ApplicationCache object's cache host is associated with, if any."
WebKit Commit Bot
Comment 8 2011-05-13 21:20:17 PDT
Comment on attachment 93540 [details] patch for landing Clearing flags on attachment: 93540 Committed r86478: <http://trac.webkit.org/changeset/86478>
WebKit Commit Bot
Comment 9 2011-05-13 21:20:22 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.