[webkit-reviews] review granted: [Bug 7451] Refactor the way we handle plugin information (previously KConfig) : [Attachment 6707] rework the way we handle plugin info

[bugzilla-request-daemon at opendarwin.org]

Darin Adler <darin at apple.com> has granted Darin Adler <darin at apple.com>'s
request for review:
Bug 7451: Refactor the way we handle plugin information (previously KConfig)
http://bugzilla.opendarwin.org/show_bug.cgi?id=7451

Attachment 6707: rework the way we handle plugin info
http://bugzilla.opendarwin.org/attachment.cgi?id=6707&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
If it was me, I wouldn't bother having a PluginInfoStore class. I'd just have a
single free function in the WebCore namespace that returns a
Vector<PlugInInfo*>, perhaps named createPlugInInfoVector. I'd suggest
Vector<OwnPtr<PlugInInfo> >, except I don't think that works. There's no
requirement here to have a way to read the plug-in info for one plug-in at a
time.

If PluginInfoStore is going to be noncopyable, it should be inheriting from the
Noncopyable class. But I don't see any reason it needs to be non-copyable. You
should remove the constructor from PluginInfoStore. And perhaps remove the
class as I suggest above.

+    PluginInfo *createPluginInfoForPluginAtIndex(unsigned);

* should be by PluginInfo.

I think that Plugin is supposed to be PlugIn whenever possible, because it's
"plug-in", not "plugin".

+    pluginInfo->file = String([plugin filename]);

I don't think you need the explicit String() here.

Nice improvement. r=me if you want to land it as is, or you could consider some
of my suggestions.