[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-commits
Subject: [kdevelop/5.3] kdevplatform: Sublime: on View destruction delete widget instantly, instead deleteLat
From: Friedrich W. H. Kossebau <null () kde ! org>
Date: 2018-09-01 10:34:30
Message-ID: E1fw3Ec-0005Qw-RC () code ! kde ! org
[Download RAW message or body]
Git commit 8127e3fe628c3da712d82d525b56f4c7722e5763 by Friedrich W. H. Kossebau.
Committed on 01/09/2018 at 10:34.
Pushed by kossebau into branch '5.3'.
Sublime: on View destruction delete widget instantly, instead deleteLater
Summary:
Having a defined destruction order allows to make more assumptions
in other code referring to the widgets and avoids extra work to track
the addditional lifetime of the widget objects (like e.g. needed with
the KTextEditor::View widget objects, which require the
KTextEditor::MainWindow to exists as long as there is a View object).
With the Sublime::View and the QWidget objects both bound to the same
thread which is the UI objects thread by current design, there is also
no need to defer the deletion into the QWidget object thread.
Test Plan: Have been running this for more than a month, no issues seen.
Reviewers: #kdevelop, kfunk
Reviewed By: #kdevelop, kfunk
Subscribers: kfunk, kdevelop-devel
Tags: #kdevelop
Differential Revision: https://phabricator.kde.org/D15198
M +13 -41 kdevplatform/shell/ktexteditorpluginintegration.cpp
M +0 -10 kdevplatform/shell/ktexteditorpluginintegration.h
M +0 -3 kdevplatform/shell/mainwindow.cpp
M +0 -12 kdevplatform/shell/tests/test_ktexteditorpluginintegration.cpp
M +6 -1 kdevplatform/sublime/view.cpp
https://commits.kde.org/kdevelop/8127e3fe628c3da712d82d525b56f4c7722e5763
diff --git a/kdevplatform/shell/ktexteditorpluginintegration.cpp \
b/kdevplatform/shell/ktexteditorpluginintegration.cpp index d24024d837..254d0d8cd2 \
100644
--- a/kdevplatform/shell/ktexteditorpluginintegration.cpp
+++ b/kdevplatform/shell/ktexteditorpluginintegration.cpp
@@ -231,7 +231,8 @@ KTextEditor::Document *Application::openUrl(const QUrl &url, \
const QString &enco }
MainWindow::MainWindow(KDevelop::MainWindow *mainWindow)
- : m_mainWindow(mainWindow)
+ : QObject(mainWindow)
+ , m_mainWindow(mainWindow)
, m_interface(new KTextEditor::MainWindow(this))
{
connect(mainWindow, &Sublime::MainWindow::viewAdded, this, [this] (Sublime::View \
*view) { @@ -265,9 +266,6 @@ QWidget *MainWindow::createToolView(KTextEditor::Plugin* \
plugin, const QString &
KXMLGUIFactory *MainWindow::guiFactory() const
{
- if (!m_mainWindow) {
- return nullptr;
- }
return m_mainWindow->guiFactory();
}
@@ -279,12 +277,10 @@ QWidget *MainWindow::window() const
QList<KTextEditor::View *> MainWindow::views() const
{
QList<KTextEditor::View *> views;
- if (m_mainWindow) {
- foreach (auto area, m_mainWindow->areas()) {
- foreach (auto view, area->views()) {
- if (auto kteView = toKteView(view)) {
- views << kteView;
- }
+ foreach (auto area, m_mainWindow->areas()) {
+ foreach (auto view, area->views()) {
+ if (auto kteView = toKteView(view)) {
+ views << kteView;
}
}
}
@@ -293,18 +289,11 @@ QList<KTextEditor::View *> MainWindow::views() const
KTextEditor::View *MainWindow::activeView() const
{
- if (!m_mainWindow) {
- return nullptr;
- }
return toKteView(m_mainWindow->activeView());
}
KTextEditor::View *MainWindow::activateView(KTextEditor::Document *doc)
{
- if (!m_mainWindow) {
- return nullptr;
- }
-
foreach (auto area, m_mainWindow->areas()) {
foreach (auto view, area->views()) {
if (auto kteView = toKteView(view)) {
@@ -327,9 +316,7 @@ QObject *MainWindow::pluginView(const QString &id) const
QWidget *MainWindow::createViewBar(KTextEditor::View *view)
{
Q_UNUSED(view);
- if (!m_mainWindow) {
- return nullptr;
- }
+
// we reuse the central view bar for every view
return m_mainWindow->viewBarContainer();
}
@@ -337,9 +324,7 @@ QWidget *MainWindow::createViewBar(KTextEditor::View *view)
void MainWindow::deleteViewBar(KTextEditor::View *view)
{
auto viewBar = m_viewBars.take(view);
- if (m_mainWindow) {
- m_mainWindow->viewBarContainer()->removeViewBar(viewBar);
- }
+ m_mainWindow->viewBarContainer()->removeViewBar(viewBar);
delete viewBar;
}
@@ -347,18 +332,15 @@ void MainWindow::showViewBar(KTextEditor::View *view)
{
auto viewBar = m_viewBars.value(view);
Q_ASSERT(viewBar);
- if (m_mainWindow) {
- m_mainWindow->viewBarContainer()->showViewBar(viewBar);
- }
+
+ m_mainWindow->viewBarContainer()->showViewBar(viewBar);
}
void MainWindow::hideViewBar(KTextEditor::View *view)
{
auto viewBar = m_viewBars.value(view);
Q_ASSERT(viewBar);
- if (m_mainWindow) {
- m_mainWindow->viewBarContainer()->hideViewBar(viewBar);
- }
+ m_mainWindow->viewBarContainer()->hideViewBar(viewBar);
}
void MainWindow::addWidgetToViewBar(KTextEditor::View *view, QWidget *widget)
@@ -366,9 +348,7 @@ void MainWindow::addWidgetToViewBar(KTextEditor::View *view, \
QWidget *widget) Q_ASSERT(widget);
m_viewBars[view] = widget;
- if (m_mainWindow) {
- m_mainWindow->viewBarContainer()->addViewBar(widget);
- }
+ m_mainWindow->viewBarContainer()->addViewBar(widget);
}
KTextEditor::MainWindow *MainWindow::interface() const
@@ -389,12 +369,6 @@ void MainWindow::removePluginView(const QString &id)
emit m_interface->pluginViewDeleted(id, view);
}
-void MainWindow::startDestroy()
-{
- m_mainWindow = nullptr;
- deleteLater();
-}
-
Plugin::Plugin(KTextEditor::Plugin *plugin, QObject *parent)
: IPlugin({}, parent)
, m_plugin(plugin)
@@ -449,9 +423,7 @@ void initialize()
void MainWindow::splitView(Qt::Orientation orientation)
{
- if (m_mainWindow) {
- m_mainWindow->split(orientation);
- }
+ m_mainWindow->split(orientation);
}
}
diff --git a/kdevplatform/shell/ktexteditorpluginintegration.h \
b/kdevplatform/shell/ktexteditorpluginintegration.h index 545237c69d..2b1af0dfe3 \
100644
--- a/kdevplatform/shell/ktexteditorpluginintegration.h
+++ b/kdevplatform/shell/ktexteditorpluginintegration.h
@@ -95,16 +95,6 @@ public:
void addPluginView(const QString &id, QObject *pluginView);
void removePluginView(const QString &id);
- /**
- * To be called when the actual mainwindow is destroyed.
- * Some KTextEditor::View objects relying on this wrapper object at that point \
in time
- * only have got called deleteLater() on them, so will still get their \
destruction
- * done only in the next handling of QEvent::DeferredDelete.
- * To outlive them, this method triggers deleteLater for this wrapper object as \
well,
- * but already prepares for the actual mainwindow no longer being available.
- */
- void startDestroy();
-
private:
KDevelop::MainWindow *m_mainWindow;
KTextEditor::MainWindow *m_interface;
diff --git a/kdevplatform/shell/mainwindow.cpp b/kdevplatform/shell/mainwindow.cpp
index cf84f3d80f..1cce8259b9 100644
--- a/kdevplatform/shell/mainwindow.cpp
+++ b/kdevplatform/shell/mainwindow.cpp
@@ -159,9 +159,6 @@ MainWindow::~ MainWindow()
Core::self()->shutdown();
}
- // The window wrapper has to stay alive until the last KTextEditor::Views are \
gone
- // but needs to know this mainwindow is next an ex-mainwindow.
- d->kateWrapper()->startDestroy();
delete d;
}
diff --git a/kdevplatform/shell/tests/test_ktexteditorpluginintegration.cpp \
b/kdevplatform/shell/tests/test_ktexteditorpluginintegration.cpp index \
0903b28e94..93f2c9aa53 100644
--- a/kdevplatform/shell/tests/test_ktexteditorpluginintegration.cpp
+++ b/kdevplatform/shell/tests/test_ktexteditorpluginintegration.cpp
@@ -95,21 +95,9 @@ void TestKTextEditorPluginIntegration::cleanupTestCase()
TestCore::shutdown();
QVERIFY(!plugin);
- // Test uncovers issue in shutdown behaviour when not triggered by last closed \
mainwindow, but directly:
- // Core::shutdown() deletes itself via deleteLater, for which \
TestCore::shutdown() adds a QTest::qWait(1)
- // so Core instance should be gone after the call returns.
- // Core in its destructor deletes the Sublime::Controller instance.
- // That one in the destructor deletes any still existing mainwindows, of which \
we have here in the test one.
- // The KTE::MainWindow wrapper trying to outlive the KTE::View instances as \
needed now is only deleted with
- // a deleteLater() from the mainwindow. Thus still living here.
- QEXPECT_FAIL("", "Chain of deleteLater too long ATM", Continue);
QVERIFY(!window);
QVERIFY(!application);
- // workaround for now, remove again if issue of too long deleteLater chain above \
is fixed
- QTest::qWait(1);
- QVERIFY(!window);
-
// editor lives by design until QCoreApplication terminates, then autodeletes
}
diff --git a/kdevplatform/sublime/view.cpp b/kdevplatform/sublime/view.cpp
index bb8ad72411..9f3486bfe3 100644
--- a/kdevplatform/sublime/view.cpp
+++ b/kdevplatform/sublime/view.cpp
@@ -59,7 +59,7 @@ View::~View()
if (d->widget && d->ws == View::TakeOwnership ) {
d->widget->hide();
d->widget->setParent(nullptr);
- d->widget->deleteLater();
+ delete d->widget;
}
}
@@ -73,6 +73,11 @@ QWidget *View::widget(QWidget *parent)
if (!d->widget)
{
d->widget = createWidget(parent);
+ // if we own this widget, we will also delete it and ideally would \
disconnect + // the following connect before doing that. For that though we \
would need to store + // a reference to the connection.
+ // As the d object still exists in the destructor when we delete the widget
+ // this lambda method though can be still safely executed, so we spare \
ourselves such disconnect.
connect(d->widget, &QWidget::destroyed, this, [&] { d->unsetWidget(); });
}
return d->widget;
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic