From 8b952b387041341f556edcf0bb34576a2fc88568 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Mr=C3=A1zek?= Date: Sun, 6 Nov 2016 21:58:54 +0100 Subject: NOISSUE Refactor and sanitize MultiMC startup/shutdown * Always create main window. * Properly handle netowrk manager - it was created twice, leading to potential crashes. --- api/logic/Env.cpp | 121 ++++++++---------------- api/logic/Env.h | 24 ++--- api/logic/minecraft/MinecraftVersionList.cpp | 3 +- api/logic/minecraft/SkinUpload.cpp | 2 +- api/logic/minecraft/auth/YggdrasilTask.cpp | 3 +- api/logic/minecraft/forge/ForgeXzDownload.cpp | 9 +- api/logic/minecraft/legacy/LegacyUpdate.cpp | 6 +- api/logic/minecraft/legacy/LwjglVersionList.cpp | 3 +- api/logic/net/Download.cpp | 3 +- api/logic/net/PasteUpload.cpp | 3 +- api/logic/screenshots/ImgurAlbumCreation.cpp | 6 +- api/logic/screenshots/ImgurUpload.cpp | 3 +- api/logic/wonko/WonkoIndex_test.cpp | 2 +- application/MultiMC.cpp | 40 ++++++-- application/MultiMC.h | 4 +- application/WonkoGui.h | 3 +- application/main.cpp | 25 ++--- 17 files changed, 116 insertions(+), 144 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 + +class Env::Private +{ +public: + QNetworkAccessManager m_qnam; + shared_qobject_ptr m_metacache; + std::shared_ptr m_iconlist; + QMap> m_versionLists; + shared_qobject_ptr m_wonkoIndex; + QString m_wonkoRootUrl; +}; + /* * The *NEW* global rat nest of an object. Handle with care. */ Env::Env() { - m_qnam = std::make_shared(); + 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 Env::icons() { - return m_iconlist; + return d->m_iconlist; } void Env::registerIconList(std::shared_ptr 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(); - } - 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 Env::wonkoIndex() +shared_qobject_ptr Env::wonkoIndex() { - if (!m_wonkoIndex) + if (!d->m_wonkoIndex) { - m_wonkoIndex = std::make_shared(); + 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 qnam(); + QNetworkAccessManager &qnam() const; - std::shared_ptr metacache(); + shared_qobject_ptr metacache(); std::shared_ptr icons(); @@ -51,16 +52,11 @@ public: void registerIconList(std::shared_ptr iconlist); - std::shared_ptr wonkoIndex(); + shared_qobject_ptr 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 m_qnam; - std::shared_ptr m_metacache; - std::shared_ptr m_iconlist; - QMap> m_versionLists; - std::shared_ptr 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(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(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 cookies = qvariant_cast>(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(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()); } diff --git a/application/MultiMC.cpp b/application/MultiMC.cpp index 40a11a26..c6ac2b63 100644 --- a/application/MultiMC.cpp +++ b/application/MultiMC.cpp @@ -286,12 +286,17 @@ MultiMC::MultiMC(int &argc, char **argv) : QApplication(argc, argv) auto inst = instances()->getInstanceById(m_instanceIdToLaunch); if(inst) { - minecraftlist(); + // minimized main window + showMainWindow(true); launch(inst, true, nullptr); return; } } - showMainWindow(); + if(!m_mainWindow) + { + // normal main window + showMainWindow(false); + } } MultiMC::~MultiMC() @@ -340,9 +345,6 @@ void MultiMC::initNetwork() // init the http meta cache ENV.initHttpMetaCache(); - // create the global network manager - ENV.m_qnam.reset(new QNetworkAccessManager(this)); - // init proxy settings { QString proxyTypeStr = settings()->get("ProxyType").toString(); @@ -1052,7 +1054,6 @@ void MultiMC::onExit() { // m_instances->saveGroupList(); } - ENV.destroy(); if(logFile) { logFile->flush(); @@ -1126,6 +1127,12 @@ void MultiMC::controllerSucceeded() } } extras.controller.reset(); + // quit when there are no more windows. + if(m_openWindows == 0) + { + m_status = Status::Succeeded; + quit(); + } } void MultiMC::controllerFailed(const QString& error) @@ -1139,9 +1146,15 @@ void MultiMC::controllerFailed(const QString& error) // on failure, do... nothing extras.controller.reset(); + // quit when there are no more windows. + if(m_openWindows == 0) + { + m_status = Status::Failed; + quit(); + } } -MainWindow * MultiMC::showMainWindow() +MainWindow* MultiMC::showMainWindow(bool minimized) { if(m_mainWindow) { @@ -1154,9 +1167,18 @@ MainWindow * MultiMC::showMainWindow() m_mainWindow = new MainWindow(); m_mainWindow->restoreState(QByteArray::fromBase64(MMC->settings()->get("MainWindowState").toByteArray())); m_mainWindow->restoreGeometry(QByteArray::fromBase64(MMC->settings()->get("MainWindowGeometry").toByteArray())); - m_mainWindow->show(); + if(minimized) + { + m_mainWindow->showMinimized(); + } + else + { + m_mainWindow->show(); + } + m_mainWindow->checkSetDefaultJava(); m_mainWindow->checkInstancePathForProblems(); + m_openWindows++; } return m_mainWindow; } @@ -1177,13 +1199,13 @@ InstanceWindow *MultiMC::showInstanceWindow(InstancePtr instance, QString page) else { window = new InstanceWindow(instance); + m_openWindows ++; connect(window, &InstanceWindow::isClosing, this, &MultiMC::on_windowClose); } if(!page.isEmpty()) { window->selectPage(page); } - m_openWindows ++; if(extras.controller) { extras.controller->setParentWidget(window); diff --git a/application/MultiMC.h b/application/MultiMC.h index 4deb95d1..7f45e873 100644 --- a/application/MultiMC.h +++ b/application/MultiMC.h @@ -142,7 +142,9 @@ public: bool openJsonEditor(const QString &filename); InstanceWindow *showInstanceWindow(InstancePtr instance, QString page = QString()); - MainWindow *showMainWindow(); + MainWindow *showMainWindow(bool minimized = false); + +public slots: void launch(InstancePtr instance, bool online = true, BaseProfilerFactory *profiler = nullptr); private slots: diff --git a/application/WonkoGui.h b/application/WonkoGui.h index 2b87b819..ad0bee89 100644 --- a/application/WonkoGui.h +++ b/application/WonkoGui.h @@ -1,11 +1,12 @@ #pragma once #include +#include "QObjectPtr.h" class QWidget; class QString; -using WonkoIndexPtr = std::shared_ptr; +using WonkoIndexPtr = shared_qobject_ptr; using WonkoVersionListPtr = std::shared_ptr; using WonkoVersionPtr = std::shared_ptr; diff --git a/application/main.cpp b/application/main.cpp index fde5eba7..5b557170 100644 --- a/application/main.cpp +++ b/application/main.cpp @@ -9,22 +9,23 @@ int main(int argc, char *argv[]) // initialize Qt MultiMC app(argc, argv); - Q_INIT_RESOURCE(instances); - Q_INIT_RESOURCE(multimc); - Q_INIT_RESOURCE(backgrounds); - Q_INIT_RESOURCE(versions); - - Q_INIT_RESOURCE(pe_dark); - Q_INIT_RESOURCE(pe_light); - Q_INIT_RESOURCE(pe_blue); - Q_INIT_RESOURCE(pe_colored); - Q_INIT_RESOURCE(OSX); - Q_INIT_RESOURCE(iOS); - switch (app.status()) { case MultiMC::Initialized: + { + Q_INIT_RESOURCE(instances); + Q_INIT_RESOURCE(multimc); + Q_INIT_RESOURCE(backgrounds); + Q_INIT_RESOURCE(versions); + + Q_INIT_RESOURCE(pe_dark); + Q_INIT_RESOURCE(pe_light); + Q_INIT_RESOURCE(pe_blue); + Q_INIT_RESOURCE(pe_colored); + Q_INIT_RESOURCE(OSX); + Q_INIT_RESOURCE(iOS); return app.exec(); + } case MultiMC::Failed: return 1; case MultiMC::Succeeded: -- cgit v1.2.3