[Webkit-unassigned] [Bug 54823] [GTK] Implemented missing API for setting and getting application cache directory path database.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=54823


Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #83093|review?                     |review-
               Flag|                            |




--- Comment #3 from Martin Robinson <mrobinson at webkit.org>  2011-02-21 14:50:06 PST ---
(From update of attachment 83093)
View in context: https://bugs.webkit.org/attachment.cgi?id=83093&action=review

As long as the Mac port exposes this, I think it's a good idea for us to expose it. Please consider my suggestions below though.

> Source/WebKit/gtk/webkit/webkitapplicationcache.cpp:28
> +static gchar* webkit_application_cache_directory_path = NULL;

Wouldn't it make more sense to make this a CString? Please use WebKit style when naming your variables.

> Source/WebKit/gtk/webkit/webkitapplicationcache.cpp:47
> + * Returns the current path to the directory WebKit will write Application
> + * Cache databases. By default this path will be in the user data
> + * directory.
> + *

Please explain what's stored in the application cache. Instead of saying user data directory, I recommend actually mentioning the GLib function we use to calculate the default directory.

> Source/WebKit/gtk/webkit/webkitapplicationcache.cpp:62
> +    if (path.isEmpty())
> +        return "";
> +
> +    g_free(webkit_application_cache_directory_path);
> +    webkit_application_cache_directory_path = g_strdup(path.utf8().data());
> +    return webkit_application_cache_directory_path;

No need to special case the empty string. When converting from a String to a raw string you need to use fileSystemRepresentation from FileSystemGtk.cpp in WebCore. I recommend checking if the current path is equal before freeing and updating the cached version.

> Source/WebKit/gtk/webkit/webkitapplicationcache.cpp:73
> + * Sets the current path to the directory WebKit will write Application
> + * Cache databases.

No need to capitalize Application Cache here and above.

> Source/WebKit/gtk/webkit/webkitapplicationcache.cpp:80
> +    WTF::String corePath = WTF::String::fromUTF8(path);

You need to use filenameToString when converting from a path to WebKit string. Filenames are not necessarily in UTF-8.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.