Merge lp:~mzanetti/qtmir/exception-settings into lp:qtmir

[Michael Zanetti]

read exception list from gsettings instead of a hardcoded list

 * Are there any related MPs required for this MP to build/function as expected? Please list.

no

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

yes

 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?

no

[lp:~mzanetti/qtmir/exception-settings]

326. By Michael Zanetti on 2015-03-13

update build dep

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:326
http://jenkins.qa.ubuntu.com/job/qtmir-ci/236/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/87/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/87/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qtmir-ci/236/rebuild

review: Needs Fixing (continuous-integration)

[lp:~mzanetti/qtmir/exception-settings]

327. By Michael Zanetti on 2015-03-13

camellCase it because the emitted key in the signal is converted to camelCase

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:327
http://jenkins.qa.ubuntu.com/job/qtmir-ci/237/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/88/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/88/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qtmir-ci/237/rebuild

review: Needs Fixing (continuous-integration)

[Daniel d'Andrada (dandrader)]

Tests are crashing.

review: Needs Fixing

[Daniel d'Andrada (dandrader)]

And the checklist is missing:
https://wiki.ubuntu.com/Process/Merges/Checklists/QtMir

review: Needs Fixing

[lp:~mzanetti/qtmir/exception-settings]

328. By Michael Zanetti on 2015-03-16

fix tests

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:328
http://jenkins.qa.ubuntu.com/job/qtmir-ci/238/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/89
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/89
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/89/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qtmir-ci/238/rebuild

review: Approve (continuous-integration)

[Daniel d'Andrada (dandrader)]

review: Abstain

[Gerry Boland (gerboland)]

Code looks ok, but there's no tests!

review: Needs Fixing

[lp:~mzanetti/qtmir/exception-settings]

329. By Michael Zanetti on 2015-03-24

properly mock settings and add a test for it

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:329
http://jenkins.qa.ubuntu.com/job/qtmir-ci/240/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/91/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/91/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qtmir-ci/240/rebuild

review: Needs Fixing (continuous-integration)

[lp:~mzanetti/qtmir/exception-settings]

330. By Michael Zanetti on 2015-03-25

merge trunk

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:330
http://jenkins.qa.ubuntu.com/job/qtmir-ci/241/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/92
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/92
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/92/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qtmir-ci/241/rebuild

review: Approve (continuous-integration)

[Gerry Boland (gerboland)]

Code looks ok, testing on device now

[Gerry Boland (gerboland)]

Ok, this works. I'm not 100% happy about having this setting defined by qtmir, as I want the lifecycle decision logic in the shell/unity8. Once that is done, the gsettings key will change, but that's ok as we know the users

review: Approve

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2015-03-18 10:12:16 +0000
+++ CMakeLists.txt	2015-03-25 08:59:38 +0000
@@ -77,6 +77,7 @@
 pkg_check_modules(GIO gio-2.0)
 pkg_check_modules(GIO_UNIX gio-unix-2.0)
 pkg_check_modules(LTTNG lttng-ust)
+pkg_check_modules(GSETTINGS_QT REQUIRED gsettings-qt)
 pkg_check_modules(QTDBUSTEST libqtdbustest-1 REQUIRED)
 pkg_check_modules(QTDBUSMOCK libqtdbusmock-1 REQUIRED)
 

=== modified file 'debian/control'
--- debian/control	2015-03-04 17:08:47 +0000
+++ debian/control	2015-03-25 08:59:38 +0000
@@ -16,6 +16,7 @@
                libfontconfig1-dev,
                libgles2-mesa-dev,
                libglib2.0-dev,
+               libgsettings-qt-dev,
                liblttng-ust-dev,
                libmirclient-dev (>= 0.6.0),
                libmircommon-dev,

=== modified file 'debian/qtdeclarative5-qtmir-plugin.install'
--- debian/qtdeclarative5-qtmir-plugin.install	2014-07-07 19:33:56 +0000
+++ debian/qtdeclarative5-qtmir-plugin.install	2015-03-25 08:59:38 +0000
@@ -1,1 +1,2 @@
 usr/lib/*/qt5/qml/*
+usr/share/glib-2.0/schemas

=== modified file 'src/modules/Unity/Application/CMakeLists.txt'
--- src/modules/Unity/Application/CMakeLists.txt	2015-01-08 12:37:08 +0000
+++ src/modules/Unity/Application/CMakeLists.txt	2015-03-25 08:59:38 +0000
@@ -9,6 +9,7 @@
     ${UBUNTU_APP_LAUNCH_INCLUDE_DIRS}
     ${CMAKE_SOURCE_DIR}/src/platforms/mirserver
     ${CMAKE_SOURCE_DIR}/src/common
+    ${GSETTINGS_QT_INCLUDE_DIRS}
 
     ${LTTNG_INCLUDE_DIRS}
     ${Qt5Gui_PRIVATE_INCLUDE_DIRS}
@@ -41,12 +42,14 @@
     sharedwakelock.cpp
     upstart/applicationcontroller.cpp
     tracepoints.c
+    settings.cpp
 # We need to run moc on these headers
     /usr/include/unity/shell/application/ApplicationInfoInterface.h
     /usr/include/unity/shell/application/ApplicationManagerInterface.h
 # Feed the automoc monster
     session_interface.h
     applicationcontroller.h
+    settings_interface.h
     )
 
 add_library(unityapplicationplugin SHARED
@@ -64,6 +67,7 @@
     ${PROCESS_CPP_LDFLAGS}
     ${UBUNTU_APP_LAUNCH_LDFLAGS}
     ${LTTNG_LDFLAGS}
+    ${GSETTINGS_QT_LDFLAGS}
 
     ${GL_LIBRARIES}
 
@@ -82,3 +86,5 @@
 
 # install
 add_qml_plugin(Unity.Application 0.1 Unity/Application TARGETS unityapplicationplugin)
+install(FILES com.canonical.qtmir.gschema.xml DESTINATION ${CMAKE_INSTALL_DATADIR}/glib-2.0/schemas)
+

=== modified file 'src/modules/Unity/Application/application_manager.cpp'
--- src/modules/Unity/Application/application_manager.cpp	2015-02-25 11:27:56 +0000
+++ src/modules/Unity/Application/application_manager.cpp	2015-03-25 08:59:38 +0000
@@ -25,6 +25,7 @@
 #include "taskcontroller.h"
 #include "upstart/applicationcontroller.h"
 #include "tracepoints.h" // generated from tracepoints.tp
+#include "settings.h"
 
 // mirserver
 #include "mirserver.h"
@@ -152,6 +153,7 @@
     QSharedPointer<DesktopFileReader::Factory> fileReaderFactory(new DesktopFileReader::Factory());
     QSharedPointer<ProcInfo> procInfo(new ProcInfo());
     QSharedPointer<SharedWakelock> sharedWakelock(new SharedWakelock);
+    QSharedPointer<Settings> settings(new Settings());
 
     // FIXME: We should use a QSharedPointer to wrap this ApplicationManager object, which requires us
     // to use the data() method to pass the raw pointer to the QML engine. However the QML engine appears
@@ -163,7 +165,8 @@
                                              taskController,
                                              sharedWakelock,
                                              fileReaderFactory,
-                                             procInfo
+                                             procInfo,
+                                             settings
                                          );
 
     connectToSessionListener(appManager, sessionListener);
@@ -199,18 +202,19 @@
         const QSharedPointer<SharedWakelock>& sharedWakelock,
         const QSharedPointer<DesktopFileReader::Factory>& desktopFileReaderFactory,
         const QSharedPointer<ProcInfo>& procInfo,
+        const QSharedPointer<SettingsInterface>& settings,
         QObject *parent)
     : ApplicationManagerInterface(parent)
     , m_mirServer(mirServer)
     , m_focusedApplication(nullptr)
     , m_mainStageApplication(nullptr)
     , m_sideStageApplication(nullptr)
-    , m_lifecycleExceptions(QStringList() << "com.ubuntu.music")
     , m_dbusWindowStack(new DBusWindowStack(this))
     , m_taskController(taskController)
     , m_desktopFileReaderFactory(desktopFileReaderFactory)
     , m_procInfo(procInfo)
     , m_sharedWakelock(sharedWakelock)
+    , m_settings(settings)
     , m_suspended(false)
     , m_forceDashActive(false)
 {
@@ -219,6 +223,11 @@
 
     m_roleNames.insert(RoleSession, "session");
     m_roleNames.insert(RoleFullscreen, "fullscreen");
+
+    if (settings.data()) {
+        m_lifecycleExceptions = m_settings->get("lifecycleExemptAppids").toStringList();
+        connect(m_settings.data(), &Settings::changed, this, &ApplicationManager::onSettingsChanged);
+    }
 }
 
 ApplicationManager::~ApplicationManager()
@@ -712,6 +721,13 @@
     }
 }
 
+void ApplicationManager::onSettingsChanged(const QString &key)
+{
+    if (key == "lifecycleExemptAppids") {
+        m_lifecycleExceptions = m_settings->get("lifecycleExemptAppids").toStringList();
+    }
+}
+
 void ApplicationManager::authorizeSession(const quint64 pid, bool &authorized)
 {
     tracepoint(qtmir, authorizeSession);

=== modified file 'src/modules/Unity/Application/application_manager.h'
--- src/modules/Unity/Application/application_manager.h	2015-01-09 11:23:34 +0000
+++ src/modules/Unity/Application/application_manager.h	2015-03-25 08:59:38 +0000
@@ -48,6 +48,7 @@
 class ProcInfo;
 class SharedWakelock;
 class TaskController;
+class SettingsInterface;
 
 class ApplicationManager : public unity::shell::application::ApplicationManagerInterface
 {
@@ -86,6 +87,7 @@
             const QSharedPointer<SharedWakelock> &sharedWakelock,
             const QSharedPointer<DesktopFileReader::Factory> &desktopFileReaderFactory,
             const QSharedPointer<ProcInfo> &processInfo,
+            const QSharedPointer<SettingsInterface> &settings,
             QObject *parent = 0);
     virtual ~ApplicationManager();
 
@@ -136,6 +138,7 @@
 
 private Q_SLOTS:
     void onAppDataChanged(const int role);
+    void onSettingsChanged(const QString &key);
 
 private:
     void setFocused(Application *application);
@@ -163,6 +166,7 @@
     QSharedPointer<DesktopFileReader::Factory> m_desktopFileReaderFactory;
     QSharedPointer<ProcInfo> m_procInfo;
     QSharedPointer<SharedWakelock> m_sharedWakelock;
+    QSharedPointer<SettingsInterface> m_settings;
     static ApplicationManager* the_application_manager;
     QList<pid_t> m_hiddenPIDs;
     bool m_suspended;

=== added file 'src/modules/Unity/Application/com.canonical.qtmir.gschema.xml'
--- src/modules/Unity/Application/com.canonical.qtmir.gschema.xml	1970-01-01 00:00:00 +0000
+++ src/modules/Unity/Application/com.canonical.qtmir.gschema.xml	2015-03-25 08:59:38 +0000
@@ -0,0 +1,10 @@
+<schemalist>
+  <schema path="/com/canonical/qtmir/" id="com.canonical.qtmir" gettext-domain="qtmir">
+    <key type="as" name="lifecycle-exempt-appids">
+      <default>[
+        'com.ubuntu.music'
+      ]</default>
+      <summary>List of apps that should be excluded from the app lifecycle</summary>
+    </key>
+  </schema>
+</schemalist>

=== added file 'src/modules/Unity/Application/settings.cpp'
--- src/modules/Unity/Application/settings.cpp	1970-01-01 00:00:00 +0000
+++ src/modules/Unity/Application/settings.cpp	2015-03-25 08:59:38 +0000
@@ -0,0 +1,39 @@
+/*
+ * Copyright (C) 2015 Canonical, Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it under
+ * the terms of the GNU Lesser 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+// Ubuntu
+#include <QGSettings>
+#include <QVariant>
+
+// local
+#include "settings.h"
+
+namespace qtmir
+{
+
+Settings::Settings(QObject *parent)
+    :SettingsInterface(parent)
+    ,m_settings(new QGSettings("com.canonical.qtmir", "/com/canonical/qtmir/"))
+{
+    connect(m_settings, &QGSettings::changed, this, &Settings::changed);
+}
+
+QVariant Settings::get(const QString &key) const
+{
+    return m_settings->get(key);
+}
+
+} // namespace qtmir

=== added file 'src/modules/Unity/Application/settings.h'
--- src/modules/Unity/Application/settings.h	1970-01-01 00:00:00 +0000
+++ src/modules/Unity/Application/settings.h	2015-03-25 08:59:38 +0000
@@ -0,0 +1,45 @@
+/*
+ * Copyright (C) 2015 Canonical, Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it under
+ * the terms of the GNU Lesser 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef SETTINGS_H
+#define SETTINGS_H
+
+//Qt
+#include <QObject>
+
+// local
+#include "settings_interface.h"
+
+class QGSettings;
+
+namespace qtmir
+{
+
+class Settings: public SettingsInterface
+{
+    Q_OBJECT
+public:
+    explicit Settings(QObject *parent = 0);
+
+    QVariant get(const QString &key) const override;
+
+private:
+    QGSettings *m_settings;
+};
+
+}
+
+#endif // SETTINGS_H

=== added file 'src/modules/Unity/Application/settings_interface.h'
--- src/modules/Unity/Application/settings_interface.h	1970-01-01 00:00:00 +0000
+++ src/modules/Unity/Application/settings_interface.h	2015-03-25 08:59:38 +0000
@@ -0,0 +1,40 @@
+/*
+ * Copyright (C) 2015 Canonical, Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it under
+ * the terms of the GNU Lesser 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef SETTINGS_INTERFACE_H
+#define SETTINGS_INTERFACE_H
+
+//Qt
+#include <QObject>
+
+namespace qtmir
+{
+
+class SettingsInterface: public QObject
+{
+    Q_OBJECT
+public:
+    explicit SettingsInterface(QObject *parent = 0): QObject(parent) {}
+
+    virtual QVariant get(const QString &key) const = 0;
+
+Q_SIGNALS:
+    void changed(const QString &key);
+};
+
+}
+
+#endif // SETTINGS_INTERFACE_H

=== modified file 'tests/modules/ApplicationManager/application_manager_test.cpp'
--- tests/modules/ApplicationManager/application_manager_test.cpp	2015-03-18 10:12:16 +0000
+++ tests/modules/ApplicationManager/application_manager_test.cpp	2015-03-25 08:59:38 +0000
@@ -2235,6 +2235,65 @@
     applicationManager.focusApplication(webbrowserAppId);
 }
 
+TEST_F(ApplicationManagerTests,lifecycle_exempt_appId_is_not_suspended)
+{
+    using namespace ::testing;
+    quint64 a_procId = 5921;
+    const char an_app_id[] = "some_app";
+    QByteArray a_cmd( "/usr/bin/app1 --desktop_file_hint=some_app");
+    std::shared_ptr<mir::scene::Surface> aSurface(nullptr);
+
+    ON_CALL(procInfo,command_line(_)).WillByDefault(Return(a_cmd));
+
+    ON_CALL(appController,appIdHasProcessId(_,_)).WillByDefault(Return(false));
+
+    bool authed = true;
+
+    std::shared_ptr<mir::scene::Session> first_session = std::make_shared<MockSession>("Oo", a_procId);
+    std::shared_ptr<mir::scene::Session> second_session = std::make_shared<MockSession>("oO", a_procId);
+    applicationManager.authorizeSession(a_procId, authed);
+
+    onSessionStarting(first_session);
+    applicationManager.onSessionCreatedSurface(first_session.get(), aSurface);
+    onSessionStarting(second_session);
+
+    Application * the_app = applicationManager.findApplication(an_app_id);
+    applicationManager.focusApplication(an_app_id);
+
+    // Add to other apps to the list (Not "some_app")
+    QVariantList lifecycleExemptAppIds;
+    lifecycleExemptAppIds << "one_app" << "another_app";
+    ON_CALL(settings,get(_)).WillByDefault(Return(lifecycleExemptAppIds));
+    settings.changed("lifecycleExemptAppids");
+
+    EXPECT_EQ(Application::Running, the_app->state());
+
+    applicationManager.setSuspended(true);
+
+    // And expect "some_app" to get suspended
+    EXPECT_EQ(Application::Suspended, the_app->state());
+
+    applicationManager.setSuspended(false);
+
+    EXPECT_EQ(Application::Running, the_app->state());
+
+    // Now add "some_app" to the exception list
+    lifecycleExemptAppIds << "some_app";
+    ON_CALL(settings,get(_)).WillByDefault(Return(lifecycleExemptAppIds));
+    settings.changed("lifecycleExemptAppids");
+
+    EXPECT_EQ(Application::Running, the_app->state());
+
+    applicationManager.setSuspended(true);
+
+    // And expect it to be running still
+    EXPECT_EQ(Application::Running, the_app->state());
+
+    applicationManager.setSuspended(false);
+
+    EXPECT_EQ(Application::Running, the_app->state());
+}
+
 /*
  * Test lifecycle exempt applications have their wakelocks released when shell tries to suspend them
  */

=== added file 'tests/modules/common/mock_settings.h'
--- tests/modules/common/mock_settings.h	1970-01-01 00:00:00 +0000
+++ tests/modules/common/mock_settings.h	2015-03-25 08:59:38 +0000
@@ -0,0 +1,41 @@
+/*
+ * Copyright (C) 2015 Canonical, Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it under
+ * the terms of the GNU Lesser 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef MOCK_SETTINGS_H
+#define MOCK_SETTINGS_H
+
+#include <Unity/Application/settings_interface.h>
+
+#include <gmock/gmock.h>
+
+namespace testing
+{
+struct MockSettings : public qtmir::SettingsInterface
+{
+    MockSettings()
+    {
+        QVariantList lifecycleExemptAppIds;
+        lifecycleExemptAppIds << "com.ubuntu.music";
+        ON_CALL(*this, get(_))
+                .WillByDefault(
+                    Return(lifecycleExemptAppIds));
+
+    }
+    MOCK_CONST_METHOD1(get, QVariant(const QString &));
+};
+
+} // namespace testing
+#endif // MOCK_SETTINGS_H

=== modified file 'tests/modules/common/qtmir_test.h'
--- tests/modules/common/qtmir_test.h	2015-02-06 09:47:21 +0000
+++ tests/modules/common/qtmir_test.h	2015-03-25 08:59:38 +0000
@@ -36,6 +36,7 @@
 #include "mock_prompt_session_manager.h"
 #include "mock_prompt_session.h"
 #include "mock_shared_wakelock.h"
+#include "mock_settings.h"
 
 namespace ms = mir::scene;
 using namespace qtmir;
@@ -102,7 +103,8 @@
             QSharedPointer<DesktopFileReader::Factory>(
                 &desktopFileReaderFactory,
                 [](DesktopFileReader::Factory*){}),
-            QSharedPointer<ProcInfo>(&procInfo,[](ProcInfo *){})
+            QSharedPointer<ProcInfo>(&procInfo,[](ProcInfo *){}),
+            QSharedPointer<MockSettings>(&settings,[](MockSettings *){})
         }
         , sessionManager{
             mirServer,
@@ -149,6 +151,7 @@
     testing::NiceMock<testing::MockProcInfo> procInfo;
     testing::NiceMock<testing::MockDesktopFileReaderFactory> desktopFileReaderFactory;
     testing::NiceMock<testing::MockSharedWakelock> sharedWakelock;
+    testing::NiceMock<testing::MockSettings> settings;
     QSharedPointer<FakeMirServer> mirServer;
     MirShell *mirShell{nullptr};
     QSharedPointer<TaskController> taskController;