ubuntu-sdk-team team mailing list archive

[Lukáš Tinkl <lukas.tinkl@xxxxxxxxxxxxx>]

Lukáš Tinkl has proposed merging lp:~lukas-kde/ubuntu-ui-toolkit/asyncDbusClipboard into lp:ubuntu-ui-toolkit/staging.

Commit message:
Unbreak the startup race between unity8/qtmir and UITK trying talk to content-hub

Requested reviews:
  Ubuntu SDK team (ubuntu-sdk-team)
Related bugs:
  Bug #1663106 in unity8 (Ubuntu): "[regression] Logging in to Unity8 takes 25 seconds (the default DBus timeout)"
  https://bugs.launchpad.net/ubuntu/+source/unity8/+bug/1663106

For more details, see:
https://code.launchpad.net/~lukas-kde/ubuntu-ui-toolkit/asyncDbusClipboard/+merge/316965

Unbreak the startup race between unity8/qtmir and UITK trying talk to content-hub

Do not initialize the UCContentHub class until the content hub service becomes available on DBUS; make the onPasteboardChanged() method async too, this is also blocking for ~25 seconds.

Fixes lp:1663106 - [regression] Logging in to Unity8 takes 25 seconds (the default DBus timeout)
-- 
Your team Ubuntu SDK team is requested to review the proposed merge of lp:~lukas-kde/ubuntu-ui-toolkit/asyncDbusClipboard into lp:ubuntu-ui-toolkit/staging.

=== modified file 'src/UbuntuToolkit/privates/uccontenthub.cpp'
--- src/UbuntuToolkit/privates/uccontenthub.cpp	2017-01-26 14:29:30 +0000
+++ src/UbuntuToolkit/privates/uccontenthub.cpp	2017-02-10 14:08:48 +0000
@@ -23,6 +23,7 @@
 #include <QtDBus/QDBusConnection>
 #include <QtDBus/QDBusInterface>
 #include <QtDBus/QDBusReply>
+#include <QtDBus/QDBusServiceWatcher>
 #include <QtQuick/QQuickItem>
 
 Q_LOGGING_CATEGORY(ucContentHub, "ubuntu.components.UCContentHub", QtMsgType::QtWarningMsg)
@@ -41,10 +42,12 @@
 
 UCContentHub::UCContentHub(QObject *parent)
     : QObject(parent),
