desktop: fix saving of column-widths of device and site tables

Qt's memory management scheme is completely broken and messes
with common expectations.

QObjects are organized as a tree. The children are destroyed
in the destructor of QObject. This means that they are destructed
after the destructor of the parent object has run and its
sub-object were destructed. Obviously, this makes no sense as
the child objects should be able to access their parent at
any time.

To restore the commonly expected deterministic order of
construction and destruction, one might simply do away with
Qt's silly object tree and organise things using classical
subobjects. However, that breaks with the Qt-generated UI
classes: The objects generated by these classes are *not*
destructed with the UI class. Instead, they are attached
to the widget's QObject tree. Thus these are again destructed
*after* the widget! Who comes up with such a scheme?

In our case this means that we cannot have models used for
TableViews as subobjects, because the TableView needs the
model to save the column widths in the destructor. Which,
as detailed above is called *after* the desctructor of the
widget! Thus, turn these models into heap-allocated objects
and add them to the QObject tree.

Funilly, this exposes another insanity of Qt's QObject tree:
Children are destructed in order of construction! One would
expect that if objects are constructed in the sequence
A, B, C one can expect that C can, at any time, access B and A.
Not so in Qt: The destruction order is likewise A, B, C!

Thus, take care to init the widgets before the model. Jeez.

Finally, print a warning in the column-saving code of
TableWidget, so that these kind of subtleties are caught
in the future.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>

7 files changed, 29 insertions, 18 deletions

diff --git a/desktop-widgets/tab-widgets/TabDiveComputer.cpp b/desktop-widgets/tab-widgets/TabDiveComputer.cpp
index 104bee0c6..d4b0f5cce 100644
--- a/desktop-widgets/tab-widgets/TabDiveComputer.cpp
+++ b/desktop-widgets/tab-widgets/TabDiveComputer.cpp
@@ -1,12 +1,17 @@
 // SPDX-License-Identifier: GPL-2.0
 #include "TabDiveComputer.h"
 #include "ui_TabDiveExtraInfo.h"
+#include "qt-models/divecomputermodel.h"
 
 TabDiveComputer::TabDiveComputer(QWidget *parent) : TabBase(parent)
 {
 	ui.setupUi(this);
-	sortedModel.setSourceModel(&model);
-	ui.devices->setModel(&sortedModel);
+
+	DiveComputerModel *model = new DiveComputerModel(this);
+	sortedModel = new DiveComputerSortedModel(this);
+
+	sortedModel->setSourceModel(model);
+	ui.devices->setModel(sortedModel);
 	ui.devices->view()->setSelectionBehavior(QAbstractItemView::SelectRows);
 	ui.devices->view()->setSelectionMode(QAbstractItemView::SingleSelection);
 	ui.devices->view()->setSortingEnabled(true);
@@ -29,5 +34,5 @@ void TabDiveComputer::tableClicked(const QModelIndex &index)
 		return;
 
 	if (index.column() == DiveComputerModel::REMOVE)
-		sortedModel.remove(index);
+		sortedModel->remove(index);
 }
diff --git a/desktop-widgets/tab-widgets/TabDiveComputer.h b/desktop-widgets/tab-widgets/TabDiveComputer.h
index bdbb9097c..f5f6900c3 100644
--- a/desktop-widgets/tab-widgets/TabDiveComputer.h
+++ b/desktop-widgets/tab-widgets/TabDiveComputer.h
@@ -4,7 +4,8 @@
 
 #include "TabBase.h"
 #include "ui_TabDiveComputer.h"
-#include "qt-models/divecomputermodel.h"
+
+class DiveComputerSortedModel;
 
 class TabDiveComputer : public TabBase {
 	Q_OBJECT
@@ -16,8 +17,7 @@ public slots:
 	void tableClicked(const QModelIndex &index);
 private:
 	Ui::TabDiveComputer ui;
-	DiveComputerModel model;
-	DiveComputerSortedModel sortedModel;
+	DiveComputerSortedModel *sortedModel;
 };
 
 #endif
