NEW133137
DocumentLoader shouldn't keep ResourceResponses indefinitely unless needed.
https://bugs.webkit.org/show_bug.cgi?id=133137
Summary DocumentLoader shouldn't keep ResourceResponses indefinitely unless needed.
Andreas Kling
Reported 2014-05-20 14:42:09 PDT
DocumentLoader shouldn't keep ResourceResponses indefinitely unless needed.
Attachments
Patch (3.76 KB, patch)
2014-05-20 14:46 PDT, Andreas Kling
ggaren: review+
buildbot: commit-queue-
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
Andreas Kling
Comment 1 2014-05-20 14:46:27 PDT
Build Bot
Comment 2 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
Build Bot
Comment 3 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
Geoffrey Garen
Comment 4 2014-05-20 18:13:28 PDT
Comment on attachment 231795 [details] Patch r=me
Andreas Kling
Comment 5 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.
Alexey Proskuryakov
Comment 6 2014-05-21 19:48:13 PDT
Can you elaborate on why it is ok to not send the delegates in this particular case?
Andreas Kling
Comment 7 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?
Alexey Proskuryakov
Comment 8 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.
Note You need to log in before you can comment on or make changes to this bug.