Bug 21949

Summary: No need to clobber all ResourceRequest fields in FrameLoader::reload()
Product: WebKit Reporter: Darin Fisher (:fishd, Google) <fishd>
Component: Page LoadingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
v1 patch andersca: review+

Description Darin Fisher (:fishd, Google) 2008-10-29 11:35:24 PDT
No need to clobber all ResourceRequest fields in FrameLoader::reload()

Presently, FrameLoader::reload clobbers all fields of the initialRequest when there is an unreachableURL associated with the current DocumentLoader.  This causes problems for the Chromium port since we store some extra data on the ResourceRequest object that we need to have preserved across a FrameLoader::reload call.

FrameLoader::reloadAllowingStaleData is careful to only call setURL when there is an unreachableURL, and I think that FrameLoader::reload should do the same.  I suspect that it was not intentional to make FrameLoader::reload behave differently in this respect.

Please see the attached patch for further details.
Comment 1 Darin Fisher (:fishd, Google) 2008-10-29 11:39:57 PDT
Created attachment 24751 [details]
v1 patch
Comment 2 Darin Fisher (:fishd, Google) 2008-10-29 11:42:06 PDT
To clarify, this causes problems for us because the HTTP method is forced to be GET.  If the previous request was something else, then that information is lost.  I guess this doesn't cause a problem for Safari, but it still seems like an error to me.
Comment 3 Darin Fisher (:fishd, Google) 2008-10-30 13:00:59 PDT
Comment on attachment 24751 [details]
v1 patch

Anders, can you please take a look at this patch?  Thanks!
Comment 4 Anders Carlsson 2008-11-04 09:25:15 PST
Comment on attachment 24751 [details]
v1 patch

Looks good, I can't think of a reason why it would break things (but I've been wrong before ;)

r=me!
Comment 5 Darin Fisher (:fishd, Google) 2008-11-04 11:41:18 PST
http://trac.webkit.org/changeset/38107