Merge lp:~alan-griffiths/qtmir/new-migrate-to-mir-Server-API into lp:qtmir

[Alan Griffiths]

Migration of qtmir from the legacy Mir API

Migration of qtmir from the legacy Mir API

This is somewhat minimalist - there's possibilities of code cleanup and improved naming. But it moves the dependencies over to the mir::Server based APIs and seems to run OK on krillin/15.04(r39).

I'll look at the cleanup work next week.

[Alan Griffiths (alan-griffiths)]

 * Are there any related MPs required for this MP to build/function as expected? Please list.
N
 * Did you perform an exploratory manual test run of your code change and any related functionality?
Y
 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

[PS Jenkins bot (ps-jenkins)]

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

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

review: Needs Fixing (continuous-integration)

[lp:~alan-griffiths/qtmir/new-migrate-to-mir-Server-API]

289. By Alan Griffiths on 2014-12-01

Fix initialization order in tests

[Alan Griffiths (alan-griffiths)]

"Cleanup" is lp:~alan-griffiths/qtmir/refactor-after-migrate-to-mir-Server-API

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Gerry Boland (gerboland)]

I approve of the code-changes, but testing on a desktop machine, it crashes on startup: http://pastebin.ubuntu.com/9332816/

review: Needs Fixing

[Gerry Boland (gerboland)]

For more info, I just checked out this branch on my vivid machine, built it, installed it, and tried to log into a unity8 desktop session. The backtrace was what I managed to get manually, may not be actual issue.

[lp:~alan-griffiths/qtmir/new-migrate-to-mir-Server-API]

290. By Alan Griffiths on 2014-12-02

Fix startup race condition

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

Also tested on with desktop U8

[Gerry Boland (gerboland)]

Yep, working nicely now, thanks

 * Did you perform an exploratory manual test run of the code change and any related functionality?
Y
 * Did CI run pass? If not, please explain why.
Y

review: Approve

=== modified file 'src/platforms/mirserver/display.cpp'
--- src/platforms/mirserver/display.cpp	2014-10-16 18:27:04 +0000
+++ src/platforms/mirserver/display.cpp	2014-12-02 12:30:45 +0000
@@ -27,7 +27,7 @@
 
 // TODO: Listen for display changes and update the list accordingly
 
