Summary: | Returning NULL from willSendRequest should cancel a load from the memory cache | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Andy Estes <aestes> | ||||
Component: | Page Loading | Assignee: | Andy Estes <aestes> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | abarth, darin, japhet, laszlo.gombos, webkit-bug-importer, webkit.review.bot, zarvai | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Andy Estes
2013-04-05 17:23:24 PDT
Created attachment 196709 [details]
Patch
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? (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! Committed r147829: <http://trac.webkit.org/changeset/147829> 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 That result looks like what you’d get if the willSendRequest function did not return null. Maybe something wrong with the test harness? I agree with Darin. Zoltan, does the Qt DRT implement testRunner.setWillSendRequestReturnsNull()? |