Further Download related cleanups

* Moved the memory buffer and mutex handling into the Download class to
  simplify the code updating the UI in ServerDialog and UpdaterWindow.

* Replaced the "DownloadUpdate" callback function with simply polling
  Download::getState, since in the end polling was happening anyway.

This changes also fixes handling of the Enter key while downloading
updates, which no longer cancels the update process. Also, when pressing
Escape while things are being downloaded, the first press cancels and
only the second press goes back to login.

Introduced a ThreadSafe template class, which wraps any type and makes
it only accessible by calling lock(). This ensures the data is never
accessed without locking the relevant mutex.

8 files changed, 422 insertions, 532 deletions

diff --git a/src/gui/serverdialog.cpp b/src/gui/serverdialog.cpp
index 90469662..4c54f2ce 100644
--- a/src/gui/serverdialog.cpp
+++ b/src/gui/serverdialog.cpp
@@ -43,7 +43,6 @@
 
 #include "utils/gettext.h"
 #include "utils/stringutils.h"
-#include "utils/xml.h"
 
 #include <guichan/font.hpp>
 
@@ -59,13 +58,11 @@ ServersListModel::ServersListModel(ServerInfos *servers, ServerDialog *parent):
 
 int ServersListModel::getNumberOfElements()
 {
-    MutexLocker lock(mParent->getMutex());
     return mServers->size();
 }
 
 std::string ServersListModel::getElementAt(int elementIndex)
 {
-    MutexLocker lock(mParent->getMutex());
     const ServerInfo &server = mServers->at(elementIndex);
     std::string myServer;
     myServer += server.hostname;
@@ -162,7 +159,6 @@ ServerDialog::ServerDialog(ServerInfo *serverInfo, const std::string &dir):
     loadCustomServers();
 
     mServersListModel = new ServersListModel(&mServers, this);
-
     mServersList = new ServersListBox(mServersListModel);
 
     auto *usedScroll = new ScrollArea(mServersList);
@@ -225,14 +221,6 @@ ServerDialog::ServerDialog(ServerInfo *serverInfo, const std::string &dir):
 
 ServerDialog::~ServerDialog()
 {
-    if (mDownload)
-    {
-        mDownload->cancel();
-
-        // Make sure thread is gone before deleting the ServersListModel
-        mDownload.reset();
-    }
-
     delete mServersListModel;
 }
 
@@ -345,7 +333,6 @@ void ServerDialog::valueChanged(const gcn::SelectionEvent &)
     // Update the server and post fields according to the new selection
     const ServerInfo &myServer = mServersListModel->getServer(index);
     mDescription->setCaption(myServer.description);
-
     mDeleteButton->setEnabled(myServer.save);
     mModifyButton->setEnabled(myServer.save);
 }
@@ -362,36 +349,42 @@ void ServerDialog::mouseClicked(gcn::MouseEvent &mouseEvent)
 
 void ServerDialog::logic()
 {
-    {
-        MutexLocker lock(&mMutex);
-        if (mDownloadStatus == DOWNLOADING_COMPLETE)
-        {
-            mDownloadStatus = DOWNLOADING_OVER;
+    Window::logic();
 
-            mDescription->setCaption(mServers[0].description);
-            mDownloadText->setCaption(std::string());
-        }
-        else if (mDownloadStatus == DOWNLOADING_IN_PROGRESS)
-        {
-            mDownloadText->setCaption(strprintf(_("Downloading server list..."
-                                                 "%2.0f%%"),
-                                      mDownloadProgress * 100));
-        }
-        else if (mDownloadStatus == DOWNLOADING_IDLE)
-        {
-            mDownloadText->setCaption(_("Waiting for server..."));
-        }
-        else if (mDownloadStatus == DOWNLOADING_PREPARING)
+    if (mDownloadDone)
+        return;
+
+    auto state = mDownload->getState();
+
+    switch (state.status) {
+    case DownloadStatus::IN_PROGRESS:
+        mDownloadText->setCaption(strprintf(_("Downloading server list..."
+                                              "%2.0f%%"),
+                                            state.progress * 100));
+        break;
+
+    case DownloadStatus::CANCELED:
+    case DownloadStatus::ERROR:
+        mDownloadDone = true;
+        logger->log("Error retrieving server list: %s", mDownload->getError());
+        mDownloadText->setCaption(_("Error retrieving server list!"));
+        break;
+
+    case DownloadStatus::COMPLETE:
+        mDownloadDone = true;
+        loadServers();
+
+        if (mServers.empty())
         {
-            mDownloadText->setCaption(_("Preparing download"));
+            mDownloadText->setCaption(_("No servers found!"));
         }
-        else if (mDownloadStatus == DOWNLOADING_ERROR)
+        else
         {
-            mDownloadText->setCaption(_("Error retreiving server list!"));
+            mDownloadText->setCaption(std::string());
+            mDescription->setCaption(mServers[0].description);
         }
+        break;
     }
-
-    Window::logic();
 }
 
 void ServerDialog::downloadServerList()
@@ -406,8 +399,7 @@ void ServerDialog::downloadServerList()
     if (listFile.empty())
         listFile = "https://www.manasource.org/serverlist.xml";
 
-    mDownload = std::make_unique<Net::Download>(this, listFile,
-                                                &ServerDialog::downloadUpdate);
+    mDownload = std::make_unique<Net::Download>(listFile);
     mDownload->setFile(mDir + "/serverlist.xml");
     mDownload->start();
 }
@@ -433,89 +425,92 @@ void ServerDialog::loadServers()
 
     for (auto serverNode : rootNode.children())
     {
-        if (serverNode.name() != "server")
-            continue;
+        if (serverNode.name() == "server")
+            loadServer(serverNode);
+    }
+}
 
