Fix layout test failures. Looks like keying the plugins based on OriginalFil...

Fix layout test failures.  Looks like keying the plugins based on OriginalFileName didn't work, since different filenames might still have the same value (i.e. QuickTime).  Instead I did what Firefox does, which is collect the list of directories in one pass, then crawl them in another.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=7907

[Evan Martin, 2009-01-13 00:23:53]

+plugin peeps

simpler code makes me happy.

http://codereview.chromium.org/17367/diff/1/9
File webkit/glue/plugins/plugin_list.cc (right):

http://codereview.chromium.org/17367/diff/1/9#newcode357
Line 357: return true;
it seems this function could just return a const reference to plugins_, since it
doesn't return the bool value.

http://codereview.chromium.org/17367/diff/1/5
File webkit/glue/plugins/plugin_list.h (right):

http://codereview.chromium.org/17367/diff/1/5#newcode157
Line 157: // Search the registry at the given path and load plugins listed
there.
does this load plugins or just get the directory names?  maybe thsi comment
needs to be updated?

http://codereview.chromium.org/17367/diff/1/5#newcode171
Line 171: // The list of WebPluginInfo.
this could be more clear -- is it all plugins that are known, or all that have
been loaded, or what?

http://codereview.chromium.org/17367/diff/1/6
File webkit/glue/plugins/webplugin_delegate_impl.cc (right):

http://codereview.chromium.org/17367/diff/1/6#newcode149
Line 149: StringToLowerASCII(plugin_info.path.BaseName().value());
Seems you could convert to std::string here and not stick in L"" on all the
things you compare it agains.

[jam, 2009-01-13 00:36:08]

http://codereview.chromium.org/17367/diff/1/9
File webkit/glue/plugins/plugin_list.cc (right):

http://codereview.chromium.org/17367/diff/1/9#newcode357
Line 357: return true;
On 2009/01/13 00:23:53, Evan Martin wrote:
> it seems this function could just return a const reference to plugins_, since
it
> doesn't return the bool value.

it won't make a difference, the pointer is going straight into an IPC message.

http://codereview.chromium.org/17367/diff/1/5
File webkit/glue/plugins/plugin_list.h (right):

http://codereview.chromium.org/17367/diff/1/5#newcode157
Line 157: // Search the registry at the given path and load plugins listed
there.
On 2009/01/13 00:23:53, Evan Martin wrote:
> does this load plugins or just get the directory names?  maybe thsi comment
> needs to be updated?

Done.

http://codereview.chromium.org/17367/diff/1/5#newcode171
Line 171: // The list of WebPluginInfo.
On 2009/01/13 00:23:53, Evan Martin wrote:
> this could be more clear -- is it all plugins that are known, or all that have
> been loaded, or what?

Done.

http://codereview.chromium.org/17367/diff/1/6
File webkit/glue/plugins/webplugin_delegate_impl.cc (right):

http://codereview.chromium.org/17367/diff/1/6#newcode149
Line 149: StringToLowerASCII(plugin_info.path.BaseName().value());
On 2009/01/13 00:23:53, Evan Martin wrote:
> Seems you could convert to std::string here and not stick in L"" on all the
> things you compare it agains.

I don't think it makes too much of a difference either way, but I did it.

[Evan Martin, 2009-01-13 00:40:46]

whoops, meant to LGTM before

plugin peeps cc'd for reference.  :)

[Avi (use Gerrit), 2009-01-13 14:51:36]

http://codereview.chromium.org/17367/diff/44/228
File webkit/glue/plugins/plugin_list.cc (right):

http://codereview.chromium.org/17367/diff/44/228#newcode101
Line 101: directories_to_scan.insert((*extra_plugin_paths_)[i].DirName());
This smells. Someone asks for a specific plugin to be loaded and we load it and
all its siblings?

http://codereview.chromium.org/17367/diff/44/224
File webkit/glue/plugins/plugin_list.h (right):

http://codereview.chromium.org/17367/diff/44/224#newcode91
Line 91: void LoadPluginsFromDir(const FilePath& path);
Too late now, but why abbrev here when we don't abbreviate "directory" in other
function names?