-      m_dbusIface(0),
-      m_contentHubIface(0),
-      m_canPaste(false),
-      m_targetItem(0)
+      m_watcher(new QDBusServiceWatcher(contentHubService, QDBusConnection::sessionBus(), QDBusServiceWatcher::WatchForRegistration, this))
+{
+    connect(m_watcher, &QDBusServiceWatcher::serviceRegistered, this, &UCContentHub::init);
+}
+
+void UCContentHub::init()
 {
     m_dbusIface = new QDBusInterface(dbusService,
                                      dbusObjectPath,
@@ -76,24 +79,13 @@
             SLOT(onPasteboardChanged())
         );
 
-        m_canPaste = checkPasteFormats();
-    }
-}
-
-UCContentHub::~UCContentHub()
-{
-    if (m_dbusIface) {
-        delete m_dbusIface;
-    }
-
-    if (m_contentHubIface) {
-        delete m_contentHubIface;
+        onPasteboardChanged();
     }
 }
 
 void UCContentHub::requestPaste(QQuickItem *targetItem)
 {
-    if (!m_contentHubIface->isValid()) {
+    if (!m_contentHubIface || !m_contentHubIface->isValid()) {
         CONTENT_HUB_TRACE("Invalid Content Hub DBusInterface");
         return;
     }
@@ -106,12 +98,12 @@
     m_contentHubIface->call(QStringLiteral("RequestPasteByAppId"), appProfile);
 }
 
-bool UCContentHub::canPaste()
+bool UCContentHub::canPaste() const
 {
     return m_canPaste;
 }
 
-void UCContentHub::onPasteSelected(QString appId, QByteArray mimedata, bool pasteAsRichText)
+void UCContentHub::onPasteSelected(const QString &appId, const QByteArray &mimedata, bool pasteAsRichText)
 {
     if (getAppProfile() != appId) {
         return;
@@ -138,15 +130,39 @@
 
 void UCContentHub::onPasteboardChanged()
 {
-    if (checkPasteFormats() != m_canPaste) {
-        m_canPaste = !m_canPaste;
+    if (!m_contentHubIface || !m_contentHubIface->isValid()) {
+        CONTENT_HUB_TRACE("Invalid Content Hub DBusInterface");
+        return;
+    }
+
+    QDBusPendingCall pcall = m_contentHubIface->asyncCall(QStringLiteral("PasteFormats"));
+    QDBusPendingCallWatcher * watcher = new QDBusPendingCallWatcher(pcall, this);
+    connect(watcher, &QDBusPendingCallWatcher::finished, [this](QDBusPendingCallWatcher * call) {
+        QDBusPendingReply<QStringList> reply = *call;
+        call->deleteLater();
+        if (reply.isValid()) {
+            // TODO: ContentHub clipboard keeps a list of all available paste formats.
+            // Probably apps could make use of this information to check if a specific
+            // data type is available, instead of only checking if list is empty or not.
+            // (LP: #1657111)
+            setCanPaste(!reply.value().isEmpty());
+        } else {
+            CONTENT_HUB_TRACE("Invalid return from DBus call PasteFormats");
+        }
+    });
+}
+
+void UCContentHub::setCanPaste(bool value)
+{
+    if (value != m_canPaste) {
+        m_canPaste = value;
         Q_EMIT canPasteChanged();
     }
 }
 
-QString UCContentHub::getAppProfile()
+QString UCContentHub::getAppProfile() const
 {
-    if (!m_dbusIface->isValid()) {
+    if (!m_dbusIface || !m_dbusIface->isValid()) {
         CONTENT_HUB_TRACE("Invalid DBus DBusInterface");
         return QString();
     }
@@ -194,25 +210,4 @@
     return mimeData;
 }
 
-bool UCContentHub::checkPasteFormats()
-{
-    if (!m_contentHubIface->isValid()) {
-        CONTENT_HUB_TRACE("Invalid Content Hub DBusInterface");
-        return false;
-    }
-
-    QDBusReply<QStringList> reply = m_contentHubIface->call(QStringLiteral("PasteFormats"));
-    if (reply.isValid()) {
-        // TODO: ContentHub clipboard keeps a list of all available paste formats.
-        // Probably apps could make use of this information to check if a specific
-        // data type is available, instead of only checking if list is empty or not.
-        // (LP: #1657111)
-        return !reply.value().isEmpty();
-    } else {
-        CONTENT_HUB_TRACE("Invalid return from DBus call PasteFormats");
-    }
-
-    return false;
-}
-
 UT_NAMESPACE_END

=== modified file 'src/UbuntuToolkit/privates/uccontenthub_p.h'
--- src/UbuntuToolkit/privates/uccontenthub_p.h	2017-01-17 19:30:01 +0000
+++ src/UbuntuToolkit/privates/uccontenthub_p.h	2017-02-10 14:08:48 +0000
@@ -25,6 +25,7 @@
 
 class QMimeData;
 class QDBusInterface;
+class QDBusServiceWatcher;
 class QQuickItem;
 
 UT_NAMESPACE_BEGIN
@@ -32,35 +33,37 @@
 class UBUNTUTOOLKIT_EXPORT UCContentHub : public QObject
 {
     Q_OBJECT
-
+    friend class tst_UCContentHub;
     Q_PROPERTY(bool canPaste READ canPaste NOTIFY canPasteChanged)
 
 public:
-    UCContentHub(QObject* parent = 0);
-    ~UCContentHub();
+    UCContentHub(QObject* parent = nullptr);
+    ~UCContentHub() = default;
 
     Q_INVOKABLE void requestPaste(QQuickItem *targetItem);
 
-    bool canPaste();
-    QString getAppProfile();
+    bool canPaste() const;
+    QString getAppProfile() const;
     QMimeData* deserializeMimeData(const QByteArray &serializedMimeData);
 
 Q_SIGNALS:
     void pasteSelected(QQuickItem *targetItem, const QString &data);
     void canPasteChanged();
 
-public Q_SLOTS:
-    void onPasteSelected(QString appId, QByteArray mimedata, bool pasteAsRichText);
+private Q_SLOTS:
+    void init();
+    void onPasteSelected(const QString &appId, const QByteArray &mimedata, bool pasteAsRichText);
     void onPasteboardChanged();
 
 private:
-    bool checkPasteFormats();
-
-    QDBusInterface *m_dbusIface;
-    QDBusInterface *m_contentHubIface;
-
-    bool m_canPaste;
-    QQuickItem *m_targetItem;
+    void setCanPaste(bool value);
+    QDBusInterface *m_dbusIface{nullptr};
+    QDBusInterface *m_contentHubIface{nullptr};
+
+    bool m_canPaste{false};
+    QQuickItem *m_targetItem{nullptr};
+
+    QDBusServiceWatcher * m_watcher{nullptr};
 };
 
 UT_NAMESPACE_END