NOISSUE Refactor and sanitize MultiMC startup/shutdown

* Always create main window.
* Properly handle netowrk manager - it was created twice, leading to potential crashes.

13 files changed, 67 insertions, 121 deletions

diff --git a/api/logic/Env.cpp b/api/logic/Env.cpp
index b8e07343..b769c9c4 100644
--- a/api/logic/Env.cpp
+++ b/api/logic/Env.cpp
@@ -10,20 +10,30 @@
 #include "wonko/WonkoIndex.h"
 #include <QDebug>
 
+
+class Env::Private
+{
+public:
+	QNetworkAccessManager m_qnam;
+	shared_qobject_ptr<HttpMetaCache> m_metacache;
+	std::shared_ptr<IIconList> m_iconlist;
+	QMap<QString, std::shared_ptr<BaseVersionList>> m_versionLists;
+	shared_qobject_ptr<WonkoIndex> m_wonkoIndex;
+	QString m_wonkoRootUrl;
+};
+
 /*
  * The *NEW* global rat nest of an object. Handle with care.
  */
 
 Env::Env()
 {
-	m_qnam = std::make_shared<QNetworkAccessManager>();
+	d = new Private();
 }
 
-void Env::destroy()
+Env::~Env()
 {
-	m_metacache.reset();
-	m_qnam.reset();
-	m_versionLists.clear();
+	delete d;
 }
 
 Env& Env::Env::getInstance()
@@ -32,85 +42,26 @@ Env& Env::Env::getInstance()
 	return instance;
 }
 
-std::shared_ptr< HttpMetaCache > Env::metacache()
+shared_qobject_ptr< HttpMetaCache > Env::metacache()
 {
-	Q_ASSERT(m_metacache != nullptr);
-	return m_metacache;
+	return d->m_metacache;
 }
 
-std::shared_ptr< QNetworkAccessManager > Env::qnam()
+QNetworkAccessManager& Env::qnam() const
 {
-	return m_qnam;
+	return d->m_qnam;
 }
 
 std::shared_ptr<IIconList> Env::icons()
 {
-	return m_iconlist;
+	return d->m_iconlist;
 }
 
 void Env::registerIconList(std::shared_ptr<IIconList> iconlist)
 {
-	m_iconlist = iconlist;
+	d->m_iconlist = iconlist;
 }
 
