Merge lp:~gerboland/qtmir/RTM-fix-lifecycle-exempt-keeps-wakelock into lp:qtmir/rtm-14.09

[Gerry Boland]

Refactor wakelock handling. Lifecycle exempt apps now release wakelock when shell tries to suspend them

The previous Wakelock RAII design was faulty as it was wrapping an asynchronous service. It made it possible for wakelocks to be acquired and not be released.

This refactors SharedWakelock to hold a single instance of Wakelock, and Wakelock always holds a DBus connection.

Testing now includes testing the DBus calls are actually emitted.

Adds dependency on libqtdbusmock1-dev and libqtdbustest1-dev

[lp:~gerboland/qtmir/RTM-fix-lifecycle-exempt-keeps-wakelock]

296. By Gerry Boland on 2015-02-27

Add test

[Gerry Boland (gerboland)]

2 wakelocks somehow acquired with this by the Music app, something wrong

review: Needs Fixing

[lp:~gerboland/qtmir/RTM-fix-lifecycle-exempt-keeps-wakelock]

297. By Gerry Boland on 2015-03-05

Refactor wakelock handling. Lifecycle exempt apps now release wakelock when shell tries to suspend them

The previous Wakelock RAII design was faulty as it was wrapping an asynchronous service. It made it possible for wakelocks to be acquired and not be released.

This refactors SharedWakelock to hold a single instance of Wakelock, and Wakelock always holds a DBus connection.

Testing now includes testing the DBus calls are actually emitted.

Adds dependency on libqtdbusmock1-dev and libqtdbustest1-dev

298. By Gerry Boland on 2015-03-05

Fix acquire(), release(), acquire() being called in quick succession, and only after do 2 cookies get sent from dbus

299. By Gerry Boland on 2015-03-05

Fix and add test for SharedWakelock acq/rel/acq not loosing a cookie

[Gerry Boland (gerboland)]

Is good now.

review: Abstain

[lp:~gerboland/qtmir/RTM-fix-lifecycle-exempt-keeps-wakelock]

300. By Gerry Boland on 2015-03-06

Delete unnecesary line

301. By Gerry Boland on 2015-03-06

Racey test not so reliable, this helps

[Albert Astals Cid (aacid)]

 * Did you perform an exploratory manual test run of the code change and any related functionality?
Yes

 * Did CI run pass?
No CI, but i compiled and ran the tests on a krillin

review: Approve

=== modified file 'debian/control'
--- debian/control	2015-01-12 12:51:39 +0000
+++ debian/control	2015-03-06 14:19:39 +0000
@@ -16,6 +16,8 @@
                libmirserver-dev (>= 0.8.1),
                libmtdev-dev,
                libprocess-cpp-dev,
+               libqtdbusmock1-dev (>= 0.2),
+               libqtdbustest1-dev (>= 0.2),
                libqt5sensors5-dev,
                libubuntu-app-launch2-dev,
                libubuntu-application-api-dev (>= 2.1.0),
 
=== modified file 'src/common/abstractdbusservicemonitor.cpp'
--- src/common/abstractdbusservicemonitor.cpp	2015-01-16 09:48:21 +0000
+++ src/common/abstractdbusservicemonitor.cpp	2015-03-06 14:19:39 +0000
@@ -41,14 +41,13 @@
 };
 
 AbstractDBusServiceMonitor::AbstractDBusServiceMonitor(const QString &service, const QString &path,
-                                                       const QString &interface, const Bus bus,
+                                                       const QString &interface, const QDBusConnection connection,
                                                        QObject *parent)
     : QObject(parent)
     , m_service(service)
     , m_path(path)
     , m_interface(interface)