-        ServerInfo server;
+void ServerDialog::loadServer(XML::Node serverNode)
+{
+    ServerInfo server;
 
-        std::string type = serverNode.getProperty("type", "unknown");
+    std::string type = serverNode.getProperty("type", "unknown");
 
-        server.type = ServerInfo::parseType(type);
+    server.type = ServerInfo::parseType(type);
 
-        // Ignore unknown server types
-        if (server.type == ServerType::UNKNOWN
+    // Ignore unknown server types
+    if (server.type == ServerType::UNKNOWN
 #ifndef MANASERV_SUPPORT
-            || server.type == ServerType::MANASERV
+        || server.type == ServerType::MANASERV
 #endif
         )
-        {
-            logger->log("Ignoring server entry with unknown type: %s",
-                        type.c_str());
-            continue;
-        }
+    {
+        logger->log("Ignoring server entry with unknown type: %s",
+                    type.c_str());
+        return;
+    }
 
-        server.name = serverNode.getProperty("name", std::string());
+    server.name = serverNode.getProperty("name", std::string());
 
-        std::string version = serverNode.getProperty("minimumVersion",
-                                               std::string());
+    std::string version = serverNode.getProperty("minimumVersion",
+                                                 std::string());
 
-        bool meetsMinimumVersion = compareStrI(version, PACKAGE_VERSION) <= 0;
+    bool meetsMinimumVersion = compareStrI(version, PACKAGE_VERSION) <= 0;
 
-        // For display in the list
-        if (meetsMinimumVersion)
-            version.clear();
-        else if (version.empty())
-            version = _("requires a newer version");
-        else
-            version = strprintf(_("requires v%s"), version.c_str());
+    // For display in the list
+    if (meetsMinimumVersion)
+        version.clear();
+    else if (version.empty())
+        version = _("requires a newer version");
+    else
+        version = strprintf(_("requires v%s"), version.c_str());
 
-        for (auto subNode : serverNode.children())
+    for (auto subNode : serverNode.children())
+    {
+        if (subNode.name() == "connection")
         {
-            if (subNode.name() == "connection")
-            {
-                server.hostname = subNode.getProperty("hostname", std::string());
-                server.port = subNode.getProperty("port", 0);
-                if (server.port == 0)
-                {
-                    // If no port is given, use the default for the given type
-                    server.port = ServerInfo::defaultPortForServerType(server.type);
-                }
-            }
-            else if (subNode.name() == "description")
-            {
-                server.description = subNode.textContent();
-            }
-            else if (subNode.name() == "persistentIp")
+            server.hostname = subNode.getProperty("hostname", std::string());
+            server.port = subNode.getProperty("port", 0);
+            if (server.port == 0)
             {
-                const auto text = subNode.textContent();
-                server.persistentIp = text == "1" || text == "true";
+                // If no port is given, use the default for the given type
+                server.port = ServerInfo::defaultPortForServerType(server.type);
             }
         }
+        else if (subNode.name() == "description")
+        {
+            server.description = subNode.textContent();
+        }
+        else if (subNode.name() == "persistentIp")
+        {
+            const auto text = subNode.textContent();
+            server.persistentIp = text == "1" || text == "true";
+        }
+    }
 
-        server.version.first = gui->getFont()->getWidth(version);
-        server.version.second = version;
+    server.version.first = gui->getFont()->getWidth(version);
+    server.version.second = version;
 
-        // Add the server to the local list if it's not already present
-        bool found = false;
-        int i = 0;
-        for (auto &s : mServers)
+    // Add the server to the local list if it's not already present
+    bool found = false;
+    int i = 0;
+    for (auto &s : mServers)
+    {
+        if (s == server)
         {
-            if (s == server)
-            {
-                // Use the name listed in the server list
-                s.name = server.name;
-                s.version = server.version;
-                s.description = server.description;
-                mServersListModel->setVersionString(i, version);
-                found = true;
-                break;
-            }
-            ++i;
+            // Use the name listed in the server list
+            s.name = server.name;
+            s.version = server.version;
+            s.description = server.description;
+            mServersListModel->setVersionString(i, version);
+            found = true;
+            break;
         }
-
-        if (!found)
-            mServers.push_back(server);
+        ++i;
     }
+
+    if (!found)
+        mServers.push_back(server);
 }
 
 void ServerDialog::loadCustomServers()
@@ -558,38 +553,6 @@ void ServerDialog::saveCustomServers(const ServerInfo &currentServer, int index)
     // Restore the correct description
     if (index < 0)
         index = 0;
-    mDescription->setCaption(mServers[index].description);
-}
-
-int ServerDialog::downloadUpdate(void *ptr, DownloadStatus status,
-                                 size_t dltotal, size_t dlnow)
-{
-    if (status == DOWNLOAD_STATUS_CANCELLED)
-        return -1;
-
-    auto *sd = reinterpret_cast<ServerDialog*>(ptr);
-    MutexLocker lock(&sd->mMutex);
-
-    if (status == DOWNLOAD_STATUS_COMPLETE)
-    {
-        sd->loadServers();
-        sd->mDownloadStatus = DOWNLOADING_COMPLETE;
-    }
-    else if (status < 0)
-    {
-        logger->log("Error retreiving server list: %s",
-                    sd->mDownload->getError());
-        sd->mDownloadStatus = DOWNLOADING_ERROR;
-    }
-    else
-    {
-        float progress = 0.0f;
-        if (dltotal > 0)
-            progress = static_cast<float>(dlnow) / dltotal;
-
-        sd->mDownloadStatus = DOWNLOADING_IN_PROGRESS;
-        sd->mDownloadProgress = progress;
-    }
-
-    return 0;
+    if (static_cast<size_t>(index) < mServers.size())
+        mDescription->setCaption(mServers[index].description);
 }
diff --git a/src/gui/serverdialog.h b/src/gui/serverdialog.h
index bf2a9512..e0d74006 100644
--- a/src/gui/serverdialog.h
+++ b/src/gui/serverdialog.h
@@ -25,8 +25,7 @@
 
 #include "net/download.h"
 #include "net/serverinfo.h"
-
-#include "utils/mutex.h"
+#include "utils/xml.h"
 
 #include <guichan/actionlistener.hpp>
 #include <guichan/keylistener.hpp>
