Summary: | [CURL] didReceiveResponse() only called for HTTP loads | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kevin Ollivier <kevino> | ||||||
Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | alp | ||||||
Priority: | P2 | Keywords: | Curl, Gtk, Wx | ||||||
Version: | 523.x (Safari 3) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Kevin Ollivier
2007-07-17 19:10:50 PDT
Created attachment 17785 [details]
Patch to call didReceiveResponse and set the URL for local files
*** Bug 14583 has been marked as a duplicate of this bug. *** Comment on attachment 17785 [details]
Patch to call didReceiveResponse and set the URL for local files
This bug fix works and is much needed.
I'm concerned about one thing in the implementation though:
writeCallback() is a hot path. We should first research if we can call didReceiveResponse() earlier than writeCallback(). If this isn't an option, the didReceiveResponse() call in writeCallback() needs to be made as lightweight as possible -- perhaps a boolean check, rather than querying CURL for the effective URL, constructing a KURL and updating the response again and again. It only needs to be done once, right?
Think you could devise a lighter solution?
Created attachment 17792 [details]
New attempt that checks a bool value
I've updated the implementation to check a bool value before running any of the local files code. It now no longer runs multiple times during one resource load, nor will it call CURL APIs, etc. unless necessary.
I added a TODO in the comments about finding a better approach, but so far I've been unable to find anything in the CURL API that would suggest a better solution, and the only other way I can see to approach this would be to avoid CURL entirely for local files, which I think is just going to add to the number of code paths we need to support. I'd really like to take this route for now so that we can get the fix in there, as I know it is biting existing wx port users at least.
Comment on attachment 17792 [details]
New attempt that checks a bool value
r=me
|