Use KPluginLoader to load our decoration plugins

This simplifies the plugin loading. Decorations just have to use
K_PLUGIN_FACTORY to specify how the KDecorationFactory needs to be
created. The KWIN_DECORATION macro is adjusted to generate the
boiler plate code, but it now needs to specify the name for the
pluginfactory and the KDecorationFactory.

This also transits the decoration abi version check to use
K_EXPORT_PLUGIN_VERSION which also simplifies the loading.

As a result the complete canLoad handling in DecorationPlugins is
removed.

REVIEW: 115930

clients/aurorae/src/aurorae.cpp

@@ -37,6 +37,10 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #include <KPluginInfo>
 #include <KServiceTypeTrader>
 
+K_PLUGIN_FACTORY(AuroraePluginFactory,
+                 registerPlugin<Aurorae::AuroraeFactory>(QString(), &Aurorae::AuroraeFactory::createInstance);)
+K_EXPORT_PLUGIN_VERSION(KWIN_DECORATION_API_VERSION)
+
 namespace Aurorae
 {
 
@@ -158,6 +162,11 @@ AuroraeFactory *AuroraeFactory::instance()
     return s_instance;
 }
 
+QObject *AuroraeFactory::createInstance(QWidget*, QObject*, const QList< QVariant >&)
+{
+    return instance();
+}
+
 void AuroraeFactory::updateConfiguration()
 {
     const KConfig conf(QStringLiteral("auroraerc"));
@@ -562,15 +571,4 @@ void AuroraeClient::render(QPaintDevice *device, const QRegion &sourceRegion)
 
 } // namespace Aurorae
 
-extern "C"
-{
-    KDECORATIONS_EXPORT KDecorationFactory *create_factory() {
-        return Aurorae::AuroraeFactory::instance();
-    }
-    KDECORATIONS_EXPORT int decoration_version() {
-        return KWIN_DECORATION_API_VERSION;
-    }
-}
-
-
 #include "aurorae.moc"

clients/aurorae/src/aurorae.h

@@ -44,6 +44,7 @@ public:
     ~AuroraeFactory();
 
     static AuroraeFactory* instance();
+    static QObject *createInstance(QWidget *, QObject *, const QList<QVariant> &);
     KDecoration *createDecoration(KDecorationBridge*);
     bool supports(Ability ability) const;
     virtual QList< BorderSize > borderSizes() const;

clients/oxygen/CMakeLists.txt

@@ -24,7 +24,7 @@ kconfig_add_kcfg_files(kwin_oxygen_SRCS oxygenconfiguration.kcfgc )
 
 add_library(kwin3_oxygen MODULE ${kwin_oxygen_SRCS})
 target_link_libraries(kwin3_oxygen Qt5::Widgets )
-target_link_libraries(kwin3_oxygen KF5::I18n KF5::WindowSystem KF5::Style)
+target_link_libraries(kwin3_oxygen KF5::I18n KF5::WindowSystem KF5::Style KF5::Service)
 
 target_link_libraries(kwin3_oxygen kdecorations)
 

clients/oxygen/oxygenfactory.cpp

@@ -25,7 +25,6 @@
 //////////////////////////////////////////////////////////////////////////////
 
 #include "oxygenfactory.h"
-#include "oxygenfactory.moc"
 #include "oxygenclient.h"
 #include "oxygenexceptionlist.h"
 
@@ -34,7 +33,7 @@
 #include <KWindowInfo>
 #include <kdeversion.h>
 
-KWIN_DECORATION(Oxygen::Factory)
+KWIN_DECORATION(OxygenPluginFactory, Oxygen::Factory)
 
 namespace Oxygen
 {
@@ -202,3 +201,5 @@ namespace Oxygen
     }
 
 }
+
+#include "oxygenfactory.moc"

kcmkwin/kwindecoration/decorationmodel.cpp

@@ -104,8 +104,6 @@ void DecorationModel::findDecorations()
                             findAuroraeThemes();
                             continue;
                         }
