Bug 14651 – [CURL] didReceiveResponse() only called for HTTP loads

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Kevin Ollivier]

The current implementation of the CURL loader calls didReceiveResponse only when receiving HTTP headers. This doesn't handle URLs using other protocols, such as file://. This causes problems with loading css files from disk because WebCore/loader/CachedCSSStyleSheet.cpp depends on m_response being set to determine the absolute URL of the css file.

I don't have a patch yet, as I'm not quite sure where the best place to fire didReceiveResponse would be for other protocols. Ideally, it would be from the first successful curl_multi_perform call for each handle, but I'm not sure if there's currently a way to track that.

Anyone more familiar with the CURL loader have any suggestions on how best to implement this?

[Kevin Ollivier]

Created attachment 17785 [details]
Patch to call didReceiveResponse and set the URL for local files

[Alp Toker]

*** Bug 14583 has been marked as a duplicate of this bug. ***

[Alp Toker]

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?

[Kevin Ollivier]

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.

[Alp Toker]

Comment on attachment 17792 [details]
New attempt that checks a bool value

r=me

[Alp Toker]

Landed in r28569.

I removed this extraneous line before landing:
  KURL url(hdr);

(qmake-based ports may need to rebuild since the ResourceRequest.h change leaves the build in an inconsistent/crashy state.)