Merge lp:~mardy/signon-ui/lp1135038 into lp:signon-ui

[Alberto Mardegan]

Do not quit if some accounts are failing

The inactivity timer should not trigger if there are some account failures (we
even had a comment warning about that!), because signon-ui is also the host for
the com.canonical.indicators.webcredentials service.

Create a new InactivityTimer to monitor both the Service and IndicatorService
objects.

Do not quit if some accounts are failing

The inactivity timer should not trigger if there are some account failures (we
even had a comment warning about that!), because signon-ui is also the host for
the com.canonical.indicators.webcredentials service.

Create a new InactivityTimer to monitor both the Service and IndicatorService
objects.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:78
http://s-jenkins:8080/job/signon-ui-ci/14/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/signon-ui-ci/./build=pbuilder,distribution=raring,flavor=amd64/14/console
    FAILURE: http://s-jenkins:8080/job/signon-ui-ci/./build=pbuilder,distribution=raring,flavor=i386/14/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/signon-ui-ci/14/rebuild

review: Needs Fixing (continuous-integration)

[lp:~mardy/signon-ui/lp1135038]

79. By Alberto Mardegan on 2013-03-14

Use xvfb-run

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:79
http://s-jenkins:8080/job/signon-ui-ci/15/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/signon-ui-ci/./build=pbuilder,distribution=raring,flavor=amd64/15/console
    SUCCESS: http://s-jenkins:8080/job/signon-ui-ci/./build=pbuilder,distribution=raring,flavor=i386/15/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/signon-ui-ci/15/rebuild

review: Approve (continuous-integration)

[David King (amigadave)]

review: Approve

=== modified file '.bzrignore'
--- .bzrignore	2012-11-15 08:10:18 +0000
+++ .bzrignore	2013-03-14 13:21:22 +0000
@@ -13,6 +13,7 @@
 core
 debian/
 Makefile
+Makefile.*
 moc_*
 qrc_*.cpp
 signon-ui-test
@@ -20,6 +21,7 @@
 src/signon_ui_adaptor.*
 src/webcredentials_adaptor.*
 tests/unit/signon-ui-unittest
+tests/unit/tst_inactivity_timer
 tests/.config/Trolltech.conf
 tests/functional/.cache/
 tests/functional/.config/

=== added file 'src/inactivity-timer.cpp'
--- src/inactivity-timer.cpp	1970-01-01 00:00:00 +0000
+++ src/inactivity-timer.cpp	2013-03-14 13:21:22 +0000
@@ -0,0 +1,68 @@
+/*
+ * This file is part of signon-ui
+ *
+ * Copyright (C) 2013 Canonical Ltd.
+ *
+ * Contact: Alberto Mardegan <[e-mail]>
+ *
+ * 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 warranties of
+ * MERCHANTABILITY, SATISFACTORY QUALITY, 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/>.
+ */
+
+#include "inactivity-timer.h"
+
+#include "debug.h"
+
+using namespace SignOnUi;
+
+InactivityTimer::InactivityTimer(int interval, QObject *parent):
+    QObject(parent),
+    m_interval(interval)
+{
+    m_timer.setSingleShot(true);
+    QObject::connect(&m_timer, SIGNAL(timeout()),
+                     this, SLOT(onTimeout()));
+}
+
+void InactivityTimer::watchObject(QObject *object)
+{
+    connect(object, SIGNAL(isIdleChanged()), SLOT(onIdleChanged()));
+    m_watchedObjects.append(object);
+
+    /* Force an initial check */
+    onIdleChanged();
+}
+
+void InactivityTimer::onIdleChanged()
+{
+    if (allObjectsAreIdle()) {
+        m_timer.start(m_interval);
+    }
+}
+
+void InactivityTimer::onTimeout()
+{
+    TRACE();
+    if (allObjectsAreIdle()) {
+        Q_EMIT timeout();
+    }
+}
+
+bool InactivityTimer::allObjectsAreIdle() const
+{
+    foreach (const QObject *object, m_watchedObjects) {
+        if (!object->property("isIdle").toBool()) {
+            return false;
+        }
+    }
+    return true;
+}

=== added file 'src/inactivity-timer.h'
--- src/inactivity-timer.h	1970-01-01 00:00:00 +0000
+++ src/inactivity-timer.h	2013-03-14 13:21:22 +0000
@@ -0,0 +1,58 @@
+/*
+ * This file is part of signon-ui
+ *
+ * Copyright (C) 2013 Canonical Ltd.
+ *
+ * Contact: Alberto Mardegan <[e-mail]>
+ *
+ * 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 warranties of
+ * MERCHANTABILITY, SATISFACTORY QUALITY, 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/>.
+ */
+
+#ifndef SIGNON_UI_INACTIVITY_TIMER_H
+#define SIGNON_UI_INACTIVITY_TIMER_H
+
+#include <QList>
+#include <QObject>
+#include <QTimer>
+
+namespace SignOnUi {
+
+class InactivityTimer: public QObject
+{
+    Q_OBJECT
+
+public:
+    InactivityTimer(int interval, QObject *parent = 0);
+    ~InactivityTimer() {}
+
+    void watchObject(QObject *object);
+
+Q_SIGNALS:
+    void timeout();
+
+private Q_SLOTS:
+    void onIdleChanged();
+    void onTimeout();
+
+private:
+    bool allObjectsAreIdle() const;
+
+private:
+    QList<QObject*> m_watchedObjects;
+    QTimer m_timer;
+    int m_interval;
+};
+
+} // namespace
+
+#endif // SIGNON_UI_INACTIVITY_TIMER_H

=== modified file 'src/indicator-service.cpp'
--- src/indicator-service.cpp	2013-01-23 07:48:35 +0000
+++ src/indicator-service.cpp	2013-03-14 13:21:22 +0000
@@ -121,14 +121,23 @@
 
 void IndicatorServicePrivate::RemoveFailures(const QSet<uint> &accountIds)
 {
+    Q_Q(IndicatorService);
     m_failures.subtract(accountIds);
     notifyPropertyChanged("Failures");
+    if (q->isIdle()) {
+        Q_EMIT q->isIdleChanged();
+    }
 }
 
 void IndicatorServicePrivate::ReportFailure(uint accountId,
                                             const QVariantMap &notification)
 {
+    Q_Q(IndicatorService);
+    bool wasIdle = q->isIdle();
     m_failures.insert(accountId);
+    if (wasIdle) {
+        Q_EMIT q->isIdleChanged();
+    }
 
     /* If the original client data is provided, we remember it: it can
      * be used to replay the authentication later.
@@ -249,6 +258,8 @@
 
 void IndicatorServicePrivate::onReauthenticatorFinished(bool success)
 {
+    Q_Q(IndicatorService);
+
     Reauthenticator *reauthenticator =
         qobject_cast<Reauthenticator*>(sender());
 
@@ -277,6 +288,7 @@
 
         if (m_failures.isEmpty()) {
             ClearErrorStatus();
+            Q_EMIT q->isIdleChanged();
         }
     }
 
@@ -342,4 +354,10 @@
     return d->m_errorStatus;
 }
 
+bool IndicatorService::isIdle() const
+{
+    Q_D(const IndicatorService);
+    return d->m_failures.isEmpty();
+}
+
 #include "indicator-service.moc"

=== modified file 'src/indicator-service.h'
--- src/indicator-service.h	2012-03-06 13:54:30 +0000
+++ src/indicator-service.h	2013-03-14 13:21:22 +0000
@@ -36,6 +36,7 @@
 class IndicatorService: public QObject
 {
     Q_OBJECT
+    Q_PROPERTY(bool isIdle READ isIdle NOTIFY isIdleChanged)
 
 public:
 
@@ -52,6 +53,10 @@
 
     QSet<uint> failures() const;
     bool errorStatus() const;
+    bool isIdle() const;
+
+Q_SIGNALS:
+    void isIdleChanged();
 
 private:
     IndicatorServicePrivate *d_ptr;

=== modified file 'src/main.cpp'
--- src/main.cpp	2013-02-01 17:11:24 +0000
+++ src/main.cpp	2013-03-14 13:21:22 +0000
@@ -20,6 +20,7 @@
 
 #include "debug.h"
 #include "i18n.h"
+#include "inactivity-timer.h"
 #include "indicator-service.h"
 #include "my-network-proxy-factory.h"
 #include "service.h"
@@ -75,24 +76,26 @@
     QNetworkProxyFactory::setApplicationProxyFactory(proxyFactory);
 
     Service *service = new Service();
-    if (daemonTimeout > 0)
-        service->setTimeout(daemonTimeout);
     QDBusConnection connection = QDBusConnection::sessionBus();
     connection.registerService(QLatin1String(serviceName));
     connection.registerObject(QLatin1String(objectPath),
                               service,
                               QDBusConnection::ExportAllContents);
-    /* FIXME: before quitting we should check if the IndicatorService is idle
-     * too. However, since this feature is used for unit tests only, for the
-     * time being this is fine.
-     */
-    QObject::connect(service, SIGNAL(idleTimeout()), &app, SLOT(quit()));
 
     IndicatorService *indicatorService = new IndicatorService();
     connection.registerService(QLatin1String(WEBCREDENTIALS_BUS_NAME));
     connection.registerObject(QLatin1String(WEBCREDENTIALS_OBJECT_PATH),
                               indicatorService->serviceObject());
 
+    InactivityTimer *inactivityTimer = 0;
+    if (daemonTimeout > 0) {
+        inactivityTimer = new InactivityTimer(daemonTimeout * 1000);
+        inactivityTimer->watchObject(service);
+        inactivityTimer->watchObject(indicatorService);
+        QObject::connect(inactivityTimer, SIGNAL(timeout()),
+                         &app, SLOT(quit()));
+    }
+
     int ret = app.exec();
 
     connection.unregisterService(QLatin1String(WEBCREDENTIALS_BUS_NAME));
@@ -103,6 +106,8 @@
     connection.unregisterObject(QLatin1String(objectPath));
     delete service;
 
+    delete inactivityTimer;
+
     return ret;
 }
 