@@ -90,7 +89,6 @@ class ServerDialog : public Window,
 {
     public:
         ServerDialog(ServerInfo *serverInfo, const std::string &dir);
-
         ~ServerDialog() override;
 
         /**
@@ -110,10 +108,8 @@ class ServerDialog : public Window,
         void logic() override;
 
     protected:
-        friend class ServersListModel;
-        Mutex *getMutex() { return &mMutex; }
-
         friend class CustomServerDialog;
+
         /**
          * Saves the new server entry in the custom server list.
          * Removes the given entry when the serverInfo is empty.
@@ -128,12 +124,10 @@ class ServerDialog : public Window,
          */
         void downloadServerList();
         void loadServers();
+        void loadServer(XML::Node serverNode);
 
         void loadCustomServers();
 
-        static int downloadUpdate(void *ptr, DownloadStatus status,
-                                  size_t dltotal, size_t dlnow);
-
         Label  *mDescription;
         Button *mQuitButton;
         Button *mConnectButton;
@@ -146,25 +140,9 @@ class ServerDialog : public Window,
 
         const std::string &mDir;
 
-        enum ServerDialogDownloadStatus
-        {
-            DOWNLOADING_ERROR,
-            DOWNLOADING_PREPARING,
-            DOWNLOADING_IDLE,
-            DOWNLOADING_IN_PROGRESS,
-            DOWNLOADING_COMPLETE,
-            DOWNLOADING_OVER
-        };
-
-        /** Status of the current download. */
-        ServerDialogDownloadStatus mDownloadStatus = DOWNLOADING_PREPARING;
-
         std::unique_ptr<Net::Download> mDownload;
+        bool mDownloadDone = false;
         Label *mDownloadText;
-
-        Mutex mMutex;
-        float mDownloadProgress = 0.0f;
-
         ServerInfos mServers;
         ServerInfo *mServerInfo;
 };