-    , m_busConnection((bus == SystemBus) ? QDBusConnection::systemBus()
-                                         : QDBusConnection::sessionBus())
+    , m_busConnection(connection)
     , m_watcher(new QDBusServiceWatcher(service, m_busConnection))
     , m_dbusInterface(nullptr)
 {
 
=== modified file 'src/common/abstractdbusservicemonitor.h'
--- src/common/abstractdbusservicemonitor.h	2015-01-16 09:48:21 +0000
+++ src/common/abstractdbusservicemonitor.h	2015-03-06 14:19:39 +0000
@@ -31,17 +31,11 @@
 class Q_DECL_EXPORT AbstractDBusServiceMonitor : public QObject
 {
     Q_OBJECT
-    Q_ENUMS(Bus)
     Q_PROPERTY(bool serviceAvailable READ serviceAvailable NOTIFY serviceAvailableChanged)
 
 public:
-    enum Bus {
-        SessionBus,
-        SystemBus,
-    };
-
     explicit AbstractDBusServiceMonitor(const QString &service, const QString &path, const QString &interface,
-                                        const Bus bus = SessionBus,
+                                        const QDBusConnection connection = QDBusConnection::sessionBus(),
                                         QObject *parent = 0);
     ~AbstractDBusServiceMonitor();
 
 
=== modified file 'src/modules/Unity/Application/Application.pro'
--- src/modules/Unity/Application/Application.pro	2015-01-16 09:48:21 +0000
+++ src/modules/Unity/Application/Application.pro	2015-03-06 14:19:39 +0000
@@ -12,7 +12,7 @@
 QMAKE_CXXFLAGS = -std=c++11 -Werror -Wall
 QMAKE_LFLAGS = -std=c++11 -Wl,-no-undefined
 
-PKGCONFIG += mirserver glib-2.0 gio-unix-2.0 process-cpp ubuntu-app-launch-2
+PKGCONFIG += mirserver glib-2.0 gio-unix-2.0 process-cpp ubuntu-app-launch-2 libqtdbusmock-1 libqtdbustest-1
 
 INCLUDEPATH += ../../../platforms/mirserver ../../../common
 LIBS += -L../../../platforms/mirserver -lqpa-mirserver
 
=== modified file 'src/modules/Unity/Application/application.cpp'
--- src/modules/Unity/Application/application.cpp	2015-01-16 09:48:21 +0000
+++ src/modules/Unity/Application/application.cpp	2015-03-06 14:19:39 +0000
@@ -319,7 +319,9 @@
 void Application::setFocused(bool focused)
 {
     qCDebug(QTMIR_APPLICATIONS) << "Application::setFocused - appId=" << appId() << "focused=" << focused;
-    holdWakelock(true);
+    if (focused) {
+        holdWakelock(true);
+    }
 
     if (m_focused != focused) {
         m_focused = focused;
 
=== modified file 'src/modules/Unity/Application/application_manager.cpp'
--- src/modules/Unity/Application/application_manager.cpp	2015-01-16 09:48:21 +0000
+++ src/modules/Unity/Application/application_manager.cpp	2015-03-06 14:19:39 +0000
@@ -372,9 +372,12 @@
         return false;
     qCDebug(QTMIR_APPLICATIONS) << "ApplicationManager::suspendApplication - appId=" << application->appId();
 
-    // Present in exceptions list, return.
-    if (!m_lifecycleExceptions.filter(application->appId().section('_',0,0)).empty())
+    // Present in exceptions list, explicitly release wakelock and return. There's no need to keep the wakelock
+    // as the process is never suspended and thus has no cleanup to perform when (for example) the display is blanked
+    if (!m_lifecycleExceptions.filter(application->appId().section('_',0,0)).empty()) {
+        m_sharedWakelock->release(application);
         return false;
+    }
 
     if (m_forceDashActive && application->appId() == "unity8-dash") {
         return false;
 
=== modified file 'src/modules/Unity/Application/sharedwakelock.cpp'
--- src/modules/Unity/Application/sharedwakelock.cpp	2015-01-16 09:48:21 +0000
+++ src/modules/Unity/Application/sharedwakelock.cpp	2015-03-06 14:19:39 +0000
@@ -29,57 +29,129 @@
 const char cookieFile[] = "/tmp/qtmir_powerd_cookie";
 
 /**
- * @brief The Wakelock class - on creation acquires a system wakelock, on destruction releases it
- * Designed in the spirit of RAII. Should the PowerD service vanish from the bus, the wakelock
- * will be re-acquired when it re-joins the bus.
+ * @brief The Wakelock class - wraps a single system wakelock
+ * Should the PowerD service vanish from the bus, the wakelock will be re-acquired when it re-joins the bus.
  */
 class Wakelock : public AbstractDBusServiceMonitor
 {
     Q_OBJECT
 public:
-    Wakelock() noexcept
-        : AbstractDBusServiceMonitor("com.canonical.powerd", "/com/canonical/powerd", "com.canonical.powerd", SystemBus)
+    Wakelock(const QDBusConnection &connection) noexcept
+        : AbstractDBusServiceMonitor("com.canonical.powerd", "/com/canonical/powerd", "com.canonical.powerd", connection)
+        , m_wakelockEnabled(false)
     {
         // (re-)acquire wake lock when powerd (re-)appears on the bus
         QObject::connect(this, &Wakelock::serviceAvailableChanged,
-                         this, &Wakelock::acquireWakelock);
-
-        if (!serviceAvailable()) {
-            qWarning() << "com.canonical.powerd DBus interface not available, waiting for it";
-            return;
-        }
+                         this, &Wakelock::onServiceAvailableChanged);
 
         // WORKAROUND: if shell crashed while it held a wakelock, due to bug lp:1409722 powerd will not have released
         // the wakelock for it. As workaround, we save the cookie to file and restore it if possible.
-        QFile cookie(cookieFile);
-        if (cookie.exists() && cookie.open(QFile::ReadOnly | QFile::Text)) {
-            m_cookie = cookie.readAll();
-        } else {
-            acquireWakelock(true);
+        QFile cookieCache(cookieFile);
+        if (cookieCache.exists() && cookieCache.open(QFile::ReadOnly | QFile::Text)) {
+            m_wakelockEnabled = true;
+            m_cookie = cookieCache.readAll();
         }
     }
 
     virtual ~Wakelock() noexcept
     {
+        release();
+    }
+
+    Q_SIGNAL void enabledChanged(bool);
+    bool enabled() const
+    {
+        return m_wakelockEnabled;
+    }
+
+    void acquire()
+    {
+        if (m_wakelockEnabled) { // wakelock already requested/set
+            return;
+        }
+        m_wakelockEnabled = true;
+
+        acquireWakelock();
+    }
+
+    void release()
+    {
         QFile::remove(cookieFile);
 
+        if (!m_wakelockEnabled) { // no wakelock already requested/set
+            return;
+        }
+        m_wakelockEnabled = false;
+        Q_EMIT enabledChanged(false);
+
         if (!serviceAvailable()) {
-            qWarning() << "com.canonical.powerd DBus interface not available";
+            qWarning() << "com.canonical.powerd DBus interface not available, presuming no wakelocks held";
             return;
         }
 
         if (!m_cookie.isEmpty()) {
-            dbusInterface()->asyncCall("clearSysState", m_cookie);
+            dbusInterface()->asyncCall("clearSysState", QString(m_cookie));
+            qCDebug(QTMIR_SESSIONS) << "Wakelock released" << m_cookie;
+            m_cookie.clear();
         }
-        qCDebug(QTMIR_SESSIONS) << "Wakelock released";
     }
 
 private Q_SLOTS:
-    void acquireWakelock(bool available)
+    void onServiceAvailableChanged(bool available)
     {
-        if (!available) {
-            m_cookie.clear(); // clear cookie so that when powerd re-appears, new cookie will be set
+        // Assumption is if service vanishes & reappears on the bus, it has lost its wakelock state and
+        // we must re-acquire if necessary
+        if (!m_wakelockEnabled) {
+            return;
+        }
+
+        if (available) {
+            acquireWakelock();
+        } else {
+            m_cookie.clear();
             QFile::remove(cookieFile);
+        }
+    }
+
+    void onWakeLockAcquired(QDBusPendingCallWatcher *call)
+    {
+        QDBusPendingReply<QString> reply = *call;
+        if (reply.isError()) {
+            qCDebug(QTMIR_SESSIONS) << "Wakelock was NOT acquired, error:"
+                                    << QDBusError::errorString(reply.error().type());
+            if (m_wakelockEnabled) {
+                m_wakelockEnabled = false;
+                Q_EMIT enabledChanged(false);
+            }
+
+            call->deleteLater();
+            return;
+        }
+        QByteArray cookie = reply.argumentAt<0>().toLatin1();
+        call->deleteLater();
+
+        if (!m_wakelockEnabled || !m_cookie.isEmpty()) {
+            // notified wakelock was created, but we either don't want it, or already have one - release it immediately
+            dbusInterface()->asyncCall("clearSysState", QString(cookie));
+            return;
+        }
+
+        m_cookie = cookie;
+
+        // see WORKAROUND above for why we save cookie to disk
+        QFile cookieCache(cookieFile);
+        cookieCache.open(QFile::WriteOnly | QFile::Text);
+        cookieCache.write(m_cookie);
+
+        qCDebug(QTMIR_SESSIONS) << "Wakelock acquired" << m_cookie;
+        Q_EMIT enabledChanged(true);
+    }
+
+private:
+    void acquireWakelock()
+    {
+        if (!serviceAvailable()) {
+            qWarning() << "com.canonical.powerd DBus interface not available, waiting for it";
             return;
         }
 
@@ -90,29 +162,8 @@
                          this, &Wakelock::onWakeLockAcquired);
     }
 
-    void onWakeLockAcquired(QDBusPendingCallWatcher *call)
-    {
-        if (m_cookie.isEmpty()) { // don't overwrite existing cookie
-            QDBusPendingReply<QString> reply = *call;
-            if (reply.isError()) {
-                qCDebug(QTMIR_SESSIONS) << "Wakelock was NOT acquired, error:"
-                                        << QDBusError::errorString(reply.error().type());
-            } else {
-                m_cookie = reply.argumentAt<0>();
-
-                // see WORKAROUND above for why we save cookie to disk
-                QFile cookie(cookieFile);
-                cookie.open(QFile::WriteOnly | QFile::Text);
-                cookie.write(m_cookie.toLatin1());
-
-                qCDebug(QTMIR_SESSIONS) << "Wakelock acquired" << m_cookie;
-            }
-        }
-        call->deleteLater();
-    }
-
-private:
-    QString m_cookie;
+    QByteArray m_cookie;
+    bool m_wakelockEnabled;
 
     Q_DISABLE_COPY(Wakelock)
 };
@@ -132,36 +183,48 @@
  * Note a caller cannot have multiple shares of the wakelock. Multiple calls to acquire are ignored.
  */
 
-QObject* SharedWakelock::createWakelock()
-{
-    return new Wakelock;
-}
-
+SharedWakelock::SharedWakelock(const QDBusConnection &connection)
+    : m_wakelock(new Wakelock(connection))
+{
+    connect(m_wakelock.data(), &Wakelock::enabledChanged,
+            this, &SharedWakelock::enabledChanged);
+}
+
+// Define empty deconstructor here, as QScopedPointer<Wakelock> requires the destructor of the Wakelock class
+// to be defined first.
+SharedWakelock::~SharedWakelock()
+{
+}
+
+bool SharedWakelock::enabled() const
+{
+    return m_wakelock->enabled();
+}
 
 void SharedWakelock::acquire(const QObject *caller)
 {
-    if (m_owners.contains(caller) || caller == nullptr) {
+    if (caller == nullptr || m_owners.contains(caller)) {
         return;
     }
 
     // register a slot to remove itself from owners list if destroyed
     QObject::connect(caller, &QObject::destroyed, this, &SharedWakelock::release);
 
-    if (m_wakelock.isNull()) {
-        m_wakelock.reset(createWakelock());
-    }
+    m_wakelock->acquire();
 
     m_owners.insert(caller);
 }
 
 void SharedWakelock::release(const QObject *caller)
 {
-    if (!m_owners.remove(caller) || caller == nullptr) {
+    if (caller == nullptr || !m_owners.remove(caller)) {
         return;
     }
 
-    if (m_owners.empty() && m_wakelock) {
-        m_wakelock.reset();
+    QObject::disconnect(caller, &QObject::destroyed, this, 0);
+
+    if (m_owners.empty()) {
+        m_wakelock->release();
     }
 }
 
 
=== modified file 'src/modules/Unity/Application/sharedwakelock.h'
--- src/modules/Unity/Application/sharedwakelock.h	2015-01-16 09:48:21 +0000
+++ src/modules/Unity/Application/sharedwakelock.h	2015-03-06 14:19:39 +0000
@@ -19,26 +19,31 @@
 #ifndef WAKELOCK_H
 #define WAKELOCK_H
 
-#include <QObject>
+#include <QDBusConnection>
 #include <QSet>
 #include <QScopedPointer>
 
 namespace qtmir {
 
+class Wakelock;
 class SharedWakelock : public QObject
 {
     Q_OBJECT
+    Q_PROPERTY(bool enabled READ enabled NOTIFY enabledChanged)
 public:
-    SharedWakelock() = default;
-    virtual ~SharedWakelock() noexcept = default;
-
-    void acquire(const QObject *caller);
-    Q_SLOT void release(const QObject *caller);
+    SharedWakelock(const QDBusConnection& connection = QDBusConnection::systemBus());
+    virtual ~SharedWakelock();
+
+    virtual bool enabled() const;
+
+    virtual void acquire(const QObject *caller);
+    Q_SLOT virtual void release(const QObject *caller);
+
+Q_SIGNALS:
+    void enabledChanged(bool enabled);
 
 protected:
-    virtual QObject* createWakelock(); // override to test
-
-    QScopedPointer<QObject> m_wakelock;
+    QScopedPointer<Wakelock> m_wakelock;
     QSet<const QObject *> m_owners;
 
 private:
 
=== modified file 'tests/modules/Application/application_test.cpp'
--- tests/modules/Application/application_test.cpp	2015-01-16 09:48:21 +0000
+++ tests/modules/Application/application_test.cpp	2015-03-06 14:19:39 +0000
@@ -34,7 +34,7 @@
 {
     using namespace ::testing;
 
-    EXPECT_CALL(sharedWakelock, createWakelock()).Times(1);
+    EXPECT_CALL(sharedWakelock, acquire(_)).Times(1);
 
     startApplication(123, "app");
     applicationManager.focusApplication("app");
@@ -50,14 +50,14 @@
     applicationManager.focusApplication("app");
 
     Q_EMIT session->suspended();
-    EXPECT_FALSE(sharedWakelock.wakelockHeld());
+    EXPECT_FALSE(sharedWakelock.enabled());
 }
 
 TEST_F(ApplicationTests, checkResumeAcquiresWakeLock)
 {
     using namespace ::testing;
 
-    EXPECT_CALL(sharedWakelock, createWakelock()).Times(1);
+    EXPECT_CALL(sharedWakelock, acquire(_)).Times(1);
 
     auto app = startApplication(123, "app");
     auto session = app->session();
@@ -69,7 +69,7 @@
 {
     using namespace ::testing;
 
-    EXPECT_CALL(sharedWakelock, createWakelock()).Times(1);
+    EXPECT_CALL(sharedWakelock, acquire(_)).Times(1);
     const QString appId = "app";
 
     auto app = startApplication(123, "app");
@@ -89,7 +89,7 @@
 {
     using namespace ::testing;
 
-    EXPECT_CALL(sharedWakelock, createWakelock()).Times(0);
+    EXPECT_CALL(sharedWakelock, acquire(_)).Times(0);
 
     startApplication(123, "unity8-dash");
     applicationManager.focusApplication("unity8-dash");
@@ -105,14 +105,14 @@
     applicationManager.focusApplication("unity8-dash");
 
     Q_EMIT session->suspended();
-    EXPECT_FALSE(sharedWakelock.wakelockHeld());
+    EXPECT_FALSE(sharedWakelock.enabled());
 }
 
 TEST_F(ApplicationTests, checkDashResumeDoesNotAcquireWakeLock)
 {
     using namespace ::testing;
 
-    EXPECT_CALL(sharedWakelock, createWakelock()).Times(0);
+    EXPECT_CALL(sharedWakelock, acquire(_)).Times(0);
 
     auto app = startApplication(123, "unity8-dash");
     auto session = app->session();
 
=== modified file 'tests/modules/ApplicationManager/application_manager_test.cpp'
--- tests/modules/ApplicationManager/application_manager_test.cpp	2014-10-20 18:48:53 +0000
+++ tests/modules/ApplicationManager/application_manager_test.cpp	2015-03-06 14:19:39 +0000
@@ -2135,3 +2135,81 @@
         cv.wait(lk, [&] { return done; } );
     }
 }
+
+/*
+ * Test lifecycle exempt applications have their wakelocks released when shell tries to suspend them
+ */
+TEST_F(ApplicationManagerTests,lifecycleExemptAppsHaveWakelockReleasedOnAttemptedSuspend)
+{
+    using namespace ::testing;
+
+    const QString appId("com.ubuntu.music"); // member of lifecycle exemption list
+    const quint64 procId = 12345;
+
+    // Set up Mocks & signal watcher
+    auto mockDesktopFileReader = new NiceMock<MockDesktopFileReader>(appId, QFileInfo());
+    ON_CALL(*mockDesktopFileReader, loaded()).WillByDefault(Return(true));
+    ON_CALL(*mockDesktopFileReader, appId()).WillByDefault(Return(appId));
+
+    ON_CALL(desktopFileReaderFactory, createInstance(appId, _)).WillByDefault(Return(mockDesktopFileReader));
+
+    EXPECT_CALL(appController, startApplicationWithAppIdAndArgs(appId, _))
+            .Times(1)
+            .WillOnce(Return(true));
+
+    auto application = applicationManager.startApplication(appId, ApplicationManager::NoFlag);
+    applicationManager.onProcessStarting(appId);
+    std::shared_ptr<mir::scene::Session> session = std::make_shared<MockSession>("", procId);
+    bool authed = true;
+    applicationManager.authorizeSession(procId, authed);
+    onSessionStarting(session);
+
+    // App creates surface, focuses it so state is running
+    std::shared_ptr<mir::scene::Surface> surface(nullptr);
+    applicationManager.onSessionCreatedSurface(session.get(), surface);
+    applicationManager.focusApplication(appId);
+
+    applicationManager.unfocusCurrentApplication();
+
+    EXPECT_FALSE(sharedWakelock.enabled());
+    EXPECT_EQ(application->state(), Application::Running);
+}
+
+/*
+ * Test lifecycle exempt applications have their wakelocks released on suspend
+ */
+TEST_F(ApplicationManagerTests,lifecycleExemptAppsHaveWakelockReleasedOnUnSuspend)
+{
+    using namespace ::testing;
+
+    const QString appId("com.ubuntu.music"); // member of lifecycle exemption list
+    const quint64 procId = 12345;
+
+    // Set up Mocks & signal watcher
+    auto mockDesktopFileReader = new NiceMock<MockDesktopFileReader>(appId, QFileInfo());
+    ON_CALL(*mockDesktopFileReader, loaded()).WillByDefault(Return(true));
+    ON_CALL(*mockDesktopFileReader, appId()).WillByDefault(Return(appId));
+
+    ON_CALL(desktopFileReaderFactory, createInstance(appId, _)).WillByDefault(Return(mockDesktopFileReader));
+
+    EXPECT_CALL(appController, startApplicationWithAppIdAndArgs(appId, _))
+            .Times(1)
+            .WillOnce(Return(true));
+
+    auto application = applicationManager.startApplication(appId, ApplicationManager::NoFlag);
+    applicationManager.onProcessStarting(appId);
+    std::shared_ptr<mir::scene::Session> session = std::make_shared<MockSession>("", procId);
+    bool authed = true;
+    applicationManager.authorizeSession(procId, authed);
+    onSessionStarting(session);
+
+    // App creates surface, focuses it so state is running
+    std::shared_ptr<mir::scene::Surface> surface(nullptr);
+    applicationManager.onSessionCreatedSurface(session.get(), surface);
+    applicationManager.focusApplication(appId);
+
+    applicationManager.setSuspended(true);
+
+    EXPECT_FALSE(sharedWakelock.enabled());
+    EXPECT_EQ(application->state(), Application::Running);
+}
 
=== added file 'tests/modules/SharedWakelock/SharedWakelock.pro'
--- tests/modules/SharedWakelock/SharedWakelock.pro	1970-01-01 00:00:00 +0000
+++ tests/modules/SharedWakelock/SharedWakelock.pro	2015-03-06 14:19:39 +0000
@@ -0,0 +1,11 @@
+include(../../test-includes.pri)
+include(../common/common.pri)
+
+TARGET = sharedwakelock_test
+PKGCONFIG += libqtdbusmock-1 libqtdbustest-1
+
+INCLUDEPATH += \
+    ../../../src/modules/Unity/Application
+
+SOURCES += \
+    sharedwakelock_test.cpp
 
=== modified file 'tests/modules/SharedWakelock/sharedwakelock_test.cpp'
--- tests/modules/SharedWakelock/sharedwakelock_test.cpp	2015-01-16 09:48:21 +0000
+++ tests/modules/SharedWakelock/sharedwakelock_test.cpp	2015-03-06 14:19:39 +0000
@@ -17,154 +17,373 @@
 
 #include <Unity/Application/sharedwakelock.h>
 
-#include "mock_shared_wakelock.h"
+#include <libqtdbusmock/DBusMock.h>
 
-#include <gmock/gmock.h>
+#include <QCoreApplication>
+#include <QSignalSpy>
 #include <gtest/gtest.h>
 
 using namespace qtmir;
-using testing::MockSharedWakelock;
-
-TEST(SharedWakelock, acquireCreatesAWakelock)
-{
-    using namespace ::testing;
-
-    testing::NiceMock<MockSharedWakelock> sharedWakelock;
-    QScopedPointer<QObject> app1(new QObject);
-
-    EXPECT_CALL(sharedWakelock, createWakelock()).Times(1);
-    sharedWakelock.acquire(app1.data());
-}
-
-TEST(SharedWakelock, acquireThenReleaseDestroysTheWakelock)
-{
-    using namespace ::testing;
-
-    testing::NiceMock<MockSharedWakelock> sharedWakelock;
-    QScopedPointer<QObject> app1(new QObject);
-
-    sharedWakelock.acquire(app1.data());
-    sharedWakelock.release(app1.data());
-    EXPECT_FALSE(sharedWakelock.wakelockHeld());
-}
-
-TEST(SharedWakelock, doubleAcquireBySameOwnerOnlyCreatesASingleWakelock)
-{
-    using namespace ::testing;
-
-    testing::NiceMock<MockSharedWakelock> sharedWakelock;
-    QScopedPointer<QObject> app1(new QObject);
-
-    EXPECT_CALL(sharedWakelock, createWakelock()).Times(1).WillOnce(Return(new QObject));
-    sharedWakelock.acquire(app1.data());
-    sharedWakelock.acquire(app1.data());
-}
-
-TEST(SharedWakelock, doubleAcquireThenReleaseBySameOwnerDestroysWakelock)
-{
-    using namespace ::testing;
-
-    testing::NiceMock<MockSharedWakelock> sharedWakelock;
-    QScopedPointer<QObject> app1(new QObject);
-
-    EXPECT_CALL(sharedWakelock, createWakelock()).Times(1).WillOnce(Return(new QObject));
-    sharedWakelock.acquire(app1.data());
-    sharedWakelock.acquire(app1.data());
-    sharedWakelock.release(app1.data());
-    EXPECT_FALSE(sharedWakelock.wakelockHeld());
-}
-
-TEST(SharedWakelock, acquireByDifferentOwnerOnlyCreatesASingleWakelock)
-{
-    using namespace ::testing;
-
-    testing::NiceMock<MockSharedWakelock> sharedWakelock;
-    QScopedPointer<QObject> app1(new QObject);
-    QScopedPointer<QObject> app2(new QObject);
-
-    EXPECT_CALL(sharedWakelock, createWakelock()).Times(1).WillOnce(Return(new QObject));
-    sharedWakelock.acquire(app1.data());
-    sharedWakelock.acquire(app2.data());
-}
-
-TEST(SharedWakelock, twoOwnersWhenOneReleasesStillHoldWakelock)
-{
-    using namespace ::testing;
-
-    testing::NiceMock<MockSharedWakelock> sharedWakelock;
-    QScopedPointer<QObject> app1(new QObject);
-    QScopedPointer<QObject> app2(new QObject);
-
-    EXPECT_CALL(sharedWakelock, createWakelock()).Times(1).WillOnce(Return(new QObject));
-    sharedWakelock.acquire(app1.data());
-    sharedWakelock.acquire(app2.data());
-    sharedWakelock.release(app1.data());
-    EXPECT_TRUE(sharedWakelock.wakelockHeld());
-}
-
-TEST(SharedWakelock, twoOwnersWhenBothReleaseWakelockReleased)
-{
-    using namespace ::testing;
-
-    testing::NiceMock<MockSharedWakelock> sharedWakelock;
-    QScopedPointer<QObject> app1(new QObject);
-    QScopedPointer<QObject> app2(new QObject);
-
-    EXPECT_CALL(sharedWakelock, createWakelock()).Times(1).WillOnce(Return(new QObject));
-    sharedWakelock.acquire(app1.data());
-    sharedWakelock.acquire(app2.data());
-    sharedWakelock.release(app2.data());
-    sharedWakelock.release(app1.data());
-    EXPECT_FALSE(sharedWakelock.wakelockHeld());
-}
-
-TEST(SharedWakelock, doubleReleaseOfSingleOwnerIgnored)
-{
-    using namespace ::testing;
-
-    testing::NiceMock<MockSharedWakelock> sharedWakelock;
-    QScopedPointer<QObject> app1(new QObject);
-    QScopedPointer<QObject> app2(new QObject);
-
-    EXPECT_CALL(sharedWakelock, createWakelock()).Times(1).WillOnce(Return(new QObject));
-    sharedWakelock.acquire(app1.data());
-    sharedWakelock.acquire(app2.data());
-    sharedWakelock.release(app1.data());
-    EXPECT_TRUE(sharedWakelock.wakelockHeld());
-
-    sharedWakelock.release(app1.data());
-    EXPECT_TRUE(sharedWakelock.wakelockHeld());
-}
-
-TEST(SharedWakelock, nullOwnerAcquireIgnored)
-{
-    using namespace ::testing;
-
-    testing::NiceMock<MockSharedWakelock> sharedWakelock;
-
-    EXPECT_CALL(sharedWakelock, createWakelock()).Times(0);
-    sharedWakelock.acquire(nullptr);
-}
-
-TEST(SharedWakelock, nullOwnerReleaseIgnored)
-{
-    using namespace ::testing;
-
-    testing::NiceMock<MockSharedWakelock> sharedWakelock;
-
-    EXPECT_CALL(sharedWakelock, createWakelock()).Times(0);
-    sharedWakelock.release(nullptr);
-}
-
-TEST(SharedWakelock, ifOwnerDestroyedWakelockReleased)
-{
-    using namespace ::testing;
-
-    testing::NiceMock<MockSharedWakelock> sharedWakelock;
-    QScopedPointer<QObject> app1(new QObject);
-
-    EXPECT_CALL(sharedWakelock, createWakelock()).Times(1).WillOnce(Return(new QObject));
-    sharedWakelock.acquire(app1.data());
-    app1.reset();
-    EXPECT_FALSE(sharedWakelock.wakelockHeld());
+using namespace testing;
+using namespace QtDBusTest;
+using namespace QtDBusMock;
+
+const char POWERD_NAME[] = "com.canonical.powerd";
+const char POWERD_PATH[] = "/com/canonical/powerd";
+const char POWERD_INTERFACE[] = "com.canonical.powerd";
+
+class SharedWakelockTest: public Test
+{
+protected:
+    SharedWakelockTest()
+        : mock(dbus)
+    {
+        mock.registerCustomMock(POWERD_NAME,
+                                POWERD_PATH,
+                                POWERD_INTERFACE,
+                                QDBusConnection::SystemBus);
+        
+        dbus.startServices();
+    }
+    
+    virtual ~SharedWakelockTest()
+    {}
+
+    virtual OrgFreedesktopDBusMockInterface& powerdMockInterface()
+    {
+        return mock.mockInterface(POWERD_NAME,
+                                  POWERD_PATH,
+                                  POWERD_INTERFACE,
+                                  QDBusConnection::SystemBus);
+    }
+
+    void implementRequestSysState() {
+        // Defines the mock impementation of this DBus method
+        powerdMockInterface().AddMethod("com.canonical.powerd",
+                "requestSysState", "si", "s", "ret = 'cookie'").waitForFinished();
+    }
+
+    void implementClearSysState() {
+        powerdMockInterface().AddMethod("com.canonical.powerd",
+                "clearSysState", "s", "", "").waitForFinished();
+    }
+
+    void EXPECT_CALL(const QList<QVariantList> &spy, int index,
+                     const QString &name, const QVariantList &args)
+    {
+        QVariant args2(QVariant::fromValue(args));
+        ASSERT_LT(index, spy.size());
+        const QVariantList &call(spy.at(index));
+        EXPECT_EQ(name, call.at(0).toString());
+        EXPECT_EQ(args2.toString().toStdString(), call.at(1).toString().toStdString());
+    }
+    
+    DBusTestRunner dbus;
+    DBusMock mock;
+};
+
+TEST_F(SharedWakelockTest, acquireCreatesAWakelock)
+{
+    implementRequestSysState();
+
+    /* Note: we pass the DBusTestRunner constructed system DBus connection instead of letting
+     * Qt create one. This is done as Qt has a non-testing friendly way of handling the built-in
+     * system and session connections, and as DBusTestRunner restarts the DBus daemon for each
+     * testcase, it unfortunately means Qt would end up pointing to a dead bus after one testcase. */
+    SharedWakelock wakelock(dbus.systemConnection());
+
+    // Verify the DBus method is called & wakelock
+    QSignalSpy wakelockDBusMethodSpy(&powerdMockInterface(), SIGNAL(MethodCalled(const QString &, const QVariantList &)));
+    QSignalSpy wakelockEnabledSpy(&wakelock, SIGNAL( enabledChanged(bool) ));
+
+    QScopedPointer<QObject> object(new QObject);
+    wakelock.acquire(object.data());
+    wakelockDBusMethodSpy.wait();
+
+    EXPECT_FALSE(wakelockDBusMethodSpy.empty());
+    EXPECT_CALL(wakelockDBusMethodSpy, 0, "requestSysState",
+                QVariantList() << QString("active") << 1);
+
+    // Ensure a wakelock created
+    wakelockEnabledSpy.wait();
+    EXPECT_FALSE(wakelockEnabledSpy.empty());
+    EXPECT_TRUE(wakelock.enabled());
+}
+
+TEST_F(SharedWakelockTest, acquireThenReleaseDestroysTheWakelock)
+{
+    implementRequestSysState();
+    implementClearSysState();
+
+    SharedWakelock wakelock(dbus.systemConnection());
+
+    QSignalSpy wakelockEnabledSpy(&wakelock, SIGNAL( enabledChanged(bool) ));
+
+    QScopedPointer<QObject> object(new QObject);
+    wakelock.acquire(object.data());
+    wakelockEnabledSpy.wait();
+
+    // Verify the DBus method is called
+    QSignalSpy wakelockDBusMethodSpy(&powerdMockInterface(), SIGNAL(MethodCalled(const QString &, const QVariantList &)));
+    wakelock.release(object.data());
+    wakelockDBusMethodSpy.wait();
+
+    EXPECT_FALSE(wakelockDBusMethodSpy.empty());
+    EXPECT_CALL(wakelockDBusMethodSpy, 0, "clearSysState",
+                QVariantList() << QString("cookie"));
+
+    EXPECT_FALSE(wakelock.enabled());
+}
+
+TEST_F(SharedWakelockTest, doubleAcquireBySameOwnerOnlyCreatesASingleWakelock)
+{
+    implementRequestSysState();
+
+    SharedWakelock wakelock(dbus.systemConnection());
+
+    // Verify the DBus method is called & wakelock
+    QSignalSpy wakelockDBusMethodSpy(&powerdMockInterface(), SIGNAL(MethodCalled(const QString &, const QVariantList &)));
+
+    QScopedPointer<QObject> object(new QObject);
+    wakelock.acquire(object.data());
+    wakelock.acquire(object.data());
+    wakelockDBusMethodSpy.wait();
+
+    EXPECT_EQ(wakelockDBusMethodSpy.count(), 1);
+}
+
+TEST_F(SharedWakelockTest, doubleAcquireThenReleaseBySameOwnerDestroysWakelock)
+{
+    implementRequestSysState();
+    implementClearSysState();
+
+    SharedWakelock wakelock(dbus.systemConnection());
+
+    QSignalSpy wakelockEnabledSpy(&wakelock, SIGNAL( enabledChanged(bool) ));
+    QScopedPointer<QObject> object(new QObject);
+
+    wakelock.acquire(object.data());
+    wakelock.acquire(object.data());
+    wakelock.release(object.data());
+    wakelockEnabledSpy.wait();
+    EXPECT_FALSE(wakelock.enabled());
+}
+
+TEST_F(SharedWakelockTest, acquireByDifferentOwnerOnlyCreatesASingleWakelock)
+{
+    implementRequestSysState();
+
+    SharedWakelock wakelock(dbus.systemConnection());
+
+    // Verify the DBus method is called & wakelock
+    QSignalSpy wakelockDBusMethodSpy(&powerdMockInterface(), SIGNAL(MethodCalled(const QString &, const QVariantList &)));
+
+    QScopedPointer<QObject> object1(new QObject);
+    QScopedPointer<QObject> object2(new QObject);
+    wakelock.acquire(object1.data());
+    wakelock.acquire(object2.data());
+
+    wakelockDBusMethodSpy.wait();
+    EXPECT_EQ(wakelockDBusMethodSpy.count(), 1);
+}
+
+TEST_F(SharedWakelockTest, twoOwnersWhenBothReleaseWakelockReleased)
+{
+    implementRequestSysState();
+
+    SharedWakelock wakelock(dbus.systemConnection());
+
+    QScopedPointer<QObject> object1(new QObject);
+    QScopedPointer<QObject> object2(new QObject);
+    wakelock.acquire(object1.data());
+    wakelock.acquire(object2.data());
+
+    QSignalSpy wakelockEnabledSpy(&wakelock, SIGNAL( enabledChanged(bool) ));
+
+    wakelock.release(object1.data());
+    wakelock.release(object2.data());
+
+    wakelockEnabledSpy.wait();
+    EXPECT_FALSE(wakelock.enabled());
+}
+
+TEST_F(SharedWakelockTest, doubleReleaseOfSingleOwnerIgnored)
+{
+    implementRequestSysState();
+
+    SharedWakelock wakelock(dbus.systemConnection());
+
+    QSignalSpy wakelockEnabledSpy(&wakelock, SIGNAL( enabledChanged(bool) ));
+
+    QScopedPointer<QObject> object1(new QObject);
+    QScopedPointer<QObject> object2(new QObject);
+    wakelock.acquire(object1.data());
+    wakelock.acquire(object2.data());
+    wakelock.release(object1.data());
+
+    wakelockEnabledSpy.wait();
+
+    wakelock.release(object1.data());
+
+    EXPECT_TRUE(wakelock.enabled());
+}
+
+TEST_F(SharedWakelockTest, wakelockAcquireReleaseFlood)
+{
+    implementRequestSysState();
+    implementClearSysState();
+
+    SharedWakelock wakelock(dbus.systemConnection());
+
+    QSignalSpy wakelockEnabledSpy(&wakelock, SIGNAL( enabledChanged(bool) ));
+
+    QScopedPointer<QObject> object(new QObject);
+    wakelock.acquire(object.data());
+    wakelock.release(object.data());
+    wakelock.acquire(object.data());
+    wakelock.release(object.data());
+    wakelock.acquire(object.data());
+    wakelock.release(object.data());
+    while (wakelockEnabledSpy.wait(100)) {}
+    EXPECT_FALSE(wakelock.enabled());
+}
+
+TEST_F(SharedWakelockTest, wakelockAcquireReleaseAcquireWithDelays)
+{
+    powerdMockInterface().AddMethod("com.canonical.powerd",
+            "requestSysState", "si", "s",
+            "i=100000\n" // delay the response from the mock dbus instance
+            "while i>0:\n"
+            "    i-=1\n"
+            "ret = 'cookie'").waitForFinished();
+
+    implementClearSysState();
+
+    SharedWakelock wakelock(dbus.systemConnection());
+
+    QSignalSpy wakelockDBusMethodSpy(&powerdMockInterface(), SIGNAL(MethodCalled(const QString &, const QVariantList &)));
+
+    QScopedPointer<QObject> object(new QObject);
+    wakelock.acquire(object.data());
+    wakelock.release(object.data());
+    wakelock.acquire(object.data());
+
+    while (wakelockDBusMethodSpy.wait(100)) {}
+    EXPECT_TRUE(wakelock.enabled());
+
+    // there must be at least one clearSysState call, but is not necessarily the second call
+    // should the dbus response be slow enough, it may be the third call. Is hard to reliably
+    // reproduce the racey condition for all platforms.
+    bool found = false;
+    for (int i=0; i<wakelockDBusMethodSpy.count(); i++) {
+        const QVariantList &call(wakelockDBusMethodSpy.at(i));
+
+        if (call.at(0).toString() == "clearSysState") {
+            found = true;
+        }
+    }
+    EXPECT_TRUE(found);
+}
+
+TEST_F(SharedWakelockTest, nullOwnerAcquireIgnored)
+{
+    implementRequestSysState();
+
+    SharedWakelock wakelock(dbus.systemConnection());
+
+    QSignalSpy wakelockEnabledSpy(&wakelock, SIGNAL( enabledChanged(bool) ));
+
+    wakelock.acquire(nullptr);
+
+    wakelockEnabledSpy.wait(200); // have to wait to see if anything happens
+
+    EXPECT_FALSE(wakelock.enabled());
+}
+
+TEST_F(SharedWakelockTest, nullReleaseAcquireIgnored)
+{
+    implementRequestSysState();
+
+    SharedWakelock wakelock(dbus.systemConnection());
+
+    wakelock.release(nullptr);
+
+    EXPECT_FALSE(wakelock.enabled());
+}
+
+TEST_F(SharedWakelockTest, ifOwnerDestroyedWakelockReleased)
+{
+    implementRequestSysState();
+    implementClearSysState();
+
+    SharedWakelock wakelock(dbus.systemConnection());
+
+    QSignalSpy wakelockEnabledSpy(&wakelock, SIGNAL( enabledChanged(bool) ));
+
+    QScopedPointer<QObject> object(new QObject);
+    wakelock.acquire(object.data());
+    wakelockEnabledSpy.wait();
+
+    // Verify the DBus method is called
+    QSignalSpy wakelockDBusMethodSpy(&powerdMockInterface(), SIGNAL(MethodCalled(const QString &, const QVariantList &)));
+
+    object.reset();
+    wakelockDBusMethodSpy.wait();
+
+    EXPECT_FALSE(wakelockDBusMethodSpy.empty());
+    EXPECT_CALL(wakelockDBusMethodSpy, 0, "clearSysState",
+                QVariantList() << QString("cookie"));
+
+    EXPECT_FALSE(wakelock.enabled());
+}
+
+TEST_F(SharedWakelockTest, reloadCachedWakelockCookie)
+{
+    const char cookieFile[] = "/tmp/qtmir_powerd_cookie";
+
+    // Custom Deleter for QScopedPointer to delete the qtmir cookie file no matter what
+    struct ScopedQFileCustomDeleter
+    {
+        static inline void cleanup(QFile *file)
+        {
+            file->remove();
+            delete file;
+        }
+    };
+    QScopedPointer<QFile, ScopedQFileCustomDeleter> cookieCache(new QFile(cookieFile));
+    cookieCache->open(QFile::WriteOnly | QFile::Text);
+    cookieCache->write("myCookie");
+
+    SharedWakelock wakelock(dbus.systemConnection());
+    EXPECT_TRUE(wakelock.enabled());
+}
+
+TEST_F(SharedWakelockTest, wakelockReleasedOnSharedWakelockDestroyed)
+{
+    implementRequestSysState();
+    implementClearSysState();
+
+    auto wakelock = new SharedWakelock(dbus.systemConnection());
+
+    QSignalSpy wakelockEnabledSpy(wakelock, SIGNAL( enabledChanged(bool) ));
+
+    QScopedPointer<QObject> object(new QObject);
+    wakelock->acquire(object.data());
+    wakelockEnabledSpy.wait(); // wait for wakelock to be enabled
+
+    QSignalSpy wakelockDBusMethodSpy(&powerdMockInterface(), SIGNAL(MethodCalled(const QString &, const QVariantList &)));
+    delete wakelock;
+    wakelockDBusMethodSpy.wait();
+
+    EXPECT_FALSE(wakelockDBusMethodSpy.empty());
+    EXPECT_CALL(wakelockDBusMethodSpy, 0, "clearSysState",
+                QVariantList() << QString("cookie"));
+}
+
+
+// Define custom main() as these tests rely on a QEventLoop
+int main(int argc, char** argv) {
+    QCoreApplication app(argc, argv);
+    ::testing::InitGoogleTest(&argc, argv);
+    return RUN_ALL_TESTS();
 }
 
=== modified file 'tests/modules/common/common.pri'
--- tests/modules/common/common.pri	2014-10-20 18:48:53 +0000
+++ tests/modules/common/common.pri	2015-03-06 14:19:39 +0000
@@ -1,7 +1,7 @@
 CONFIG += no_keywords # keywords clash with ProcessC++
 PKGCONFIG += ubuntu-app-launch-2 process-cpp
 
-QT += quick
+QT += quick dbus
 
 HEADERS += ../common/mock_application_controller.h \
     ../common/mock_desktop_file_reader.h \
@@ -11,6 +11,7 @@
     ../common/mock_prompt_session.h \
     ../common/mock_prompt_session_manager.h \
     ../common/mock_renderable.h \
+    ../common/mock_shared_wakelock.h \
     ../common/mock_session.h \
     ../common/mock_surface.h \
     ../common/qtmir_test.h \
 
=== modified file 'tests/modules/common/mock_shared_wakelock.h'
--- tests/modules/common/mock_shared_wakelock.h	2015-01-16 09:48:21 +0000
+++ tests/modules/common/mock_shared_wakelock.h	2015-03-06 14:19:39 +0000
@@ -23,21 +23,48 @@
 
 namespace testing
 {
-struct MockSharedWakelock : public qtmir::SharedWakelock
+class MockSharedWakelock : public qtmir::SharedWakelock
 {
-    MockSharedWakelock()
-    {
-        ON_CALL(*this, createWakelock()).WillByDefault(Invoke(this, &MockSharedWakelock::doCreateWakelock));
-    }
-
-    MOCK_METHOD0(createWakelock, QObject*());
-    bool wakelockHeld() { return m_wakelock; }
-
-
-    QObject* doCreateWakelock() const
-    {
-        return new QObject;
-    }
+public:
+    MockSharedWakelock(const QDBusConnection& /*connection*/= QDBusConnection::systemBus())
+    {
+        ON_CALL(*this, enabled()).WillByDefault(Invoke(this, &MockSharedWakelock::doEnabled));
+        ON_CALL(*this, acquire(_)).WillByDefault(Invoke(this, &MockSharedWakelock::doAcquire));
+        ON_CALL(*this, release(_)).WillByDefault(Invoke(this, &MockSharedWakelock::doRelease));
+    }
+
+    MOCK_CONST_METHOD0(enabled, bool());
+    MOCK_METHOD1(acquire, void(const QObject *));
+    MOCK_METHOD1(release, void(const QObject *));
+
+    bool doEnabled()
+    {
+        return !m_owners.isEmpty();
+    }
+
+    void doAcquire(const QObject *object)
+    {
+        if (m_owners.contains(object)) {
+            return;
+        }
+        m_owners.insert(object);
+        if (m_owners.size() == 1) {
+            Q_EMIT enabledChanged(true);
+        }
+    }
+
+    void doRelease(const QObject *object)
+    {
+        if (!m_owners.remove(object)) {
+            return;
+        }
+        if (m_owners.isEmpty()) {
+            Q_EMIT enabledChanged(false);
+        }
+    }
+
+private:
+    QSet<const QObject *> m_owners;
 };
 
 } // namespace testing
 
=== modified file 'tests/modules/modules.pro'
--- tests/modules/modules.pro	2015-01-16 09:48:21 +0000
+++ tests/modules/modules.pro	2015-03-06 14:19:39 +0000
@@ -1,2 +1,2 @@
 TEMPLATE = subdirs
-SUBDIRS = Application ApplicationManager General MirSurfaceItem SessionManager TaskController DesktopFileReader
+SUBDIRS = Application ApplicationManager General MirSurfaceItem SessionManager SharedWakelock TaskController DesktopFileReader