diff --git a/desktop-widgets/tab-widgets/TabDiveSite.cpp b/desktop-widgets/tab-widgets/TabDiveSite.cpp
index 87bed8f92..60f82a0ae 100644
--- a/desktop-widgets/tab-widgets/TabDiveSite.cpp
+++ b/desktop-widgets/tab-widgets/TabDiveSite.cpp
@@ -12,8 +12,10 @@
 TabDiveSite::TabDiveSite(QWidget *parent) : TabBase(parent)
 {
 	ui.setupUi(this);
+
+	model = new DiveSiteSortedModel(this);
 	ui.diveSites->setTitle(tr("Dive sites"));
-	ui.diveSites->setModel(&model);
+	ui.diveSites->setModel(model);
 	// Default: sort by name
 	ui.diveSites->view()->sortByColumn(LocationInformationModel::NAME, Qt::AscendingOrder);
 	ui.diveSites->view()->setSortingEnabled(true);
@@ -44,7 +46,7 @@ void TabDiveSite::clear()
 
 void TabDiveSite::diveSiteClicked(const QModelIndex &index)
 {
-	struct dive_site *ds = model.getDiveSite(index);
+	struct dive_site *ds = model->getDiveSite(index);
 	if (!ds)
 		return;
 	switch (index.column()) {
@@ -80,7 +82,7 @@ void TabDiveSite::diveSiteAdded(struct dive_site *, int idx)
 	if (idx < 0)
 		return;
 	QModelIndex globalIdx = LocationInformationModel::instance()->index(idx, LocationInformationModel::NAME);
-	QModelIndex localIdx = model.mapFromSource(globalIdx);
+	QModelIndex localIdx = model->mapFromSource(globalIdx);
 	ui.diveSites->view()->setCurrentIndex(localIdx);
 	ui.diveSites->view()->edit(localIdx);
 }
@@ -91,7 +93,7 @@ void TabDiveSite::diveSiteChanged(struct dive_site *ds, int field)
 	if (idx < 0)
 		return;
 	QModelIndex globalIdx = LocationInformationModel::instance()->index(idx, field);
-	QModelIndex localIdx = model.mapFromSource(globalIdx);
+	QModelIndex localIdx = model->mapFromSource(globalIdx);
 	ui.diveSites->view()->scrollTo(localIdx);
 }
 
@@ -102,7 +104,7 @@ void TabDiveSite::on_purgeUnused_clicked()
 
 void TabDiveSite::on_filterText_textChanged(const QString &text)
 {
-	model.setFilter(text);
+	model->setFilter(text);
 }
 
 QVector<dive_site *> TabDiveSite::selectedDiveSites()
@@ -111,7 +113,7 @@ QVector<dive_site *> TabDiveSite::selectedDiveSites()
 	QVector<dive_site *> sites;
 	sites.reserve(indices.size());
 	for (const QModelIndex &idx: indices) {
-		struct dive_site *ds = model.getDiveSite(idx);
+		struct dive_site *ds = model->getDiveSite(idx);
 		sites.append(ds);
 	}
 	return sites;
diff --git a/desktop-widgets/tab-widgets/TabDiveSite.h b/desktop-widgets/tab-widgets/TabDiveSite.h
index c5d69eecc..d123dc28e 100644
--- a/desktop-widgets/tab-widgets/TabDiveSite.h
+++ b/desktop-widgets/tab-widgets/TabDiveSite.h
@@ -4,7 +4,8 @@
 
 #include "TabBase.h"
 #include "ui_TabDiveSite.h"
-#include "qt-models/divelocationmodel.h"
+
+class DiveSiteSortedModel;
 
 class TabDiveSite : public TabBase {
 	Q_OBJECT
@@ -22,7 +23,7 @@ private slots:
 	void selectionChanged(const QItemSelection &selected, const QItemSelection &deselected);
 private:
 	Ui::TabDiveSite ui;
-	DiveSiteSortedModel model;
+	DiveSiteSortedModel *model;
 	QVector<dive_site *> selectedDiveSites();
 	void hideEvent(QHideEvent *) override;
 	void showEvent(QShowEvent *) override;
diff --git a/desktop-widgets/tableview.cpp b/desktop-widgets/tableview.cpp
index 2a6a2d535..c5dbd453e 100644
--- a/desktop-widgets/tableview.cpp
+++ b/desktop-widgets/tableview.cpp
@@ -81,11 +81,14 @@ TableView::~TableView()
 		s.remove("");
 	} else if (ui.tableView->model()) {
 		for (int i = 0; i < ui.tableView->model()->columnCount(); i++) {
-			if (ui.tableView->columnWidth(i) == defaultColumnWidth(i))
+			if (ui.tableView->columnWidth(i) == defaultColumnWidth(i)) {
 				s.remove(QString("colwidth%1").arg(i));
-			else
+			} else {
 				s.setValue(QString("colwidth%1").arg(i), ui.tableView->columnWidth(i));
+			}
 		}
+	} else {
+		qWarning("TableView %s without model", qPrintable(objectName()));
 	}
 	s.endGroup();
 }
diff --git a/qt-models/divelocationmodel.cpp b/qt-models/divelocationmodel.cpp
index f8bad01a7..f353e7832 100644
--- a/qt-models/divelocationmodel.cpp
+++ b/qt-models/divelocationmodel.cpp
@@ -215,7 +215,7 @@ bool DiveSiteSortedModel::lessThan(const QModelIndex &i1, const QModelIndex &i2)
 	}
 }
 
-DiveSiteSortedModel::DiveSiteSortedModel()
+DiveSiteSortedModel::DiveSiteSortedModel(QObject *parent) : QSortFilterProxyModel(parent)
 {
 	setSourceModel(LocationInformationModel::instance());
 }
diff --git a/qt-models/divelocationmodel.h b/qt-models/divelocationmodel.h
index 623296de2..57471dee6 100644
--- a/qt-models/divelocationmodel.h
+++ b/qt-models/divelocationmodel.h
@@ -49,7 +49,7 @@ private:
 	bool setData(const QModelIndex &index, const QVariant &value, int role) override;
 #endif // SUBSURFACE_MOBILE
 public:
-	DiveSiteSortedModel();
+	DiveSiteSortedModel(QObject *parent = nullptr);
 	QStringList allSiteNames() const;
 	void setFilter(const QString &text);
 	struct dive_site *getDiveSite(const QModelIndex &idx);