Bug 133137

Summary: DocumentLoader shouldn't keep ResourceResponses indefinitely unless needed.
Product: WebKit Reporter: Andreas Kling <kling>
Component: Page LoadingAssignee: Andreas Kling <kling>
Status: NEW ---    
Severity: Normal CC: ap, beidson, buildbot, commit-queue, ggaren, japhet, kling, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
ggaren: review+, buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2 none

Description Andreas Kling 2014-05-20 14:42:09 PDT 
DocumentLoader shouldn't keep ResourceResponses indefinitely unless needed.
Comment 1 Andreas Kling 2014-05-20 14:46:27 PDT 
Created attachment 231795 [details]
Patch
Comment 2 Build Bot 2014-05-20 16:52:08 PDT 
Comment on attachment 231795 [details]
Patch

Attachment 231795 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4714788051484672

New failing tests:
loader/go-back-cached-main-resource.html
Comment 3 Build Bot 2014-05-20 16:52:11 PDT 
Created attachment 231799 [details]
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-16  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 4 Geoffrey Garen 2014-05-20 18:13:28 PDT 
Comment on attachment 231795 [details]
Patch

r=me
Comment 5 Andreas Kling 2014-05-21 18:07:47 PDT 
The test is failing because the resource delegate callbacks no longer fire under WK2 as expected. I think we could land this with a platform-specific expectation for WK2.
Comment 6 Alexey Proskuryakov 2014-05-21 19:48:13 PDT 
Can you elaborate on why it is ok to not send the delegates in this particular case?
Comment 7 Andreas Kling 2014-05-21 20:04:37 PDT 
(In reply to comment #6)
> Can you elaborate on why it is ok to not send the delegates in this particular case?

It's my understanding, from talking with Geoff, that these callbacks exist exclusively for servicing the WebKit1 Mac API, and that they are not needed for WK2.

Is that not the case?
Comment 8 Alexey Proskuryakov 2014-05-21 22:24:55 PDT 
It depends on which level the failure occurs at - we could be breaking internal behaviors too. Also, there are a lot of very important WebKit1 clients.

I don't think that "WebKit1 only" is a license to kill. The test seems like a very straightforward one - I expect that the only reason why the patch didn't break dozens of tests is that we don't enable page cache in regression tests by default. But page cache being enabled is the norm outside regression tests.