Remove implicit auto-delete from web tasks

6 files changed, 118 insertions, 83 deletions

diff --git a/src/musicbrainz/tagfetcher.cpp b/src/musicbrainz/tagfetcher.cpp
index 1435aa2fa5..3c80410a52 100644
--- a/src/musicbrainz/tagfetcher.cpp
+++ b/src/musicbrainz/tagfetcher.cpp
@@ -36,30 +36,17 @@ void TagFetcher::startFetch(
             &TagFetcher::slotFingerprintReady);
 }
 
-void TagFetcher::abortAcoustIdTask() {
+void TagFetcher::cancel() {
+    m_pTrack.reset();
+    m_fingerprintWatcher.cancel();
     if (m_pAcoustIdTask) {
-        disconnect(m_pAcoustIdTask);
-        m_pAcoustIdTask->deleteBeforeFinished();
-        m_pAcoustIdTask = nullptr;
+        m_pAcoustIdTask->invokeAbort();
     }
-}
-
-void TagFetcher::abortMusicBrainzTask() {
     if (m_pMusicBrainzTask) {
-        disconnect(m_pMusicBrainzTask);
-        m_pMusicBrainzTask->deleteBeforeFinished();
-        m_pMusicBrainzTask = nullptr;
+        m_pMusicBrainzTask->invokeAbort();
     }
 }
 
-void TagFetcher::cancel() {
-    // qDebug()<< "Cancel tagfetching";
-    m_fingerprintWatcher.cancel();
-    abortAcoustIdTask();
-    abortMusicBrainzTask();
-    m_pTrack.reset();
-}
-
 void TagFetcher::slotFingerprintReady() {
     if (!m_pTrack ||
             !m_fingerprintWatcher.isFinished()) {
@@ -74,14 +61,13 @@ void TagFetcher::slotFingerprintReady() {
         return;
     }
 
-    abortAcoustIdTask();
-
     emit fetchProgress(tr("Identifying track through Acoustid"));
     DEBUG_ASSERT(!m_pAcoustIdTask);
-    m_pAcoustIdTask = new mixxx::AcoustIdLookupTask(
+    m_pAcoustIdTask = make_parented<mixxx::AcoustIdLookupTask>(
             &m_network,
             fingerprint,
-            m_pTrack->getDurationInt());
+            m_pTrack->getDurationInt(),
+            this);
     connect(m_pAcoustIdTask,
             &mixxx::AcoustIdLookupTask::succeeded,
             this,
@@ -104,7 +90,12 @@ void TagFetcher::slotFingerprintReady() {
 
 void TagFetcher::slotAcoustIdTaskSucceeded(
         QList<QUuid> recordingIds) {
-    abortAcoustIdTask();
+    VERIFY_OR_DEBUG_ASSERT(m_pAcoustIdTask) {
+        return;
+    }
+    m_pAcoustIdTask->deleteAfterFinished();
+    m_pAcoustIdTask = nullptr;
+
     if (!m_pTrack) {
         return;
     }
@@ -118,9 +109,10 @@ void TagFetcher::slotAcoustIdTaskSucceeded(
 
     emit fetchProgress(tr("Retrieving metadata from MusicBrainz"));
     DEBUG_ASSERT(!m_pMusicBrainzTask);
-    m_pMusicBrainzTask = new mixxx::MusicBrainzRecordingsTask(
+    m_pMusicBrainzTask = make_parented<mixxx::MusicBrainzRecordingsTask>(
             &m_network,
-            std::move(recordingIds));
+            std::move(recordingIds),
+            this);
     connect(m_pMusicBrainzTask,
             &mixxx::MusicBrainzRecordingsTask::succeeded,
             this,
@@ -143,7 +135,11 @@ void TagFetcher::slotAcoustIdTaskSucceeded(
 
 void TagFetcher::slotAcoustIdTaskFailed(
         mixxx::network::JsonWebResponse response) {
-    cancel();
+    VERIFY_OR_DEBUG_ASSERT(m_pAcoustIdTask) {
+        return;
+    }
+    m_pAcoustIdTask->deleteAfterFinished();
+    m_pAcoustIdTask = nullptr;
     emit networkError(
             response.statusCode,
             "AcoustID",
@@ -152,7 +148,11 @@ void TagFetcher::slotAcoustIdTaskFailed(
 }
 
 void TagFetcher::slotAcoustIdTaskAborted() {
-    abortAcoustIdTask();
+    VERIFY_OR_DEBUG_ASSERT(m_pAcoustIdTask) {
+        return;
+    }
+    m_pAcoustIdTask->deleteAfterFinished();
+    m_pAcoustIdTask = nullptr;
 }
 
 void TagFetcher::slotAcoustIdTaskNetworkError(
@@ -162,7 +162,11 @@ void TagFetcher::slotAcoustIdTaskNetworkError(
         QByteArray errorContent) {
     Q_UNUSED(requestUrl);
     Q_UNUSED(errorContent);
-    cancel();
+    VERIFY_OR_DEBUG_ASSERT(m_pAcoustIdTask) {
+        return;
+    }
+    m_pAcoustIdTask->deleteAfterFinished();
+    m_pAcoustIdTask = nullptr;
     emit networkError(
             mixxx::network::kHttpStatusCodeInvalid,
             "AcoustID",
@@ -171,7 +175,11 @@ void TagFetcher::slotAcoustIdTaskNetworkError(
 }
 
 void TagFetcher::slotMusicBrainzTaskAborted() {
-    abortMusicBrainzTask();
+    VERIFY_OR_DEBUG_ASSERT(m_pMusicBrainzTask) {
+        return;
+    }
+    m_pMusicBrainzTask->deleteAfterFinished();
+    m_pMusicBrainzTask = nullptr;
 }
 
 void TagFetcher::slotMusicBrainzTaskNetworkError(
@@ -181,7 +189,11 @@ void TagFetcher::slotMusicBrainzTaskNetworkError(
         QByteArray errorContent) {
     Q_UNUSED(requestUrl);
     Q_UNUSED(errorContent);
-    cancel();
+    VERIFY_OR_DEBUG_ASSERT(m_pMusicBrainzTask) {
+        return;
+    }
+    m_pMusicBrainzTask->deleteAfterFinished();
+    m_pMusicBrainzTask = nullptr;
     emit networkError(
             mixxx::network::kHttpStatusCodeInvalid,
             "MusicBrainz",
@@ -193,7 +205,11 @@ void TagFetcher::slotMusicBrainzTaskFailed(
         mixxx::network::WebResponse response,
         int errorCode,
         QString errorMessage) {
-    cancel();
+    VERIFY_OR_DEBUG_ASSERT(m_pMusicBrainzTask) {
+        return;
+    }
+    m_pMusicBrainzTask->deleteAfterFinished();
+    m_pMusicBrainzTask = nullptr;
     emit networkError(
             response.statusCode,
             "MusicBrainz",
@@ -204,7 +220,12 @@ void TagFetcher::slotMusicBrainzTaskFailed(
 void TagFetcher::slotMusicBrainzTaskSucceeded(
         QList<mixxx::musicbrainz::TrackRelease> guessedTrackReleases) {
     auto pOriginalTrack = std::move(m_pTrack);
-    abortMusicBrainzTask();
+    DEBUG_ASSERT(!m_pTrack);
+    VERIFY_OR_DEBUG_ASSERT(m_pMusicBrainzTask) {
+        return;
+    }
+    m_pMusicBrainzTask->deleteAfterFinished();
+    m_pMusicBrainzTask = nullptr;
     if (!pOriginalTrack) {
         // aborted
         return;
diff --git a/src/musicbrainz/tagfetcher.h b/src/musicbrainz/tagfetcher.h
index 622cebe11c..3caf4ac7e3 100644
--- a/src/musicbrainz/tagfetcher.h
+++ b/src/musicbrainz/tagfetcher.h
@@ -2,23 +2,24 @@
 
 #include <QFutureWatcher>
 #include <QObject>
-#include <QPointer>
 
 #include "musicbrainz/web/acoustidlookuptask.h"
 #include "musicbrainz/web/musicbrainzrecordingstask.h"
 #include "track/track.h"
+#include "util/parented_ptr.h"
 
 class TagFetcher : public QObject {
-  Q_OBJECT
+    Q_OBJECT
 
-  // Implements retrieval of metadata in 3 subsequent stages:
-  //   1. Chromaprint -> AcoustID fingerprint
-  //   2. AcoustID -> MusicBrainz recording UUIDs
-  //   3. MusicBrainz -> MusicBrainz track releases
+    // Implements retrieval of metadata in 3 subsequent stages:
+    //   1. Chromaprint -> AcoustID fingerprint
+    //   2. AcoustID -> MusicBrainz recording UUIDs
+    //   3. MusicBrainz -> MusicBrainz track releases
 
   public:
     explicit TagFetcher(
             QObject* parent = nullptr);
+    ~TagFetcher() override = default;
 
     void startFetch(
             TrackPointer pTrack);
@@ -66,16 +67,13 @@ class TagFetcher : public QObject {
             QByteArray errorContent);
 
   private:
-    void abortAcoustIdTask();
-    void abortMusicBrainzTask();
-
     QNetworkAccessManager m_network;
 
     QFutureWatcher<QString> m_fingerprintWatcher;
 
-    QPointer<mixxx::AcoustIdLookupTask> m_pAcoustIdTask;
+    parented_ptr<mixxx::AcoustIdLookupTask> m_pAcoustIdTask;
 
-    QPointer<mixxx::MusicBrainzRecordingsTask> m_pMusicBrainzTask;
+    parented_ptr<mixxx::MusicBrainzRecordingsTask> m_pMusicBrainzTask;
 
     TrackPointer m_pTrack;
 };
diff --git a/src/musicbrainz/web/acoustidlookuptask.cpp b/src/musicbrainz/web/acoustidlookuptask.cpp
index 6f8a2acdfd..a9807f6b8b 100644
--- a/src/musicbrainz/web/acoustidlookuptask.cpp
+++ b/src/musicbrainz/web/acoustidlookuptask.cpp
@@ -67,11 +67,13 @@ network::JsonWebRequest lookupRequest() {
 AcoustIdLookupTask::AcoustIdLookupTask(
         QNetworkAccessManager* networkAccessManager,
         const QString& fingerprint,
-        int duration)
+        int duration,
+        QObject* parent)
         : network::JsonWebTask(
                   networkAccessManager,
                   kBaseUrl,
-                  lookupRequest()),
+                  lookupRequest(),
+                  parent),
           m_urlQuery(lookupUrlQuery(fingerprint, duration)) {
 }
 
@@ -110,7 +112,7 @@ QNetworkReply* AcoustIdLookupTask::sendNetworkRequest(
 }
 
 void AcoustIdLookupTask::onFinished(
-        network::JsonWebResponse response) {
+        network::JsonWebResponse&& response) {
     if (!response.isStatusCodeSuccess()) {
         kLogger.warning()
                 << "Request failed with HTTP status code"
@@ -198,7 +200,7 @@ void AcoustIdLookupTask::onFinished(
                 const auto recordingId =
                         QUuid(recordingObject.value(QLatin1String("id")).toString());
                 VERIFY_OR_DEBUG_ASSERT(!recordingId.isNull()) {
-                     continue;
+                    continue;
                 }
                 recordingIds.append(recordingId);
             }
@@ -209,15 +211,15 @@ void AcoustIdLookupTask::onFinished(
 
 void AcoustIdLookupTask::emitSucceeded(
         QList<QUuid>&& recordingIds) {
-    const auto signal = QMetaMethod::fromSignal(
-            &AcoustIdLookupTask::succeeded);
-    DEBUG_ASSERT(receivers(signal.name()) <= 1); // unique connection
-    if (isSignalConnected(signal)) {
-        emit succeeded(
-                std::move(recordingIds));
-    } else {
+    VERIFY_OR_DEBUG_ASSERT(
+            isSignalFuncConnected(&AcoustIdLookupTask::succeeded)) {
+        kLogger.warning()
+                << "Unhandled succeeded signal";
         deleteLater();
+        return;
     }
+    emit succeeded(
+            std::move(recordingIds));
 }
 
 } // namespace mixxx
diff --git a/src/musicbrainz/web/acoustidlookuptask.h b/src/musicbrainz/web/acoustidlookuptask.h
index 34dc457491..20a1a504a5 100644
--- a/src/musicbrainz/web/acoustidlookuptask.h
+++ b/src/musicbrainz/web/acoustidlookuptask.h
@@ -15,7 +15,8 @@ class AcoustIdLookupTask : public network::JsonWebTask {
     AcoustIdLookupTask(
             QNetworkAccessManager* networkAccessManager,
             const QString& fingerprint,
-            int duration);
+            int duration,
+            QObject* parent = nullptr);
     ~AcoustIdLookupTask() override = default;
 
   signals:
@@ -31,7 +32,7 @@ class AcoustIdLookupTask : public network::JsonWebTask {
 
   private:
     void onFinished(
-            network::JsonWebResponse response) override;
+            network::JsonWebResponse&& response) override;
 
     void emitSucceeded(
             QList<QUuid>&& recordingIds);
diff --git a/src/musicbrainz/web/musicbrainzrecordingstask.cpp b/src/musicbrainz/web/musicbrainzrecordingstask.cpp
index cccdaf673c..054338e68c 100644
--- a/src/musicbrainz/web/musicbrainzrecordingstask.cpp
+++ b/src/musicbrainz/web/musicbrainzrecordingstask.cpp
@@ -45,7 +45,7 @@ QUrlQuery createUrlQuery() {
 }
 
 QNetworkRequest createNetworkRequest(
-    const QUuid& recordingId) {
+        const QUuid& recordingId) {
     DEBUG_ASSERT(kBaseUrl.isValid());
     DEBUG_ASSERT(!recordingId.isNull());
     QUrl url = kBaseUrl;
@@ -65,14 +65,22 @@ QNetworkRequest createNetworkRequest(
 
 MusicBrainzRecordingsTask::MusicBrainzRecordingsTask(
         QNetworkAccessManager* networkAccessManager,
-        QList<QUuid>&& recordingIds)
+        QList<QUuid>&& recordingIds,
+        QObject* parent)
         : network::WebTask(
-                  networkAccessManager),
+                  networkAccessManager,
+                  parent),
           m_queuedRecordingIds(std::move(recordingIds)),
           m_parentTimeoutMillis(0) {
     musicbrainz::registerMetaTypesOnce();
 }
 
+MusicBrainzRecordingsTask::~MusicBrainzRecordingsTask() {
+    VERIFY_OR_DEBUG_ASSERT(!m_pendingNetworkReply) {
+        m_pendingNetworkReply->deleteLater();
+    }
+}
+
 bool MusicBrainzRecordingsTask::doStart(
         QNetworkAccessManager* networkAccessManager,
         int parentTimeoutMillis) {
@@ -176,13 +184,13 @@ void MusicBrainzRecordingsTask::slotNetworkReplyFinished() {
                 << "GET reply"
                 << "statusCode:" << statusCode
                 << "body:" << body;
-        const auto error = musicbrainz::Error(reader);
+        auto error = musicbrainz::Error(reader);
         emitFailed(
                 network::WebResponse(
                         networkReply->url(),
                         statusCode),
                 error.code,
-                error.message);
+                std::move(error.message));
         return;
     }
 
@@ -223,31 +231,35 @@ void MusicBrainzRecordingsTask::slotNetworkReplyFinished() {
 
 void MusicBrainzRecordingsTask::emitSucceeded(
         QList<musicbrainz::TrackRelease>&& trackReleases) {
-    const auto signal = QMetaMethod::fromSignal(
-            &MusicBrainzRecordingsTask::succeeded);
-    DEBUG_ASSERT(receivers(signal.name()) <= 1); // unique connection
-    if (isSignalConnected(signal)) {
-        emit succeeded(
-                std::move(trackReleases));
-    } else {
+    VERIFY_OR_DEBUG_ASSERT(
+            isSignalFuncConnected(&MusicBrainzRecordingsTask::succeeded)) {
+        kLogger.warning()
+                << "Unhandled succeeded signal";
         deleteLater();
+        return;
     }
+    emit succeeded(
+            std::move(trackReleases));
 }
+
 void MusicBrainzRecordingsTask::emitFailed(
-        network::WebResponse response,
+        network::WebResponse&& response,
         int errorCode,
-        QString errorMessage) {
-    const auto signal = QMetaMethod::fromSignal(
-            &MusicBrainzRecordingsTask::succeeded);
-    DEBUG_ASSERT(receivers(signal.name()) <= 1); // unique connection
-    if (isSignalConnected(signal)) {
-        emit failed(
-                response,
-                errorCode,
-                errorMessage);
-    } else {
+        QString&& errorMessage) {
+    VERIFY_OR_DEBUG_ASSERT(
+            isSignalFuncConnected(&MusicBrainzRecordingsTask::failed)) {
+        kLogger.warning()
+                << "Unhandled failed signal"
+                << response
+                << errorCode
+                << errorMessage;
         deleteLater();
+        return;
     }
+    emit failed(
+            std::move(response),
+            errorCode,
+            std::move(errorMessage));
 }
 
 } // namespace mixxx
diff --git a/src/musicbrainz/web/musicbrainzrecordingstask.h b/src/musicbrainz/web/musicbrainzrecordingstask.h
index 614315d758..e3f65e55a8 100644
--- a/src/musicbrainz/web/musicbrainzrecordingstask.h
+++ b/src/musicbrainz/web/musicbrainzrecordingstask.h
@@ -17,8 +17,9 @@ class MusicBrainzRecordingsTask : public network::WebTask {
   public:
     MusicBrainzRecordingsTask(
             QNetworkAccessManager* networkAccessManager,
-            QList<QUuid>&& recordingIds);
-    ~MusicBrainzRecordingsTask() override = default;
+            QList<QUuid>&& recordingIds,
+            QObject* parent = nullptr);
+    ~MusicBrainzRecordingsTask() override;
 
   signals:
     void succeeded(
@@ -41,9 +42,9 @@ class MusicBrainzRecordingsTask : public network::WebTask {
     void emitSucceeded(
             QList<musicbrainz::TrackRelease>&& trackReleases);
     void emitFailed(
-            network::WebResponse response,
+            network::WebResponse&& response,
             int errorCode,
-            QString errorMessage);
+            QString&& errorMessage);
 
     void continueWithNextRequest();