[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)

Attachment 6707: rework the way we handle plugin info

------- 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

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