From b3d3c45149f391b90dd26e620df71cf025002cdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= Date: Fri, 21 Feb 2014 10:48:24 +0100 Subject: [PATCH] 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 | 20 ++- clients/aurorae/src/aurorae.h | 1 + clients/oxygen/CMakeLists.txt | 2 +- clients/oxygen/oxygenfactory.cpp | 5 +- kcmkwin/kwindecoration/decorationmodel.cpp | 2 - libkdecorations/kdecoration.h | 33 ++++- libkdecorations/kdecoration_plugins_p.cpp | 162 ++++----------------- libkdecorations/kdecoration_plugins_p.h | 8 - 8 files changed, 71 insertions(+), 162 deletions(-) diff --git a/clients/aurorae/src/aurorae.cpp b/clients/aurorae/src/aurorae.cpp index 1c31b41889..9601f38d04 100644 --- a/clients/aurorae/src/aurorae.cpp +++ b/clients/aurorae/src/aurorae.cpp @@ -37,6 +37,10 @@ along with this program. If not, see . #include #include +K_PLUGIN_FACTORY(AuroraePluginFactory, + registerPlugin(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" diff --git a/clients/aurorae/src/aurorae.h b/clients/aurorae/src/aurorae.h index 1d105bd6b7..a85467a062 100644 --- a/clients/aurorae/src/aurorae.h +++ b/clients/aurorae/src/aurorae.h @@ -44,6 +44,7 @@ public: ~AuroraeFactory(); static AuroraeFactory* instance(); + static QObject *createInstance(QWidget *, QObject *, const QList &); KDecoration *createDecoration(KDecorationBridge*); bool supports(Ability ability) const; virtual QList< BorderSize > borderSizes() const; diff --git a/clients/oxygen/CMakeLists.txt b/clients/oxygen/CMakeLists.txt index bb5484aef9..9e6dcdfe1a 100644 --- a/clients/oxygen/CMakeLists.txt +++ b/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) diff --git a/clients/oxygen/oxygenfactory.cpp b/clients/oxygen/oxygenfactory.cpp index 2f44a6a16d..19559dbeda 100644 --- a/clients/oxygen/oxygenfactory.cpp +++ b/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 #include -KWIN_DECORATION(Oxygen::Factory) +KWIN_DECORATION(OxygenPluginFactory, Oxygen::Factory) namespace Oxygen { @@ -202,3 +201,5 @@ namespace Oxygen } } + +#include "oxygenfactory.moc" diff --git a/kcmkwin/kwindecoration/decorationmodel.cpp b/kcmkwin/kwindecoration/decorationmodel.cpp index 351e47013b..9e8b1365a6 100644 --- a/kcmkwin/kwindecoration/decorationmodel.cpp +++ b/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; diff --git a/libkdecorations/kdecoration.h b/libkdecorations/kdecoration.h index 99ece35033..d7118cd762 100644 --- a/libkdecorations/kdecoration.h +++ b/libkdecorations/kdecoration.h @@ -33,19 +33,36 @@ DEALINGS IN THE SOFTWARE. #include #include #include +#include #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 &) { return new classname(); } \ + K_PLUGIN_FACTORY(pluginfactoryname, registerPlugin(QString(), &createDecorationFactory);) \ + K_EXPORT_PLUGIN_VERSION(KWIN_DECORATION_API_VERSION) #define KWIN_DECORATION_BRIDGE_API_VERSION 1 extern "C" { diff --git a/libkdecorations/kdecoration_plugins_p.cpp b/libkdecorations/kdecoration_plugins_p.cpp index 325dae9a5b..03b8595751 100644 --- a/libkdecorations/kdecoration_plugins_p.cpp +++ b/libkdecorations/kdecoration_plugins_p.cpp @@ -29,6 +29,8 @@ DEALINGS IN THE SOFTWARE. #include #include #include +#include +#include #include #include #include @@ -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("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("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 *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&) diff --git a/libkdecorations/kdecoration_plugins_p.h b/libkdecorations/kdecoration_plugins_p.h index bf36fc4860..8d737ad1fd 100644 --- a/libkdecorations/kdecoration_plugins_p.h +++ b/libkdecorations/kdecoration_plugins_p.h @@ -38,7 +38,6 @@ DEALINGS IN THE SOFTWARE. #include "kdecoration.h" #include -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;