From bdf187ac419457af14e72ef442d3ebe58407652b Mon Sep 17 00:00:00 2001 From: "craig.p.drummond" Date: Mon, 17 Mar 2014 19:48:11 +0000 Subject: [PATCH] When song is updated in context view, abort any current network jobs. BUG: 442 --- ChangeLog | 1 + context/albumview.cpp | 3 +++ context/artistview.cpp | 5 +++-- context/contextengine.cpp | 2 +- context/contextwidget.cpp | 5 ++++- context/songview.cpp | 8 +++++--- context/ultimatelyricsprovider.cpp | 6 +----- gui/coverdialog.cpp | 8 +------- models/streamsearchmodel.cpp | 4 +--- network/networkaccessmanager.cpp | 18 +++++++++++++++++- network/networkaccessmanager.h | 9 +++++++-- online/podcastsearchdialog.cpp | 6 ++---- online/podcastservice.cpp | 9 ++------- online/soundcloudservice.cpp | 4 +--- streams/streamfetcher.cpp | 15 ++++----------- 15 files changed, 53 insertions(+), 50 deletions(-) diff --git a/ChangeLog b/ChangeLog index 90daaad25..afad56cb5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -65,6 +65,7 @@ 3. Fix display of online-service track album-artist in copy dialog. 4. Ensure library is loaded the very first time Cantata is run (ie. after connected to MPD via initial settings wizard). +5. When song is updated in context view, abort any current network jobs. 1.3.2 ----- diff --git a/context/albumview.cpp b/context/albumview.cpp index eb17f963c..b4caab5e8 100644 --- a/context/albumview.cpp +++ b/context/albumview.cpp @@ -107,6 +107,7 @@ void AlbumView::refresh() void AlbumView::update(const Song &song, bool force) { if (song.isStandardStream() && song.album.isEmpty() && !song.name.isEmpty() && song.name!=currentSong.name) { + abort(); currentSong=song; clearDetails(); setHeader(song.name); @@ -120,12 +121,14 @@ void AlbumView::update(const Song &song, bool force) if (song.isEmpty() || song.albumArtist().isEmpty() || song.album.isEmpty()) { currentSong=song; clearDetails(); + abort(); return; } if (force || song.albumArtist()!=currentSong.albumArtist() || song.album!=currentSong.album) { currentSong=song; currentArtist=currentSong.basicArtist(); + abort(); if (!isVisible()) { needToUpdate=true; return; diff --git a/context/artistview.cpp b/context/artistview.cpp index 97b869ab9..b7d2a121d 100644 --- a/context/artistview.cpp +++ b/context/artistview.cpp @@ -134,6 +134,7 @@ void ArtistView::update(const Song &s, bool force) currentSong=s; engine->cancel(); clear(); + abort(); return; } @@ -143,6 +144,7 @@ void ArtistView::update(const Song &s, bool force) if (artistChanged) { artistAlbumsFirstTracks.clear(); + abort(); } if (!isVisible()) { if (artistChanged) { @@ -370,8 +372,7 @@ void ArtistView::abort() { engine->cancel(); if (currentSimilarJob) { - disconnect(currentSimilarJob, SIGNAL(finished()), this, SLOT(handleSimilarReply())); - currentSimilarJob->abort(); + currentSimilarJob->cancelAndDelete(); currentSimilarJob=0; } hideSpinner(); diff --git a/context/contextengine.cpp b/context/contextengine.cpp index 04e739fa5..c98ffebdf 100644 --- a/context/contextengine.cpp +++ b/context/contextengine.cpp @@ -61,7 +61,7 @@ QStringList ContextEngine::fixQuery(const QStringList &query) const void ContextEngine::cancel() { if (job) { - job->deleteLater(); + job->cancelAndDelete(); job=0; } } diff --git a/context/contextwidget.cpp b/context/contextwidget.cpp index 5efd819fe..9a1f7b74e 100644 --- a/context/contextwidget.cpp +++ b/context/contextwidget.cpp @@ -643,6 +643,9 @@ void ContextWidget::update(const Song &s) } } + if (s.albumArtist()!=currentSong.albumArtist()) { + cancel(); + } artist->update(sng); album->update(sng); song->update(sng); @@ -673,7 +676,7 @@ bool ContextWidget::eventFilter(QObject *o, QEvent *e) void ContextWidget::cancel() { if (job) { - job->deleteLater(); + job->cancelAndDelete(); job=0; } } diff --git a/context/songview.cpp b/context/songview.cpp index 7e382ef86..266444e86 100644 --- a/context/songview.cpp +++ b/context/songview.cpp @@ -332,9 +332,7 @@ void SongView::scroll() void SongView::abort() { if (job) { - connect(job, SIGNAL(finished()), this, SLOT(downloadFinished())); - job->abort(); - job->deleteLater(); + job->cancelAndDelete(); job=0; } currentProvider=-1; @@ -361,12 +359,16 @@ void SongView::update(const Song &s, bool force) if (s.isEmpty() || s.title.isEmpty() || s.artist.isEmpty()) { currentSong=s; clear(); + abort(); return; } Song song(s); bool songChanged = song.artist!=currentSong.artist || song.title!=currentSong.title; + if (songChanged) { + abort(); + } if (!isVisible()) { if (songChanged) { needToUpdate=true; diff --git a/context/ultimatelyricsprovider.cpp b/context/ultimatelyricsprovider.cpp index f67fbc3dd..9ea0e3dae 100644 --- a/context/ultimatelyricsprovider.cpp +++ b/context/ultimatelyricsprovider.cpp @@ -283,11 +283,7 @@ void UltimateLyricsProvider::abort() QHash::ConstIterator end(requests.constEnd()); for (; it!=end; ++it) { - disconnect(it.key(), SIGNAL(finished()), this, SLOT(lyricsFetched())); - disconnect(it.key(), SIGNAL(finished()), this, SLOT(wikiMediaSearchResponse())); - disconnect(it.key(), SIGNAL(finished()), this, SLOT(wikiMediaLyricsFetched())); - it.key()->deleteLater(); - it.key()->close(); + it.key()->cancelAndDelete(); } requests.clear(); songs.clear(); diff --git a/gui/coverdialog.cpp b/gui/coverdialog.cpp index cb603773b..9c78417b1 100644 --- a/gui/coverdialog.cpp +++ b/gui/coverdialog.cpp @@ -858,13 +858,7 @@ void CoverDialog::checkStatus() void CoverDialog::cancelQuery() { foreach (NetworkJob *job, currentQuery) { - if (DL_Query==job->property(constTypeProperty).toInt()) { - disconnect(job, SIGNAL(finished()), this, SLOT(queryJobFinished())); - } else { - disconnect(job, SIGNAL(finished()), this, SLOT(downloadJobFinished())); - } - job->close(); - job->deleteLater(); + job->cancelAndDelete(); } currentQuery.clear(); setSearching(false); diff --git a/models/streamsearchmodel.cpp b/models/streamsearchmodel.cpp index 14fbe6007..f11f25003 100644 --- a/models/streamsearchmodel.cpp +++ b/models/streamsearchmodel.cpp @@ -334,9 +334,7 @@ void StreamSearchModel::cancelAll() if (!jobs.isEmpty()) { QList jobList=jobs.keys(); foreach (NetworkJob *j, jobList) { - j->abort(); - j->deleteLater(); - disconnect(j, SIGNAL(finished()), this, SLOT(jobFinished())); + j->cancelAndDelete(); } jobs.clear(); emit loaded(); diff --git a/network/networkaccessmanager.cpp b/network/networkaccessmanager.cpp index 29a675556..e35df2115 100644 --- a/network/networkaccessmanager.cpp +++ b/network/networkaccessmanager.cpp @@ -68,10 +68,26 @@ NetworkJob::NetworkJob(QNetworkReply *j) } NetworkJob::~NetworkJob() +{ + cancelJob(); +} + +void NetworkJob::cancelAndDelete() +{ + cancelJob(); + deleteLater(); +} + +void NetworkJob::cancelJob() { if (job) { disconnect(job, SIGNAL(finished()), this, SLOT(jobFinished())); + disconnect(job, SIGNAL(readyRead()), this, SLOT(handleReadyRead())); + disconnect(job, SIGNAL(error(QNetworkReply::NetworkError)), this, SIGNAL(error(QNetworkReply::NetworkError))); + disconnect(job, SIGNAL(uploadProgress(qint64, qint64)), this, SIGNAL(uploadProgress(qint64, qint64))); + disconnect(job, SIGNAL(downloadProgress(qint64, qint64)), this, SLOT(downloadProg(qint64, qint64))); disconnect(job, SIGNAL(destroyed(QObject *)), this, SLOT(jobDestroyed(QObject *))); + job->close(); job->abort(); job->deleteLater(); job=0; @@ -201,6 +217,6 @@ void NetworkAccessManager::timerEvent(QTimerEvent *e) { NetworkJob *job = timers.key(e->timerId()); if (job) { - job->abort(); + job->cancelAndDelete(); } } diff --git a/network/networkaccessmanager.h b/network/networkaccessmanager.h index 10a910329..0f68dab22 100644 --- a/network/networkaccessmanager.h +++ b/network/networkaccessmanager.h @@ -44,13 +44,12 @@ class NetworkJob : public QObject Q_OBJECT public: - NetworkJob(NetworkAccessManager *p, const QUrl &u); NetworkJob(QNetworkReply *j); virtual ~NetworkJob(); QNetworkReply * actualJob() const { return job; } - void abort() { if (job) job->abort(); } + void cancelAndDelete(); bool open(QIODevice::OpenMode mode) { return job && job->open(mode); } void close() { if (job) job->close(); } @@ -79,11 +78,17 @@ private Q_SLOTS: void downloadProg(qint64 bytesReceived, qint64 bytesTotal); void handleReadyRead(); +private: + NetworkJob(NetworkAccessManager *p, const QUrl &u); + void cancelJob(); + private: int numRedirects; int lastDownloadPc; QNetworkReply *job; QUrl origU; + + friend class NetworkAccessManager; }; class NetworkAccessManager : public BASE_NETWORK_ACCESS_MANAGER diff --git a/online/podcastsearchdialog.cpp b/online/podcastsearchdialog.cpp index 5d6b594d2..0153ebbf7 100644 --- a/online/podcastsearchdialog.cpp +++ b/online/podcastsearchdialog.cpp @@ -186,8 +186,7 @@ void PodcastPage::cancel() { spinner->stop(); if (job) { - disconnect(job, SIGNAL(finished()), this, SLOT(jobFinished())); - job->deleteLater(); + job->cancelAndDelete(); job=0; } } @@ -196,8 +195,7 @@ void PodcastPage::cancelImage() { imageSpinner->stop(); if (imageJob) { - disconnect(imageJob, SIGNAL(finished()), this, SLOT(imageJobFinished())); - imageJob->deleteLater(); + imageJob->cancelAndDelete(); imageJob=0; } } diff --git a/online/podcastservice.cpp b/online/podcastservice.cpp index c56c3f9e9..b58016f42 100644 --- a/online/podcastservice.cpp +++ b/online/podcastservice.cpp @@ -177,8 +177,7 @@ void PodcastService::cancelAll() { foreach (NetworkJob *j, rssJobs) { disconnect(j, SIGNAL(finished()), this, SLOT(rssJobFinished())); - j->abort(); - j->deleteLater(); + j->cancelAndDelete(); } rssJobs.clear(); setBusy(!rssJobs.isEmpty() || !downloadJobs.isEmpty()); @@ -483,11 +482,7 @@ void PodcastService::cancelDownload(const QUrl &url) void PodcastService::cancelDownload(NetworkJob *job) { - disconnect(job, SIGNAL(finished()), this, SLOT(downloadJobFinished())); - disconnect(job, SIGNAL(readyRead()), this, SLOT(downloadReadyRead())); - disconnect(job, SIGNAL(downloadPercent(int)), this, SLOT(downloadPercent(int))); - job->abort(); - job->deleteLater(); + job->cancelAndDelete(); QString dest=job->property(constDestProperty).toString(); QString partial=dest.isEmpty() ? QString() : QString(dest+constPartialExt); diff --git a/online/soundcloudservice.cpp b/online/soundcloudservice.cpp index 949dc0315..118dd0fcf 100644 --- a/online/soundcloudservice.cpp +++ b/online/soundcloudservice.cpp @@ -103,9 +103,7 @@ void SoundCloudService::setSearch(const QString &searchTerm) void SoundCloudService::cancelAll() { if (job) { - disconnect(job, SIGNAL(finished()), this, SLOT(jobFinished())); - job->abort(); - job->deleteLater(); + job->cancelAndDelete(); job=0; } setBusy(false); diff --git a/streams/streamfetcher.cpp b/streams/streamfetcher.cpp index 2b602ff25..e41a5f62f 100644 --- a/streams/streamfetcher.cpp +++ b/streams/streamfetcher.cpp @@ -239,12 +239,9 @@ void StreamFetcher::cancel() data.clear(); current=QString(); if (job) { - disconnect(job, SIGNAL(readyRead()), this, SLOT(dataReady())); - disconnect(job, SIGNAL(finished()), this, SLOT(jobFinished())); - job->abort(); - job->deleteLater(); + job->cancelAndDelete(); + job=0; } - job=0; emit status(QString()); } @@ -253,12 +250,8 @@ void StreamFetcher::dataReady() data+=job->readAll(); if (data.count()>constMaxData) { - NetworkJob *thisJob=job; - disconnect(thisJob, SIGNAL(readyRead()), this, SLOT(dataReady())); - disconnect(thisJob, SIGNAL(finished()), this, SLOT(jobFinished())); - jobFinished(thisJob); - thisJob->abort(); - thisJob->deleteLater(); + job->cancelAndDelete(); + job=0; } }