WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
146149
[GTK] Add initial WebKitWebsiteDataManager API for process configuration options
https://bugs.webkit.org/show_bug.cgi?id=146149
Summary
[GTK] Add initial WebKitWebsiteDataManager API for process configuration options
Carlos Garcia Campos
Reported
2015-06-19 09:58:10 PDT
As discussed in the mailing list, for now it will replace the different ways we have to configure data store directories.
Attachments
Patch
(45.92 KB, patch)
2015-06-19 10:11 PDT
,
Carlos Garcia Campos
no flags
Details
Formatted Diff
Diff
Updated patch
(46.36 KB, patch)
2015-06-21 23:49 PDT
,
Carlos Garcia Campos
no flags
Details
Formatted Diff
Diff
Updated patch
(55.56 KB, patch)
2015-06-23 03:17 PDT
,
Carlos Garcia Campos
svillar
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Carlos Garcia Campos
Comment 1
2015-06-19 10:11:00 PDT
Created
attachment 255195
[details]
Patch This won't apply as it depends on
bug #146145
Michael Catanzaro
Comment 2
2015-06-19 10:47:36 PDT
You forgot documentation for webkit_website_data_manager_new
Carlos Garcia Campos
Comment 3
2015-06-19 10:54:51 PDT
(In reply to
comment #2
)
> You forgot documentation for webkit_website_data_manager_new
Oops.
Carlos Garcia Campos
Comment 4
2015-06-21 23:49:33 PDT
Created
attachment 255339
[details]
Updated patch Just added the missing documentation.
Carlos Garcia Campos
Comment 5
2015-06-23 03:17:03 PDT
Created
attachment 255402
[details]
Updated patch Added base-data-directory and base-cache-directory properties, more API documentation and updated the unit tests.
Sergio Villar Senin
Comment 6
2015-06-24 01:26:47 PDT
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
Carlos Garcia Campos
Comment 7
2015-06-24 04:43:51 PDT
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?
Gustavo Noronha (kov)
Comment 8
2015-06-24 08:37:20 PDT
The API seems sane to me =)
Martin Robinson
Comment 9
2015-06-24 08:41:24 PDT
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.
Carlos Garcia Campos
Comment 10
2015-06-25 00:52:31 PDT
Committed
r185950
: <
http://trac.webkit.org/changeset/185950
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug