Bug 113251

Summary: Resources are loaded from the memory cache before willSendRequest has a chance to modify the request
Product: WebKit Reporter: Andy Estes <aestes>
Component: Page LoadingAssignee: Andy Estes <aestes>
Status: ASSIGNED ---    
Severity: Critical CC: abarth, ap, beidson, japhet, jochen
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   

Description Andy Estes 2013-03-25 16:47:28 PDT
If a request is made for a resource present in the memory cache, WebKit will load it without properly calling the willSendRequest resource load delegate. We call the delegate method, but we ignore the modified request that it returns. This means that clients cannot modify or cancel requests for memory-cached resources, nor will modified requests be served from the memory cache even if they could be. Some WebKit clients have disabled the memory cache because of this issue, which they shouldn't have to do.

We should change the loader to dispatch willSendRequest before consulting the memory cache, and then continue the loading process with the modified request that the delegate method returned.
Comment 1 Andy Estes 2013-03-25 16:53:29 PDT
<rdar://problem/13228856>
Comment 2 Andy Estes 2013-03-25 17:25:23 PDT
It looks like we've added a new callback on FrameLoaderClient called willRequestResource that behaves similar to willSendRequest but is called on cached resources: <http://trac.webkit.org/changeset/135872>
Comment 3 Brady Eidson 2013-03-25 18:57:24 PDT
Come now, we definitely have at least one dupe of this already, right?
Comment 4 jochen 2013-03-26 00:56:57 PDT
Note, however, that the chromium API does not allow for modifying the request. If you want to modify the request, you should double-check that this doesn't break the loader.
Comment 5 Andy Estes 2013-03-26 01:11:17 PDT
(In reply to comment #3)
> Come now, we definitely have at least one dupe of this already, right?

I couldn't find one, but maybe you can dig something up! I found <https://bugs.webkit.org/show_bug.cgi?id=56647>, which seems to be arguing in the opposite direction of what I'm proposing, and the only client I know that's encountered this issue solved it by disabling the memory cache via WebKit1 SPI.
Comment 6 Andy Estes 2013-03-26 01:13:05 PDT
(In reply to comment #4)
> Note, however, that the chromium API does not allow for modifying the request. If you want to modify the request, you should double-check that this doesn't break the loader.

Thanks, that's useful to know. Apple's API does already support request modification, so the loader should support it in the existing willSendRequest code path, but I'll tread carefully.
Comment 7 jochen 2013-03-26 01:14:11 PDT
(In reply to comment #6)
> (In reply to comment #4)
> > Note, however, that the chromium API does not allow for modifying the request. If you want to modify the request, you should double-check that this doesn't break the loader.
> 
> Thanks, that's useful to know. Apple's API does already support request modification, so the loader should support it in the existing willSendRequest code path, but I'll tread carefully.

Sorry if that was unclear. In willSendRequest, we also allow for modifying requests, just not in willRequestResource
Comment 8 Andy Estes 2013-03-26 01:17:55 PDT
(In reply to comment #7)
> Sorry if that was unclear. In willSendRequest, we also allow for modifying requests, just not in willRequestResource

Ah, that makes sense. Thanks!
Comment 9 Andy Estes 2013-04-05 17:24:10 PDT
For now I'm just going to handle the case where the delegate returns NULL. I'm going to do that in <https://bugs.webkit.org/show_bug.cgi?id=114075>, and leave this bug open to handle the general case of request modification.