Summary: | Prevent Redundant AppCache Downloads of Disallowed AppCache | ||
---|---|---|---|
Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> |
Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> |
Status: | NEW --- | ||
Severity: | Normal | CC: | ap, joepeck, michaeln |
Priority: | P2 | ||
Version: | 528+ (Nightly build) | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
Attachments: |
Description
Joseph Pecoraro
2011-07-22 10:57:57 PDT
Created attachment 101820 [details]
[PATCH] Part 1: Remove the old implementation
Created attachment 101821 [details]
[PATCH] Part 2: Implement "Disallowed Cache"
This is the bulk of the work. ChangeLog explains the details.
Created attachment 101822 [details]
[PATCH] Part 3: When Disallowed still Fallback to Newest Cache
I think this is the controversial patch. ChangeLog explains it a bit.
Created attachment 101823 [details]
[PATCH] Part 4: Move Existing Quota Tests to http/tests/appcache/quotas
So other ports Skip list don't need to be adjusted every time a new "origin quota" test is added.
Created attachment 101824 [details]
[PATCH] Part 5: Add Tests for Disallowed Quota Behavior
I think this does a good job of displaying the expected behavior.
Please let me know if you think there is an area where I am missing
obvious test coverage.
These patches are very nearly complete. I hope the tests show that. • I still have some questions as FIXMEs in Part 2 • I think discussion would be required on whether or not we perform the policy in Part 3. • The tests in Part 5 should hopefully display clearly what is implemented. • I have a minor open issue about including the disallowed cache in quota calculation. I can include a follow-up patch for that, but I'd first like to have the discussion about the approach in the existing patches so I don't do unnecessary extra work if things change. Thanks! Comment on attachment 101824 [details] [PATCH] Part 5: Add Tests for Disallowed Quota Behavior View in context: https://bugs.webkit.org/attachment.cgi?id=101824&action=review > LayoutTests/http/tests/appcache/quotas/origin-quota-disallowed-without-preexisting.html:64 > + // Expected to fail. Will be ~15kb out of the 20kb limit. This comment was stale. It should just be "Expected to fail" because it won't fit into the 1kb limit. Part 3 - When Disallowed still Fallback to Newest Cache 10 This handles the case where: 11 12 - user has a successfully downloads a "newest" cache 13 - server updates manifest 14 - user disallows the update (has a "disallowed" cache) 15 - user reloads the page. it is the same as the disallowed 16 cache so we issue an "error" but we fallback to the 17 old, successful cache. If an update is "disallowed", continuing to use an old version questionable. That old version may or may not be compatible with the current server-side of the application. From the apps point of view, this client is wedged at old version. It may be better to delete the old version when the user disallows an update, so the user is disallowing the manifest and not just a particular version of the manifest. (In reply to comment #8) > If an update is "disallowed", continuing to use an old version questionable. That old version may or may not be compatible with the current server-side of the application. From the apps point of view, this client is wedged at old version. It may be better to delete the old version when the user disallows an update, so the user is disallowing the manifest and not just a particular version of the manifest. Correct, that is one of the reasons I mentioned it was controversial and would need discussion. I guess deleting the previous manifests would be safest. Comment on attachment 101820 [details]
[PATCH] Part 1: Remove the old implementation
Clearing flags on these. We can hold off on this and, as Michael Nordman pointed out, we probably would want to implement the safest behavior.
|