Merge lp:~alan-griffiths/miral/encapsulate-PromptSessionListener into lp:miral

[Alan Griffiths]

[miral-qt] localize dependency on #include <mir/scene/prompt_session_listener.h>

[Gerry Boland (gerboland)]

+ void promptProviderAdded(qtmir::PromptSession const&, miral::Application const&);
one thing to be careful of is that Qt signal/slot argument types need to be registered with the meta object system like this:

Q_DECLARE_METATYPE(namespace::type) in a header file, outside all namespaces

qRegisterMetaType<namespace::type>(); in one C++ file. It should be called before a signal using it is fired.

If you don't do this, at *runtime* the signal will fail to be emitted. You get a line printed to the console as an error. Lame I know.

I see neither argument being registered in the code (I missed this in reviewing rev 400 & 401).

Have you any tips on testing prompt sessions manually?

review: Needs Fixing

[Alan Griffiths (alan-griffiths)]

> + void promptProviderAdded(qtmir::PromptSession const&, miral::Application
> const&);
> one thing to be careful of is that Qt signal/slot argument types need to be
> registered with the meta object system like this:
>
> Q_DECLARE_METATYPE(namespace::type) in a header file, outside all namespaces
>
> qRegisterMetaType<namespace::type>(); in one C++ file. It should be called
> before a signal using it is fired.
>
> If you don't do this, at *runtime* the signal will fail to be emitted. You get
> a line printed to the console as an error. Lame I know.
>
> I see neither argument being registered in the code (I missed this in
> reviewing rev 400 & 401).

For the avoidance of doubt, this is a pre-existing problem? Nothing to do with *this* branch?

> Have you any tips on testing prompt sessions manually?

Sorry, the use of prompt sessions is a mystery to me.

[lp:~alan-griffiths/miral/encapsulate-PromptSessionListener]

411. By Alan Griffiths on 2016-10-26

merge :parent

412. By Alan Griffiths on 2016-10-26

Fix Qt type registration

[Gerry Boland (gerboland)]

That will do for now, let's land this, and if something broke, we can fix later

review: Approve

=== modified file 'miral-qt/src/platforms/mirserver/mirserverhooks.cpp'
--- miral-qt/src/platforms/mirserver/mirserverhooks.cpp	2016-09-15 22:40:51 +0000
+++ miral-qt/src/platforms/mirserver/mirserverhooks.cpp	2016-10-26 13:57:17 +0000
@@ -22,15 +22,37 @@
 #include "mirserverstatuslistener.h"
 #include "promptsessionlistener.h"
 #include "screenscontroller.h"
+#include "logging.h"
 
 // mir
 #include <mir/server.h>
 #include <mir/graphics/cursor.h>
+#include <mir/scene/prompt_session_listener.h>
 
 namespace mg = mir::graphics;
+namespace ms = mir::scene;
 
 namespace
 {
+struct PromptSessionListenerImpl : PromptSessionListener, mir::scene::PromptSessionListener
+{
+    using PromptSessionListener::PromptSessionListener;
+    ~PromptSessionListenerImpl();
+
+    void starting(std::shared_ptr<mir::scene::PromptSession> const& prompt_session) override;
+    void stopping(std::shared_ptr<mir::scene::PromptSession> const& prompt_session) override;
+    void suspending(std::shared_ptr<mir::scene::PromptSession> const& prompt_session) override;
+    void resuming(std::shared_ptr<mir::scene::PromptSession> const& prompt_session) override;
+
+    void prompt_provider_added(mir::scene::PromptSession const& prompt_session,
+                               std::shared_ptr<mir::scene::Session> const& prompt_provider) override;
+    void prompt_provider_removed(mir::scene::PromptSession const& prompt_session,
+                                 std::shared_ptr<mir::scene::Session> const& prompt_provider) override;
+
+private:
+    QHash<const mir::scene::PromptSession *, qtmir::PromptSession> m_mirPromptToSessionHash;
+};
+
 struct HiddenCursorWrapper : mg::Cursor
 {
     HiddenCursorWrapper(std::shared_ptr<mg::Cursor> const& wrapped) :
@@ -72,7 +94,7 @@
 
     server.override_the_prompt_session_listener([this]
         {
-            auto const result = std::make_shared<PromptSessionListener>();
+            auto const result = std::make_shared<PromptSessionListenerImpl>();
             self->m_promptSessionListener = result;
             return result;
         });
@@ -115,3 +137,48 @@
         new ScreensController(screensModel, theMirDisplay(), self->m_mirDisplayConfigurationController.lock()));
 }
 
