Merge lp:~renatofilho/address-book-app/fix-1469005 into lp:address-book-app

[Renato Araujo Oliveira Filho]

Use Runnable implementation instead of thread to avoid overload cpu usage.
Implement async copyImage support.

[lp:~renatofilho/address-book-app/fix-1469005]

484. By Renato Araujo Oliveira Filho on 2015-10-19

Use Runnable implementation instead of thread to avoid overload cpu usage.
Implement async copyImage support.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:484
http://jenkins.qa.ubuntu.com/job/address-book-app-ci/963/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/address-book-app-vivid-i386-ci/266
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/4710
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/3803
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4707
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4707/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/24363

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/address-book-app-ci/963/rebuild

review: Approve (continuous-integration)

[lp:~renatofilho/address-book-app/fix-1469005]

485. By Renato Araujo Oliveira Filho on 2015-10-28

Trunk merged.

486. By Renato Araujo Oliveira Filho on 2015-10-28

Merged: ~renatofilho/address-book-app/update-autopilot-with-new-bottom-edge

Unmerged revisions

=== modified file 'src/imports/Ubuntu/AddressBook/ContactEditor/AvatarImport.qml'
--- src/imports/Ubuntu/AddressBook/ContactEditor/AvatarImport.qml	2015-10-26 13:18:11 +0000
+++ src/imports/Ubuntu/AddressBook/ContactEditor/AvatarImport.qml	2015-10-28 19:43:31 +0000
@@ -59,7 +59,6 @@
                     id: peerPicker
 
                     anchors.fill: parent
-                    visible: dialogue.done
                     contentType: ContentType.Pictures
                     handler: ContentHandler.Source
 
@@ -77,13 +76,11 @@
             Connections {
                 id: signalConnections
 
-                target: dialogue.activeTransfer
                 onStateChanged: {
                     var done = ((dialogue.activeTransfer.state === ContentTransfer.Charged) ||
                                 (dialogue.activeTransfer.state === ContentTransfer.Aborted))
 
                     if (dialogue.activeTransfer.state === ContentTransfer.Charged) {
-                        dialogue.hide()
                         if (dialogue.activeTransfer.items.length > 0) {
                             root.avatarReceived(dialogue.activeTransfer.items[0].url)
                         }
@@ -108,7 +105,7 @@
                 repeat: true
                 running: false
                 onTriggered: {
-                   if(Qt.application.active) {
+                    if(Qt.application.state === Qt.ApplicationActive) {
                        PopupUtils.close(root.importDialog)
                    }
                 }

=== modified file 'src/imports/Ubuntu/AddressBook/ContactEditor/ContactDetailAvatarEditor.qml'
--- src/imports/Ubuntu/AddressBook/ContactEditor/ContactDetailAvatarEditor.qml	2015-10-26 13:18:11 +0000
+++ src/imports/Ubuntu/AddressBook/ContactEditor/ContactDetailAvatarEditor.qml	2015-10-28 19:43:31 +0000
@@ -27,6 +27,8 @@
 
     readonly property alias busy: activityIndicator.running
     readonly property string defaultAvatar: "image://theme/add"
+    property string temporaryAvatar: ""
+    property string temporaryAvatarId: ""
 
     function isEmpty() {
         return false;
@@ -101,7 +103,7 @@
         id: activityIndicator
 
         anchors.centerIn: avatar
-        running: (avatarImport.importDialog != null)
+        running: (avatarImport.importDialog != null) || (root.temporaryAvatarId != "")
         visible: running
     }
 
@@ -109,12 +111,25 @@
         id: avatarImport
 
         onAvatarReceived: {
+            Contacts.removeFile(root.temporaryAvatar)
+
             // remove the previous image, this is nessary to make sure that the new image
             // get updated otherwise if the new image has the same name the image will not
             // be updated
             avatarImage.source = ""
-            // Update with the new value
-            avatarImage.source = Contacts.copyImage(root.contact, avatarUrl);
+            // copy and resize image
+            root.temporaryAvatarId = Contacts.copyImage(avatarUrl, null);
+        }
+    }
+
+    Connections {
+        target: Contacts
+        onImageCopyDone: {
+            if (root.temporaryAvatarId === id) {
+                root.temporaryAvatar = fileName
+                avatarImage.source = fileName
+                root.temporaryAvatarId = ""
+            }
         }
     }
 
@@ -126,6 +141,12 @@
             avatarImport.requestNewAvatar()
         }
     }
+
+    Component.onDestruction: {
+        console.debug("Delete temporary avatar image:" + root.temporaryAvatar)
+        Contacts.removeFile("file:///" + root.temporaryAvatar)
+        root.temporaryAvatar = ""
+    }
 }
 
 

