Summary: | [GTK] The webkit cache model needs to be set when WebFrameLoaderClient::didPerformFirstNavigation() is called | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Joone Hur <joone> | ||||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | gustavo, mrobinson, xan.lopez | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | Linux | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 50004 | ||||||||||
Attachments: |
|
Description
Joone Hur
2010-12-02 18:13:50 PST
Created attachment 75449 [details]
Proposed Patch
Comment on attachment 75449 [details] Proposed Patch View in context: https://bugs.webkit.org/attachment.cgi?id=75449&action=review > WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:696 > + if (cacheModel != WEBKIT_CACHE_MODEL_DOCUMENT_VIEWER && cacheModel != WEBKIT_CACHE_MODEL_WEB_BROWSER) I think it makes sense to expalin this logic with a comment. The Mac port explictly tracks whether or not an API consumer has set a cache model via the automaticallyDetectsCacheModel setting. That might provide a self-documenting alternative. Created attachment 75656 [details]
Proposed Patch2
Martin suggested me to add WEBKIT_CACHE_MODEL_DEFAULT enumeration value in order to check if the cache model is not determined, so I applied this idea to the patch. Thanks!
Comment on attachment 75656 [details] Proposed Patch2 View in context: https://bugs.webkit.org/attachment.cgi?id=75656&action=review I think some of the comments should be shuffled around and expanded. > WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:698 > + // Page cache capacity (in pages). Comment from Mac port: > + // (Research indicates that value / page drops substantially after 3 pages.) This can should probably move to the relevant line in webkit_set_cache_model (the one that sets 3 cache size to 3 pages for WEBKIT_CACHE_MODEL_WEB_BROWSER). > WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:699 > + // FIXME: Expose this with an API and/or calculate based on available resources I think you're right that we can just remove this line now. > WebKit/gtk/webkit/webkitwebview.h:54 > + WEBKIT_CACHE_MODEL_DEFAULT, We should include some documentation with webkit_web_view_set_cache_model explaining that the default is WEB_BROWSER. Alternatively, it makes sense to use an enum comment block: http://library.gnome.org/devel/gtk-doc-manual/stable/documenting_symbols.html.en Created attachment 75660 [details]
Proposed Patch3
The enum comment block has been added. Thanks!
Committed r73353: <http://trac.webkit.org/changeset/73353> Comment on attachment 75660 [details]
Proposed Patch3
Landed after some minor changes.
|