Bug 109566
| Summary: | Invalid resource identifier in callbacks after a 304 response in the main resource | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> |
| Component: | Page Loading | Assignee: | Nobody <webkit-unassigned> |
| Status: | NEW | ||
| Severity: | Normal | CC: | abarth, ap, a.renevier, beidson, csaavedra, japhet |
| Priority: | P1 | ||
| Version: | 528+ (Nightly build) | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Bug Depends on: | |||
| Bug Blocks: | 109287 | ||
Carlos Garcia Campos
The problem is that when the clients are switched to revalidate the resource, the revalidated resource is set for the main resource loader and the loader of such result is NULL. This makes MainResourceLoader::identifier() to return 0. The sequence of events is something like this:
PROVISIONAL
assignIdentifierToInitialRequest: 2
dispatchDidReceiveResponse: 0
COMMITTED
dispatchDidFinishLoading: 0
FINISHED
dispatchDidReceiveResponse: 2
dispatchDidFinishLoading: 2
didReceiveContentLength is also called after the 304, which I think it's wrong.
This happens because in SubresourceLoader::didReceiveResponse(), when the response is a 304, MemoryCache::revalidationSucceeded() is called. That makes the clients to be switched and responseReceived() and dataReceived() are called in CachedRawResource::didAddClient() and notifyFinished() in CachedResource::didAddClient(). These callbacks are emitted with resource identifier = 0, because the current resource in main resource loader doesnt' have a loader. After this, SubresourceLoader::didReceiveResponse() continues and ResourceLoader::didReceiveResponse() is called with the valid resource identifier, but the frame load has been committed and finished already.
I think we shouldn't call responseReceived(), dataReceived() and notifyFinished() in case of 304 response or while switching clients, and committed and finished should be emitted after the resource callbacks.
This can be reproduced with http://renevier.net/misc/webkit_109225.php, see also bug https://bugs.webkit.org/show_bug.cgi?id=109287
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Nate Chapin
Is this still on trunk? I thought I had resolved this in http://trac.webkit.org/changeset/141615
Carlos Garcia Campos
(In reply to comment #1)
> Is this still on trunk? I thought I had resolved this in http://trac.webkit.org/changeset/141615
hmm, looking at the patch it seems to fix the problem of reporting an invalid resource identifier, but it might still emit the callbacks in a different order, for example, the didReceiveResponse for the main resource might be called after the main frame load is committed. I haven't really tried it, though. I guess that didReceiveContentLength might also be called for the main resource after 304, which I'm not sure is right because there's no actual data transfer in such case.
Alexey Proskuryakov
What is the status if this bug? Looks like a pretty bad issue to me.
Carlos Garcia Campos
(In reply to comment #3)
> What is the status if this bug? Looks like a pretty bad issue to me.
I'll check if this is still a problem in trunk
Carlos Garcia Campos
I've tried to reproduce it again but for some reason now I don't get 304 after reloading main resource in http://renevier.net/misc/webkit_109225.php