-/*
-class NullVersion : public BaseVersion
-{
-	Q_OBJECT
-public:
-	virtual QString name()
-	{
-		return "null";
-	}
-	virtual QString descriptor()
-	{
-		return "null";
-	}
-	virtual QString typeString() const
-	{
-		return "Null";
-	}
-};
-
-class NullTask: public Task
-{
-	Q_OBJECT
-public:
-	virtual void executeTask()
-	{
-		emitFailed(tr("Nothing to do."));
-	}
-};
-
-class NullVersionList: public BaseVersionList
-{
-	Q_OBJECT
-public:
-	virtual const BaseVersionPtr at(int i) const
-	{
-		return std::make_shared<NullVersion>();
-	}
-	virtual int count() const
-	{
-		return 0;
-	};
-	virtual Task* getLoadTask()
-	{
-		return new NullTask;
-	}
-	virtual bool isLoaded()
-	{
-		return false;
-	}
-	virtual void sort()
-	{
-	}
-	virtual void updateListData(QList< BaseVersionPtr >)
-	{
-	}
-};
-*/
-
 BaseVersionPtr Env::getVersion(QString component, QString version)
 {
 	auto list = getVersionList(component);
@@ -123,8 +74,8 @@ BaseVersionPtr Env::getVersion(QString component, QString version)
 
 std::shared_ptr< BaseVersionList > Env::getVersionList(QString component)
 {
-	auto iter = m_versionLists.find(component);
-	if(iter != m_versionLists.end())
+	auto iter = d->m_versionLists.find(component);
+	if(iter != d->m_versionLists.end())
 	{
 		return *iter;
 	}
@@ -134,21 +85,22 @@ std::shared_ptr< BaseVersionList > Env::getVersionList(QString component)
 
 void Env::registerVersionList(QString name, std::shared_ptr< BaseVersionList > vlist)
 {
-	m_versionLists[name] = vlist;
+	d->m_versionLists[name] = vlist;
 }
 
-std::shared_ptr<WonkoIndex> Env::wonkoIndex()
+shared_qobject_ptr<WonkoIndex> Env::wonkoIndex()
 {
-	if (!m_wonkoIndex)
+	if (!d->m_wonkoIndex)
 	{
-		m_wonkoIndex = std::make_shared<WonkoIndex>();
+		d->m_wonkoIndex.reset(new WonkoIndex());
 	}
-	return m_wonkoIndex;
+	return d->m_wonkoIndex;
 }
 
 
 void Env::initHttpMetaCache()
 {
+	auto &m_metacache = d->m_metacache;
 	m_metacache.reset(new HttpMetaCache("metacache"));
 	m_metacache->addBase("asset_indexes", QDir("assets/indexes").absolutePath());
 	m_metacache->addBase("asset_objects", QDir("assets/objects").absolutePath());
@@ -192,8 +144,7 @@ void Env::updateProxySettings(QString proxyTypeStr, QString addr, int port, QStr
 
 	qDebug() << "Detecting proxy settings...";
 	QNetworkProxy proxy = QNetworkProxy::applicationProxy();
-	if (m_qnam.get())
-		m_qnam->setProxy(proxy);
+	d->m_qnam.setProxy(proxy);
 	QString proxyDesc;
 	if (proxy.type() == QNetworkProxy::NoProxy)
 	{
@@ -229,4 +180,14 @@ void Env::updateProxySettings(QString proxyTypeStr, QString addr, int port, QStr
 	qDebug() << proxyDesc;
 }
 
-#include "Env.moc"
+QString Env::wonkoRootUrl() const
+{
+	return d->m_wonkoRootUrl;
+}
+
+void Env::setWonkoRootUrl(const QString& url)
+{
+	d->m_wonkoRootUrl = url;
+}
+
+#include "Env.moc"
\ No newline at end of file
diff --git a/api/logic/Env.h b/api/logic/Env.h
index dcf1947f..989b4f3c 100644
--- a/api/logic/Env.h
+++ b/api/logic/Env.h
@@ -7,6 +7,8 @@
 
 #include "multimc_logic_export.h"
 
+#include "QObjectPtr.h"
+
 class QNetworkAccessManager;
 class HttpMetaCache;
 class BaseVersionList;
@@ -22,16 +24,15 @@ class MULTIMC_LOGIC_EXPORT Env
 {
 	friend class MultiMC;
 private:
+	class Private;
 	Env();
+	~Env();
 public:
 	static Env& getInstance();
 
-	// call when Qt stuff is being torn down
-	void destroy();
-
-	std::shared_ptr<QNetworkAccessManager> qnam();
+	QNetworkAccessManager &qnam() const;
 
-	std::shared_ptr<HttpMetaCache> metacache();
+	shared_qobject_ptr<HttpMetaCache> metacache();
 
 	std::shared_ptr<IIconList> icons();
 
@@ -51,16 +52,11 @@ public:
 
 	void registerIconList(std::shared_ptr<IIconList> iconlist);
 
-	std::shared_ptr<WonkoIndex> wonkoIndex();
+	shared_qobject_ptr<WonkoIndex> wonkoIndex();
 
-	QString wonkoRootUrl() const { return m_wonkoRootUrl; }
-	void setWonkoRootUrl(const QString &url) { m_wonkoRootUrl = url; }
+	QString wonkoRootUrl() const;
+	void setWonkoRootUrl(const QString &url);
 
 protected:
-	std::shared_ptr<QNetworkAccessManager> m_qnam;
-	std::shared_ptr<HttpMetaCache> m_metacache;
-	std::shared_ptr<IIconList> m_iconlist;
-	QMap<QString, std::shared_ptr<BaseVersionList>> m_versionLists;
-	std::shared_ptr<WonkoIndex> m_wonkoIndex;
-	QString m_wonkoRootUrl;
+	Private * d;
 };
diff --git a/api/logic/minecraft/MinecraftVersionList.cpp b/api/logic/minecraft/MinecraftVersionList.cpp
index 4e42f204..9d2e5b69 100644
--- a/api/logic/minecraft/MinecraftVersionList.cpp
+++ b/api/logic/minecraft/MinecraftVersionList.cpp
@@ -425,8 +425,7 @@ MCVListLoadTask::MCVListLoadTask(MinecraftVersionList *vlist)
 void MCVListLoadTask::executeTask()
 {
 	setStatus(tr("Loading instance version list..."));
-	auto worker = ENV.qnam();
-	vlistReply = worker->get(QNetworkRequest(QUrl("https://launchermeta.mojang.com/mc/game/version_manifest.json")));
+	vlistReply = ENV.qnam().get(QNetworkRequest(QUrl("https://launchermeta.mojang.com/mc/game/version_manifest.json")));
 	connect(vlistReply, SIGNAL(finished()), this, SLOT(list_downloaded()));
 }
 
diff --git a/api/logic/minecraft/SkinUpload.cpp b/api/logic/minecraft/SkinUpload.cpp
index 2c67c7aa..1d1e38f3 100644
--- a/api/logic/minecraft/SkinUpload.cpp
+++ b/api/logic/minecraft/SkinUpload.cpp
@@ -40,7 +40,7 @@ void SkinUpload::executeTask()
 	multiPart->append(model);
 	multiPart->append(skin);
 
-	QNetworkReply *rep = ENV.qnam()->put(request, multiPart);
+	QNetworkReply *rep = ENV.qnam().put(request, multiPart);
 	m_reply = std::shared_ptr<QNetworkReply>(rep);
 
 	setStatus(tr("Uploading skin"));
diff --git a/api/logic/minecraft/auth/YggdrasilTask.cpp b/api/logic/minecraft/auth/YggdrasilTask.cpp
index c6971c9f..a0e4471f 100644
--- a/api/logic/minecraft/auth/YggdrasilTask.cpp
+++ b/api/logic/minecraft/auth/YggdrasilTask.cpp
@@ -42,13 +42,12 @@ void YggdrasilTask::executeTask()
 	// Get the content of the request we're going to send to the server.
 	QJsonDocument doc(getRequestContent());
 
-	auto worker = ENV.qnam();
 	QUrl reqUrl("https://" + URLConstants::AUTH_BASE + getEndpoint());
 	QNetworkRequest netRequest(reqUrl);
 	netRequest.setHeader(QNetworkRequest::ContentTypeHeader, "application/json");
 
 	QByteArray requestData = doc.toJson();
-	m_netReply = worker->post(netRequest, requestData);
+	m_netReply = ENV.qnam().post(netRequest, requestData);
 	connect(m_netReply, &QNetworkReply::finished, this, &YggdrasilTask::processReply);
 	connect(m_netReply, &QNetworkReply::uploadProgress, this, &YggdrasilTask::refreshTimers);
 	connect(m_netReply, &QNetworkReply::downloadProgress, this, &YggdrasilTask::refreshTimers);
diff --git a/api/logic/minecraft/forge/ForgeXzDownload.cpp b/api/logic/minecraft/forge/ForgeXzDownload.cpp
index 8b762866..68e080f2 100644
--- a/api/logic/minecraft/forge/ForgeXzDownload.cpp
+++ b/api/logic/minecraft/forge/ForgeXzDownload.cpp
@@ -61,15 +61,12 @@ void ForgeXzDownload::start()
 	request.setRawHeader(QString("If-None-Match").toLatin1(), m_entry->getETag().toLatin1());
 	request.setHeader(QNetworkRequest::UserAgentHeader, "MultiMC/5.0 (Cached)");
 
-	auto worker = ENV.qnam();
-	QNetworkReply *rep = worker->get(request);
+	QNetworkReply *rep = ENV.qnam().get(request);
 
 	m_reply.reset(rep);
-	connect(rep, SIGNAL(downloadProgress(qint64, qint64)),
-			SLOT(downloadProgress(qint64, qint64)));
+	connect(rep, SIGNAL(downloadProgress(qint64, qint64)), SLOT(downloadProgress(qint64, qint64)));
 	connect(rep, SIGNAL(finished()), SLOT(downloadFinished()));
-	connect(rep, SIGNAL(error(QNetworkReply::NetworkError)),
-			SLOT(downloadError(QNetworkReply::NetworkError)));
+	connect(rep, SIGNAL(error(QNetworkReply::NetworkError)), SLOT(downloadError(QNetworkReply::NetworkError)));
 	connect(rep, SIGNAL(readyRead()), SLOT(downloadReadyRead()));
 }
 
diff --git a/api/logic/minecraft/legacy/LegacyUpdate.cpp b/api/logic/minecraft/legacy/LegacyUpdate.cpp
index a0d1933f..cbca1019 100644
--- a/api/logic/minecraft/legacy/LegacyUpdate.cpp
+++ b/api/logic/minecraft/legacy/LegacyUpdate.cpp
@@ -195,7 +195,7 @@ void LegacyUpdate::lwjglStart()
 	QString url = version->url();
 	QUrl realUrl(url);
 	QString hostname = realUrl.host();
-	auto worker = ENV.qnam();
+	auto worker = &ENV.qnam();
 	QNetworkRequest req(realUrl);
 	req.setRawHeader("Host", hostname.toLatin1());
 	req.setHeader(QNetworkRequest::UserAgentHeader, "MultiMC/5.0 (Cached)");
@@ -203,7 +203,7 @@ void LegacyUpdate::lwjglStart()
 
 	m_reply = std::shared_ptr<QNetworkReply>(rep);
 	connect(rep, &QNetworkReply::downloadProgress, this, &LegacyUpdate::progress);
-	connect(worker.get(), &QNetworkAccessManager::finished, this, &LegacyUpdate::lwjglFinished);
+	connect(worker, &QNetworkAccessManager::finished, this, &LegacyUpdate::lwjglFinished);
 }
 
 void LegacyUpdate::lwjglFinished(QNetworkReply *reply)
@@ -219,7 +219,7 @@ void LegacyUpdate::lwjglFinished(QNetworkReply *reply)
 				   "a row. YMMV");
 		return;
 	}
-	auto worker = ENV.qnam();
+	auto worker = &ENV.qnam();
 	// Here i check if there is a cookie for me in the reply and extract it
 	QList<QNetworkCookie> cookies =
 		qvariant_cast<QList<QNetworkCookie>>(reply->header(QNetworkRequest::SetCookieHeader));
diff --git a/api/logic/minecraft/legacy/LwjglVersionList.cpp b/api/logic/minecraft/legacy/LwjglVersionList.cpp
index bb017368..427032a4 100644
--- a/api/logic/minecraft/legacy/LwjglVersionList.cpp
+++ b/api/logic/minecraft/legacy/LwjglVersionList.cpp
@@ -82,11 +82,10 @@ void LWJGLVersionList::loadList()
 	Q_ASSERT_X(!m_loading, "loadList", "list is already loading (m_loading is true)");
 
 	setLoading(true);
-	auto worker = ENV.qnam();
 	QNetworkRequest req(QUrl(RSS_URL));
 	req.setRawHeader("Accept", "application/rss+xml, text/xml, */*");
 	req.setRawHeader("User-Agent", "MultiMC/5.0 (Uncached)");
-	reply = worker->get(req);
+	reply = ENV.qnam().get(req);
 	connect(reply, SIGNAL(finished()), SLOT(netRequestComplete()));
 }
 
