Merge lp:~pete-woods/libdbusmenu-qt/sync-mode-option into lp:libdbusmenu-qt

Proposed by Pete Woods on 2014-03-05
Status: Merged
Approved by: Antti Kaijanmäki on 2014-03-14
Approved revision: 263
Merged at revision: 263
Proposed branch: lp:~pete-woods/libdbusmenu-qt/sync-mode-option
Merge into: lp:libdbusmenu-qt
Diff against target: 188 lines (+77/-37)
3 files modified
debian/changelog (+13/-0)
src/dbusmenuimporter.cpp (+48/-37)
src/dbusmenuimporter.h (+16/-0)
To merge this branch: bzr merge lp:~pete-woods/libdbusmenu-qt/sync-mode-option
Reviewer Review Type Date Requested Status
Antti Kaijanmäki (community) 2014-03-06 Approve on 2014-03-14
PS Jenkins bot (community) continuous-integration Approve on 2014-03-14
Marcus Tomlinson 2014-03-05 Pending
Review via email: mp+209445@code.launchpad.net

Commit message

Add option to make DBus calls synchronous

Description of the change

* Is your branch in sync with latest trunk (e.g. bzr pull lp:trunk -> no changes)
  * Yes
 * Did you build your software in a clean sbuild/pbuilder chroot or ppa?
  * Yes
 * Did you build your software in a clean sbuild/pbuilder armhf chroot or ppa?
  * Yes
 * Has your component "TestPlan” been executed successfully on emulator, N4?
  * Yes
 * Has a 5 minute exploratory testing run been executed on N4?
  * Yes
 * If you changed the packaging (debian), did you subscribe a core-dev to this MP?
  * N/A
 * If you changed the UI, did you subscribe the design-reviewers to this MP?
  * No change
 * What components might get impacted by your changes?
  * Unity HUD
 * Have you requested review by the teams of these owning components?
  * Yes

Check List:
https://wiki.ubuntu.com/Process/Merges/Checklists/libdbusmenu-qt

Test Plan:
https://wiki.ubuntu.com/Process/Merges/TestPlan/libdbusmenu-qt

Silo:
https://launchpad.net/~ci-train-ppa-service/+archive/landing-002/

To post a comment you must log in.
Antti Kaijanmäki (kaijanmaki) wrote :

LGTM. Waiting for silo.

Antti Kaijanmäki (kaijanmaki) wrote :

 * Are any changes against your component pending/needed to land the MP under review in a functional state and are those called out explicitly by the submitter?
YES.

 * Did you do exploratory testing related to the component you own with the MP changeset included?
YES.

 * Has the submitter requested review by all the relevant teams/reviewres?
YES.

 * If you are the reviewer owning the component the MP is against, have you checked that submitter has accurately filled out the submitter checklist and has taken no shortcut?
