[Webkit-unassigned] [Bug 17220] Illogical dependency between PluginView and PluginDatabase

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=17220





------- Comment #11 from dobey at wayofthemonkey.com  2008-02-11 11:02 PDT -------
(In reply to comment #10)
> (From update of attachment 19067 [edit])
> r=me, although I'm not entirely happy with these changes.

I presume you meant to set review+ instead of clearing the review flag? Seems
odd to say r=me, and clear the flag. :)

> We'd normally omit the name for the Element* and const KURL& parameters,
> because the type names make it clear what they are.
> We'd normally omit the name for the PluginPackage* parameter, because the type
> name makes it clear what it is.

These can easily be removed. They were already there however, and I merely just
copied the API between files, and changed the method name. :)

>  32 #include "KURL.h"
> 
> It does not seem good to add KURL.h to the PluginDatabase.h header. Why are you
> doing that? Please don't add more includes if you can avoid it!

PluginDatabase.h still uses KURL in findPlugin().

As per comment #5 from Jon:
PluginDatabase.h shouldn't include PluginView.h, but will then require KURL.h.
You can then remove the "class KURL;" declaration.

> I don't think it's clear what you mean by "illogical dependency". I don't see
> anything illogical here. It'll be easier to understand your changes if you use
> more-precise language.

PluginDatabase does not manage a database of PluginViews. It manages a database
of PluginPackages. Asking it for a PluginView is therefore illogical, as
explained in comment #8. It lacks the logical hierarchy of a well-designed API.
This change makes PluginView API consistent with the PluginPackage API, where
we call PluginPackage::createPackage.


-- 
Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.