Summary: | [GTK] Add initial WebKitWebsiteDataManager API for process configuration options | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | gustavo, mcatanzaro | ||||||||
Priority: | P2 | Keywords: | Gtk | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | 146145 | ||||||||||
Bug Blocks: | |||||||||||
Attachments: |
|
Description
Carlos Garcia Campos
2015-06-19 09:58:10 PDT
Created attachment 255195 [details] Patch This won't apply as it depends on bug #146145 You forgot documentation for webkit_website_data_manager_new (In reply to comment #2) > You forgot documentation for webkit_website_data_manager_new Oops. Created attachment 255339 [details]
Updated patch
Just added the missing documentation.
Created attachment 255402 [details]
Updated patch
Added base-data-directory and base-cache-directory properties, more API documentation and updated the unit tests.
Comment on attachment 255402 [details] Updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=255402&action=review Provided we agree on the API this looks good to me > Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.h:156 > +webkit_web_context_get_website_data_manager (WebKitWebContext *context); Shouln't we use const here because of the immutable thing? > Source/WebKit2/UIProcess/API/gtk/WebKitWebsiteDataManager.cpp:158 > + priv->webSQLDirectory.reset(g_build_filename(priv->baseDataDirectory.get(), "databases", nullptr)); Perhaps cache the value of "priv->baseDataDirectory.get()" in a local variable Comment on attachment 255402 [details] Updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=255402&action=review Thanks for the review! Please, not that this depends on patch attached to bug #146145 >> Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.h:156 >> +webkit_web_context_get_website_data_manager (WebKitWebContext *context); > > Shouln't we use const here because of the immutable thing? Well, const in C is not exactly the same as in C++. The properties in WebKitWebsiteDataManager are construct-only, so the object can't be modified no matter if the pointer is const or not. >> Source/WebKit2/UIProcess/API/gtk/WebKitWebsiteDataManager.cpp:158 >> + priv->webSQLDirectory.reset(g_build_filename(priv->baseDataDirectory.get(), "databases", nullptr)); > > Perhaps cache the value of "priv->baseDataDirectory.get()" in a local variable What for? to make lines shorter? The API seems sane to me =) Comment on attachment 255402 [details] Updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=255402&action=review The API looks fine to me. > Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:382 > + _("The WebKitWebsiteDataManager associated to this context"), English nit: associated to -> associated with PS: Sorry about English. > Source/WebKit2/UIProcess/API/gtk/WebKitWebsiteDataManager.cpp:43 > + * a default value will be used automatically for the confoguration options Nit: confoguration -> configuration > Source/WebKit2/UIProcess/API/gtk/WebKitWebsiteDataManager.cpp:355 > + * Returns: the base directory for Website data, or %NULL if Website is capitalized here, but not earlier. Committed r185950: <http://trac.webkit.org/changeset/185950> |