YES.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2014-02-18 14:33:02 +0000
3+++ debian/changelog 2014-03-14 14:12:57 +0000
4@@ -1,3 +1,16 @@
5+libdbusmenu-qt (0.9.3-0ubuntu1) UNRELEASED; urgency=medium
6+
7+ [ Pete Woods ]
8+ * Add importer parameter to allow control over DBus behavior.
9+
10+ -- Pete Woods <pete.woods@canonical.com> Wed, 05 Mar 2014 09:36:46 +0000
11+
12+libdbusmenu-qt (0.9.2+14.04.20140305-0ubuntu1) trusty; urgency=low
13+
14+ * New rebuild forced
15+
16+ -- Ubuntu daily release <ps-jenkins@lists.canonical.com> Wed, 05 Mar 2014 09:29:14 +0000
17+
18 libdbusmenu-qt (0.9.2+14.04.20140218.2-0ubuntu1) trusty; urgency=low
19
20 [ Pete Woods ]
21
22=== modified file 'src/dbusmenuimporter.cpp'
23--- src/dbusmenuimporter.cpp 2014-02-18 11:52:15 +0000
24+++ src/dbusmenuimporter.cpp 2014-03-14 14:12:57 +0000
25@@ -89,6 +89,8 @@
26
27 bool m_mustEmitMenuUpdated;
28
29+ DBusMenuImporterType m_type;
30+
31 QDBusPendingCallWatcher *refresh(int id)
32 {
33 #ifdef BENCHMARK
34@@ -279,9 +281,51 @@
35 QVariant empty = QVariant::fromValue(QDBusVariant(QString()));
36 m_interface->asyncCall("Event", id, eventId, empty, 0u);
37 }
38+
39+ bool waitForWatcher(QDBusPendingCallWatcher * _watcher, int maxWait)
40+ {
41+ QPointer<QDBusPendingCallWatcher> watcher(_watcher);
42+
43+ if(m_type == ASYNCHRONOUS) {
44+ QTimer timer;
45+ timer.setSingleShot(true);
46+ QEventLoop loop;
47+ loop.connect(&timer, SIGNAL(timeout()), SLOT(quit()));
48+ loop.connect(watcher, SIGNAL(finished(QDBusPendingCallWatcher *)), SLOT(quit()));
49+ timer.start(maxWait);
50+ loop.exec();
51+ timer.stop();
52+
53+ if (!watcher) {
54+ // Watcher died. This can happen if importer got deleted while we were
55+ // waiting. See:
56+ // https://bugs.kde.org/show_bug.cgi?id=237156
57+ return false;
58+ }
59+
60+ if(!watcher->isFinished()) {
61+ // Timed out
62+ return false;
63+ }
64+ } else {
65+ watcher->waitForFinished();
66+ }
67+
68+ if (watcher->isError()) {
69+ DMWARNING << watcher->error().message();
70+ return false;
71+ }
72+
73+ return true;
74+ }
75 };
76
77 DBusMenuImporter::DBusMenuImporter(const QString &service, const QString &path, QObject *parent)
78+: DBusMenuImporter(service, path, ASYNCHRONOUS, parent)
79+{
80+}
81+
82+DBusMenuImporter::DBusMenuImporter(const QString &service, const QString &path, DBusMenuImporterType type, QObject *parent)
83 : QObject(parent)
84 , d(new DBusMenuImporterPrivate)
85 {
86@@ -292,6 +336,8 @@
87 d->m_menu = 0;
88 d->m_mustEmitMenuUpdated = false;
89
90+ d->m_type = type;
91+
92 connect(&d->m_mapper, SIGNAL(mapped(int)), SLOT(sendClickedEvent(int)));
93
94 d->m_pendingLayoutUpdateTimer = new QTimer(this);
95@@ -443,41 +489,6 @@
96 QMetaObject::invokeMethod(menu(), "aboutToShow");
97 }
98
99-static bool waitForWatcher(QDBusPendingCallWatcher * _watcher, int maxWait)
100-{
101- QPointer<QDBusPendingCallWatcher> watcher(_watcher);
102-
103- {
104- QTimer timer;
105- timer.setSingleShot(true);
106- QEventLoop loop;
107- loop.connect(&timer, SIGNAL(timeout()), SLOT(quit()));
108- loop.connect(watcher, SIGNAL(finished(QDBusPendingCallWatcher *)), SLOT(quit()));
109- timer.start(maxWait);
110- loop.exec();
111- timer.stop();
112- }
113-
114- if (!watcher) {
115- // Watcher died. This can happen if importer got deleted while we were
116- // waiting. See:
117- // https://bugs.kde.org/show_bug.cgi?id=237156
118- return false;
119- }
120-
121- if(!watcher->isFinished()) {
122- // Timed out
123- return false;
124- }
125-
126- if (watcher->isError()) {
127- DMWARNING << watcher->error().message();
128- return false;
129- }
130-
131- return true;
132-}
133-
134 void DBusMenuImporter::slotMenuAboutToShow()
135 {
136 QMenu *menu = qobject_cast<QMenu*>(sender());
137@@ -501,7 +512,7 @@
138
139 QPointer<QObject> guard(this);
140
141- if (!waitForWatcher(watcher, ABOUT_TO_SHOW_TIMEOUT)) {
142+ if (!d->waitForWatcher(watcher, ABOUT_TO_SHOW_TIMEOUT)) {
143 DMWARNING << "Application did not answer to AboutToShow() before timeout";
144 }
145
146@@ -541,7 +552,7 @@
147 if (needRefresh || menu->actions().isEmpty()) {
148 d->m_idsRefreshedByAboutToShow << id;
149 watcher = d->refresh(id);
150- if (!waitForWatcher(watcher, REFRESH_TIMEOUT)) {
151+ if (!d->waitForWatcher(watcher, REFRESH_TIMEOUT)) {
152 DMWARNING << "Application did not refresh before timeout";
153 }
154 }
155
156=== modified file 'src/dbusmenuimporter.h'
157--- src/dbusmenuimporter.h 2013-12-18 09:29:06 +0000
158+++ src/dbusmenuimporter.h 2014-03-14 14:12:57 +0000
159@@ -35,6 +35,16 @@
160 class QMenu;
161
162 class DBusMenuImporterPrivate;
163+
164+/**
165+ * Determine whether internal method calls should allow the Qt event loop
166+ * to execute or not
167+ */
168+enum DBusMenuImporterType {
169+ ASYNCHRONOUS,
170+ SYNCHRONOUS
171+};
172+
173 /**
174 * A DBusMenuImporter instance can recreate a menu serialized over DBus by
175 * DBusMenuExporter
176@@ -48,6 +58,12 @@
177 */
178 DBusMenuImporter(const QString &service, const QString &path, QObject *parent = 0);
179
180+ /**
181+ * Creates a DBusMenuImporter listening over DBus on service, path, with either async
182+ * or sync DBus calls
183+ */
184+ DBusMenuImporter(const QString &service, const QString &path, DBusMenuImporterType type, QObject *parent = 0);
185+
186 virtual ~DBusMenuImporter();
187
188 /**

Subscribers

People subscribed via source and target branches

to all changes: