Commit 0c8e191b authored by aestes@apple.com's avatar aestes@apple.com

Returning NULL from willSendRequest should cancel a load from the memory cache

https://bugs.webkit.org/show_bug.cgi?id=114075

Reviewed by Darin Adler.

Source/WebCore:

When a resource is loaded from the memory cache and the client does not
implement the didLoadResourceFromMemoryCache delegate method, WebKit
synthesizes the typical sequence of resource load callbacks. One of
these is willSendRequest, which gives the client the opportunity to
modify the request. We should respect these modifications.

Handling any arbitrary modification is difficult given where in the
loading process we check the memory cache (see <http://webkit.org/b/113251>),
but we can handle the common case where the client cancels the load by
returning a NULL request.

Test: fast/loader/willsendrequest-returns-null-for-memory-cache-load.html

* loader/FrameLoader.cpp:
(WebCore::FrameLoader::willLoadMediaElementURL): Passed request to sendRemainingDelegateMessages.
(WebCore::FrameLoader::commitProvisionalLoad): Ditto.
(WebCore::FrameLoader::loadResourceSynchronously): Ditto.
(WebCore::FrameLoader::loadedResourceFromMemoryCache): Added an out
parameter to pass back a request potentially modified by
requestFromDelegate, which is also passed to sendRemainingDelegateMessages.
* loader/FrameLoader.h:
* loader/ResourceLoadNotifier.cpp:
(WebCore::ResourceLoadNotifier::dispatchDidFailLoading): Added a
function to dispatch didFailLoading and to call the correct InspectorInstrumentation.
(WebCore::ResourceLoadNotifier::sendRemainingDelegateMessages): If the
request is NULL, call dispatchDidFailLoading and return. This matches
the delegate callback sequence of a non memory cache load that is
cancelled by willSendRequest.
* loader/ResourceLoadNotifier.h:
* loader/cache/CachedResourceLoader.cpp:
(WebCore::CachedResourceLoader::shouldContinueAfterNotifyingLoadedFromMemoryCache):
Return false if the memory cache load is cancelled by the client.
(WebCore::CachedResourceLoader::requestResource): Return early if
shouldContinueAfterNotifyingLoadedFromMemoryCache is false.

LayoutTests:

* fast/loader/resources/cached-image.html: Added.
* fast/loader/willsendrequest-returns-null-for-memory-cache-load-expected.txt: Added.
* fast/loader/willsendrequest-returns-null-for-memory-cache-load.html: Added.
* platform/wk2/TestExpectations: Expect the test to fail in WKTR due to <http://webkit.org/b/114074>.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@147829 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 9ae791c1
2013-04-05 Andy Estes <aestes@apple.com>
Returning NULL from willSendRequest should cancel a load from the memory cache
https://bugs.webkit.org/show_bug.cgi?id=114075
Reviewed by Darin Adler.
* fast/loader/resources/cached-image.html: Added.
* fast/loader/willsendrequest-returns-null-for-memory-cache-load-expected.txt: Added.
* fast/loader/willsendrequest-returns-null-for-memory-cache-load.html: Added.
* platform/wk2/TestExpectations: Expect the test to fail in WKTR due to <http://webkit.org/b/114074>.
2013-04-05 Chris Fleizach <cfleizach@apple.com>
AX: VoiceOver can't press on items
<script>
if (window.testRunner)
testRunner.setWillSendRequestReturnsNull(true);
</script>
<img src="compass.jpg" onload="console.log('FAIL: image was incorrectly loaded')">
resources/compass.jpg - willSendRequest <NSURLRequest URL resources/compass.jpg, main document URL willsendrequest-returns-null-for-memory-cache-load.html, http method GET> redirectResponse (null)
<unknown> - didFinishLoading
resources/compass.jpg - didReceiveResponse <NSURLResponse resources/compass.jpg, http status code 0>
resources/compass.jpg - didFinishLoading
resources/cached-image.html - willSendRequest <NSURLRequest URL resources/cached-image.html, main document URL willsendrequest-returns-null-for-memory-cache-load.html, http method GET> redirectResponse (null)
resources/cached-image.html - didReceiveResponse <NSURLResponse resources/cached-image.html, http status code 0>
resources/compass.jpg - willSendRequest <NSURLRequest URL resources/compass.jpg, main document URL (null), http method GET> redirectResponse (null)
resources/compass.jpg - didFailLoadingWithError: <NSError domain NSURLErrorDomain, code -999, failing URL "resources/compass.jpg">
<!DOCTYPE html>
<html>
<head>
<script>
if (window.testRunner) {
testRunner.dumpAsText();
testRunner.waitUntilDone();
testRunner.dumpResourceLoadCallbacks();
}
function loadCachedImageInIFrame()
{
var iframe = document.createElement("iframe");
document.body.appendChild(iframe);
iframe.onload = function() {
if (window.testRunner)
testRunner.notifyDone();
}
iframe.src = "resources/cached-image.html";
}
</script>
</head>
<body>
<img src="resources/compass.jpg" onload="loadCachedImageInIFrame()">
</body>
</html>
\ No newline at end of file
......@@ -340,6 +340,10 @@ plugins/npruntime/embed-property-iframe-equality.html
webkit.org/b/105952 fast/loader/submit-form-while-parsing-2.html [ Pass Failure ]
# [WebKit2] WKTR's output for dumpResourceLoadCallbacks is inconsistent with DRT
# https://bugs.webkit.org/show_bug.cgi?id=114074
webkit.org/b/114074 fast/loader/willsendrequest-returns-null-for-memory-cache-load.html [ Failure ]
### END OF (1) Classified failures with bug reports
########################################
......
2013-04-05 Andy Estes <aestes@apple.com>
Returning NULL from willSendRequest should cancel a load from the memory cache
https://bugs.webkit.org/show_bug.cgi?id=114075
Reviewed by Darin Adler.
When a resource is loaded from the memory cache and the client does not
implement the didLoadResourceFromMemoryCache delegate method, WebKit
synthesizes the typical sequence of resource load callbacks. One of
these is willSendRequest, which gives the client the opportunity to
modify the request. We should respect these modifications.
Handling any arbitrary modification is difficult given where in the
loading process we check the memory cache (see <http://webkit.org/b/113251>),
but we can handle the common case where the client cancels the load by
returning a NULL request.
Test: fast/loader/willsendrequest-returns-null-for-memory-cache-load.html
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::willLoadMediaElementURL): Passed request to sendRemainingDelegateMessages.
(WebCore::FrameLoader::commitProvisionalLoad): Ditto.
(WebCore::FrameLoader::loadResourceSynchronously): Ditto.
(WebCore::FrameLoader::loadedResourceFromMemoryCache): Added an out
parameter to pass back a request potentially modified by
requestFromDelegate, which is also passed to sendRemainingDelegateMessages.
* loader/FrameLoader.h:
* loader/ResourceLoadNotifier.cpp:
(WebCore::ResourceLoadNotifier::dispatchDidFailLoading): Added a
function to dispatch didFailLoading and to call the correct InspectorInstrumentation.
(WebCore::ResourceLoadNotifier::sendRemainingDelegateMessages): If the
request is NULL, call dispatchDidFailLoading and return. This matches
the delegate callback sequence of a non memory cache load that is
cancelled by willSendRequest.
* loader/ResourceLoadNotifier.h:
* loader/cache/CachedResourceLoader.cpp:
(WebCore::CachedResourceLoader::shouldContinueAfterNotifyingLoadedFromMemoryCache):
Return false if the memory cache load is cancelled by the client.
(WebCore::CachedResourceLoader::requestResource): Return early if
shouldContinueAfterNotifyingLoadedFromMemoryCache is false.
2013-04-05 Chris Fleizach <cfleizach@apple.com>
AX: VoiceOver can't press on items
......@@ -1416,7 +1416,7 @@ bool FrameLoader::willLoadMediaElementURL(KURL& url)
unsigned long identifier;
ResourceError error;
requestFromDelegate(request, identifier, error);
notifier()->sendRemainingDelegateMessages(m_documentLoader.get(), identifier, ResourceResponse(url, String(), -1, String(), String()), 0, -1, -1, error);
notifier()->sendRemainingDelegateMessages(m_documentLoader.get(), identifier, request, ResourceResponse(url, String(), -1, String(), String()), 0, -1, -1, error);
url = request.url();
......@@ -1752,7 +1752,7 @@ void FrameLoader::commitProvisionalLoad()
// FIXME: If we get a resource with more than 2B bytes, this code won't do the right thing.
// However, with today's computers and networking speeds, this won't happen in practice.
// Could be an issue with a giant local file.
notifier()->sendRemainingDelegateMessages(m_documentLoader.get(), identifier, response, 0, static_cast<int>(response.expectedContentLength()), 0, error);
notifier()->sendRemainingDelegateMessages(m_documentLoader.get(), identifier, request, response, 0, static_cast<int>(response.expectedContentLength()), 0, error);
}
// FIXME: Why only this frame and not parent frames?
......@@ -2586,7 +2586,7 @@ unsigned long FrameLoader::loadResourceSynchronously(const ResourceRequest& requ
}
}
int encodedDataLength = response.resourceLoadInfo() ? static_cast<int>(response.resourceLoadInfo()->encodedDataLength) : -1;
notifier()->sendRemainingDelegateMessages(m_documentLoader.get(), identifier, response, data.data(), data.size(), encodedDataLength, error);
notifier()->sendRemainingDelegateMessages(m_documentLoader.get(), identifier, request, response, data.data(), data.size(), encodedDataLength, error);
return identifier;
}
......@@ -2878,8 +2878,10 @@ void FrameLoader::requestFromDelegate(ResourceRequest& request, unsigned long& i
request = newRequest;
}
void FrameLoader::loadedResourceFromMemoryCache(CachedResource* resource)
void FrameLoader::loadedResourceFromMemoryCache(CachedResource* resource, ResourceRequest& newRequest)
{
newRequest = ResourceRequest(resource->url());
Page* page = m_frame->page();
if (!page)
return;
......@@ -2898,8 +2900,7 @@ void FrameLoader::loadedResourceFromMemoryCache(CachedResource* resource)
return;
}
ResourceRequest request(resource->url());
if (m_client->dispatchDidLoadResourceFromMemoryCache(m_documentLoader.get(), request, resource->response(), resource->encodedSize())) {
if (m_client->dispatchDidLoadResourceFromMemoryCache(m_documentLoader.get(), newRequest, resource->response(), resource->encodedSize())) {
InspectorInstrumentation::didLoadResourceFromMemoryCache(page, m_documentLoader.get(), resource);
m_documentLoader->didTellClientAboutLoad(resource->url());
return;
......@@ -2907,9 +2908,9 @@ void FrameLoader::loadedResourceFromMemoryCache(CachedResource* resource)
unsigned long identifier;
ResourceError error;
requestFromDelegate(request, identifier, error);
requestFromDelegate(newRequest, identifier, error);
InspectorInstrumentation::markResourceAsCached(page, identifier);
notifier()->sendRemainingDelegateMessages(m_documentLoader.get(), identifier, resource->response(), 0, resource->encodedSize(), 0, error);
notifier()->sendRemainingDelegateMessages(m_documentLoader.get(), identifier, newRequest, resource->response(), 0, resource->encodedSize(), 0, error);
}
void FrameLoader::applyUserAgent(ResourceRequest& request)
......
......@@ -181,7 +181,7 @@ public:
void didLayout(LayoutMilestones);
void didFirstLayout();
void loadedResourceFromMemoryCache(CachedResource*);
void loadedResourceFromMemoryCache(CachedResource*, ResourceRequest& newRequest);
void tellClientAboutPastMemoryCacheLoads();
void checkLoadComplete();
......
......@@ -148,8 +148,23 @@ void ResourceLoadNotifier::dispatchDidFinishLoading(DocumentLoader* loader, unsi
InspectorInstrumentation::didFinishLoading(m_frame, loader, identifier, finishTime);
}
void ResourceLoadNotifier::sendRemainingDelegateMessages(DocumentLoader* loader, unsigned long identifier, const ResourceResponse& response, const char* data, int dataLength, int encodedDataLength, const ResourceError& error)
void ResourceLoadNotifier::dispatchDidFailLoading(DocumentLoader* loader, unsigned long identifier, const ResourceError& error)
{
m_frame->loader()->client()->dispatchDidFailLoading(loader, identifier, error);
InspectorInstrumentation::didFailLoading(m_frame, loader, identifier, error);
}
void ResourceLoadNotifier::sendRemainingDelegateMessages(DocumentLoader* loader, unsigned long identifier, const ResourceRequest& request, const ResourceResponse& response, const char* data, int dataLength, int encodedDataLength, const ResourceError& error)
{
// If the request is null, willSendRequest cancelled the load. We should
// only dispatch didFailLoading in this case.
if (request.isNull()) {
ASSERT(error.isCancellation());
dispatchDidFailLoading(loader, identifier, error);
return;
}
if (!response.isNull())
dispatchDidReceiveResponse(loader, identifier, response);
......@@ -159,7 +174,7 @@ void ResourceLoadNotifier::sendRemainingDelegateMessages(DocumentLoader* loader,
if (error.isNull())
dispatchDidFinishLoading(loader, identifier, 0);
else
m_frame->loader()->client()->dispatchDidFailLoading(loader, identifier, error);
dispatchDidFailLoading(loader, identifier, error);
}
} // namespace WebCore
......@@ -62,8 +62,9 @@ public:
void dispatchDidReceiveResponse(DocumentLoader*, unsigned long identifier, const ResourceResponse&, ResourceLoader* = 0);
void dispatchDidReceiveData(DocumentLoader*, unsigned long identifier, const char* data, int dataLength, int encodedDataLength);
void dispatchDidFinishLoading(DocumentLoader*, unsigned long identifier, double finishTime);
void dispatchDidFailLoading(DocumentLoader*, unsigned long identifier, const ResourceError&);
void sendRemainingDelegateMessages(DocumentLoader*, unsigned long identifier, const ResourceResponse&, const char* data, int dataLength, int encodedDataLength, const ResourceError&);
void sendRemainingDelegateMessages(DocumentLoader*, unsigned long identifier, const ResourceRequest&, const ResourceResponse&, const char* data, int dataLength, int encodedDataLength, const ResourceError&);
private:
Frame* m_frame;
......
......@@ -421,6 +421,19 @@ bool CachedResourceLoader::canRequest(CachedResource::Type type, const KURL& url
return true;
}
bool CachedResourceLoader::shouldContinueAfterNotifyingLoadedFromMemoryCache(CachedResource* resource)
{
if (!resource || !frame() || resource->status() != CachedResource::Cached)
return true;
ResourceRequest newRequest;
frame()->loader()->loadedResourceFromMemoryCache(resource, newRequest);
// FIXME <http://webkit.org/b/113251>: If the delegate modifies the request's
// URL, it is no longer appropriate to use this CachedResource.
return !newRequest.isNull();
}
CachedResourceHandle<CachedResource> CachedResourceLoader::requestResource(CachedResource::Type type, CachedResourceRequest& request)
{
KURL url = request.resourceRequest().url();
......@@ -468,8 +481,9 @@ CachedResourceHandle<CachedResource> CachedResourceLoader::requestResource(Cache
resource = revalidateResource(request, resource.get());
break;
case Use:
if (!shouldContinueAfterNotifyingLoadedFromMemoryCache(resource.get()))
return 0;
memoryCache()->resourceAccessed(resource.get());
notifyLoadedFromMemoryCache(resource.get());
break;
}
......@@ -803,15 +817,6 @@ void CachedResourceLoader::performPostLoadActions()
#endif
}
void CachedResourceLoader::notifyLoadedFromMemoryCache(CachedResource* resource)
{
if (!resource || !frame() || resource->status() != CachedResource::Cached)
return;
// FIXME: If the WebKit client changes or cancels the request, WebCore does not respect this and continues the load.
frame()->loader()->loadedResourceFromMemoryCache(resource);
}
void CachedResourceLoader::incrementRequestCount(const CachedResource* res)
{
if (res->ignoreForRequestCount())
......
......@@ -153,7 +153,7 @@ private:
enum RevalidationPolicy { Use, Revalidate, Reload, Load };
RevalidationPolicy determineRevalidationPolicy(CachedResource::Type, ResourceRequest&, bool forPreload, CachedResource* existingResource, CachedResourceRequest::DeferOption) const;
void notifyLoadedFromMemoryCache(CachedResource*);
bool shouldContinueAfterNotifyingLoadedFromMemoryCache(CachedResource*);
bool checkInsecureContent(CachedResource::Type, const KURL&) const;
void garbageCollectDocumentResourcesTimerFired(Timer<CachedResourceLoader>*);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment