Merge lp:~mandel/ubuntu-system-settings/mem-leak-fixes into lp:ubuntu-system-settings

[Manuel de la Peña]

Fix mem leak in the update page.

The Download pointer returned by the download manager (as stated by the documentation) is owned by the caller and the memory should be managed by the client. This MR cleans the resource in the following cases:

* finished
* canceled
* error

There is also a small change that removes the definitions for a better cpp approach.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1220
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/1836/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/477
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-vivid-i386-ci/106
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/421
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/477
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/477/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/16448

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/1836/rebuild

review: Needs Fixing (continuous-integration)

[Sebastien Bacher (seb128)]

Those change looks good, thanks (CI is failing due to issue with the infrastructure)

review: Approve

[Sebastien Bacher (seb128)]

recent changes look fine as well to me. Ken I read that you had issues testing the version before those changes, could you comment on what those were?

review: Approve

[Manuel de la Peña (mandel)]

I have tested this branch several times in vivid and everything works as expected. Would be nice to get kens feedback too :)

=== modified file 'debian/changelog'
--- debian/changelog	2014-12-10 16:01:15 +0000
+++ debian/changelog	2014-12-11 09:46:48 +0000
@@ -38,6 +38,12 @@
 
  -- Ubuntu daily release <[e-mail]>  Tue, 02 Dec 2014 19:39:55 +0000
 
+ubuntu-system-settings (0.3+15.04.20141201-0ubuntu2) UNRELEASED; urgency=medium
+
+  * Fix mem leak in the update page. 
+
+ -- Manuel de la Pena <[e-mail]>  Wed, 03 Dec 2014 11:11:00 +0100
+
 ubuntu-system-settings (0.3+15.04.20141201-0ubuntu1) vivid; urgency=low
 
   [ Michał Sawicz ]

=== modified file 'plugins/system-update/download_tracker.cpp'
--- plugins/system-update/download_tracker.cpp	2014-09-30 02:05:36 +0000
+++ plugins/system-update/download_tracker.cpp	2014-12-11 09:46:48 +0000
@@ -18,22 +18,26 @@
  * Authored by: Diego Sarmentero <[e-mail]>
  */
 
+#include <QProcessEnvironment>
+
+#include <ubuntu/download_manager/download_struct.h>
+#include <ubuntu/download_manager/error.h>
+
 #include "download_tracker.h"
 #include "network/network.h"
-#include <ubuntu/download_manager/download_struct.h>
-#include <ubuntu/download_manager/error.h>
-#include <QProcessEnvironment>
 
-#define DOWNLOAD_COMMAND "post-download-command"
-#define APP_ID "app_id"
-#define PKCON_COMMAND "pkcon"
+namespace {
+    const QString DOWNLOAD_COMMAND = "post-download-command";
+    const QString APP_ID = "app_id";
+    const QString PKCON_COMMAND = "pkcon";
+}
 
 namespace UpdatePlugin {
 
 DownloadTracker::DownloadTracker(QObject *parent) :
     QObject(parent),
-    m_clickToken(""),
-    m_downloadUrl(""),
+    m_clickToken(QString::null),
+    m_downloadUrl(QString::null),
     m_download(nullptr),
     m_manager(nullptr),
     m_progress(0)
@@ -42,7 +46,7 @@
 
 void DownloadTracker::setDownload(const QString& url)
 {
-    if (url != "") {
+    if (!url.isEmpty()) {
         m_downloadUrl = url;
         startService();
     }
@@ -50,7 +54,7 @@
 
 void DownloadTracker::setClickToken(const QString& token)
 {
-    if (token != "") {
+    if (!token.isEmpty()) {
         m_clickToken = token;
         startService();
     }
@@ -58,7 +62,7 @@
 
 void DownloadTracker::setPackageName(const QString& package)
 {
-    if (package != "") {
+    if (!package.isEmpty()) {
         m_packageName = package;
         startService();
     }
@@ -90,9 +94,9 @@
 {
     m_download = download;
     connect(m_download, SIGNAL(finished(const QString &)), this,
-            SIGNAL(finished(const QString &)));
+            SLOT(onDownloadFinished(const QString &)));
     connect(m_download, SIGNAL(canceled(bool)), this,
-            SIGNAL(canceled(bool)));
+            SLOT(onDownloadCanceled(bool)));
     connect(m_download, SIGNAL(paused(bool)), this,
             SIGNAL(paused(bool)));
     connect(m_download, SIGNAL(resumed(bool)), this,
@@ -112,6 +116,27 @@
 void DownloadTracker::registerError(Error* error)
 {
     Q_EMIT errorFound(error->errorString());
+
+    // we need to ensure that the resources are cleaned
+    m_download->deleteLater();
+    m_download = nullptr;
+}
+
+void DownloadTracker::onDownloadFinished(const QString& path)
+{
+    // once a download is finished we need to clean the resources
+    m_download->deleteLater();
+    m_download = nullptr;
+    Q_EMIT finished(path);
+}
+
+void DownloadTracker::onDownloadCanceled(bool wasCanceled)
+{
+    if (wasCanceled) {
+        m_download->deleteLater();
+        m_download = nullptr;
+    }
+    Q_EMIT canceled(wasCanceled);
 }
 
 void DownloadTracker::pause()

=== modified file 'plugins/system-update/download_tracker.h'
--- plugins/system-update/download_tracker.h	2014-09-30 02:05:36 +0000
+++ plugins/system-update/download_tracker.h	2014-12-11 09:46:48 +0000
@@ -63,6 +63,8 @@
     void bindDownload(Download* download);
     void setProgress(qulonglong received, qulonglong total);
     void registerError(Ubuntu::DownloadManager::Error* error);
+    void onDownloadFinished(const QString& path);
+    void onDownloadCanceled(bool wasCanceled);
 
 Q_SIGNALS:
     void error(const QString &errorMessage);
@@ -76,12 +78,12 @@
     void errorFound(const QString &error);
 
 private:
-    QString m_clickToken;
-    QString m_downloadUrl;
-    QString m_packageName;
-    Download* m_download;
-    Manager* m_manager;
-    int m_progress;
+    QString m_clickToken = QString::null;
+    QString m_downloadUrl = QString::null;
+    QString m_packageName = QString::null;
+    Download* m_download = nullptr;
+    Manager* m_manager = nullptr;
+    int m_progress = 0;
 
     void startService();
     QString getPkconCommand();