From 70d2fb2378d636ef6d052da08417b27c99182fb0 Mon Sep 17 00:00:00 2001 From: David Edmundson Date: Fri, 10 Feb 2017 10:52:48 +0100 Subject: [PATCH 1/2] Avoid a crash on Kwin decoration KCM teardown Summary: As described in https://bugreports.qt.io/browse/QTBUG-57714 exposing a QWidget as a context item to QtQuick can crash. Especially as the engine's context property gets deleted whilst deleting the parent item. This patch reworks the code so that the models are exposed to QML directly rather than going through a QWidget. CCBUG: 373628 Test Plan: Pressing back whilst in the decoration KCM used to crash every time, now it doesn't. I still have the buttons. Reviewers: #plasma, graesslin Reviewed By: #plasma, graesslin Subscribers: graesslin, cfeck, plasma-devel Tags: #plasma Differential Revision: https://phabricator.kde.org/D4533 --- kcmkwin/kwindecoration/kcm.cpp | 20 ++++---------------- kcmkwin/kwindecoration/kcm.h | 7 ------- kcmkwin/kwindecoration/qml/Buttons.qml | 6 +++--- 3 files changed, 7 insertions(+), 26 deletions(-) diff --git a/kcmkwin/kwindecoration/kcm.cpp b/kcmkwin/kwindecoration/kcm.cpp index 035bc8b2aa..231b3e6a80 100644 --- a/kcmkwin/kwindecoration/kcm.cpp +++ b/kcmkwin/kwindecoration/kcm.cpp @@ -100,7 +100,10 @@ ConfigurationModule::ConfigurationModule(QWidget *parent, const QVariantList &ar m_quickView->rootContext()->setContextProperty(QStringLiteral("decorationsModel"), m_proxyModel); updateColors(); m_quickView->rootContext()->setContextProperty("_borderSizesIndex", 3); // 3 is normal - m_quickView->rootContext()->setContextProperty("configurationModule", this); + m_quickView->rootContext()->setContextProperty("leftButtons", m_leftButtons); + m_quickView->rootContext()->setContextProperty("rightButtons", m_rightButtons); + m_quickView->rootContext()->setContextProperty("availableButtons", m_availableButtons); + m_quickView->rootContext()->setContextProperty("titleFont", QFontDatabase::systemFont(QFontDatabase::TitleFont)); m_quickView->setResizeMode(QQuickView::SizeRootObjectToView); m_quickView->setSource(QUrl::fromLocalFile(QStandardPaths::locate(QStandardPaths::GenericDataLocation, QStringLiteral("kwin/kcm_kwindecoration/main.qml")))); @@ -406,21 +409,6 @@ void ConfigurationModule::showKNS(const QString &config) delete downloadDialog; } -QAbstractItemModel *ConfigurationModule::leftButtons() const -{ - return m_leftButtons; -} - -QAbstractItemModel *ConfigurationModule::rightButtons() const -{ - return m_rightButtons; -} - -QAbstractItemModel *ConfigurationModule::availableButtons() const -{ - return m_availableButtons; -} - bool ConfigurationModule::eventFilter(QObject *watched, QEvent *e) { if (watched != m_ui) { diff --git a/kcmkwin/kwindecoration/kcm.h b/kcmkwin/kwindecoration/kcm.h index 1d7ee73784..99fa431143 100644 --- a/kcmkwin/kwindecoration/kcm.h +++ b/kcmkwin/kwindecoration/kcm.h @@ -47,17 +47,10 @@ public: class ConfigurationModule : public KCModule { Q_OBJECT - Q_PROPERTY(QAbstractItemModel* leftButtons READ leftButtons CONSTANT) - Q_PROPERTY(QAbstractItemModel* rightButtons READ rightButtons CONSTANT) - Q_PROPERTY(QAbstractItemModel* availableButtons READ availableButtons CONSTANT) public: explicit ConfigurationModule(QWidget *parent = nullptr, const QVariantList &args = QVariantList()); virtual ~ConfigurationModule(); - QAbstractItemModel *leftButtons() const; - QAbstractItemModel *rightButtons() const; - QAbstractItemModel *availableButtons() const; - bool eventFilter(QObject *watched, QEvent *e) override; public Q_SLOTS: diff --git a/kcmkwin/kwindecoration/qml/Buttons.qml b/kcmkwin/kwindecoration/qml/Buttons.qml index 2e6487306d..64ab0f9be3 100644 --- a/kcmkwin/kwindecoration/qml/Buttons.qml +++ b/kcmkwin/kwindecoration/qml/Buttons.qml @@ -63,7 +63,7 @@ Item { id: leftButtonsView anchors.left: parent.left; height: buttonPreviewRow.height - model: configurationModule.leftButtons + model: leftButtons key: "decoButtonLeft" } Item { @@ -81,7 +81,7 @@ Item { id: rightButtonsView anchors.right: parent.right; height: buttonPreviewRow.height - model: configurationModule.rightButtons + model: rightButtons key: "decoButtonRight" } DropArea { @@ -144,7 +144,7 @@ Item { GridView { id: availableGrid Layout.fillWidth: true - model: configurationModule.availableButtons + model: availableButtons interactive: false cellWidth: iconLabel.implicitWidth cellHeight: units.iconSizes.small + iCannotBelieveIDoThis.implicitHeight + 4*units.smallSpacing From cb481b492271860586c69298395e2ba118239406 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= Date: Fri, 17 Feb 2017 07:09:29 +0100 Subject: [PATCH 2/2] Find minimum required Breeze version instead of current version Summary: So far KWin put a version requirement on Breeze which is the current version number of KWin. This is technically correct, but distributions seem to have problems with it. E.g. I noticed twice in Debian testing that kwin defaults to Plastik which happens when Breeze is not found. I reported it, it got fixed, now it's again broken. Thus I think that distributions have problems with the requirement of building Breeze prior to KWin. As KWin does not depend on the minor version, let's depend only on the actual required version, which is 5.8 in the 5.8 branch, 5.9 in the 5.9 branch and master. Reviewers: #kwin, #plasma Subscribers: plasma-devel, kwin Tags: #kwin Differential Revision: https://phabricator.kde.org/D4644 --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ab636cd35b..a683812788 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -120,7 +120,7 @@ set_package_properties(KScreenLocker PROPERTIES TYPE REQUIRED PURPOSE "For screenlocker integration in kwin_wayland") -find_package(Breeze ${PROJECT_VERSION} CONFIG) +find_package(Breeze 5.8.0 CONFIG) set_package_properties(Breeze PROPERTIES TYPE OPTIONAL PURPOSE "For setting the default window decoration plugin")