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

[bugzilla-daemon at webkit.org]

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


Simon Hausmann <hausmann at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #33941|review?                     |review+
               Flag|                            |




--- Comment #30 from Simon Hausmann <hausmann at webkit.org>  2009-08-03 14:34:32 PDT ---
(From update of attachment 33941)
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==.

> +/*!
> +    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.

> +/*!
> +    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...


> +
> +/*!
> +    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.

> +/*!
> +    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 :)

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