RESOLVED FIXED80905
Implement ApplicationCacheHost::canCacheInPageCache() for chromium. 
https://bugs.webkit.org/show_bug.cgi?id=80905
Summary Implement ApplicationCacheHost::canCacheInPageCache() for chromium.
Gavin Peters
Reported 2012-03-12 16:12:00 PDT
Implement ApplicationCacheHost::canCacheInPageCache() for chromium.
Attachments
Patch (1.07 KB, patch)
2012-03-12 16:15 PDT, Gavin Peters 		no flags
DetailsFormatted DiffDiff
Patch (2.30 KB, patch)
2012-03-12 16:18 PDT, Gavin Peters 		no flags
DetailsFormatted DiffDiff
Patch (1.07 KB, patch)
2012-03-12 16:24 PDT, Gavin Peters 		no flags
DetailsFormatted DiffDiff
Patch (1.98 KB, patch)
2012-03-12 16:32 PDT, Gavin Peters 		no flags
DetailsFormatted DiffDiff
Patch (1.98 KB, patch)
2012-03-13 08:26 PDT, Gavin Peters 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Gavin Peters
Comment 1 2012-03-12 16:15:34 PDT
Created attachment 131438 [details] Patch
Gavin Peters
Comment 2 2012-03-12 16:18:20 PDT
Created attachment 131440 [details] Patch
Gavin Peters
Comment 3 2012-03-12 16:21:11 PDT
Wait a second while I learn webkit-patch upload.
Gavin Peters
Comment 4 2012-03-12 16:24:48 PDT
Created attachment 131442 [details] Patch
Gavin Peters
Comment 5 2012-03-12 16:32:44 PDT
Created attachment 131446 [details] Patch
Gavin Peters
Comment 6 2012-03-12 16:34:31 PDT
It took me a while, but I managed to get webkit-patch to upload a real patch. I am not turning on the PageCache with this; Settings stops it as I note in my comment. However, this will make the statistics I'm gathering in PageCache.cpp to reevaluate that decision much better. Note this patch is made relative to Bug 80904, so can't land until after that patch.
WebKit Review Bot
Comment 7 2012-03-12 17:09:57 PDT
Comment on attachment 131446 [details] Patch Attachment 131446 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11943230
Gavin Peters
Comment 8 2012-03-12 20:46:44 PDT
Comment on attachment 131446 [details] Patch The chromium buildbot failure is expected; given that 80904 hasn't landed yet. I'll upload another patch to double-check EWS after any review.
Gavin Peters
Comment 9 2012-03-13 08:26:04 PDT
Created attachment 131619 [details] Patch
Gavin Peters
Comment 10 2012-03-13 08:26:38 PDT
Bug 80904 has landed, so this upload is the same as the last one: except I expect it to pass EWS because the tree has changed.
Eric Seidel (no email)
Comment 11 2012-03-13 11:58:03 PDT
Comment on attachment 131619 [details] Patch I assume you've audited the callers?
Gavin Peters
Comment 12 2012-03-13 12:06:14 PDT
There are two callers: both are in PageCache.cpp. The first is in the debug printing/histogram gathering code in the PageCache. This change will make that family of histograms better reflect what PageCache performance would be if we used the PageCache. The second is in PageCache::canCachePageContainingThisFrame(). That's moot because frameLoader->client()->canCachePage() will always fail, so it will always return the same result. As well, page->settings()->usesPageCache() is usually false in chrome (modulo a command line argument that combined with the above does nothing). If I ever want to make frameLoader->client()->canCachePage() report better stats, I'll have to either disable the command line flag to "turn on pagecache" or make page-cache work well enough not to ruin the lives of those brave canaries who have set this (currently nonfunctional) command line argument.
WebKit Review Bot
Comment 13 2012-03-13 13:48:15 PDT
Comment on attachment 131619 [details] Patch Clearing flags on attachment: 131619 Committed r110600: <http://trac.webkit.org/changeset/110600>
WebKit Review Bot
Comment 14 2012-03-13 13:48:28 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.