-                        if (!m_plugins->canLoad(libName))
-                            continue;
                         DecorationModelData data;
                         data.name = desktopFile.readName();
                         data.libraryName = libName;

libkdecorations/kdecoration.h

@@ -33,19 +33,36 @@ DEALINGS IN THE SOFTWARE.
 #include <QtGui/QIcon>
 #include <netwm_def.h>
 #include <QtGui/QMouseEvent>
+#include <KPluginFactory>
 
 #define KWIN_DECORATION_API_VERSION 1
 
 /**
- * Defines the class to be used for decoration factory.
- * The class must be namespace complete.
- * E.g.  KWIN_EFFECT( Oxygen::Factory )
+ * Defines the KDecorationFactory implementation provided by the
+ * decoration plugin and  defines a KPluginFactory sub class.
+ * The KPluginFactory sub class returns a new instance of the specified KDecorationFactory.
+ *
+ * @param pluginfactoryname The name to be used for the KPluginFactory, passed to K_PLUGIN_FACTORY
+ * @param classname The class name of the KDecorationFactory subclass
+ *
+ * Example:
+ * @code
+ * KWIN_DECORATION(MyDecoPluginFactory, MyDeco::Factory)
+ * @endcode
+ *
+ * In case the decoration plugin wants to have more control over the created factory (e.g. a singleton)
+ * it is recommended to not use this convenience macro, but specify an own K_PLUGIN_FACTORY. In that
+ * case it is also required to add
+ * @code
+ * K_EXPORT_PLUGIN_VERSION(KWIN_DECORATION_API_VERSION)
+ * @endcode
+ *
+ * to add the correct plugin version. This is also handled by this macro in the default case.
  **/
-#define KWIN_DECORATION( classname ) \
-    extern "C" { \
-        KDECORATIONS_EXPORT KDecorationFactory* create_factory() { return new classname(); } \
-        KDECORATIONS_EXPORT int decoration_version() { return KWIN_DECORATION_API_VERSION; } \
-    }
+#define KWIN_DECORATION( pluginfactoryname, classname ) \
+    QObject *createDecorationFactory(QWidget *, QObject *, const QList<QVariant> &) { return new classname(); } \
+    K_PLUGIN_FACTORY(pluginfactoryname, registerPlugin<classname>(QString(), &createDecorationFactory);) \
+    K_EXPORT_PLUGIN_VERSION(KWIN_DECORATION_API_VERSION)
 
 #define KWIN_DECORATION_BRIDGE_API_VERSION 1
 extern "C" {

libkdecorations/kdecoration_plugins_p.cpp

@@ -29,6 +29,8 @@ DEALINGS IN THE SOFTWARE.
 #include <kconfig.h>
 #include <klocale.h>
 #include <klibrary.h>
+#include <KPluginFactory>
+#include <KPluginLoader>
 #include <kconfiggroup.h>
 #include <KDE/KLocalizedString>
 #include <assert.h>
@@ -40,10 +42,7 @@ DEALINGS IN THE SOFTWARE.
 #include "kdecorationfactory.h"
 
 KDecorationPlugins::KDecorationPlugins(const KSharedConfigPtr &cfg)
-    :   create_ptr(nullptr),
-        library(nullptr),
-        fact(nullptr),
-        old_library(nullptr),
+    :   fact(nullptr),
         old_fact(nullptr),
         pluginStr(QStringLiteral("kwin3_undefined ")),
         config(cfg)
@@ -52,16 +51,8 @@ KDecorationPlugins::KDecorationPlugins(const KSharedConfigPtr &cfg)
 
 KDecorationPlugins::~KDecorationPlugins()
 {
-    if (library) {
-        assert(fact != nullptr);
-        delete fact;
-        library->unload();
-    }
-    if (old_library) {
-        assert(old_fact != nullptr);
-        delete old_fact;
-        old_library->unload();
-    }
+    delete fact;
+    delete old_fact;
 }
 
 QString KDecorationPlugins::currentPlugin()
@@ -95,90 +86,6 @@ KDecoration* KDecorationPlugins::createDecoration(KDecorationBridge* bridge)
     return nullptr;
 }
 
