[webkit-reviews] review denied: [Bug 62091] Add API to WebKit-GTK to allow setting localStorage database path : [Attachment 96023] Add API to allow setting local storage database path
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sat Jun 4 20:45:43 PDT 2011
Martin Robinson <mrobinson at webkit.org> has denied review:
Bug 62091: Add API to WebKit-GTK to allow setting localStorage database path
https://bugs.webkit.org/show_bug.cgi?id=62091
Attachment 96023: Add API to allow setting local storage database path
https://bugs.webkit.org/attachment.cgi?id=96023&action=review
------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=96023&action=review
Thanks for your contribution! One thing this patch is missing is a ChangeLog.
See here for information about generating one:
http://www.webkit.org/coding/contributing.html
Another thing is that this patch doesn't properly handle
webkit_web_settings_copy.
> webkit-1.4.0-fix/Source/WebKit/gtk/webkit/webkitwebsettings.cpp:96
> + gchar* local_storage_database_path;
I think that html5_local_storage_database_path might be more consistent here.
> webkit-1.4.0-fix/Source/WebKit/gtk/webkit/webkitwebsettings.cpp:151
> + PROP_LOCAL_STORAGE_DATABASE_PATH,
Ditto.
> webkit-1.4.0-fix/Source/WebKit/gtk/webkit/webkitwebsettings.cpp:580
> + * Path to HTML5 localStorage databases.
This should probably be expanded. You should probably link to the local storage
standard and mention what the default value is.
> webkit-1.4.0-fix/Source/WebKit/gtk/webkit/webkitwebsettings.cpp:590
> + g_build_filename(g_get_user_data_dir(),
"webkit", "databases", NULL),
Isn't this a memory leak?
> webkit-1.4.0-fix/Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:407
> + "local-storage-database-path",
g_build_filename(g_get_user_data_dir(), "webkit", "databases", NULL),
Another memory leak here.
More information about the webkit-reviews
mailing list