[webkit-reviews] review granted: [Bug 17825] PluginDatabase implementation for GTK+ : [Attachment 19727] PluginDatabase implementation for GTK+

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has granted Rodney Dawes
<dobey at wayofthemonkey.com>'s request for review:
Bug 17825: PluginDatabase implementation for GTK+
http://bugs.webkit.org/show_bug.cgi?id=17825

Attachment 19727: PluginDatabase implementation for GTK+
http://bugs.webkit.org/attachment.cgi?id=19727&action=edit

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
 42	for (Vector<String>::const_iterator it = m_pluginPaths.begin(); it !=
end; ++it) {
 43	    GDir* dir = g_dir_open((it->utf8()).data(), 0, NULL);
 44	    if (dir) {

You could reverse this and do

if (!dir)
    continue;

That would remove a level of nesting.

 45		const char* name;
 46		while ((name = g_dir_read_name(dir))) {

I think you can declare name right in the assignment:

while (const char* name = g_dir_read_name(dir)) {

 47		    if (g_str_has_suffix(name, "." G_MODULE_SUFFIX)) {

You could reverse this as well:

if (!g_str_has_suffix(...))
    continue;

 48			gchar* filename = g_build_filename((it->utf8()).data(),
name, NULL);
 49			PluginPackage* pluginPackage =
PluginPackage::createPackage(filename, time(NULL));

Our code style guidelines say we should be using 0 instead of NULL in C++
source files (even for calling C functions).

 71 #elif defined(GDK_WINDOWING_WIN32)
 72	path = g_build_filename(g_get_home_dir(), "Application Data",
"Mozilla", "plugins", NULL);
 73	paths.append(path);
 74	g_free(path);
 75 #endif

Should you be looking at the MozillaPlugins registry key as well?

 77	const gchar* mozPath = g_getenv("MOZ_PLUGIN_PATH");
 78	if (mozPath) {
 79	    gchar** pluginPaths = g_strsplit(mozPath, ":", -1);

This doesn't seem quite right to do on Windows. Does Gecko really use a
colon-separated MOZ_PLUGIN_PATH environment variable on Windows?

r=me even if the above issues are not addressed, but it would be nice to fix
them since it'd be so easy!