Bug 18393 - Move POST cache policy from platform dependent file
Summary: Move POST cache policy from platform dependent file
Status: ASSIGNED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: David Carson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-04-09 12:11 PDT by David Carson
Modified: 2010-06-10 15:38 PDT (History)
0 users

See Also:


Attachments
Patch to move cache setting (2.23 KB, patch)
2008-04-09 12:18 PDT, David Carson
darin: review-
Details | Formatted Diff | Diff
new patch (2.71 KB, patch)
2008-04-24 14:28 PDT, David Carson
darin: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Carson 2008-04-09 12:11:06 PDT
The cache policy for POST is set in the file ResourceHandleMac.mm
This means that every port of WebKit needs to set the policy in their implementation.
Comment 1 David Carson 2008-04-09 12:18:25 PDT
Created attachment 20439 [details]
Patch to move cache setting
Comment 2 Darin Adler 2008-04-12 23:24:21 PDT
Comment on attachment 20439 [details]
Patch to move cache setting

+        Move setting cache policy out of platform dependant implementation.

Misspelling of dependent here.

     // FIXME: Where's the code that implements what the comment above says?
+    // ANSWER: The cache policy is set in loadItem()

This is a bit silly. Lets remove the "FIXME" if there's nothing that needs fixing.

                 if (ResourceHandle::willLoadFromCache(request))
+                    request.setCachePolicy(ReturnCacheDataDontLoad);
                     action = NavigationAction(itemURL, loadType, false);
                 else {

This won't compile unless you add braces.

review- because it won't compile.
Comment 3 David Carson 2008-04-24 14:28:33 PDT
Created attachment 20799 [details]
new patch

I was waaay to hasty with the last patch. Attached is a new patch that will compile and corrects the items pointed out in the review.
Comment 4 Darin Adler 2008-06-08 12:38:33 PDT
Comment on attachment 20799 [details]
new patch

This patch, as is, breaks the Mac platform. That's because the request sent into sendSynchronousRequest won't have the cache policy set to ReturnCacheDataDontLoad.

The rearranged code sets the policy after the call, whereas the old code set it before!

I think that the old behavior where the cache policy was set to ReturnCacheDataDontLoad for the navigation itself was actually a bug -- I think the intent was to set it only for the "willLoadFromCache" check itself.

review- because we don't want to break the Mac. I won't try to rework this myself, but it should be relatively straightforward.