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

[Andy Estes]

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

[Andy Estes]

<rdar://problem/13228856>

[Andy Estes]

Created attachment 196709 [details]
Patch

[Darin Adler]

Comment on attachment 196709 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=196709&action=review

> Source/WebCore/loader/cache/CachedResourceLoader.h:156
> +    bool shouldContinueAfterNotifyingLoadedFromMemoryCache(CachedResource*);

I might have suggested using a bool out argument instead of return value. Then you would not have had to change the function name. But probably OK this way.

> LayoutTests/fast/loader/resources/cached-image.html:3
> +        testRunner.setWillSendRequestReturnsNull(true);

Strange that this is a function call rather than a property.

> LayoutTests/fast/loader/resources/cached-image.html:5
> +<img src="compass.jpg" onload="console.log('FAIL: image was incorrectly loaded')">

Do we really need a new image for this? Can’t we use an existing image that’s already somewhere in the test tree?

[Andy Estes]

(In reply to comment #3)
> (From update of attachment 196709 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=196709&action=review
> > LayoutTests/fast/loader/resources/cached-image.html:5
> > +<img src="compass.jpg" onload="console.log('FAIL: image was incorrectly loaded')">
> 
> Do we really need a new image for this? Can’t we use an existing image that’s already somewhere in the test tree?

I was running into a case where DRT would print absolute file paths in its resource load delegate messages when the path to the resource was not a subpath of the main resource. Adding a new image (the only image in fast/loader/resources/) was my workaround. I'm going to investigate fixing this in <https://bugs.webkit.org/show_bug.cgi?id=114074>.

Thanks for the review!

[Andy Estes]

Committed r147829: <http://trac.webkit.org/changeset/147829>

[Zoltan Arvai]

On Qt WK1 we have different result for the test. What is your opinion about this result?

--- /ramdisk/qt-linux-release/build/layout-test-results/fast/loader/willsendrequest-returns-null-for-memory-cache-load-expected.txt
+++ /ramdisk/qt-linux-release/build/layout-test-results/fast/loader/willsendrequest-returns-null-for-memory-cache-load-actual.txt
@@ -4,6 +4,8 @@
 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">
+resources/compass.jpg - willSendRequest <NSURLRequest URL resources/compass.jpg, main document URL , http method GET> redirectResponse (null)
+resources/compass.jpg - didReceiveResponse <NSURLResponse resources/compass.jpg, http status code 0>
+resources/compass.jpg - didFinishLoading
+CONSOLE MESSAGE: line 5: FAIL: image was incorrectly loaded

[Darin Adler]

That result looks like what you’d get if the willSendRequest function did not return null. Maybe something wrong with the test harness?

[Andy Estes]

I agree with Darin. Zoltan, does the Qt DRT implement testRunner.setWillSendRequestReturnsNull()?