[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
bugzilla-request-daemon at opendarwin.org
Fri Feb 24 19:09:07 PST 2006
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.
More information about the webkit-reviews
mailing list