Bug 17825

Summary: PluginDatabase implementation for GTK+
Product: WebKit Reporter: Rodney Dawes <dobey>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
PluginDatabase implementation for GTK+
aroben: review+
Updated patch for adam's comments aroben: review+

Description Rodney Dawes 2008-03-13 08:59:56 PDT
The attached patch implements the PluginDatabase class for GTK+.
Comment 1 Rodney Dawes 2008-03-13 09:00:27 PDT
Created attachment 19727 [details]
PluginDatabase implementation for GTK+
Comment 2 Adam Roben (:aroben) 2008-03-14 09:30:13 PDT
Comment on attachment 19727 [details]
PluginDatabase implementation for GTK+

 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!
Comment 3 Rodney Dawes 2008-03-14 13:11:47 PDT
Created attachment 19772 [details]
Updated patch for adam's comments
Comment 4 Adam Roben (:aroben) 2008-03-17 07:37:53 PDT
Comment on attachment 19772 [details]
Updated patch for adam's comments

r=me
Comment 5 Adam Roben (:aroben) 2008-03-17 07:51:49 PDT
Committed in r31094