diff --git a/src/gui/updaterwindow.cpp b/src/gui/updaterwindow.cpp
index 852d3ade..34169195 100644
--- a/src/gui/updaterwindow.cpp
+++ b/src/gui/updaterwindow.cpp
@@ -123,7 +123,6 @@ UpdaterWindow::UpdaterWindow(const std::string &updateHost,
     Window(_("Updating...")),
     mUpdateHost(updateHost),
     mUpdatesDir(updatesDir),
-    mCurrentFile("news.txt"),
     mLoadUpdates(applyUpdates),
     mLinkHandler(std::make_unique<ItemLinkHandler>(this))
 {
@@ -161,41 +160,22 @@ UpdaterWindow::UpdaterWindow(const std::string &updateHost,
     setVisible(true);
     mCancelButton->requestFocus();
 
-    // Try to download the updates list
-    download();
+    startDownload("news.txt", true);
 }
 
 UpdaterWindow::~UpdaterWindow()
 {
     if (mLoadUpdates)
         loadUpdates();
-
-    if (mDownload)
-    {
-        mDownload->cancel();
-
-        // Make sure thread is gone before freeing the memory buffer
-        mDownload.reset();
-    }
-
-    free(mMemoryBuffer);
-}
-
-void UpdaterWindow::setProgress(float progress)
-{
-    // Do delayed progress bar update, since Guichan isn't thread-safe
-    MutexLocker lock(&mDownloadMutex);
-    mDownloadProgress = progress;
 }
 
 void UpdaterWindow::setLabel(const std::string &str)
 {
-    // Do delayed label text update, since Guichan isn't thread-safe
-    MutexLocker lock(&mDownloadMutex);
-    mNewLabelCaption = str;
+    mLabel->setCaption(str);
+    mLabel->adjustSize();
 }
 
-void UpdaterWindow::enable()
+void UpdaterWindow::enablePlay()
 {
     mCancelButton->setEnabled(false);
     mPlayButton->setEnabled(true);
@@ -205,20 +185,9 @@ void UpdaterWindow::enable()
 void UpdaterWindow::action(const gcn::ActionEvent &event)
 {
     if (event.getId() == "cancel")
-    {
-        // Register the user cancel
-        mUserCancel = true;
-        // Skip the updating process
-        if (mDownloadStatus != UPDATE_COMPLETE)
-        {
-            mDownload->cancel();
-            mDownloadStatus = UPDATE_ERROR;
-        }
-    }
+        cancel();
     else if (event.getId() == "play")
-    {
-        Client::setState(STATE_LOAD_DATA);
-    }
+        play();
 }
 
 void UpdaterWindow::keyPressed(gcn::KeyEvent &keyEvent)
@@ -227,114 +196,58 @@ void UpdaterWindow::keyPressed(gcn::KeyEvent &keyEvent)
 
     if (key.getValue() == Key::ESCAPE)
     {
-        action(gcn::ActionEvent(nullptr, mCancelButton->getActionEventId()));
-        Client::setState(STATE_WORLD_SELECT);
+        if (!cancel())
+        {
+            mLoadUpdates = false;
+            Client::setState(STATE_WORLD_SELECT);
+        }
     }
     else if (key.getValue() == Key::ENTER)
     {
-        if (mDownloadStatus == UPDATE_COMPLETE ||
-                mDownloadStatus == UPDATE_ERROR)
-        {
-            action(gcn::ActionEvent(nullptr, mPlayButton->getActionEventId()));
-        }
-        else
-        {
-            action(gcn::ActionEvent(nullptr, mCancelButton->getActionEventId()));
-        }
+        play();
     }
 }
 
-void UpdaterWindow::loadNews()
+bool UpdaterWindow::cancel()
 {
-    if (!mMemoryBuffer)
+    // Skip the updating process
+    if (mDialogState != DialogState::DONE)
     {
-        logger->log("Couldn't load news");
-        return;
+        mDownload->cancel();
+        return true;
     }
-
-    mBrowserBox->clearRows();
-    mBrowserBox->addRows(std::string_view(mMemoryBuffer, mDownloadedBytes));
-
-    // Free the memory buffer now that we don't need it anymore
-    free(mMemoryBuffer);
-    mMemoryBuffer = nullptr;
-
-    mScrollArea->setVerticalScrollAmount(0);
+    return false;
 }
 
-int UpdaterWindow::updateProgress(void *ptr, DownloadStatus status,
-                                  size_t dltotal, size_t dlnow)
+void UpdaterWindow::play()
 {
-    auto *uw = reinterpret_cast<UpdaterWindow *>(ptr);
-
-    if (status == DOWNLOAD_STATUS_COMPLETE)
-    {
-        uw->mDownloadComplete = true;
-    }
-    else if (status == DOWNLOAD_STATUS_ERROR ||
-             status == DOWNLOAD_STATUS_CANCELLED)
-    {
-        uw->mDownloadStatus = UPDATE_ERROR;
-    }
-
-    float progress = 0.0f;
-    if (dltotal > 0)
-        progress = static_cast<float>(dlnow) / dltotal;
-
-    uw->setLabel(
-            uw->mCurrentFile + " (" + toString((int) (progress * 100)) + "%)");
-    uw->setProgress(progress);
-
-    if (Client::getState() != STATE_UPDATE)
-    {
-        // If the action was canceled return an error code to stop the mThread
-        return -1;
-    }
-
-    return 0;
+    if (mPlayButton->isEnabled())
+        Client::setState(STATE_LOAD_DATA);
 }
 
-size_t UpdaterWindow::memoryWrite(char *ptr, size_t size, size_t nmemb, void *stream)
+void UpdaterWindow::loadNews()
 {
-    auto *uw = reinterpret_cast<UpdaterWindow *>(stream);
-    const size_t totalMem = size * nmemb;
-    uw->mMemoryBuffer = (char*) realloc(uw->mMemoryBuffer,
-                                        uw->mDownloadedBytes + totalMem);
-    if (uw->mMemoryBuffer)
-    {
-        memcpy(uw->mMemoryBuffer + uw->mDownloadedBytes, ptr, totalMem);
-        uw->mDownloadedBytes += totalMem;
-    }
+    mBrowserBox->clearRows();
+    mBrowserBox->addRows(mDownload->getBuffer());
 
-    return totalMem;
+    mScrollArea->setVerticalScrollAmount(0);
 }
 
-void UpdaterWindow::download()
+void UpdaterWindow::startDownload(const std::string &fileName,
+                                  bool storeInMemory,
+                                  std::optional<unsigned long> adler32)
 {
-    mDownload = std::make_unique<Net::Download>(this,
-                                                mUpdateHost + "/" + mCurrentFile,
-                                                &UpdaterWindow::updateProgress);
+    mDownload = std::make_unique<Net::Download>(mUpdateHost + "/" + fileName);
+    mCurrentFile = fileName;
 
-    if (mStoreInMemory)
-    {
-        mDownload->setWriteFunction(UpdaterWindow::memoryWrite);
-    }
+    if (storeInMemory)
+        mDownload->setUseBuffer();
     else
-    {
-        std::optional<unsigned long> adler32;
-        if (mDownloadStatus == UPDATE_RESOURCES)
-            adler32 = mCurrentChecksum;
-
-        mDownload->setFile(mUpdatesDir + "/" + mCurrentFile, adler32);
-    }
+        mDownload->setFile(mUpdatesDir + "/" + fileName, adler32);
 
-    if (mDownloadStatus != UPDATE_RESOURCES)
+    if (mDialogState != DialogState::DOWNLOAD_RESOURCES)
         mDownload->noCache();
 
-    setLabel(mCurrentFile + " (0%)");
-    mDownloadComplete = false;
-
-    // TODO: check return
     mDownload->start();
 }
 
@@ -359,127 +272,132 @@ void UpdaterWindow::loadUpdates()
 
 void UpdaterWindow::logic()
 {
-    const std::string xmlUpdateFile = "resources.xml";
-    const std::string txtUpdateFile = "resources2.txt";
+    Window::logic();
 
-    // Update Scroll logic
-    mScrollArea->logic();
+    if (mDialogState == DialogState::DONE)
+        return;
 
-    // Synchronize label caption when necessary
-    {
-        MutexLocker lock(&mDownloadMutex);
+    const auto state = mDownload->getState();
+    float progress = 0.0f;
 
-        if (mLabel->getCaption() != mNewLabelCaption)
-        {
-            mLabel->setCaption(mNewLabelCaption);
-            mLabel->adjustSize();
-        }
+    switch (state.status) {
+    case DownloadStatus::IN_PROGRESS: {
+        setLabel(mCurrentFile + " (" + toString((int) (state.progress * 100)) + "%)");
+        progress = state.progress;
+        break;
+    }
+
+    case DownloadStatus::CANCELED:
+        mDialogState = DialogState::DONE;
+
+        enablePlay();
+        setLabel(_("Download canceled"));
+        break;
+
+    case DownloadStatus::ERROR: {
+        mDialogState = DialogState::DONE;
 
-        mProgressBar->setProgress(mDownloadProgress);
+        std::string error = "##1";
+        error += mDownload->getError();
+        error += "\n\n##1";
+        error += _("The update process is incomplete. "
+                   "It is strongly recommended that you try again later.");
+        mBrowserBox->addRows(error);
+
+        int maxScroll = mScrollArea->getVerticalMaxScroll();
+        mScrollArea->setVerticalScrollAmount(maxScroll);
+
+        enablePlay();
+        setLabel(_("Error while downloading"));
+        break;
+    }
+
+    case DownloadStatus::COMPLETE:
+        downloadCompleted();
+        break;
     }
 
-    switch (mDownloadStatus)
+    mProgressBar->setProgress(progress);
+}
+
+void UpdaterWindow::downloadCompleted()
+{
+    switch (mDialogState)
     {
-        case UPDATE_ERROR: {
-            std::string error = "##1";
-            error += mDownload->getError();
-            error += "\n\n";
-            error += _("The update process is incomplete. "
-                       "It is strongly recommended that you try again later.");
-            mBrowserBox->addRows(error);
-
-            mScrollArea->setVerticalScrollAmount(
-                    mScrollArea->getVerticalMaxScroll());
-            mDownloadStatus = UPDATE_COMPLETE;
-            break;
-        }
-        case UPDATE_NEWS:
-            if (mDownloadComplete)
-            {
-                // Parse current memory buffer as news and dispose of the data
-                loadNews();
+    case DialogState::DOWNLOAD_NEWS:
+        loadNews();
 
-                mCurrentFile = xmlUpdateFile;
-                mStoreInMemory = false;
-                mDownloadStatus = UPDATE_LIST;
-                download(); // download() changes mDownloadComplete to false
-            }
-            break;
-        case UPDATE_LIST:
-            if (mDownloadComplete)
+        mDialogState = DialogState::DOWNLOAD_LIST;
+        startDownload(xmlUpdateFile, false);
+        break;
+
+    case DialogState::DOWNLOAD_LIST:
+        if (mCurrentFile == xmlUpdateFile)
+        {
+            mUpdateFiles = loadXMLFile(mUpdatesDir + "/" + xmlUpdateFile);
+            if (mUpdateFiles.empty())
             {
-                if (mCurrentFile == xmlUpdateFile)
-                {
-                    mUpdateFiles = loadXMLFile(mUpdatesDir + "/" + xmlUpdateFile);
-                    if (mUpdateFiles.empty())
-                    {
-                        logger->log("Warning this server does not have a %s"
-                                    " file falling back to %s",
-                                    xmlUpdateFile.c_str(), txtUpdateFile.c_str());
-
-                        // If the resources.xml file fails, fall back onto a older version
-                        mCurrentFile = txtUpdateFile;
-                        mStoreInMemory = false;
-                        mDownloadStatus = UPDATE_LIST;
-                        download();
-                        break;
-                    }
-                }
-                else if (mCurrentFile == txtUpdateFile)
-                {
-                    mUpdateFiles = loadTxtFile(mUpdatesDir + "/" + txtUpdateFile);
-                }
-                mStoreInMemory = false;
-                mDownloadStatus = UPDATE_RESOURCES;
+                logger->log("Warning this server does not have a %s"
+                            " file falling back to %s",
+                            xmlUpdateFile, txtUpdateFile);
+
+                // If the resources.xml file fails, fall back onto a older version
+                mDialogState = DialogState::DOWNLOAD_LIST;
+                startDownload(txtUpdateFile, false);
+                break;
             }
-            break;
-        case UPDATE_RESOURCES:
-            if (mDownloadComplete)
+        }
+        else if (mCurrentFile == txtUpdateFile)
+        {
+            mUpdateFiles = loadTxtFile(mUpdatesDir + "/" + txtUpdateFile);
+        }
+
+        mDialogState = DialogState::DOWNLOAD_RESOURCES;
+        break;
+
+    case DialogState::DOWNLOAD_RESOURCES:
+        if (mUpdateIndex < mUpdateFiles.size())
+        {
+            const UpdateFile &thisFile = mUpdateFiles[mUpdateIndex];
+            if (!thisFile.required)
             {
-                if (mUpdateIndex < mUpdateFiles.size())
+                if (!(thisFile.type == "music" && config.downloadMusic))
                 {
-                    const UpdateFile &thisFile = mUpdateFiles[mUpdateIndex];
-                    if (!thisFile.required)
-                    {
-                        if (!(thisFile.type == "music" && config.downloadMusic))
-                        {
-                            mUpdateIndex++;
-                            break;
-                        }
-                    }
-                    mCurrentFile = thisFile.name;
-                    std::stringstream ss(thisFile.hash);
-                    ss >> std::hex >> mCurrentChecksum;
-
-                    std::string filename = mUpdatesDir + "/" + mCurrentFile;
-                    FILE *file = fopen(filename.c_str(), "r+b");
-
-                    if (!file || Net::Download::fadler32(file) != mCurrentChecksum)
-                    {
-                        if (file)
-                            fclose(file);
-                        download();
-                    }
-                    else
-                    {
-                        fclose(file);
-                        logger->log("%s already here", mCurrentFile.c_str());
-                    }
                     mUpdateIndex++;
-                }
-                else
-                {
-                    // Download of updates completed
-                    mDownloadStatus = UPDATE_COMPLETE;
+                    break;
                 }
             }
-            break;
-        case UPDATE_COMPLETE:
-            enable();
+
+            unsigned long checksum;
+            std::stringstream ss(thisFile.hash);
+            ss >> std::hex >> checksum;
+
+            std::string filename = mUpdatesDir + "/" + thisFile.name;
+            FILE *file = fopen(filename.c_str(), "r+b");
+
+            if (!file || Net::Download::fadler32(file) != checksum)
+            {
+                if (file)
+                    fclose(file);
+                startDownload(thisFile.name, false, checksum);
+            }
+            else
+            {
+                fclose(file);
+                logger->log("%s already here", thisFile.name.c_str());
+            }
+            mUpdateIndex++;
+        }
+        else
+        {
+            // Download of updates completed
+            mDialogState = DialogState::DONE;
+            enablePlay();
             setLabel(_("Completed"));
-            mDownloadStatus = UPDATE_IDLE;
-            break;
-        case UPDATE_IDLE:
-            break;
+        }
+        break;
+
+    case DialogState::DONE:
+        break;
     }
 }
diff --git a/src/gui/updaterwindow.h b/src/gui/updaterwindow.h
index 5c749e18..249da067 100644
--- a/src/gui/updaterwindow.h
+++ b/src/gui/updaterwindow.h
@@ -25,8 +25,6 @@
 
 #include "net/download.h"
 
-#include "utils/mutex.h"
-
 #include <guichan/actionlistener.hpp>
 #include <guichan/keylistener.hpp>
 
@@ -73,27 +71,6 @@ class UpdaterWindow : public Window, public gcn::ActionListener,
 
     ~UpdaterWindow() override;
 
-    /**
-     * Set's progress bar status
-     */
-    void setProgress(float progress);
-
-    /**
-     * Set's label above progress
-     */
-    void setLabel(const std::string &);
-
-    /**
-     * Enables play button
-     */
-    void enable();
-
-    /**
-     * Loads and display news. Assumes the news file contents have been loaded
-     * into the memory buffer.
-     */
-    void loadNews();
-
     void action(const gcn::ActionEvent &event) override;
 
     void keyPressed(gcn::KeyEvent &keyEvent) override;
@@ -101,38 +78,38 @@ class UpdaterWindow : public Window, public gcn::ActionListener,
     void logic() override;
 
 private:
-    void download();
+    bool cancel();
+    void play();
 
-    /**
-     * Loads the updates this window has gotten into the resource manager
-     */
-    void loadUpdates();
+    void setLabel(const std::string &);
+    void enablePlay();
 
+    void startDownload(const std::string &fileName,
+                       bool storeInMemory,
+                       std::optional<unsigned long> adler32 = {});
+    void downloadCompleted();
 
     /**
-     * A download callback for progress updates.
+     * Loads and display news. Assumes the news file contents have been loaded
+     * into the memory buffer.
      */
-    static int updateProgress(void *ptr, DownloadStatus status,
-                              size_t dltotal, size_t dlnow);
+    void loadNews();
 
     /**
-     * A libcurl callback for writing to memory.
+     * Loads the updates this window has gotten into the resource manager
      */
-    static size_t memoryWrite(char *ptr, size_t size, size_t nmemb,
-                              void *stream);
+    void loadUpdates();
 
-    enum UpdateDownloadStatus
+    enum class DialogState
     {
-        UPDATE_ERROR,
-        UPDATE_IDLE,
-        UPDATE_LIST,
-        UPDATE_COMPLETE,
-        UPDATE_NEWS,
-        UPDATE_RESOURCES
+        DOWNLOAD_NEWS,
+        DOWNLOAD_LIST,
+        DOWNLOAD_RESOURCES,
+        DONE,
     };
 
     /** Status of the current download. */
-    UpdateDownloadStatus mDownloadStatus = UPDATE_NEWS;
+    DialogState mDialogState = DialogState::DOWNLOAD_NEWS;
 
     /** Host where we get the updated files. */
     std::string mUpdateHost;
@@ -143,33 +120,6 @@ private:
     /** The file currently downloading. */
     std::string mCurrentFile;
 
-    /** The new label caption to be set in the logic method. */
-    std::string mNewLabelCaption;
-
-    /** The new progress value to be set in the logic method. */
-    float mDownloadProgress = 0.0f;
-
-    /** The mutex used to guard access to mNewLabelCaption and mDownloadProgress. */
-    Mutex mDownloadMutex;
-
-    /** The Adler32 checksum of the file currently downloading. */
-    unsigned long mCurrentChecksum = 0;
-
-    /** A flag to indicate whether to use a memory buffer or a regular file. */
-    bool mStoreInMemory = true;
-
-    /** Flag that show if current download is complete. */
-    bool mDownloadComplete = true;
-
-    /** Flag that show if the user has canceled the update. */
-    bool mUserCancel = false;
-
-    /** Byte count currently downloaded in mMemoryBuffer. */
-    size_t mDownloadedBytes = 0;
-
-    /** Buffer for files downloaded to memory. */
-    char *mMemoryBuffer = nullptr;
-
     /** Download handle. */
     std::unique_ptr<Net::Download> mDownload;
 
diff --git a/src/net/download.cpp b/src/net/download.cpp
index 2cfdc3a1..92656ac8 100644
--- a/src/net/download.cpp
+++ b/src/net/download.cpp
@@ -59,29 +59,25 @@ unsigned long Download::fadler32(FILE *file)
     return adler;
 }
 
-Download::Download(void *ptr,
-                   const std::string &url,
-                   DownloadUpdate updateFunction)
-    : mPtr(ptr)
-    , mUrl(url)
-    , mUpdateFunction(updateFunction)
+Download::Download(const std::string &url)
+    : mUrl(url)
 {
-    mError = (char*) malloc(CURL_ERROR_SIZE);
     mError[0] = 0;
-
-    mOptions.cancel = false;
 }
 
 Download::~Download()
 {
+    mCancel = true;
     SDL_WaitThread(mThread, nullptr);
 
     curl_slist_free_all(mHeaders);
-    free(mError);
+    free(mBuffer);
 }
 
 void Download::addHeader(const char *header)
 {
+    assert(!mThread);   // Cannot add headers after starting download
+
     mHeaders = curl_slist_append(mHeaders, header);
 }
 
@@ -94,19 +90,24 @@ void Download::noCache()
 void Download::setFile(const std::string &filename,
                        std::optional<unsigned long> adler32)
 {
-    mOptions.memoryWrite = false;
+    assert(!mThread);   // Cannot set file after starting download
+
+    mMemoryWrite = false;
     mFileName = filename;
     mAdler = adler32;
 }
 
-void Download::setWriteFunction(curl_write_callback write)
+void Download::setUseBuffer()
 {
-    mOptions.memoryWrite = true;
-    mWriteFunction = write;
+    assert(!mThread);   // Cannot set write function after starting download
+
+    mMemoryWrite = true;
 }
 
 bool Download::start()
 {
+    assert(!mThread);   // Download already started
+
     logger->log("Starting download: %s", mUrl.c_str());
 
     mThread = SDL_CreateThread(downloadThread, "Download", this);
@@ -115,8 +116,7 @@ bool Download::start()
     {
         logger->log("%s", DOWNLOAD_ERROR_MESSAGE_THREAD);
         strncpy(mError, DOWNLOAD_ERROR_MESSAGE_THREAD, CURL_ERROR_SIZE - 1);
-        mUpdateFunction(mPtr, DOWNLOAD_STATUS_THREAD_ERROR, 0, 0);
-
+        mState.lock()->status = DownloadStatus::ERROR;
         return false;
     }
 
@@ -126,23 +126,49 @@ bool Download::start()
 void Download::cancel()
 {
     logger->log("Canceling download: %s", mUrl.c_str());
-    mOptions.cancel = true;
+    mCancel = true;
 }
 
-const char *Download::getError() const
+std::string_view Download::getBuffer() const
 {
-    return mError;
+    assert(mMemoryWrite);   // Buffer not used
+    return std::string_view(mBuffer, mDownloadedBytes);
 }
 
+/**
+ * A libcurl callback for reporting progress.
+ */
 int Download::downloadProgress(void *clientp,
                                curl_off_t dltotal, curl_off_t dlnow,
                                curl_off_t ultotal, curl_off_t ulnow)
 {
     auto *d = reinterpret_cast<Download*>(clientp);
-    DownloadStatus status = d->mOptions.cancel ? DOWNLOAD_STATUS_CANCELLED
-                                               : DOWNLOAD_STATUS_IN_PROGRESS;
 
-    return d->mUpdateFunction(d->mPtr, status, (size_t) dltotal, (size_t) dlnow);
+    auto state = d->mState.lock();
+    state->status = DownloadStatus::IN_PROGRESS;
+    state->progress = 0.0f;
+    if (dltotal > 0)
+        state->progress = static_cast<float>(dlnow) / dltotal;
+
+    return d->mCancel;
+}
+
+/**
+ * A libcurl callback for writing to memory.
+ */
+size_t Download::writeBuffer(char *ptr, size_t size, size_t nmemb, void *stream)
+{
+    auto *d = reinterpret_cast<Download *>(stream);
+
+    const size_t totalMem = size * nmemb;
+    d->mBuffer = (char *) realloc(d->mBuffer, d->mDownloadedBytes + totalMem);
+    if (d->mBuffer)
+    {
+        memcpy(d->mBuffer + d->mDownloadedBytes, ptr, totalMem);
+        d->mDownloadedBytes += totalMem;
+    }
+
+    return totalMem;
 }
 
 int Download::downloadThread(void *ptr)
@@ -151,13 +177,11 @@ int Download::downloadThread(void *ptr)
     bool complete = false;
     std::string outFilename;
 
-    if (!d->mOptions.memoryWrite)
+    if (!d->mMemoryWrite)
         outFilename = d->mFileName + ".part";
 
-    for (int attempts = 0; attempts < 3 && !complete && !d->mOptions.cancel; ++attempts)
+    for (int attempts = 0; attempts < 3 && !complete && !d->mCancel; ++attempts)
     {
-        d->mUpdateFunction(d->mPtr, DOWNLOAD_STATUS_STARTING, 0, 0);
-
         CURL *curl = curl_easy_init();
         if (!curl)
             break;
@@ -169,11 +193,11 @@ int Download::downloadThread(void *ptr)
 
         FILE *file = nullptr;
 
-        if (d->mOptions.memoryWrite)
+        if (d->mMemoryWrite)
         {
             curl_easy_setopt(curl, CURLOPT_FAILONERROR, 1);
-            curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, d->mWriteFunction);
-            curl_easy_setopt(curl, CURLOPT_WRITEDATA, d->mPtr);
+            curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, &Download::writeBuffer);
+            curl_easy_setopt(curl, CURLOPT_WRITEDATA, ptr);
         }
         else
         {
@@ -189,7 +213,7 @@ int Download::downloadThread(void *ptr)
         curl_easy_setopt(curl, CURLOPT_ERRORBUFFER, d->mError);
         curl_easy_setopt(curl, CURLOPT_URL, d->mUrl.c_str());
         curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 0);
-        curl_easy_setopt(curl, CURLOPT_XFERINFOFUNCTION, downloadProgress);
+        curl_easy_setopt(curl, CURLOPT_XFERINFOFUNCTION, &Download::downloadProgress);
         curl_easy_setopt(curl, CURLOPT_XFERINFODATA, ptr);
         curl_easy_setopt(curl, CURLOPT_NOSIGNAL, 1);
         curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, 15);
@@ -199,7 +223,7 @@ int Download::downloadThread(void *ptr)
 
         if (res == CURLE_ABORTED_BY_CALLBACK)
         {
-            d->mOptions.cancel = true;
+            d->mCancel = true;
 
             if (file)
             {
@@ -224,9 +248,9 @@ int Download::downloadThread(void *ptr)
             break;
         }
 
-        if (!d->mOptions.memoryWrite)
+        if (!d->mMemoryWrite)
         {
-            // Don't check resources.xml checksum
+            // Check the checksum if available
             if (d->mAdler)
             {
                 unsigned long adler = fadler32(file);
@@ -274,13 +298,13 @@ int Download::downloadThread(void *ptr)
             fclose(file);
     }
 
-    if (!d->mOptions.cancel)
-    {
-        if (complete)
-            d->mUpdateFunction(d->mPtr, DOWNLOAD_STATUS_COMPLETE, 0, 0);
-        else
-            d->mUpdateFunction(d->mPtr, DOWNLOAD_STATUS_ERROR, 0, 0);
-    }
+    auto state = d->mState.lock();
+    if (d->mCancel)
+        state->status = DownloadStatus::CANCELED;
+    else if (complete)
+        state->status = DownloadStatus::COMPLETE;
+    else
+        state->status = DownloadStatus::ERROR;
 
     return 0;
 }
diff --git a/src/net/download.h b/src/net/download.h
index be2e374f..20a7fc74 100644
--- a/src/net/download.h
+++ b/src/net/download.h
@@ -18,22 +18,22 @@
  *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "utils/mutex.h"
+
 #include <cstdio>
-#include <string>
 #include <optional>
+#include <string>
 
 #include <curl/curl.h>
 
 #pragma once
 
-enum DownloadStatus
+enum class DownloadStatus
 {
-    DOWNLOAD_STATUS_CANCELLED = -3,
-    DOWNLOAD_STATUS_THREAD_ERROR = -2,
-    DOWNLOAD_STATUS_ERROR = -1,
-    DOWNLOAD_STATUS_STARTING = 0,
-    DOWNLOAD_STATUS_IN_PROGRESS,
-    DOWNLOAD_STATUS_COMPLETE
+    IN_PROGRESS,
+    CANCELED,
+    ERROR,
+    COMPLETE
 };
 
 struct SDL_Thread;
@@ -43,17 +43,13 @@ namespace Net {
 class Download
 {
     public:
-        /**
-         * Callback function for download updates.
-         *
-         * @param ptr       Pointer passed to Download constructor
-         * @param status    Current download status
-         * @param dltotal   Total number of bytes to download
-         * @param dlnow     Number of bytes downloaded so far
-         */
-        using DownloadUpdate = int (*)(void *, DownloadStatus, size_t, size_t);
+        struct State
+        {
+            DownloadStatus status = DownloadStatus::IN_PROGRESS;
+            float progress = 0.0f;
+        };
 
-        Download(void *ptr, const std::string &url, DownloadUpdate updateFunction);
+        Download(const std::string &url);
         ~Download();
 
         void addHeader(const char *header);
@@ -66,44 +62,66 @@ class Download
         void setFile(const std::string &filename,
                      std::optional<unsigned long> adler32 = {});
 
-        void setWriteFunction(curl_write_callback write);
+        void setUseBuffer();
 
         /**
          * Starts the download thread.
-         * @returns true  if thread was created
-         *          false if the thread could not be made or download wasn't
-         *                properly setup
+         * @returns whether the thread could be created
          */
         bool start();
 
         /**
-         * Cancels the download. Returns immediately, the cancelled status will
+         * Cancels the download. Returns immediately, the canceled status will
          * be noted in the next available update call.
          */
         void cancel();
 
+        /**
+         * Returns a view on the downloaded data.
+         */
+        std::string_view getBuffer() const;
+
+        State getState();
+
         const char *getError() const;
 
         static unsigned long fadler32(FILE *file);
 
     private:
-        static int downloadThread(void *ptr);
         static int downloadProgress(void *clientp,
                                     curl_off_t dltotal, curl_off_t dlnow,
                                     curl_off_t ultotal, curl_off_t ulnow);
-        void *mPtr;
+
+        static size_t writeBuffer(char *ptr, size_t size, size_t nmemb,
+                                  void *stream);
+
+        static int downloadThread(void *ptr);
+
+        ThreadSafe<State> mState;
         std::string mUrl;
-        struct {
-            unsigned cancel : 1;
-            unsigned memoryWrite: 1;
-        } mOptions;
+        bool mCancel = false;
+        bool mMemoryWrite = false;
         std::string mFileName;
-        curl_write_callback mWriteFunction = nullptr;
         std::optional<unsigned long> mAdler;
-        DownloadUpdate mUpdateFunction;
         SDL_Thread *mThread = nullptr;
         curl_slist *mHeaders = nullptr;
-        char *mError;
+        char mError[CURL_ERROR_SIZE];
+
+        /** Byte count currently downloaded in mMemoryBuffer. */
+        size_t mDownloadedBytes = 0;
+
+        /** Buffer for files downloaded to memory. */
+        char *mBuffer = nullptr;
 };
 
+inline Download::State Download::getState()
+{
+    return *mState.lock();
+}
+
+inline const char *Download::getError() const
+{
+    return mError;
+}
+
 } // namespace Net
diff --git a/src/net/tmwa/tradehandler.cpp b/src/net/tmwa/tradehandler.cpp
index 60732eef..c129cfd4 100644
--- a/src/net/tmwa/tradehandler.cpp
+++ b/src/net/tmwa/tradehandler.cpp
@@ -131,7 +131,7 @@ void TradeHandler::handleMessage(MessageIn &msg)
                                    "doesn't exist."));
                     break;
                 case 2: // Invite request check failed...