-// tests whether the plugin can be loaded
-bool KDecorationPlugins::canLoad(QString nameStr, KLibrary **loadedLib)
-{
-    if (nameStr.isEmpty())
-        return false; // we can't load that
-
-    // Check if this library is not already loaded.
-    if (pluginStr == nameStr) {
-        if (loadedLib) {
-            *loadedLib = library;
-        }
-        return true;
-    }
-
-    KConfigGroup group(config, QStringLiteral("Style"));
-    if (group.readEntry<bool>("NoPlugin", false)) {
-        error(i18n("Loading of window decoration plugin library disabled in configuration."));
-        return false;
-    }
-
-    KLibrary libToFind(nameStr);
-    QString path = libToFind.fileName();
-    qDebug() << "kwin : path " << path << " for " << nameStr;
-
-    if (path.isEmpty()) {
-        return false;
-    }
-
-    // Try loading the requested plugin
-    KLibrary *lib = new KLibrary(path);
-
-    if (!lib)
-        return false;
-
-    // TODO this would be a nice shortcut, but for "some" reason QtCurve with wrong ABI slips through
-    // TODO figure where it's loaded w/o being unloaded and check whether that can be fixed.
-#if 0
-    if (lib->isLoaded()) {
-        if (loadedLib) {
-            *loadedLib = lib;
-        }
-        return true;
-    }
-#endif
-    // so we check whether this lib was loaded before and don't unload it in case
-    bool wasLoaded = lib->isLoaded();
-
-    KDecorationFactory*(*cptr)() = nullptr;
-    int (*vptr)()  = nullptr;
-    int deco_version = 0;
-    KLibrary::void_function_ptr version_func = lib->resolveFunction("decoration_version");
-    if (version_func) {
-        vptr = (int(*)())version_func;
-        deco_version = vptr();
-    }
-    if (deco_version != KWIN_DECORATION_API_VERSION) {
-        if (version_func)
-            qWarning() << i18n("The library %1 has wrong API version %2", path, deco_version);
-        lib->unload();
-        delete lib;
-        return false;
-    }
-
-    KLibrary::void_function_ptr create_func = lib->resolveFunction("create_factory");
-    if (create_func)
-        cptr = (KDecorationFactory * (*)())create_func;
-
-    if (!cptr) {
-        qDebug() << i18n("The library %1 is not a KWin plugin.", path);
-        lib->unload();
-        delete lib;
-        return false;
-    }
-
-    if (loadedLib) {
-        *loadedLib = lib;
-    } else {
-        if (!wasLoaded)
-            lib->unload();
-        delete lib;
-    }
-    return true;
-}
-
 // returns true if plugin was loaded successfully
 bool KDecorationPlugins::loadPlugin(QString nameStr)
 {
@@ -191,30 +98,31 @@ bool KDecorationPlugins::loadPlugin(QString nameStr)
     if (pluginStr == nameStr)
         return true;
 
-    KLibrary *oldLibrary = library;
-    KDecorationFactory* oldFactory = fact;
-
-    if (!canLoad(nameStr, &library)) {
-        // If that fails, fall back to the default plugin
-        nameStr = defaultPlugin;
-        if (!canLoad(nameStr, &library)) {
-            // big time trouble!
-            // -> exit kwin with an error message
-            error(i18n("The default decoration plugin is corrupt and could not be loaded."));
-            return false;
-        }
+    if (group.readEntry<bool>("NoPlugin", false)) {
+        error(i18n("Loading of window decoration plugin library disabled in configuration."));
+        return false;
     }
 
-    // guarded by "canLoad"
-    KLibrary::void_function_ptr create_func = library->resolveFunction("create_factory");
-    if (create_func)
-        create_ptr = (KDecorationFactory * (*)())create_func;
-    if (!create_ptr) {  // this means someone probably attempted to load "some" kwin plugin/lib as deco
-                        // and thus cheated on the "isLoaded" shortcut -> try the default and yell a warning
-        if (nameStr != defaultPlugin) {
-            qWarning() << i18n("The library %1 was attempted to be loaded as a decoration plugin but it is NOT", nameStr);
-            return loadPlugin(defaultPlugin);
+    auto createFactory = [](const QString &pluginName) -> KDecorationFactory* {
+        qDebug() << "Trying to load decoration plugin" << pluginName;
+        KPluginLoader loader(pluginName);
+        if (loader.pluginVersion() != KWIN_DECORATION_API_VERSION) {
+            qWarning() << i18n("The library %1 has wrong API version %2", loader.pluginName(), loader.pluginVersion());
+            return nullptr;
+        }
+        KPluginFactory *factory = loader.factory();
+        if (!factory) {
+            qWarning() << "Error loading decoration library: " << loader.errorString();
+            return nullptr;
         } else {
+            return factory->create<KDecorationFactory>();
+        }
+    };
+    KDecorationFactory *factory = createFactory(nameStr);
+    if (!factory) {
+        // If that fails, fall back to the default plugin
+        factory = createFactory(defaultPlugin);
+        if (!factory) {
             // big time trouble!
             // -> exit kwin with an error message
             error(i18n("The default decoration plugin is corrupt and could not be loaded."));
@@ -222,8 +130,7 @@ bool KDecorationPlugins::loadPlugin(QString nameStr)
         }
     }
 
-    fact = create_ptr();
-    fact->checkRequirements(this);   // let it check what is supported
+    factory->checkRequirements(this);   // let it check what is supported
 
     pluginStr = nameStr;
 
@@ -241,8 +148,8 @@ bool KDecorationPlugins::loadPlugin(QString nameStr)
     KGlobal::locale()->insertCatalog("kwin_art_clients");
 #endif
 
-    old_library = oldLibrary; // save for delayed destroying
-    old_fact = oldFactory;
+    old_fact = fact;
+    fact = factory;
 
     return true;
 }
@@ -250,13 +157,8 @@ bool KDecorationPlugins::loadPlugin(QString nameStr)
 void KDecorationPlugins::destroyPreviousPlugin()
 {
     // Destroy the old plugin
-    if (old_library) {
-        delete old_fact;
-        old_fact = nullptr;
-        old_library->unload();
-        delete old_library;
-        old_library = nullptr;
-    }
+    delete old_fact;
+    old_fact = nullptr;
 }
 
 void KDecorationPlugins::error(const QString&)

libkdecorations/kdecoration_plugins_p.h

@@ -38,7 +38,6 @@ DEALINGS IN THE SOFTWARE.
 #include "kdecoration.h"
 #include <kdecorations_export.h>
 
-class KLibrary;
 class KDecoration;
 class KDecorationBridge;
 class KDecorationFactory;
@@ -49,10 +48,6 @@ class KDECORATIONS_EXPORT KDecorationPlugins
 public:
     explicit KDecorationPlugins(const KSharedConfigPtr &cfg);
     virtual ~KDecorationPlugins();
-    /** Whether the plugin with @p name can be loaded
-     * if @p loadedLib is passed, the library is NOT unloaded and freed
-     * what is now your resposibility (intended for and used by below loadPlugin mainly) */
-    bool canLoad(QString name, KLibrary ** loadedLib = 0);
     bool loadPlugin(QString name);
     void destroyPreviousPlugin();
     KDecorationFactory* factory();
@@ -64,10 +59,7 @@ protected:
     virtual void error(const QString& error_msg);
     QString defaultPlugin; // FRAME normalne protected?
 private:
-    KDecorationFactory*(*create_ptr)();
-    KLibrary *library;
     KDecorationFactory* fact;
-    KLibrary *old_library;
     KDecorationFactory* old_fact;
     QString pluginStr;
     KSharedConfigPtr config;