=== modified file 'src/imports/Ubuntu/Contacts/contacts.cpp'
--- src/imports/Ubuntu/Contacts/contacts.cpp	2015-10-14 22:53:35 +0000
+++ src/imports/Ubuntu/Contacts/contacts.cpp	2015-10-28 19:43:31 +0000
@@ -23,6 +23,7 @@
 #include <QtCore/QDir>
 #include <QtCore/QUrl>
 #include <QtCore/QLockFile>
+#include <QtCore/QThreadPool>
 
 #include "config.h"
 
@@ -77,27 +78,11 @@
     return out;
 }
 
-QUrl UbuntuContacts::copyImage(QObject *contact, const QUrl &imageUrl)
+QString UbuntuContacts::copyImage(const QUrl &imageUrl)
 {
-    // keep track of threads to avoid memory leeak
-    ImageScaleThread *imgThread;
-    QVariant oldThread = contact->property("IMAGE_SCALE_THREAD");
-    if (!oldThread.isNull()) {
-        imgThread = oldThread.value<ImageScaleThread *>();
-        imgThread->updateImageUrl(imageUrl);
-    } else {
-        imgThread = new ImageScaleThread(imageUrl, contact);
-        contact->setProperty("IMAGE_SCALE_THREAD", QVariant::fromValue<ImageScaleThread*>(imgThread));
-    }
-
-    imgThread->start();
-
-    // FIXME: implement this as async function
-    while(imgThread->isRunning()) {
-        QCoreApplication::processEvents(QEventLoop::AllEvents, 3000);
-    }
-
-    return imgThread->outputFile();
+    ImageScaleThread *imgThread = new ImageScaleThread(imageUrl, this);
+    QThreadPool::globalInstance()->start(imgThread);
+    return imgThread->id();
 }
 
 bool UbuntuContacts::containsLetters(const QString &value)
@@ -112,7 +97,11 @@
 
 bool UbuntuContacts::removeFile(const QUrl &file)
 {
-    return QFile::remove(file.toLocalFile());
+    QString localFile = file.toLocalFile();
+    if (!localFile.isEmpty() && QFile::exists(localFile)) {
+        return QFile::remove(localFile);
+    }
+    return false;
 }
 
 bool UbuntuContacts::updateIsRunning() const

=== modified file 'src/imports/Ubuntu/Contacts/contacts.h'
--- src/imports/Ubuntu/Contacts/contacts.h	2015-10-13 12:50:14 +0000
+++ src/imports/Ubuntu/Contacts/contacts.h	2015-10-28 19:43:31 +0000
@@ -36,12 +36,13 @@
 
     Q_INVOKABLE QString contactInitialsFromString(const QString &value);
     Q_INVOKABLE QString normalized(const QString &value);
-    Q_INVOKABLE QUrl copyImage(QObject *contact, const QUrl &imageUrl);
+    Q_INVOKABLE QString copyImage(const QUrl &imageUrl);
     Q_INVOKABLE bool containsLetters(const QString &value);
     Q_INVOKABLE bool removeFile(const QUrl &file);
     Q_INVOKABLE bool updateIsRunning() const;
 
 Q_SIGNALS:
+    void imageCopyDone(const QString &id, const QString &fileName);
     void updateIsRunningChanged();
 
 private:

=== modified file 'src/imports/Ubuntu/Contacts/imagescalethread.cpp'
--- src/imports/Ubuntu/Contacts/imagescalethread.cpp	2015-05-07 17:27:16 +0000
+++ src/imports/Ubuntu/Contacts/imagescalethread.cpp	2015-10-28 19:43:31 +0000
@@ -20,39 +20,46 @@
 #include <QImageReader>
 #include <QFile>
 #include <QDir>
+#include <QUuid>
 #include <QDebug>
 
-ImageScaleThread::ImageScaleThread(const QUrl &imageUrl, QObject *parent)
-    : QThread(parent),
-      m_imageUrl(imageUrl),
-      m_tmpFile(0)
+ImageScaleThread::ImageScaleThread(const QUrl &imageUrl, QObject *listener)
+     : m_imageUrl(imageUrl),
+       m_id(QUuid::createUuid().toString()),
+       m_listener(listener),
+       m_tmpFile(0)
 {
 }
 
 ImageScaleThread::~ImageScaleThread()
 {
     if (m_tmpFile) {
-        delete m_tmpFile;
+        m_tmpFile->setAutoRemove(false);
+        m_tmpFile->deleteLater();
+        m_tmpFile = 0;
     }
 }
 