-                    serverNotice(_("Trade cancelled due to an unknown "
+                    serverNotice(_("Trade canceled due to an unknown "
                                    "reason."));
                     break;
                 case 3: // Trade accepted
@@ -140,11 +140,11 @@ void TradeHandler::handleMessage(MessageIn &msg)
                                                       tradePartnerName.c_str()));
                     tradeWindow->setVisible(true);
                     break;
-                case 4: // Trade cancelled
+                case 4: // Trade canceled
                     if (player_relations.hasPermission(tradePartnerName,
                                                        PlayerPermissions::SPEECH_LOG))
                     {
-                        serverNotice(strprintf(_("Trade with %s cancelled."),
+                        serverNotice(strprintf(_("Trade with %s canceled."),
                                                tradePartnerName.c_str()));
                     }
                     // otherwise ignore silently
diff --git a/src/utils/mutex.h b/src/utils/mutex.h
index 3e01eb76..a0c72e95 100644
--- a/src/utils/mutex.h
+++ b/src/utils/mutex.h
@@ -102,3 +102,42 @@ inline MutexLocker::~MutexLocker()
     if (mMutex)
         mMutex->unlock();
 }
+
+/**
+ * A template class for wrapping data that is accessed by multiple threads.
+ */
+template <typename T>
+class ThreadSafe
+{
+    class Locked : private MutexLocker
+    {
+    public:
+        Locked(T &data, Mutex &mutex)
+            : MutexLocker(&mutex)
+            , mData(data)
+        {}
+
+        Locked(Locked&& rhs) = delete;
+        Locked(const Locked&) = delete;
+        Locked& operator=(const Locked&) = delete;
+        Locked& operator=(Locked&&) = delete;
+
+        T &operator*() const { return mData; }
+        T *operator->() const { return &mData; }
+
+    private:
+        T &mData;
+    };
+
+public:
+    ThreadSafe() = default;
+    ThreadSafe(const T &data)
+        : mData(data)
+    {}
+
+    Locked lock() { return { mData, mMutex }; }
+
+private:
+    T mData;
+    Mutex mMutex;
+};