Bug 109566 - Invalid resource identifier in callbacks after a 304 response in the main resource
Summary: Invalid resource identifier in callbacks after a 304 response in the main res...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P1 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 109287
  Show dependency treegraph
 
Reported: 2013-02-12 04:52 PST by Carlos Garcia Campos
Modified: 2013-04-25 06:31 PDT (History)
6 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2013-02-12 04:52:16 PST
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
Comment 1 Nate Chapin 2013-02-12 08:46:40 PST
Is this still on trunk? I thought I had resolved this in http://trac.webkit.org/changeset/141615
Comment 2 Carlos Garcia Campos 2013-02-12 11:09:21 PST
(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.
Comment 3 Alexey Proskuryakov 2013-04-18 20:53:19 PDT
What is the status if this bug? Looks like a pretty bad issue to me.
Comment 4 Carlos Garcia Campos 2013-04-18 23:47:23 PDT
(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
Comment 5 Carlos Garcia Campos 2013-04-25 06:31:43 PDT
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