-Display::Display(const QSharedPointer<mir::DefaultServerConfiguration> &config, QObject *parent)
+Display::Display(const QSharedPointer<mir::Server> &config, QObject *parent)
   : QObject(parent)
   , m_mirConfig(config)
 {
 
=== modified file 'src/platforms/mirserver/display.h'
--- src/platforms/mirserver/display.h	2014-10-10 18:52:44 +0000
+++ src/platforms/mirserver/display.h	2014-12-02 12:30:45 +0000
@@ -23,20 +23,20 @@
 #include <qpa/qplatformscreen.h>
 
 #include "mir/graphics/display.h"
-#include "mir/default_server_configuration.h"
+#include "mir/server.h"
 
 class Display : public QObject
 {
     Q_OBJECT
 public:
-    Display(const QSharedPointer<mir::DefaultServerConfiguration> &config, QObject *parent = 0);
+    Display(const QSharedPointer<mir::Server> &config, QObject *parent = 0);
     ~Display();
 
     QList<QPlatformScreen *> screens() const { return m_screens; }
 
 private:
     QList<QPlatformScreen *> m_screens;
-    const QSharedPointer<mir::DefaultServerConfiguration> m_mirConfig;
+    const QSharedPointer<mir::Server> m_mirConfig;
 };
 
 #endif // DISPLAY_H
 
=== modified file 'src/platforms/mirserver/miropenglcontext.cpp'
--- src/platforms/mirserver/miropenglcontext.cpp	2014-07-15 11:30:11 +0000
+++ src/platforms/mirserver/miropenglcontext.cpp	2014-12-02 12:30:45 +0000
@@ -35,7 +35,7 @@
 // The Mir "Display" generates a shared GL context for all DisplayBuffers
 // (i.e. individual display output buffers) to use as a common base context.
 
-MirOpenGLContext::MirOpenGLContext(const QSharedPointer<mir::DefaultServerConfiguration> &config, const QSurfaceFormat &format)
+MirOpenGLContext::MirOpenGLContext(const QSharedPointer<mir::Server> &config, const QSurfaceFormat &format)
     : m_mirConfig(config)
 #if GL_DEBUG
     , m_logger(new QOpenGLDebugLogger(this))
 
=== modified file 'src/platforms/mirserver/miropenglcontext.h'
--- src/platforms/mirserver/miropenglcontext.h	2014-05-16 16:45:00 +0000
+++ src/platforms/mirserver/miropenglcontext.h	2014-12-02 12:30:45 +0000
@@ -27,13 +27,13 @@
 #include <QOpenGLDebugLogger>
 #endif
 
-#include <mir/default_server_configuration.h>
+#include <mir/server.h>
 
 class MirOpenGLContext : public QObject, public QPlatformOpenGLContext
 {
     Q_OBJECT
 public:
-    MirOpenGLContext(const QSharedPointer<mir::DefaultServerConfiguration> &, const QSurfaceFormat &);
+    MirOpenGLContext(const QSharedPointer<mir::Server> &, const QSurfaceFormat &);
     ~MirOpenGLContext() = default;
 
     QSurfaceFormat format() const override;
@@ -52,7 +52,7 @@
 #endif
 
 private:
-    const QSharedPointer<mir::DefaultServerConfiguration> m_mirConfig;
+    const QSharedPointer<mir::Server> m_mirConfig;
     QSurfaceFormat m_format;
 #if GL_DEBUG
     QOpenGLDebugLogger *m_logger;
 
=== modified file 'src/platforms/mirserver/mirserverconfiguration.cpp'
--- src/platforms/mirserver/mirserverconfiguration.cpp	2014-09-17 12:48:54 +0000
+++ src/platforms/mirserver/mirserverconfiguration.cpp	2014-12-02 12:30:45 +0000
@@ -14,6 +14,8 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <QCoreApplication>
+
 #include "mirserverconfiguration.h"
 
 // local
@@ -50,115 +52,76 @@
 
 MirServerConfiguration::MirServerConfiguration(int argc, char const* argv[], QObject* parent)
     : QObject(parent)
-    , DefaultServerConfiguration(std::make_shared<mo::DefaultConfiguration>(argc, argv, &ignore_unparsed_arguments))
 {
-    qCDebug(QTMIR_MIR_MESSAGES) << "MirServerConfiguration created";
-}
+    set_command_line_handler(&ignore_unparsed_arguments);
+    set_command_line(argc, argv);
 
-std::shared_ptr<ms::PlacementStrategy>
-MirServerConfiguration::the_placement_strategy()
-{
-    return shell_placement_strategy(
-        [this]
+    override_the_placement_strategy([this]
         {
             return std::make_shared<MirPlacementStrategy>(the_shell_display_layout());
         });
-}
 
-std::shared_ptr<ms::SessionListener>
-MirServerConfiguration::the_session_listener()
-{
-    return session_listener(
-        [this]
+    override_the_session_listener([]
         {
             return std::make_shared<SessionListener>();
         });
-}
 
-std::shared_ptr<ms::PromptSessionListener>
-MirServerConfiguration::the_prompt_session_listener()
-{
-    return prompt_session_listener(
-        [this]
+    override_the_prompt_session_listener([]
         {
             return std::make_shared<PromptSessionListener>();
         });
-}
 
-std::shared_ptr<ms::SurfaceConfigurator>
-MirServerConfiguration::the_surface_configurator()
-{
-    return surface_configurator(
-        [this]()
+    override_the_surface_configurator([]
         {
             return std::make_shared<SurfaceConfigurator>();
         });
-}
-
-std::shared_ptr<mir::frontend::SessionAuthorizer>
-MirServerConfiguration::the_session_authorizer()
-{
-    return session_authorizer(
-    []
-    {
-        return std::make_shared<SessionAuthorizer>();
-    });
-}
-
-std::shared_ptr<mir::compositor::Compositor>
-MirServerConfiguration::the_compositor()
-{
-    return compositor(
-        [this]()
+
+    override_the_session_authorizer([]
+        {
+            return std::make_shared<SessionAuthorizer>();
+        });
+
+    override_the_compositor([]
         {
             return std::make_shared<QtCompositor>();
         });
-}
-
-std::shared_ptr<mir::input::InputDispatcher>
-MirServerConfiguration::the_input_dispatcher()
-{
-    return input_dispatcher(
-    [this]()
-    {
-        return std::make_shared<QtEventFeeder>();
-    });
-}
-
-std::shared_ptr<mir::graphics::GLConfig>
-MirServerConfiguration::the_gl_config()
-{
-    return gl_config(
-    [this]()
-    {
+
+    override_the_input_dispatcher([]
+        {
+            return std::make_shared<QtEventFeeder>();
+        });
+
+    override_the_gl_config([]
+        {
 #ifdef QTMIR_USE_OPENGL
-        // Should desktop-GL be desired, need to bind that API before a context is created
-        eglBindAPI(EGL_OPENGL_API);
+            // Should desktop-GL be desired, need to bind that API before a context is created
+            eglBindAPI(EGL_OPENGL_API);
 #endif
-        return std::make_shared<MirGLConfig>();
-    });
-}
+            return std::make_shared<MirGLConfig>();
+        });
 
-std::shared_ptr<mir::ServerStatusListener>
-MirServerConfiguration::the_server_status_listener()
-{
-    return server_status_listener(
-        []()
+    override_the_server_status_listener([]
         {
             return std::make_shared<MirServerStatusListener>();
         });
-}
 
-std::shared_ptr<mir::shell::FocusSetter>
-MirServerConfiguration::the_shell_focus_setter()
-{
-    return shell_focus_setter(
-        [this]
+    override_the_shell_focus_setter([]
         {
             return std::make_shared<FocusSetter>();
         });
+
+    set_terminator([&](int)
+        {
+            qDebug() << "Signal caught by Mir, stopping Mir server..";
+            QCoreApplication::quit();
+        });
+
+    apply_settings();
+
+    qCDebug(QTMIR_MIR_MESSAGES) << "MirServerConfiguration created";
 }
 
+
 /************************************ Shell side ************************************/
 
 //
 
=== modified file 'src/platforms/mirserver/mirserverconfiguration.h'
--- src/platforms/mirserver/mirserverconfiguration.h	2014-09-17 12:34:40 +0000
+++ src/platforms/mirserver/mirserverconfiguration.h	2014-12-02 12:30:45 +0000
@@ -18,7 +18,7 @@
 #define MIRSERVERCONFIGURATION_H
 
 #include <QObject>
-#include <mir/default_server_configuration.h>
+#include <mir/server.h>
 
 class QtEventFeeder;
 class SessionListener;
@@ -26,7 +26,9 @@
 class SurfaceConfigurator;
 class PromptSessionListener;
 
-class MirServerConfiguration : public QObject, public mir::DefaultServerConfiguration
+// We use virtual inheritance of mir::Server to facilitate derived classes (e.g. testing)
+// calling initialization functions before MirServerConfiguration is constructed.
+class MirServerConfiguration : public QObject, public virtual mir::Server
 {
     Q_OBJECT
 
@@ -39,18 +41,6 @@
     MirServerConfiguration(int argc, char const* argv[], QObject* parent = 0);
     ~MirServerConfiguration() = default;
 
-    /* mir specific */
-    std::shared_ptr<mir::compositor::Compositor> the_compositor() override;
-    std::shared_ptr<mir::scene::PlacementStrategy> the_placement_strategy() override;
-    std::shared_ptr<mir::scene::SessionListener> the_session_listener() override;
-    std::shared_ptr<mir::scene::PromptSessionListener> the_prompt_session_listener() override;
-    std::shared_ptr<mir::scene::SurfaceConfigurator> the_surface_configurator() override;
-    std::shared_ptr<mir::frontend::SessionAuthorizer> the_session_authorizer() override;
-    std::shared_ptr<mir::input::InputDispatcher> the_input_dispatcher() override;
-    std::shared_ptr<mir::graphics::GLConfig> the_gl_config() override;
-    std::shared_ptr<mir::ServerStatusListener> the_server_status_listener() override;
-    std::shared_ptr<mir::shell::FocusSetter> the_shell_focus_setter() override;
-
     /* qt specific */
     // getters
     SessionAuthorizer *sessionAuthorizer();
 
=== modified file 'src/platforms/mirserver/mirserverintegration.cpp'
--- src/platforms/mirserver/mirserverintegration.cpp	2014-10-01 18:42:26 +0000
+++ src/platforms/mirserver/mirserverintegration.cpp	2014-12-02 12:30:45 +0000
@@ -42,7 +42,6 @@
 // Mir
 #include <mir/graphics/display.h>
 #include <mir/graphics/display_buffer.h>
-#include <mir/main_loop.h>
 
 // std
 #include <csignal>
@@ -179,17 +178,6 @@
     for (QPlatformScreen *screen : m_display->screens())
         screenAdded(screen);
 
-    // install signal handler into the Mir event loop
-    auto mainLoop = m_mirConfig->the_main_loop();
-
-    mainLoop->register_signal_handler(
-    {SIGINT, SIGTERM},
-    [&](int)
-    {
-        qDebug() << "Signal caught by Mir, stopping Mir server..";
-        QCoreApplication::quit();
-    });
-
     m_clipboard->setupDBusService();
 }
 
 
=== modified file 'src/platforms/mirserver/qmirserver.cpp'
--- src/platforms/mirserver/qmirserver.cpp	2014-05-16 16:45:00 +0000
+++ src/platforms/mirserver/qmirserver.cpp	2014-12-02 12:30:45 +0000
@@ -19,10 +19,38 @@
 #include <QCoreApplication>
 #include <QDebug>
 
+#include <mir/main_loop.h>
+
 // local
 #include "qmirserver.h"
 
 
+void MirServerWorker::run()
+{
+    auto const main_loop = config->the_main_loop();
+    // By enqueuing the notification code in the main loop, we are
+    // ensuring that the server has really and fully started before
+    // leaving wait_for_startup().
+    main_loop->enqueue(
+        this,
+        [&]
+        {
+            std::lock_guard<std::mutex> lock(mutex);
+            mir_running = true;
+            started_cv.notify_one();
+        });
+
+    config->run();
+    Q_EMIT stopped();
+}
+
+bool MirServerWorker::wait_for_mir_startup()
+{
+    std::unique_lock<decltype(mutex)> lock(mutex);
+    started_cv.wait_for(lock, std::chrono::seconds{10}, [&]{ return mir_running; });
+    return mir_running;
+}
+
 QMirServer::QMirServer(const QSharedPointer<MirServerConfiguration> &config, QObject *parent)
     : QObject(parent)
     , m_mirServer(new MirServerWorker(config))
@@ -37,6 +65,12 @@
 
     m_mirThread.start(QThread::TimeCriticalPriority);
     Q_EMIT run();
+
+    if (!m_mirServer->wait_for_mir_startup())
+    {
+        qCritical() << "ERROR: QMirServer - Mir failed to start";
+        QCoreApplication::quit();
+    }
 }
 
 QMirServer::~QMirServer()
 
=== modified file 'src/platforms/mirserver/qmirserver.h'
--- src/platforms/mirserver/qmirserver.h	2014-05-16 16:45:00 +0000
+++ src/platforms/mirserver/qmirserver.h	2014-12-02 12:30:45 +0000
@@ -22,28 +22,37 @@
 #include <QThread>
 #include <QSharedPointer>
 
-// Mir
-#include <mir/display_server.h>
-
 // local
 #include "mirserverconfiguration.h"
 
-// Wrap mir::DisplayServer with QObject, so it can be controlled via QThread
-class MirServerWorker : public QObject, public mir::DisplayServer
+#include <condition_variable>
+#include <mutex>
+
+// Wrap mir::Server with QObject, so it can be controlled via QThread
+class MirServerWorker : public QObject
 {
     Q_OBJECT
 
 public:
     MirServerWorker(const QSharedPointer<MirServerConfiguration> &config)
-        : mir::DisplayServer(*config.data())
+        : config(config)
     {}
 
+    bool wait_for_mir_startup();
+
 Q_SIGNALS:
     void stopped();
 
 public Q_SLOTS:
-    void run() { mir::DisplayServer::run(); Q_EMIT stopped(); }
-    void stop() { mir::DisplayServer::stop(); }
+    void run();
+    void stop() { config->stop(); }
+
+private:
+    std::mutex mutex;
+    std::condition_variable started_cv;
+    bool mir_running{false};
+
+    const QSharedPointer<MirServerConfiguration> config;
 };
 
 
 
=== modified file 'tests/modules/common/qtmir_test.h'
--- tests/modules/common/qtmir_test.h	2014-10-06 21:23:39 +0000
+++ tests/modules/common/qtmir_test.h	2014-12-02 12:30:45 +0000
@@ -41,31 +41,36 @@
 
 namespace qtmir {
 
-class FakeMirServerConfiguration: public MirServerConfiguration
+// Initialization of mir::Server needed for by tests
+class TestMirServerInit : virtual mir::Server
 {
+public:
+    TestMirServerInit()
+    {
+        override_the_prompt_session_manager(
+            [this]{ return the_mock_prompt_session_manager(); });
+    }
+
+    std::shared_ptr<mir::scene::MockPromptSessionManager> the_mock_prompt_session_manager()
+    {
+        return mock_prompt_session_manager;
+    }
+
+private:
     typedef testing::NiceMock<mir::scene::MockPromptSessionManager> StubPromptSessionManager;
+    std::shared_ptr<StubPromptSessionManager> const mock_prompt_session_manager
+        {std::make_shared<StubPromptSessionManager>()};
+};
+
+class FakeMirServerConfiguration: private TestMirServerInit, public MirServerConfiguration
+{
 public:
     FakeMirServerConfiguration()
     : MirServerConfiguration(0, nullptr)
-    , mock_prompt_session_manager(std::make_shared<StubPromptSessionManager>())
-    {
-    }
-
-    std::shared_ptr<ms::PromptSessionManager> the_prompt_session_manager() override
-    {
-        return prompt_session_manager([this]()
-           ->std::shared_ptr<ms::PromptSessionManager>
-           {
-               return the_mock_prompt_session_manager();
-           });
-    }
-
-    std::shared_ptr<StubPromptSessionManager> the_mock_prompt_session_manager()
-    {
-        return mock_prompt_session_manager;
-    }
-
-    std::shared_ptr<StubPromptSessionManager> mock_prompt_session_manager;
+    {
+    }
+
+    using TestMirServerInit::the_mock_prompt_session_manager;
 };
 
 } // namespace qtmir