core/reloader: fix late creation of Reloadable types

Author: outfoxxed

src/core/floatingwindow.cpp

@@ -43,7 +43,7 @@ void FloatingWindowInterface::onReload(QObject* oldInstance) {
 	QQmlEngine::setContextForObject(this->window, QQmlEngine::contextForObject(this));
 
 	auto* old = qobject_cast<FloatingWindowInterface*>(oldInstance);
-	this->window->onReload(old != nullptr ? old->window : nullptr);
+	this->window->reload(old != nullptr ? old->window : nullptr);
 }
 
 QQmlListProperty<QObject> FloatingWindowInterface::data() { return this->window->data(); }

src/core/generation.cpp

@@ -13,6 +13,7 @@
 #include <qqmlengine.h>
 #include <qqmlincubator.h>
 #include <qtimer.h>
+#include <qtmetamacros.h>
 
 #include "iconimageprovider.hpp"
 #include "incubator.hpp"
@@ -54,8 +55,10 @@ void EngineGeneration::onReload(EngineGeneration* old) {
 	QObject::connect(&this->engine, &QQmlEngine::quit, app, &QCoreApplication::quit);
 	QObject::connect(&this->engine, &QQmlEngine::exit, app, &QCoreApplication::exit);
 
-	this->root->onReload(old == nullptr ? nullptr : old->root);
+	this->root->reload(old == nullptr ? nullptr : old->root);
 	this->singletonRegistry.onReload(old == nullptr ? nullptr : &old->singletonRegistry);
+	this->reloadComplete = true;
+	emit this->reloadFinished();
 
 	if (old != nullptr) {
 		QTimer::singleShot(0, [this, old]() {

src/core/generation.hpp

@@ -41,9 +41,11 @@ class EngineGeneration: public QObject {
 	SingletonRegistry singletonRegistry;
 	QFileSystemWatcher* watcher = nullptr;
 	DelayedQmlIncubationController delayedIncubationController;
+	bool reloadComplete = false;
 
 signals:
 	void filesChanged();
+	void reloadFinished();
 
 private slots:
 	void incubationControllerDestroyed();

src/core/lazyloader.cpp

@@ -23,13 +23,11 @@ void LazyLoader::onReload(QObject* oldInstance) {
 
 	if (this->mItem != nullptr) {
 		if (auto* reloadable = qobject_cast<Reloadable*>(this->mItem)) {
-			reloadable->onReload(old == nullptr ? nullptr : old->mItem);
+			reloadable->reload(old == nullptr ? nullptr : old->mItem);
 		} else {
 			Reloadable::reloadRecursive(this->mItem, old);
 		}
 	}
-
-	this->postReload = true;
 }
 
 QObject* LazyLoader::item() {
@@ -48,14 +46,6 @@ void LazyLoader::setItem(QObject* item) {
 
 	if (item != nullptr) {
 		item->setParent(this);
-
-		if (this->postReload) {
-			if (auto* reloadable = qobject_cast<Reloadable*>(this->mItem)) {
-				reloadable->onReload(nullptr);
-			} else {
-				Reloadable::reloadRecursive(this->mItem, nullptr);
-			}
-		}
 	}
 
 	this->targetActive = this->isActive();
@@ -160,7 +150,7 @@ void LazyLoader::setSource(QString source) {
 }
 
 void LazyLoader::incubateIfReady(bool overrideReloadCheck) {
-	if (!(this->postReload || overrideReloadCheck) || !(this->targetLoading || this->targetActive)
+	if (!(this->reloadComplete || overrideReloadCheck) || !(this->targetLoading || this->targetActive)
 	    || this->mComponent == nullptr || this->incubator != nullptr)
 	{
 		return;

src/core/lazyloader.hpp

@@ -152,7 +152,6 @@ private slots:
 	void incubateIfReady(bool overrideReloadCheck = false);
 	void waitForObjectCreation();
 
-	bool postReload = false;
 	bool targetLoading = false;
 	bool targetActive = false;
 	QObject* mItem = nullptr;

src/core/main.cpp

@@ -10,7 +10,6 @@
 #include <qguiapplication.h>
 #include <qhash.h>
 #include <qlogging.h>
-#include <qobject.h>
 #include <qquickwindow.h>
 #include <qstandardpaths.h>
 #include <qstring.h>

src/core/reload.cpp

@@ -4,6 +4,43 @@
 #include <qobject.h>
 #include <qqmllist.h>
 
+#include "generation.hpp"
+
+void Reloadable::componentComplete() {
+	this->engineGeneration = EngineGeneration::findObjectGeneration(this);
+
+	if (this->engineGeneration != nullptr) {
+		// When called this way there is no chance a reload will have old data,
+		// but this will at least help prevent weird behaviors due to never getting a reload.
+		if (this->engineGeneration->reloadComplete) this->reload();
+		else {
+			QObject::connect(
+			    this->engineGeneration,
+			    &EngineGeneration::reloadFinished,
+			    this,
+			    &Reloadable::onReloadFinished
+			);
+		}
+	}
+}
+
+void Reloadable::reload(QObject* oldInstance) {
+	if (this->reloadComplete) return;
+	this->onReload(oldInstance);
+	this->reloadComplete = true;
+
+	if (this->engineGeneration != nullptr) {
+		QObject::disconnect(
+		    this->engineGeneration,
+		    &EngineGeneration::reloadFinished,
+		    this,
+		    &Reloadable::onReloadFinished
+		);
+	}
+}
+
+void Reloadable::onReloadFinished() { this->reload(nullptr); }
+
 void ReloadPropagator::onReload(QObject* oldInstance) {
 	auto* old = qobject_cast<ReloadPropagator*>(oldInstance);
 
@@ -13,7 +50,7 @@ void ReloadPropagator::onReload(QObject* oldInstance) {
 			auto* oldChild = old == nullptr || old->mChildren.length() <= i
 			                   ? nullptr
 			                   : qobject_cast<Reloadable*>(old->mChildren.at(i));
-			newChild->onReload(oldChild);
+			newChild->reload(oldChild);
 		} else {
 			Reloadable::reloadRecursive(newChild, oldInstance);
 		}

src/core/reload.hpp

@@ -7,6 +7,8 @@
 #include <qqmlparserstatus.h>
 #include <qtmetamacros.h>
 
+class EngineGeneration;
+
 ///! The base class of all types that can be reloaded.
 /// Reloadables will attempt to take specific state from previous config revisions if possible.
 /// Some examples are [ProxyWindowBase] and [PersistentProperties]
@@ -56,21 +58,28 @@ class Reloadable
 public:
 	explicit Reloadable(QObject* parent = nullptr): QObject(parent) {}
 
-	// Called unconditionally in the reload phase, with nullptr if no source could be determined.
-	// If non null the old instance may or may not be of the same type, and should be checked
-	// by `onReload`.
-	virtual void onReload(QObject* oldInstance) = 0;
+	void reload(QObject* oldInstance = nullptr);
 
-	// TODO: onReload runs after initialization for reloadable objects created late
-	void classBegin() override {}
-	void componentComplete() override {}
+	void classBegin() override {};
+	void componentComplete() override;
 
 	// Reload objects in the parent->child graph recursively.
 	static void reloadRecursive(QObject* newObj, QObject* oldRoot);
 	// Same as above but does not reload the passed object, only its children.
 	static void reloadChildrenRecursive(QObject* newRoot, QObject* oldRoot);
 
 	QString mReloadableId;
+	bool reloadComplete = false;
+	EngineGeneration* engineGeneration = nullptr;
+
+private slots:
+	void onReloadFinished();
+
+protected:
+	// Called unconditionally in the reload phase, with nullptr if no source could be determined.
+	// If non null the old instance may or may not be of the same type, and should be checked
+	// by `onReload`.
+	virtual void onReload(QObject* oldInstance) = 0;
 
 private:
 	static QObject* getChildByReloadId(QObject* parent, const QString& reloadId);

src/core/singleton.cpp

@@ -48,7 +48,7 @@ void SingletonRegistry::registerSingleton(const QUrl& url, Singleton* singleton)
 
 void SingletonRegistry::onReload(SingletonRegistry* old) {
 	for (auto [url, singleton]: this->registry.asKeyValueRange()) {
-		singleton->onReload(old == nullptr ? nullptr : old->registry.value(url));
+		singleton->reload(old == nullptr ? nullptr : old->registry.value(url));
 	}
 }
 

src/core/test/popupwindow.cpp

@@ -16,8 +16,8 @@ void TestPopupWindow::initiallyVisible() { // NOLINT
 	popup.setParentWindow(&parent);
 	popup.setVisible(true);
 
-	parent.onReload(nullptr);
-	popup.onReload(nullptr);
+	parent.reload();
+	popup.reload();
 
 	QVERIFY(popup.isVisible());
 	QVERIFY(popup.backingWindow()->isVisible());
@@ -36,8 +36,8 @@ void TestPopupWindow::reloadReparent() { // NOLINT
 	popup.setParentWindow(&parent);
 	popup.setVisible(true);
 
-	parent.onReload(nullptr);
-	popup.onReload(nullptr);
+	parent.reload();
+	popup.reload();
 
 	// second generation
 	auto newParent = ProxyWindowBase();
@@ -51,8 +51,8 @@ void TestPopupWindow::reloadReparent() { // NOLINT
 
 	auto spy = QSignalSpy(oldWindow, &QWindow::visibleChanged);
 
-	newParent.onReload(&parent);
-	newPopup.onReload(&popup);
+	newParent.reload(&parent);
+	newPopup.reload(&popup);
 
 	QVERIFY(newPopup.isVisible());
 	QVERIFY(newPopup.backingWindow()->isVisible());
@@ -69,15 +69,15 @@ void TestPopupWindow::reloadUnparent() { // NOLINT
 	popup.setParentWindow(&parent);
 	popup.setVisible(true);
 
-	parent.onReload(nullptr);
-	popup.onReload(nullptr);
+	parent.reload();
+	popup.reload();
 
 	// second generation
 	auto newPopup = ProxyPopupWindow();
 
 	// parent not set
 	newPopup.setVisible(true);
-	newPopup.onReload(&popup);
+	newPopup.reload(&popup);
 
 	QVERIFY(!newPopup.isVisible());
 	QVERIFY(!newPopup.backingWindow()->isVisible());
@@ -88,7 +88,7 @@ void TestPopupWindow::invisibleWithoutParent() { // NOLINT
 	auto popup = ProxyPopupWindow();
 
 	popup.setVisible(true);
-	popup.onReload(nullptr);
+	popup.reload();
 
 	QVERIFY(!popup.isVisible());
 }
@@ -102,8 +102,8 @@ void TestPopupWindow::moveWithParent() { // NOLINT
 	popup.setRelativeY(10);
 	popup.setVisible(true);
 
-	parent.onReload(nullptr);
-	popup.onReload(nullptr);
+	parent.reload();
+	popup.reload();
 
 	QCOMPARE(popup.x(), parent.x() + 10);
 	QCOMPARE(popup.y(), parent.y() + 10);
@@ -121,8 +121,8 @@ void TestPopupWindow::attachParentLate() { // NOLINT
 
 	popup.setVisible(true);
 
-	parent.onReload(nullptr);
-	popup.onReload(nullptr);
+	parent.reload();
+	popup.reload();
 
 	QVERIFY(!popup.isVisible());
 
@@ -139,14 +139,14 @@ void TestPopupWindow::reparentLate() { // NOLINT
 	popup.setParentWindow(&parent);
 	popup.setVisible(true);
 
-	parent.onReload(nullptr);
-	popup.onReload(nullptr);
+	parent.reload();
+	popup.reload();
 
 	QCOMPARE(popup.x(), parent.x());
 	QCOMPARE(popup.y(), parent.y());
 
 	auto parent2 = ProxyWindowBase();
-	parent2.onReload(nullptr);
+	parent2.reload();
 
 	parent2.backingWindow()->setX(10);
 	parent2.backingWindow()->setY(10);
@@ -166,8 +166,8 @@ void TestPopupWindow::xMigrationFix() { // NOLINT
 	popup.setParentWindow(&parent);
 	popup.setVisible(true);
 
-	parent.onReload(nullptr);
-	popup.onReload(nullptr);
+	parent.reload();
+	popup.reload();
 
 	QCOMPARE(popup.x(), parent.x());