=== modified file 'src/service.cpp'
--- src/service.cpp	2012-04-03 11:46:12 +0000
+++ src/service.cpp	2013-03-14 13:21:22 +0000
@@ -23,7 +23,6 @@
 #include "debug.h"
 #include "request.h"
 
-#include <QTimer>
 #include <QQueue>
 
 using namespace SignOnUi;
@@ -41,24 +40,16 @@
     ServicePrivate(Service *service);
     ~ServicePrivate();
 
-    void setTimeout(int timeout);
-
     RequestQueue &queueForWindowId(WId windowId);
     void enqueue(Request *request);
     void runQueue(RequestQueue &queue);
     void cancelUiRequest(const QString &requestId);
 
-private:
-    void resetTimer();
-
 private Q_SLOTS:
     void onRequestCompleted();
-    void onTimeout();
 
 private:
     mutable Service *q_ptr;
-    QTimer m_timer;
-    int m_timeout;
     /* each window Id has a different queue */
     QMap<WId,RequestQueue> m_requests;
 };
@@ -67,41 +58,14 @@
 
 ServicePrivate::ServicePrivate(Service *service):
     QObject(service),
-    q_ptr(service),
-    m_timeout(0)
+    q_ptr(service)
 {
-    m_timer.setSingleShot(true);
-    QObject::connect(&m_timer, SIGNAL(timeout()),
-                     this, SLOT(onTimeout()));
 }
 
 ServicePrivate::~ServicePrivate()
 {
 }
 
