Fix a possible race and some wrong assumptions about file ownership.

The download archive job does not need to cleanup the files afterwards,
that's taken care of by the meta data download. That one will provide
the infrastructure where the files are downloaded at. Also prevent the
meta job from removing the files once they are fetched in case of a
cancel event (which does not entirely mean we are going to shutdown).

Change-Id: I66eeff30ef4cabb485dd4f300b2917deb7557867
Reviewed-by: Kai Koehne <kai.koehne@digia.com>
Reviewed-by: Niels Weber <niels.weber@digia.com>

3 files changed, 19 insertions, 29 deletions

diff --git a/src/libs/installer/downloadarchivesjob.cpp b/src/libs/installer/downloadarchivesjob.cpp
index 5cada20d4..7e1196bcf 100644
--- a/src/libs/installer/downloadarchivesjob.cpp
+++ b/src/libs/installer/downloadarchivesjob.cpp
@@ -74,16 +74,9 @@ DownloadArchivesJob::DownloadArchivesJob(PackageManagerCore *core)
 
 /*!
     Destroys the DownloadArchivesJob.
-    All temporary files get deleted.
 */
 DownloadArchivesJob::~DownloadArchivesJob()
 {
-    foreach (const QString &fileName, m_temporaryFiles) {
-        QFile file(fileName);
-        if (file.exists() && !file.remove())
-            qWarning("Could not delete file %s: %s", qPrintable(fileName), qPrintable(file.errorString()));
-    }
-
     if (m_downloader)
         m_downloader->deleteLater();
 }
@@ -159,8 +152,6 @@ void DownloadArchivesJob::finishedHashDownload()
     else
         finishWithError(tr("Downloading hash signature failed."));
 
-    m_temporaryFiles.insert(tempFile);
-
     fetchNextArchive();
 }
 
@@ -253,7 +244,6 @@ void DownloadArchivesJob::registerFile()
         emit progressChanged(double(m_archivesDownloaded) / m_archivesToDownloadCount);
     }
 
-    m_temporaryFiles.insert(m_downloader->downloadedFileName());
     const QPair<QString, QString> pair = m_archivesToDownload.takeFirst();
     QInstallerCreator::BinaryFormatEngineHandler::instance()->registerArchive(pair.first,
         m_downloader->downloadedFileName());
diff --git a/src/libs/installer/downloadarchivesjob.h b/src/libs/installer/downloadarchivesjob.h
index 765d2c564..25d3daaec 100644
--- a/src/libs/installer/downloadarchivesjob.h
+++ b/src/libs/installer/downloadarchivesjob.h
@@ -45,7 +45,6 @@
 #include <kdjob.h>
 
 #include <QtCore/QPair>
-#include <QtCore/QSet>
 
 QT_BEGIN_NAMESPACE
 class QTimerEvent;
@@ -65,9 +64,10 @@ class DownloadArchivesJob : public KDJob
     Q_OBJECT
 
 public:
-    explicit DownloadArchivesJob(PackageManagerCore *core = 0);
+    explicit DownloadArchivesJob(PackageManagerCore *core);
     ~DownloadArchivesJob();
 
+    int numberOfDownloads() const { return m_archivesDownloaded; }
     void setArchivesToDownload(const QList<QPair<QString, QString> > &archives);
 
 Q_SIGNALS:
@@ -102,7 +102,6 @@ private:
     QList<QPair<QString, QString> > m_archivesToDownload;
 
     bool m_canceled;
-    QSet<QString> m_temporaryFiles;
     QByteArray m_currentHash;
     double m_lastFileProgress;
     int m_progressChangedTimerId;
diff --git a/src/libs/installer/packagemanagercore.cpp b/src/libs/installer/packagemanagercore.cpp
index b90435993..61ce92467 100644
--- a/src/libs/installer/packagemanagercore.cpp
+++ b/src/libs/installer/packagemanagercore.cpp
@@ -582,32 +582,32 @@ int PackageManagerCore::downloadNeededArchives(double partProgressSize)
 
     ProgressCoordinator::instance()->emitLabelAndDetailTextChanged(tr("\nDownloading packages..."));
 
-    // don't have it on the stack, since it keeps the temporary files
-    DownloadArchivesJob *const archivesJob = new DownloadArchivesJob(this);
-    archivesJob->setAutoDelete(false);
-    archivesJob->setArchivesToDownload(archivesToDownload);
-    connect(this, SIGNAL(installationInterrupted()), archivesJob, SLOT(cancel()));
-    connect(archivesJob, SIGNAL(outputTextChanged(QString)), ProgressCoordinator::instance(),
+    DownloadArchivesJob archivesJob(this);
+    archivesJob.setAutoDelete(false);
+    archivesJob.setArchivesToDownload(archivesToDownload);
+    connect(this, SIGNAL(installationInterrupted()), &archivesJob, SLOT(cancel()));
+    connect(&archivesJob, SIGNAL(outputTextChanged(QString)), ProgressCoordinator::instance(),
         SLOT(emitLabelAndDetailTextChanged(QString)));
-    connect(archivesJob, SIGNAL(downloadStatusChanged(QString)), ProgressCoordinator::instance(),
+    connect(&archivesJob, SIGNAL(downloadStatusChanged(QString)), ProgressCoordinator::instance(),
         SIGNAL(downloadStatusChanged(QString)));
 
-    ProgressCoordinator::instance()->registerPartProgress(archivesJob, SIGNAL(progressChanged(double)),
-        partProgressSize);
+    ProgressCoordinator::instance()->registerPartProgress(&archivesJob,
+        SIGNAL(progressChanged(double)), partProgressSize);
 
-    archivesJob->start();
-    archivesJob->waitForFinished();
+    archivesJob.start();
+    archivesJob.waitForFinished();
 
-    if (archivesJob->error() == KDJob::Canceled)
+    if (archivesJob.error() == KDJob::Canceled)
         interrupt();
-    else if (archivesJob->error() != KDJob::NoError)
-        throw Error(archivesJob->errorString());
+    else if (archivesJob.error() != KDJob::NoError)
+        throw Error(archivesJob.errorString());
 
     if (d->statusCanceledOrFailed())
         throw Error(tr("Installation canceled by user"));
+
     ProgressCoordinator::instance()->emitDownloadStatus(tr("All downloads finished."));
 
-    return archivesToDownload.count();
+    return archivesJob.numberOfDownloads();
 }
 
 /*!
@@ -1883,7 +1883,8 @@ void PackageManagerCore::interrupt()
  */
 void PackageManagerCore::setCanceled()
 {
-    cancelMetaInfoJob();
+    if (!d->m_repoFetched)
+        cancelMetaInfoJob();
     d->setStatus(PackageManagerCore::Canceled);
 }