+PromptSessionListenerImpl::~PromptSessionListenerImpl() = default;
+
+void PromptSessionListenerImpl::starting(std::shared_ptr<ms::PromptSession> const& prompt_session)
+{
+    qCDebug(QTMIR_MIR_MESSAGES) << "PromptSessionListener::starting - this=" << this << "prompt_session=" << prompt_session.get();
+    m_mirPromptToSessionHash.insert(prompt_session.get(), prompt_session);
+    Q_EMIT promptSessionStarting(prompt_session);
+}
+
+void PromptSessionListenerImpl::stopping(std::shared_ptr<ms::PromptSession> const& prompt_session)
+{
+    qCDebug(QTMIR_MIR_MESSAGES) << "PromptSessionListener::stopping - this=" << this << "prompt_session=" << prompt_session.get();
+    Q_EMIT promptSessionStopping(prompt_session);
+    m_mirPromptToSessionHash.remove(prompt_session.get());
+}
+
+void PromptSessionListenerImpl::suspending(std::shared_ptr<ms::PromptSession> const& prompt_session)
+{
+    qCDebug(QTMIR_MIR_MESSAGES) << "PromptSessionListener::suspending - this=" << this << "prompt_session=" << prompt_session.get();
+    Q_EMIT promptSessionSuspending(prompt_session);
+}
+
+void PromptSessionListenerImpl::resuming(std::shared_ptr<ms::PromptSession> const& prompt_session)
+{
+    qCDebug(QTMIR_MIR_MESSAGES) << "PromptSessionListener::resuming - this=" << this << "prompt_session=" << prompt_session.get();
+    Q_EMIT promptSessionResuming(prompt_session);
+}
+
+void PromptSessionListenerImpl::prompt_provider_added(ms::PromptSession const& prompt_session,
+                                                      std::shared_ptr<ms::Session> const& prompt_provider)
+{
+    qCDebug(QTMIR_MIR_MESSAGES) << "PromptSessionListener::prompt_provider_added - this=" << this
+                                << "prompt_session=" << &prompt_session
+                                << "prompt_provider=" << prompt_provider.get();
+    Q_EMIT promptProviderAdded(m_mirPromptToSessionHash[&prompt_session], prompt_provider);
+}
+
+void PromptSessionListenerImpl::prompt_provider_removed(ms::PromptSession const& prompt_session,
+                                                        std::shared_ptr<ms::Session> const& prompt_provider)
+{
+    qCDebug(QTMIR_MIR_MESSAGES) << "PromptSessionListener::prompt_provider_removed - this=" << this
+                                << "prompt_session=" << &prompt_session
+                                << "prompt_provider=" << prompt_provider.get();
+    Q_EMIT promptProviderRemoved(m_mirPromptToSessionHash[&prompt_session], prompt_provider);
+}

=== modified file 'miral-qt/src/platforms/mirserver/promptsessionlistener.cpp'
--- miral-qt/src/platforms/mirserver/promptsessionlistener.cpp	2016-10-06 14:05:01 +0000
+++ miral-qt/src/platforms/mirserver/promptsessionlistener.cpp	2016-10-26 13:57:17 +0000
@@ -19,60 +19,15 @@
 
 namespace ms = mir::scene;
 
-Q_DECLARE_METATYPE(std::shared_ptr<ms::PromptSession>)
-
 PromptSessionListener::PromptSessionListener(QObject *parent) :
     QObject(parent)
 {
     qCDebug(QTMIR_MIR_MESSAGES) << "PromptSessionListener::PromptSessionListener - this=" << this;
-    qRegisterMetaType<std::shared_ptr<ms::PromptSession>>("std::shared_ptr<mir::scene::PromptSession>");
+    qRegisterMetaType<qtmir::PromptSession>("qtmir::PromptSession");
+    qRegisterMetaType<miral::Application>("miral::Application");
 }
 
 PromptSessionListener::~PromptSessionListener()
 {
     qCDebug(QTMIR_MIR_MESSAGES) << "PromptSessionListener::~PromptSessionListener - this=" << this;
 }
-
-void PromptSessionListener::starting(std::shared_ptr<ms::PromptSession> const& prompt_session)
-{
-    qCDebug(QTMIR_MIR_MESSAGES) << "PromptSessionListener::starting - this=" << this << "prompt_session=" << prompt_session.get();
-    m_mirPromptToSessionHash.insert(prompt_session.get(), prompt_session);
-    Q_EMIT promptSessionStarting(prompt_session);
-}
-
-void PromptSessionListener::stopping(std::shared_ptr<ms::PromptSession> const& prompt_session)
-{
-    qCDebug(QTMIR_MIR_MESSAGES) << "PromptSessionListener::stopping - this=" << this << "prompt_session=" << prompt_session.get();
-    Q_EMIT promptSessionStopping(prompt_session);
-    m_mirPromptToSessionHash.remove(prompt_session.get());
-}
-
-void PromptSessionListener::suspending(std::shared_ptr<ms::PromptSession> const& prompt_session)
-{
-    qCDebug(QTMIR_MIR_MESSAGES) << "PromptSessionListener::suspending - this=" << this << "prompt_session=" << prompt_session.get();
-    Q_EMIT promptSessionSuspending(prompt_session);
-}
-
-void PromptSessionListener::resuming(std::shared_ptr<ms::PromptSession> const& prompt_session)
-{
-    qCDebug(QTMIR_MIR_MESSAGES) << "PromptSessionListener::resuming - this=" << this << "prompt_session=" << prompt_session.get();
-    Q_EMIT promptSessionResuming(prompt_session);
-}
-
-void PromptSessionListener::prompt_provider_added(ms::PromptSession const& prompt_session,
-                                                  std::shared_ptr<ms::Session> const& prompt_provider)
-{
-    qCDebug(QTMIR_MIR_MESSAGES) << "PromptSessionListener::prompt_provider_added - this=" << this
-                                << "prompt_session=" << &prompt_session
-                                << "prompt_provider=" << prompt_provider.get();
-    Q_EMIT promptProviderAdded(m_mirPromptToSessionHash[&prompt_session], prompt_provider);
-}
-
-void PromptSessionListener::prompt_provider_removed(ms::PromptSession const& prompt_session,
-                                                    std::shared_ptr<ms::Session> const& prompt_provider)
-{
-    qCDebug(QTMIR_MIR_MESSAGES) << "PromptSessionListener::prompt_provider_removed - this=" << this
-                                << "prompt_session=" << &prompt_session
-                                << "prompt_provider=" << prompt_provider.get();
-    Q_EMIT promptProviderRemoved(m_mirPromptToSessionHash[&prompt_session], prompt_provider);
-}

=== modified file 'miral-qt/src/platforms/mirserver/promptsessionlistener.h'
--- miral-qt/src/platforms/mirserver/promptsessionlistener.h	2016-10-07 15:45:30 +0000
+++ miral-qt/src/platforms/mirserver/promptsessionlistener.h	2016-10-26 13:57:17 +0000
@@ -23,36 +23,27 @@
 
 #include "promptsession.h"
 
-#include <mir/scene/prompt_session_listener.h>
+#include <miral/application.h>
 
-class PromptSessionListener : public QObject, public mir::scene::PromptSessionListener
+class PromptSessionListener : public QObject
 {
-    Q_OBJECT
-public:
+Q_OBJECT
+protected:
     explicit PromptSessionListener(QObject *parent = 0);
     ~PromptSessionListener();
 
-    void starting(std::shared_ptr<mir::scene::PromptSession> const& prompt_session) override;
-    void stopping(std::shared_ptr<mir::scene::PromptSession> const& prompt_session) override;
-    void suspending(std::shared_ptr<mir::scene::PromptSession> const& prompt_session) override;
-    void resuming(std::shared_ptr<mir::scene::PromptSession> const& prompt_session) override;
-
-    void prompt_provider_added(mir::scene::PromptSession const& prompt_session,
-        std::shared_ptr<mir::scene::Session> const& prompt_provider) override;
-    void prompt_provider_removed(mir::scene::PromptSession const& prompt_session,
-        std::shared_ptr<mir::scene::Session> const& prompt_provider) override;
-
+public:
 Q_SIGNALS:
     void promptSessionStarting(qtmir::PromptSession const &session);
     void promptSessionStopping(qtmir::PromptSession const &session);
     void promptSessionSuspending(qtmir::PromptSession const &session);
     void promptSessionResuming(qtmir::PromptSession const &session);
 
-    void promptProviderAdded(qtmir::PromptSession const&, std::shared_ptr<mir::scene::Session> const&);
-    void promptProviderRemoved(qtmir::PromptSession const&, std::shared_ptr<mir::scene::Session> const&);
-
-private:
-    QHash<const mir::scene::PromptSession *, qtmir::PromptSession> m_mirPromptToSessionHash;
+    void promptProviderAdded(qtmir::PromptSession const&, miral::Application const&);
+    void promptProviderRemoved(qtmir::PromptSession const&, miral::Application const&);
 };
 
+Q_DECLARE_METATYPE(qtmir::PromptSession)
+Q_DECLARE_METATYPE(miral::Application)
+
 #endif // SESSIONLISTENER_H