-void ServicePrivate::setTimeout(int timeout)
-{
-    m_timeout = timeout * 1000;
-    resetTimer();
-}
-
-void ServicePrivate::resetTimer()
-{
-    m_timer.stop();
-    if (m_timeout > 0) {
-        m_timer.start(m_timeout);
-    }
-}
-
-void ServicePrivate::onTimeout()
-{
-    Q_Q(Service);
-
-    if (m_requests.isEmpty()) {
-        Q_EMIT q->idleTimeout();
-    }
-}
-
 RequestQueue &ServicePrivate::queueForWindowId(WId windowId)
 {
     if (!m_requests.contains(windowId)) {
@@ -113,13 +77,18 @@
 
 void ServicePrivate::enqueue(Request *request)
 {
-    resetTimer();
+    Q_Q(Service);
+    bool wasIdle = q->isIdle();
 
     WId windowId = request->windowId();
 
     RequestQueue &queue = queueForWindowId(windowId);
     queue.enqueue(request);
 
+    if (wasIdle) {
+        Q_EMIT q->isIdleChanged();
+    }
+
     runQueue(queue);
 }
 
@@ -140,7 +109,7 @@
 
 void ServicePrivate::onRequestCompleted()
 {
-    resetTimer();
+    Q_Q(Service);
 
     Request *request = qobject_cast<Request*>(sender());
     WId windowId = request->windowId();
@@ -160,6 +129,10 @@
         /* start the next request */
         runQueue(queue);
     }
+
+    if (q->isIdle()) {
+        Q_EMIT q->isIdleChanged();
+    }
 }
 
 void ServicePrivate::cancelUiRequest(const QString &requestId)
@@ -193,10 +166,10 @@
 {
 }
 
-void Service::setTimeout(int timeout)
+bool Service::isIdle() const
 {
-    Q_D(Service);
-    d->setTimeout(timeout);
+    Q_D(const Service);
+    return d->m_requests.isEmpty();
 }
 
 QVariantMap Service::queryDialog(const QVariantMap &parameters)

=== modified file 'src/service.h'
--- src/service.h	2012-04-03 11:46:12 +0000
+++ src/service.h	2013-03-14 13:21:22 +0000
@@ -32,13 +32,14 @@
 class Service: public QObject, protected QDBusContext
 {
     Q_OBJECT
+    Q_PROPERTY(bool isIdle READ isIdle NOTIFY isIdleChanged)
     Q_CLASSINFO("D-Bus Interface", "com.nokia.singlesignonui")
 
 public:
     explicit Service(QObject *parent = 0);
     ~Service();
 
-    void setTimeout(int timeout);
+    bool isIdle() const;
 
 public Q_SLOTS:
     QVariantMap queryDialog(const QVariantMap &parameters);
@@ -46,7 +47,7 @@
     Q_NOREPLY void cancelUiRequest(const QString &requestId);
 
 Q_SIGNALS:
-    void idleTimeout();
+    void isIdleChanged();
 
 private:
     ServicePrivate *d_ptr;

=== modified file 'src/src.pro'
--- src/src.pro	2013-01-23 07:48:35 +0000
+++ src/src.pro	2013-03-14 13:21:22 +0000
@@ -34,6 +34,7 @@
     dialog.h \
     errors.h \
     i18n.h \
+    inactivity-timer.h \
     indicator-service.h \
     network-access-manager.h \
     reauthenticator.h \
@@ -49,6 +50,7 @@
     dialog-request.cpp \
     dialog.cpp \
     i18n.cpp \
+    inactivity-timer.cpp \
     indicator-service.cpp \
     main.cpp \
     my-network-proxy-factory.cpp \

=== added file 'tests/unit/tst_inactivity_timer.cpp'
--- tests/unit/tst_inactivity_timer.cpp	1970-01-01 00:00:00 +0000
+++ tests/unit/tst_inactivity_timer.cpp	2013-03-14 13:21:22 +0000
@@ -0,0 +1,143 @@
+/*
+ * This file is part of signon-ui
+ *
+ * Copyright (C) 2013 Canonical Ltd.
+ *
+ * Contact: Alberto Mardegan <[e-mail]>
+ *
+ * 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 warranties of
+ * MERCHANTABILITY, SATISFACTORY QUALITY, 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/>.
+ */
+
+#include "debug.h"
+#include "inactivity-timer.h"
+
+#include <QDebug>
+#include <QObject>
+#include <QSignalSpy>
+#include <QTest>
+
+using namespace SignOnUi;
+
+class InactivityTest: public QObject
+{
+    Q_OBJECT
+
+public:
+    InactivityTest() {};
+
+private Q_SLOTS:
+    void testAlwaysIdle();
+    void testIdleThenBusy();
+    void testStartBusy();
+};
+
+class TestObject: public QObject
+{
+    Q_OBJECT
+    Q_PROPERTY(bool isIdle READ isIdle NOTIFY isIdleChanged)
+
+public:
+    TestObject(): QObject(0), m_isIdle(false) {}
+    ~TestObject() {}
+
+    bool isIdle() const { return m_isIdle; }
+    void setIdle(bool idle) {
+        if (idle == m_isIdle) return;
+        m_isIdle = idle;
+        Q_EMIT isIdleChanged();
+    }
+
+Q_SIGNALS:
+    void isIdleChanged();
+
+private:
+    bool m_isIdle;
+};
+
+void InactivityTest::testAlwaysIdle()
+{
+    InactivityTimer inactivityTimer(100);
+    QSignalSpy timeout(&inactivityTimer, SIGNAL(timeout()));
+
+    TestObject object1;
+    object1.setIdle(true);
+    inactivityTimer.watchObject(&object1);
+
+    TestObject object2;
+    object2.setIdle(true);
+    inactivityTimer.watchObject(&object2);
+
+    QCOMPARE(timeout.count(), 0);
+    QTest::qWait(150);
+    QCOMPARE(timeout.count(), 1);
+}
+
+void InactivityTest::testIdleThenBusy()
+{
+    InactivityTimer inactivityTimer(100);
+    QSignalSpy timeout(&inactivityTimer, SIGNAL(timeout()));
+
+    TestObject object1;
+    object1.setIdle(true);
+    inactivityTimer.watchObject(&object1);
+
+    TestObject object2;
+    object2.setIdle(true);
+    inactivityTimer.watchObject(&object2);
+
+    QCOMPARE(timeout.count(), 0);
+
+    QTest::qWait(30);
+    QCOMPARE(timeout.count(), 0);
+    object2.setIdle(false);
+
+    QTest::qWait(100);
+    QCOMPARE(timeout.count(), 0);
+    object2.setIdle(true);
+    QTest::qWait(30);
+    QCOMPARE(timeout.count(), 0);
+
+    QTest::qWait(100);
+    QCOMPARE(timeout.count(), 1);
+}
+
+void InactivityTest::testStartBusy()
+{
+    InactivityTimer inactivityTimer(100);
+    QSignalSpy timeout(&inactivityTimer, SIGNAL(timeout()));
+
+    TestObject object1;
+    inactivityTimer.watchObject(&object1);
+
+    TestObject object2;
+    inactivityTimer.watchObject(&object2);
+
+    QCOMPARE(timeout.count(), 0);
+
+    QTest::qWait(130);
+    QCOMPARE(timeout.count(), 0);
+    object2.setIdle(true);
+
+    QTest::qWait(130);
+    QCOMPARE(timeout.count(), 0);
+    object1.setIdle(true);
+
+    QTest::qWait(30);
+    QCOMPARE(timeout.count(), 0);
+
+    QTest::qWait(100);
+    QCOMPARE(timeout.count(), 1);
+}
+
+QTEST_MAIN(InactivityTest);
+#include "tst_inactivity_timer.moc"

=== added file 'tests/unit/tst_inactivity_timer.pro'
--- tests/unit/tst_inactivity_timer.pro	1970-01-01 00:00:00 +0000
+++ tests/unit/tst_inactivity_timer.pro	2013-03-14 13:21:22 +0000
@@ -0,0 +1,37 @@
+include(../../common-project-config.pri)
+include($${TOP_SRC_DIR}/common-vars.pri)
+include($${TOP_SRC_DIR}/common-installs-config.pri)
+
+TARGET = tst_inactivity_timer
+
+CONFIG += \
+    build_all \
+    debug \
+    qtestlib
+
+QT += \
+    core
+
+SOURCES += \
+    tst_inactivity_timer.cpp \
+    $$TOP_SRC_DIR/src/debug.cpp \
+    $$TOP_SRC_DIR/src/inactivity-timer.cpp
+HEADERS += \
+    $$TOP_SRC_DIR/src/debug.h \
+    $$TOP_SRC_DIR/src/inactivity-timer.h
+
+INCLUDEPATH += \
+    . \
+    $$TOP_SRC_DIR/src
+
+QMAKE_CXXFLAGS += \
+    -fno-exceptions \
+    -fno-rtti
+
+DEFINES += \
+    DEBUG_ENABLED \
+    UNIT_TESTS
+
+check.commands = "xvfb-run -a ./$$TARGET"
+check.depends = $$TARGET
+QMAKE_EXTRA_TARGETS += check

=== renamed file 'tests/unit/unit.pro' => 'tests/unit/tst_signon_ui.pro'
=== added file 'tests/unit/unit.pro'
--- tests/unit/unit.pro	1970-01-01 00:00:00 +0000
+++ tests/unit/unit.pro	2013-03-14 13:21:22 +0000
@@ -0,0 +1,4 @@
+TEMPLATE = subdirs
+SUBDIRS = \
+    tst_inactivity_timer.pro \
+    tst_signon_ui.pro