diff --git a/api/logic/net/Download.cpp b/api/logic/net/Download.cpp
index 3d53ded0..af0b3144 100644
--- a/api/logic/net/Download.cpp
+++ b/api/logic/net/Download.cpp
@@ -95,8 +95,7 @@ void Download::start()
 
 	request.setHeader(QNetworkRequest::UserAgentHeader, "MultiMC/5.0");
 
-	auto worker = ENV.qnam();
-	QNetworkReply *rep = worker->get(request);
+	QNetworkReply *rep =  ENV.qnam().get(request);
 
 	m_reply.reset(rep);
 	connect(rep, SIGNAL(downloadProgress(qint64, qint64)), SLOT(downloadProgress(qint64, qint64)));
diff --git a/api/logic/net/PasteUpload.cpp b/api/logic/net/PasteUpload.cpp
index 4b671d6f..a03a4c64 100644
--- a/api/logic/net/PasteUpload.cpp
+++ b/api/logic/net/PasteUpload.cpp
@@ -36,8 +36,7 @@ void PasteUpload::executeTask()
 	request.setRawHeader("Content-Type", "application/x-www-form-urlencoded");
 	request.setRawHeader("Content-Length", QByteArray::number(m_text.size()));
 
-	auto worker = ENV.qnam();
-	QNetworkReply *rep = worker->post(request, buf);
+	QNetworkReply *rep = ENV.qnam().post(request, buf);
 
 	m_reply = std::shared_ptr<QNetworkReply>(rep);
 	setStatus(tr("Uploading to paste.ee"));
