Bug 99769 - Reorder some functions in SubresourceLoader to permit main resources
Summary: Reorder some functions in SubresourceLoader to permit main resources
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nate Chapin
URL:
Keywords:
Depends on:
Blocks: 49246
  Show dependency treegraph
 
Reported: 2012-10-18 15:48 PDT by Nate Chapin
Modified: 2012-10-19 10:41 PDT (History)
5 users (show)

See Also:


Attachments
patch (4.35 KB, patch)
2012-10-18 16:02 PDT, Nate Chapin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nate Chapin 2012-10-18 15:48:43 PDT
Most resource types that go through the memory cache (and therefore through SubresourceLoader) are not sensitive to the exact ordering of the callbacks they receive, particularly as it relates to ResourceLoadNotifier calls.  Main resources are not so lenient.  For main resources to be cacheable and maintain the current behavior as precisely as possible, we will need to rearrange SubresourceLoader's willSendRequest() and didReceiveData().

For willSendRequest(), there are two calls, there are a series of checks that can result in the request being canceled, plus calls to CachedResource::willSendRequest() and ResourceLoader::willSendRequest().  MainResourceLoader (which will be a CachedResourceClient) has work it expects to do before ResourceLoader::willSendRequest() is called, but the calls are out of order for that, so swap those.

For didReceiveData(), We need to populate ResourceLoader::m_resourceData before notifying CachedResource of new data.  Currently the call order is :
(Start call to ResourceLoader::didReceiveData)
addData()
frameLoader()->notifier()->didReceiveData()
(End call to ResourceLoader::didReceiveData)
sendDataToResource()

Main resources want the order to be:
addData()
sendDataToResource()
frameLoader()->notifier()->didReceiveData()

...which is not permitted via calling ResourceLoader::didReceiveData().  Therefore, call all three directly, rather than delegating to the parent class.
Comment 1 Nate Chapin 2012-10-18 16:02:17 PDT
Created attachment 169496 [details]
patch
Comment 2 Adam Barth 2012-10-19 10:32:42 PDT
Comment on attachment 169496 [details]
patch

This looks much better.  I'm sort of amazed that the old code worked.
Comment 3 WebKit Review Bot 2012-10-19 10:41:01 PDT
Comment on attachment 169496 [details]
patch

Clearing flags on attachment: 169496

Committed r131919: <http://trac.webkit.org/changeset/131919>
Comment 4 WebKit Review Bot 2012-10-19 10:41:05 PDT
All reviewed patches have been landed.  Closing bug.