| Summary: | Implement ApplicationCacheHost::canCacheInPageCache() for chromium. | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Gavin Peters <gavinp> | ||||||||||||
| Component: | New Bugs | Assignee: | Gavin Peters <gavinp> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | dglazkov, eric, japhet, webkit.review.bot | ||||||||||||
| Priority: | P2 | ||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Gavin Peters
2012-03-12 16:12:00 PDT
Created attachment 131438 [details]
Patch
Created attachment 131440 [details]
Patch
Wait a second while I learn webkit-patch upload. Created attachment 131442 [details]
Patch
Created attachment 131446 [details]
Patch
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. Comment on attachment 131446 [details] Patch Attachment 131446 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11943230 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.
Created attachment 131619 [details]
Patch
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. Comment on attachment 131619 [details]
Patch
I assume you've audited the callers?
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. Comment on attachment 131619 [details] Patch Clearing flags on attachment: 131619 Committed r110600: <http://trac.webkit.org/changeset/110600> All reviewed patches have been landed. Closing bug. |