diff --git a/api/logic/screenshots/ImgurAlbumCreation.cpp b/api/logic/screenshots/ImgurAlbumCreation.cpp
index e009ef4d..a6964681 100644
--- a/api/logic/screenshots/ImgurAlbumCreation.cpp
+++ b/api/logic/screenshots/ImgurAlbumCreation.cpp
@@ -33,14 +33,12 @@ void ImgurAlbumCreation::start()
 
 	const QByteArray data = "ids=" + ids.join(',').toUtf8() + "&title=Minecraft%20Screenshots&privacy=hidden";
 
-	auto worker = ENV.qnam();
-	QNetworkReply *rep = worker->post(request, data);
+	QNetworkReply *rep = ENV.qnam().post(request, data);
 
 	m_reply.reset(rep);
 	connect(rep, &QNetworkReply::uploadProgress, this, &ImgurAlbumCreation::downloadProgress);
 	connect(rep, &QNetworkReply::finished, this, &ImgurAlbumCreation::downloadFinished);
-	connect(rep, SIGNAL(error(QNetworkReply::NetworkError)),
-			SLOT(downloadError(QNetworkReply::NetworkError)));
+	connect(rep, SIGNAL(error(QNetworkReply::NetworkError)), SLOT(downloadError(QNetworkReply::NetworkError)));
 }
 void ImgurAlbumCreation::downloadError(QNetworkReply::NetworkError error)
 {
diff --git a/api/logic/screenshots/ImgurUpload.cpp b/api/logic/screenshots/ImgurUpload.cpp
index 48e0ec18..ef7fee6b 100644
--- a/api/logic/screenshots/ImgurUpload.cpp
+++ b/api/logic/screenshots/ImgurUpload.cpp
@@ -49,8 +49,7 @@ void ImgurUpload::start()
 	namePart.setBody(m_shot->m_file.baseName().toUtf8());
 	multipart->append(namePart);
 
-	auto worker = ENV.qnam();
-	QNetworkReply *rep = worker->post(request, multipart);
+	QNetworkReply *rep = ENV.qnam().post(request, multipart);
 
 	m_reply.reset(rep);
 	connect(rep, &QNetworkReply::uploadProgress, this, &ImgurUpload::downloadProgress);
diff --git a/api/logic/wonko/WonkoIndex_test.cpp b/api/logic/wonko/WonkoIndex_test.cpp
index 7eb51bc3..d7b92f21 100644
--- a/api/logic/wonko/WonkoIndex_test.cpp
+++ b/api/logic/wonko/WonkoIndex_test.cpp
@@ -12,7 +12,7 @@ private
 slots:
 	void test_isProvidedByEnv()
 	{
-		QVERIFY(ENV.wonkoIndex() != nullptr);
+		QVERIFY(ENV.wonkoIndex());
 		QCOMPARE(ENV.wonkoIndex(), ENV.wonkoIndex());
 	}