-void ImageScaleThread::updateImageUrl(const QUrl &imageUrl)
+QString ImageScaleThread::id()
 {
-    if (isRunning()) {
-        wait();
-    }
-    m_imageUrl = imageUrl;
+    return m_id;
 }
 
 QString ImageScaleThread::outputFile() const
 {
-    return m_tmpFile->fileName();
+    if (m_tmpFile) {
+        return m_tmpFile->fileName();
+    } else {
+        return QString();
+    }
 }
 
 void ImageScaleThread::run()
 {
     // make sure that the old image get deleted
     if (m_tmpFile) {
+        qDebug() << "Delete previous avatar" << m_tmpFile->fileName();
+        m_tmpFile->setAutoRemove(true);
         m_tmpFile->close();
         delete m_tmpFile;
     }
@@ -60,6 +67,7 @@
     // Create the temporary file
     m_tmpFile = new QTemporaryFile(QString("%1/avatar_XXXXXX.png").arg(QDir::tempPath()));
     if (!m_tmpFile->open()) {
+        qWarning() << "Fail to create avatar temporary file";
         return;
     }
 
@@ -91,5 +99,11 @@
     if (!scaledAvatar.isNull()) {
         scaledAvatar.save(m_tmpFile, "png");
     }
+
     m_tmpFile->close();
+
+    if (m_listener) {
+        QMetaObject::invokeMethod(m_listener, "imageCopyDone",
+                                  Q_ARG(QString, m_id), Q_ARG(QString, m_tmpFile->fileName()));
+    }
 }

=== modified file 'src/imports/Ubuntu/Contacts/imagescalethread.h'
--- src/imports/Ubuntu/Contacts/imagescalethread.h	2015-05-07 17:27:16 +0000
+++ src/imports/Ubuntu/Contacts/imagescalethread.h	2015-10-28 19:43:31 +0000
@@ -18,18 +18,18 @@
 #define IMAGESCALETHREAD_H
 
 #include <QObject>
-#include <QThread>
+#include <QRunnable>
 #include <QUrl>
+#include <QPointer>
 #include <QTemporaryFile>
 
-class ImageScaleThread : public QThread
+class ImageScaleThread : public QRunnable
 {
-    Q_OBJECT
 public:
-    ImageScaleThread(const QUrl &imageUrl, QObject *parent=0);
+    ImageScaleThread(const QUrl &imageUrl, QObject *listener);
     ~ImageScaleThread();
 
-    void updateImageUrl(const QUrl &imageUrl);
+    QString id();
     QString outputFile() const;
 
 protected:
@@ -37,6 +37,8 @@
 
 private:
     QUrl m_imageUrl;
+    QString m_id;
+    QPointer<QObject> m_listener;
     QTemporaryFile *m_tmpFile;
 };
 

=== modified file 'tests/autopilot/address_book_app/__init__.py'
--- tests/autopilot/address_book_app/__init__.py	2015-10-20 03:38:07 +0000
+++ tests/autopilot/address_book_app/__init__.py	2015-10-28 19:43:31 +0000
@@ -70,7 +70,8 @@
     def get_contact_edit_page(self):
         # We can have two contact editor page because of bottom edge page
         # but we will return only the active one
-        return self.wait_select_single(objectName="contactEditorPage", active=True)
+        return self.wait_select_single(pages.ABContactEditorPage,
+                                       objectName="contactEditorPage", active=True)
 
     def get_contact_view_page(self):
         return self.wait_select_single(pages.ABContactViewPage,

=== modified file 'tests/autopilot/address_book_app/pages/__init__.py'
--- tests/autopilot/address_book_app/pages/__init__.py	2015-05-11 14:21:03 +0000
+++ tests/autopilot/address_book_app/pages/__init__.py	2015-10-28 19:43:31 +0000
@@ -20,7 +20,6 @@
     'ABContactViewPage'
 ]
 
-from address_book_app.address_book \
-    import ContactEditorPage as ABContactEditorPage
+from address_book_app.pages._ab_contact_editor_page import ABContactEditorPage
 from address_book_app.pages._ab_contact_view_page import ABContactViewPage
 from address_book_app.pages._ab_contact_list_page import ABContactListPage

=== added file 'tests/autopilot/address_book_app/pages/_ab_contact_editor_page.py'
--- tests/autopilot/address_book_app/pages/_ab_contact_editor_page.py	1970-01-01 00:00:00 +0000
+++ tests/autopilot/address_book_app/pages/_ab_contact_editor_page.py	2015-10-28 19:43:31 +0000
@@ -0,0 +1,28 @@
+# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
+#
+# Copyright (C) 2014 Canonical Ltd.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 3, as published
+# by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+""" ContactEditorPage emulator for Addressbook App tests """
+
+import logging
+import time
+
+import autopilot.logging
+import ubuntuuitoolkit
+
+from address_book_app import address_book
+
+class ABContactEditorPage(address_book.ContactEditorPage):
+    """Autopilot helper for the Contact Editor page."""