Currently, the webkit cache model is set when webkit_init() is called in WebKitGtk+, but other ports set the cache model when didPerformFirstNavigation() callback is called. I think that it seems better to set the cache model at the explicit time like other ports. Also, we need to consider adding a new property(webview-cache-model) to WebKitWebSettings instead of calling the cache model APIs directly.
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.