[Webkit-unassigned] [Bug 27215] [QT] suppress (un)desired launcher output that can make layout test to fail with stderr

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jul 15 12:07:45 PDT 2009


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


Adam Treat <treat at kde.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #32796|review?                     |review-
               Flag|                            |




--- Comment #10 from Adam Treat <treat at kde.org>  2009-07-15 12:07:45 PDT ---
(From update of attachment 32796)
I like the approach of the patch and think this will work, but a few problems:

> -PluginDatabase* PluginDatabase::installedPlugins()
> +PluginDatabase* PluginDatabase::installedPlugins(bool populate)

This reason for this populate arg should be documented in code as it is not
obvious.

> +void PluginDatabase::clear()
> +{
> +    m_plugins.clear();
> +    m_pluginsByPath.clear();
> +    m_registeredMIMETypes.clear();
> +}

Sure, but no deletion needed?

> +        void setPluginDirectories(const Vector<String>& directories)
> +        {
> +            clear();
> +            m_pluginDirectories = directories;
> +        }

Looks suspect as you're changing the behavior of this method.  Where is this
method currently called from?  Any obvious side effects?

> +        bool m_pluginSetChanged;

Adding a member variable that is never used or initialized?

> +void QWEBKIT_EXPORT qt_drt_overwritePluginDirectories()
> +{
> +    PluginDatabase *db = PluginDatabase::installedPlugins(/* populate */ false);

The decorator is in wrong place according to webkit coding style guidelines.

With these things fixed I think it'll be in good shape.

Cheers,
Adam

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



More information about the webkit-unassigned mailing list