Bug 133137 - DocumentLoader shouldn't keep ResourceResponses indefinitely unless needed.
Summary: DocumentLoader shouldn't keep ResourceResponses indefinitely unless needed.
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andreas Kling
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-05-20 14:42 PDT by Andreas Kling
Modified: 2014-05-21 22:24 PDT (History)
8 users (show)

See Also:

Attachments
Patch (3.76 KB, patch)
2014-05-20 14:46 PDT, Andreas Kling 		ggaren: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2 (471.88 KB, application/zip)
2014-05-20 16:52 PDT, Build Bot 		no flags Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.