[Marble-commits] KDE/kdeedu/marble

[Dennis Nienhüser]

SVN commit 1181707 by nienhueser:

Cleanup all loaded plugin instances when the PluginManager is deleted. In contrast to the previous implementation this should deal with multiple MarbleWidget instances correctly.
Move the duplicated plugin loading code into a common template method. Rename RenderPlugin::pluginInstance to RenderPlugin::newInstance for consistency.
Delete plugin loader if the loaded plugin is not a valid Marble plugin.
Increase RenderPlugin interface version. Note that you need to have cmake recreate the .moc files now to have the plugins loaded after a rebuild.

 M  +7 -2      docs/release_notes/APIChanges-0.11.txt  
 M  +28 -40    src/lib/PluginManager.cpp  
 M  +1 -1      src/lib/RenderPlugin.h  
 M  +2 -2      src/lib/RenderPluginInterface.h  


--- trunk/KDE/kdeedu/marble/docs/release_notes/APIChanges-0.11.txt #1181706:1181707
@@ -1,8 +1,8 @@
 
 2010-08-19  Bernhard Beschow    <bbeschow at cs.tu-berlin.de>
 
-* The medhod measureTool() has been moved from MarbleMap to MarbleModel.
-  You can continue accessing the measure tool from MarbleMap using MarbleMapp::model()->measureTool().
+* The method measureTool() has been moved from MarbleMap to MarbleModel.
+  You can continue accessing the measure tool from MarbleMap using MarbleMap::model()->measureTool().
 
 * The method ViewParams *viewParams() has been removed from MarbleMap in favor of ViewportParams *viewport().
   Setting and getting the MapQuality can now be done using MarbleMap::setMapQuality() and MarbleMap::mapQuality().
@@ -21,3 +21,8 @@
 
 * Remove MarbleMap::goHome().
   Use MarbleMap::flyTo(home), where home is the home position.
+
+2010-10-01 Dennis Nienhüser <earthwings at gentoo.org>
+
+* RenderPlugin::pluginInstance() has been renamed RenderPlugin::newInstance to be consistent with other plugin types.
+  You only need to change your code if you're not using the MARBLE_PLUGIN macro in custom RenderPlugins.
--- trunk/KDE/kdeedu/marble/src/lib/PluginManager.cpp #1181706:1181707
@@ -47,18 +47,27 @@
     QMap<QPluginLoader*, NetworkPlugin *> m_networkPluginTemplates;
     QMap<QPluginLoader*, PositionProviderPlugin *> m_positionProviderPluginTemplates;
     QMap<QPluginLoader*, RunnerPlugin *> m_runnerPlugins;
+
+private:
+    void cleanup( const QList<QPluginLoader*> loaders );
 };
 
 PluginManagerPrivate::~PluginManagerPrivate()
 {
     QMap<QPluginLoader*, RunnerPlugin *>::const_iterator i = m_runnerPlugins.constBegin();
-    for ( ; i != m_runnerPlugins.constEnd(); ++i ) {
-        i.key()->unload();
-        delete i.key();
+    cleanup( m_renderPluginTemplates.keys() );
+    cleanup( m_networkPluginTemplates.keys() );
+    cleanup( m_positionProviderPluginTemplates.keys() );
+    cleanup( m_runnerPlugins.keys() );
     }
 
-    /** @todo: unload other plugin types as well */
+void PluginManagerPrivate::cleanup( const QList<QPluginLoader*> loaders )
+{
+    foreach( QPluginLoader* loader, loaders ) {
+        loader->unload();
+        delete loader;
 }
+}
 
 PluginManager::PluginManager( QObject *parent )
     : QObject(parent),
@@ -68,14 +77,6 @@
 
 PluginManager::~PluginManager()
 {
-    // The plugin instances returned by QPluginLoader are shared by all QPluginLoaders.
-    // If more than one MarbleWidget is used, deleting them here leads to dangling
-    // pointers and double deletions in the other PluginManagers.
-    // TODO: According to QPluginLoader::unload deletion happens automatically on application
-    // termination, but it should be checked that this really is the case (and remove this TODO)
-    // qDeleteAll( d->m_renderPluginTemplates );
-    // qDeleteAll( d->m_networkPluginTemplates );
-    // qDeleteAll( d->m_positionProviderPluginTemplates );
     delete d;
 }
 
@@ -91,54 +92,40 @@
         AbstractFloatItem * const floatItem = qobject_cast<AbstractFloatItem *>(*i);
         if ( floatItem ) {
             floatItemList.append( qobject_cast<AbstractFloatItem *>( floatItem->
-                                                                     pluginInstance() ));
+                                                                     newInstance() ));
         }
     }
 
     return floatItemList;
 }
 
