[Webkit-unassigned] [Bug 27651] [Qt] QWebPluginDatabase API

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=27651





--- Comment #39 from Jakub Wieczorek <faw217 at gmail.com>  2009-08-15 06:53:33 PDT ---
(In reply to comment #30)
> (From update of attachment 33941 [details])
> r=me. There's a few things that would be nice to clean up in follow-up patches.
> 
> > +bool QWebPlugin::MimeType::operator==(const MimeType& other) const
> > +{
> > +    return name == other.name
> > +           && description == other.description
> > +           && fileExtensions == other.fileExtensions;
> > +}
> 
> We really should also have an operator!= if we offer operator==.

Already fixed in http://trac.webkit.org/changeset/46763.

> 
> > +/*!
> > +    Constructs a null QWebPlugin.
> > +*/
> > +QWebPlugin::QWebPlugin()
> > +    : d(new QWebPluginPrivate(0))
> > +{
> > +}
> 
> It would be nice if this class could be constructed without a malloc, i.e. if
> the null QWebPlugin would be null due to it's null d pointer. That makes it
> cheaper and avoids unnecessary mallocs.
> In fact I would even go as far as having a private d-pointer but also the
> plugin pointer directly as member, similar to QWebElement, to reduce the number
> of mallocs even more.

Done.

> 
> > +/*!
> > +    Returns a list of MIME types that are supported by the plugin.
> > +
> > +    \sa supportsMimeType()
> > +*/
> > +QList<QWebPlugin::MimeType> QWebPlugin::mimeTypes() const
> > +{
> > +    if (!d->plugin)
> > +        return QList<MimeType>();
> > +
> > +    QList<MimeType> mimeTypes;
> > +    const MIMEToDescriptionsMap& mimeToDescriptions = d->plugin->mimeToDescriptions();
> > +    MIMEToDescriptionsMap::const_iterator end = mimeToDescriptions.end();
> > +    for (MIMEToDescriptionsMap::const_iterator it = mimeToDescriptions.begin(); it != end; ++it) {
> > +        MimeType mimeType;
> > +        mimeType.name = it->first;
> > +        mimeType.description = it->second;
> > +
> > +        QStringList fileExtensions;
> > +        Vector<String> extensions = d->plugin->mimeToExtensions().get(mimeType.name);
> > +
> > +        for (unsigned i = 0; i < extensions.size(); ++i)
> > +            fileExtensions.append(extensions[i]);
> > +
> > +        mimeType.fileExtensions = fileExtensions;
> > +        mimeTypes.append(mimeType);
> > +    }
> > +
> > +    return mimeTypes;
> > +}
> 
> It might be worth mentioning in the docs that this method is not as fast as it
> seems...

Right. It's now optimized.

> 
> 
> > +
> > +/*!
> > +    Returns true if the plugin supports a specific \a mimeType, false otherwise.
> > +
> > +    \sa mimeTypes()
> > +*/
> > +bool QWebPlugin::supportsMimeType(const QString& mimeType) const
> > +{
> > +    QList<MimeType> types = mimeTypes();
> > +    foreach (const MimeType& type, types) {
> > +        if (type.name == mimeType)
> > +            return true;
> > +    }
> 
> Would it be possible to implement this more efficiently? It seems expensive to
> call mimeTypes() every time.

Done.

> 
> > +/*!
> > +    Enables or disables the plugin.
> > +
> > +    Disabled plugins will not be picked up by WebKit when looking for a plugin supporting
> > +    a particular MIME type.
> > +
> > +    \sa isEnabled()
> > +*/
> > +void QWebPlugin::setEnabled(bool enabled)
> > +{
> > +    if (!d->plugin)
> > +        return;
> > +    d->plugin->setEnabled(enabled);
> > +}
> 
> Shouldn't the docs mention that refresh() needs to be called afterwards? But
> perhaps I'm missing something :)

refresh() doesn't need to be called afterwards.

Disabling plugins has only effect on the searching stage, i.e. when WebKit
picks a plugin for a given type, at which point this setting is checked. It is
not cached so the database doesn't need to be refreshed.

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