-QList<RenderPlugin *> PluginManager::createRenderPlugins() const
+template<class T>
+QList<T*> createPlugins( PluginManagerPrivate* d, const QMap<QPluginLoader*, T*> &loaders )
 {
-    QList<RenderPlugin *> result;
-
+    QList<T*> result;
     d->loadPlugins();
-
-    QMap<QPluginLoader*, RenderPlugin *>::const_iterator i = d->m_renderPluginTemplates.constBegin();
-    QMap<QPluginLoader*, RenderPlugin *>::const_iterator const end = d->m_renderPluginTemplates.constEnd();
+    typename QMap<QPluginLoader*, T*>::const_iterator i = loaders.constBegin();
+    typename QMap<QPluginLoader*, T*>::const_iterator const end = loaders.constEnd();
     for (; i != end; ++i) {
-        result.append( (*i)->pluginInstance() );
+        result.append( (*i)->newInstance() );
     }
     return result;
 }
 
-QList<NetworkPlugin *> PluginManager::createNetworkPlugins() const
+QList<RenderPlugin *> PluginManager::createRenderPlugins() const
 {
-    QList<NetworkPlugin *> result;
+    return createPlugins( d, d->m_renderPluginTemplates );
+}
 
-    d->loadPlugins();
-
-    QMap<QPluginLoader*, NetworkPlugin *>::const_iterator pos = d->m_networkPluginTemplates.constBegin();
-    QMap<QPluginLoader*, NetworkPlugin *>::const_iterator const end = d->m_networkPluginTemplates.constEnd();
-    for (; pos != end; ++pos ) {
-        result.append( (*pos)->newInstance() );
+QList<NetworkPlugin *> PluginManager::createNetworkPlugins() const
+{
+    return createPlugins( d, d->m_networkPluginTemplates );
     }
-    return result;
-}
 
 QList<PositionProviderPlugin *> PluginManager::createPositionProviderPlugins() const
 {
-    QList<PositionProviderPlugin *> result;
-
-    d->loadPlugins();
-
-    QMap<QPluginLoader*, PositionProviderPlugin *>::const_iterator pos = d->m_positionProviderPluginTemplates.constBegin();
-    QMap<QPluginLoader*, PositionProviderPlugin *>::const_iterator const end = d->m_positionProviderPluginTemplates.constEnd();
-    for (; pos != end; ++pos ) {
-        result.append( (*pos)->newInstance() );
+    return createPlugins( d, d->m_positionProviderPluginTemplates );
     }
-    return result;
-}
 
 QList<RunnerPlugin *> PluginManager::runnerPlugins() const
 {
@@ -208,6 +195,7 @@
             if ( !isPlugin ) {
                 mDebug() << "Plugin failure:" << fileName << "is a plugin, but it does not implement the "
                         << "right interfaces or it was compiled against an old version of Marble. Ignoring it.";
+                delete loader;
             }
         } else {
             mDebug() << "Plugin failure:" << fileName << "is not a valid Marble Plugin:"
--- trunk/KDE/kdeedu/marble/src/lib/RenderPlugin.h #1181706:1181707
@@ -158,7 +158,7 @@
 };
 
 #define MARBLE_PLUGIN(T) public:\
-    virtual RenderPlugin* pluginInstance() { return new T(); };
+    virtual RenderPlugin* newInstance() { return new T(); };
 }
 
 #endif
--- trunk/KDE/kdeedu/marble/src/lib/RenderPluginInterface.h #1181706:1181707
@@ -35,7 +35,7 @@
     /**
     * @brief Returns a new object of the plugin
     */
-    virtual RenderPlugin * pluginInstance() = 0;
+    virtual RenderPlugin * newInstance() = 0;
 
     /**
      * @brief Returns the name(s) of the backend that the plugin can render
@@ -69,6 +69,6 @@
 
 }
 
-Q_DECLARE_INTERFACE( Marble::RenderPluginInterface, "org.kde.Marble.RenderPluginInterface/1.06" )
+Q_DECLARE_INTERFACE( Marble::RenderPluginInterface, "org.kde.Marble.RenderPluginInterface/1.07" )
 
 #endif