Merge lp:~mterry/unity8/dbus-race-fix into lp:unity8

Proposed by Michael Terry
Status: Merged
Approved by: Michael Zanetti
Approved revision: 1344
Merged at revision: 1382
Proposed branch: lp:~mterry/unity8/dbus-race-fix
Merge into: lp:unity8
Diff against target: 935 lines (+337/-130)
28 files modified
plugins/LightDM/CMakeLists.txt (+2/-0)
plugins/LightDM/DBusGreeter.cpp (+3/-24)
plugins/LightDM/DBusGreeter.h (+3/-6)
plugins/LightDM/DBusGreeterList.cpp (+3/-24)
plugins/LightDM/DBusGreeterList.h (+3/-6)
plugins/LightDM/plugin.cpp (+2/-9)
plugins/Unity/DashCommunicator/dbusdashcommunicatorservice.cpp (+1/-5)
plugins/Unity/DashCommunicator/dbusdashcommunicatorservice.h (+2/-2)
plugins/Unity/Launcher/CMakeLists.txt (+5/-1)
plugins/Unity/Launcher/dbusinterface.cpp (+3/-36)
plugins/Unity/Launcher/dbusinterface.h (+2/-5)
plugins/Unity/Session/CMakeLists.txt (+5/-1)
plugins/Unity/Session/dbusunitysessionservice.cpp (+2/-8)
plugins/Unity/Session/dbusunitysessionservice.h (+2/-2)
qml/Shell.qml (+13/-0)
src/libunity8-private/CMakeLists.txt (+2/-0)
src/libunity8-private/unitydbusobject.cpp (+86/-0)
src/libunity8-private/unitydbusobject.h (+46/-0)
src/libunity8-private/unitydbusvirtualobject.cpp (+83/-0)
src/libunity8-private/unitydbusvirtualobject.h (+47/-0)
tests/mocks/LightDM/CMakeLists.txt (+2/-0)
tests/plugins/Unity/Launcher/CMakeLists.txt (+5/-1)
tests/plugins/Unity/Launcher/launchermodeltest.cpp (+1/-0)
tests/plugins/Unity/Session/CMakeLists.txt (+4/-0)
tests/plugins/Unity/Session/sessionbackendtest.cpp (+1/-0)
tests/qmltests/tst_Shell.qml (+3/-0)
tests/qmltests/tst_ShellWithPin.qml (+3/-0)
tests/qmltests/tst_TabletShell.qml (+3/-0)
To merge this branch: bzr merge lp:~mterry/unity8/dbus-race-fix
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Needs Fixing
Michael Zanetti (community) Approve
Unity Team Pending
Review via email: mp+237594@code.launchpad.net

Commit message

Fix a race between Qml loading and DBus registration that caused problems when jenkins tried to unlock the phone.

There is a small window of opportunity between when a Qml plugin registers on DBus and when the Qml is finished loading and hooking up signals. If the unlock-device script happens to hit that window, it will report success (DBus method was called), but the screen won't unlock (Qml never saw signal).

An easy fix is to just wait for the eventloop to be idle before registering on DBus. That's what I've done in this branch. In addition to fixing the proximate problem (LightDM), I've also fixed other Qml plugins that do the same thing.

Description of the change

Fix a race between Qml loading and DBus registration that caused problems when jenkins tried to unlock the phone.

There is a small window of opportunity between when a Qml plugin registers on DBus and when the Qml is finished loading and hooking up signals. If the unlock-device script happens to hit that window, it will report success (DBus method was called), but the screen won't unlock (Qml never saw signal).

An easy fix is to just wait for the eventloop to be idle before registering on DBus. That's what I've done in this branch. In addition to fixing the proximate problem (LightDM), I've also fixed other Qml plugins that do the same thing.

This unlock problem didn't appear until recently. Presumably startup timing changed. But you can test this bug before and after this branch with the following script:

adb reboot && adb wait-for-device && adb shell 'while ! gdbus call --session --dest com.canonical.UnityGreeter --object-path / --method com.canonical.UnityGreeter.HideGreeter 2>/dev/null 1>&2; do echo -n .; done' && echo -e '\nDid it work?'

== Checklist ==

 * 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

 * Did you make sure that your branch does not contain spurious tags?
 Yes

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

 * If you changed the UI, has there been a design review?
 NA

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Michael Zanetti (mzanetti) wrote :

Nowadays we have src/libunity-private which already contains D-Bus related stuff. It has some common interface for the client side of things.

Do you think you could create some reusable server-side class in there? Something like UnityDBusService, that handles registration using this timer where all plugins exposing services would just subclass it without having to worry about such races?

review: Needs Information
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Michael Terry (mterry) wrote :

OK, I've expanded this a bit, per your direction.

- I've added two classes UnityDBusObject and UnityDBusVirtualObject. Each with similar API to make it easier to create little DBus services in Unity.

- I actually let the dash communicator continue registering on DBus immediately, since it was noticeably slower to popuplate the dash if I did not. The other services seem fine in my quick testing.

- I closed a further race between the HideGreeter DBus call and PAM on startup. (this is the Shell.qml changes)

- I fixed a bug with the launcher's PropertiesChanged notification. It was leaving off the first 'interface' argument.

Resetting status to 'needs review'...

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Michael Zanetti (mzanetti) wrote :

Ok. Code looks good. Tested it and it works. I think we should at least test the shell.enabled=true/false calls. Probably adding compare(shell.locked, true) at the beginning/end of existing tests would make sense to catch potential "lockup" bugs in future code modifications. What do you say?

review: Needs Information
lp:~mterry/unity8/dbus-race-fix updated
1343. By Michael Terry

Try to guard against the shell.enabled logic from biting us

1344. By Michael Terry

Merge from trunk

Revision history for this message
Michael Terry (mterry) wrote :

I've added a bit of logic to the existing Shell qmluitests to confirm that shell starts enabled and ends enabled. It's not 100% a great test since the mock PAM plugin we use returns instantly, so there's only a millisecond where the shell is disabled anyway. But can't hurt.

What did you mean about checking shell.locked?

Revision history for this message
Michael Zanetti (mzanetti) wrote :

> I've added a bit of logic to the existing Shell qmluitests to confirm that
> shell starts enabled and ends enabled. It's not 100% a great test since the
> mock PAM plugin we use returns instantly, so there's only a millisecond where
> the shell is disabled anyway. But can't hurt.
>
> What did you mean about checking shell.locked?

sorry. I meant shell.enabled.

Ok, thanks.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/LightDM/CMakeLists.txt'
2--- plugins/LightDM/CMakeLists.txt 2014-06-18 17:26:30 +0000
3+++ plugins/LightDM/CMakeLists.txt 2014-10-15 17:55:21 +0000
4@@ -10,6 +10,7 @@
5 ${CMAKE_SOURCE_DIR}/plugins/Utils
6 ${CMAKE_SOURCE_DIR}/tests/mocks/LightDM
7 #${LIBLIGHTDM_INCLUDE_DIRS}
8+ ${libunity8-private_SOURCE_DIR}
9 ${LIBUSERMETRICSOUTPUT_INCLUDE_DIRS}
10 )
11
12@@ -28,6 +29,7 @@
13
14 target_link_libraries(LightDM-qml
15 MockLightDM-demo
16+ unity8-private
17 # TODO: Once we split out a separate greeter process, uncomment these lines
18 # ${LIBLIGHTDM_LDFLAGS}
19 ${LIBUSERMETRICSOUTPUT_LDFLAGS}
20
21=== modified file 'plugins/LightDM/DBusGreeter.cpp'
22--- plugins/LightDM/DBusGreeter.cpp 2014-08-27 15:11:21 +0000
23+++ plugins/LightDM/DBusGreeter.cpp 2014-10-15 17:55:21 +0000
24@@ -20,11 +20,9 @@
25 #include <QDBusMessage>
26 #include <QStringList>
27
28-DBusGreeter::DBusGreeter(Greeter *greeter, const QDBusConnection &connection, const QString &path)
29- : QObject(greeter),
30- m_greeter(greeter),
31- m_connection(connection),
32- m_path(path)
33+DBusGreeter::DBusGreeter(Greeter *greeter, const QString &path)
34+ : UnityDBusObject(path, "com.canonical.UnityGreeter", true, greeter),
35+ m_greeter(greeter)
36 {
37 connect(m_greeter, SIGNAL(isActiveChanged()), this, SLOT(isActiveChangedHandler()));
38 }
39@@ -49,22 +47,3 @@
40 notifyPropertyChanged("IsActive", isActive());
41 Q_EMIT isActiveChanged();
42 }
43-
44-// Manually emit a PropertiesChanged signal over DBus, because QtDBus
45-// doesn't do it for us on Q_PROPERTIES, oddly enough.
46-void DBusGreeter::notifyPropertyChanged(const QString& propertyName, const QVariant &value)
47-{
48- QDBusMessage message;
49- QVariantMap changedProps;
50-
51- changedProps.insert(propertyName, value);
52-
53- message = QDBusMessage::createSignal(m_path,
54- "org.freedesktop.DBus.Properties",
55- "PropertiesChanged");
56- message << "com.canonical.UnityGreeter";
57- message << changedProps;
58- message << QStringList();
59-
60- m_connection.send(message);
61-}
62
63=== modified file 'plugins/LightDM/DBusGreeter.h'
64--- plugins/LightDM/DBusGreeter.h 2014-09-05 11:57:35 +0000
65+++ plugins/LightDM/DBusGreeter.h 2014-10-15 17:55:21 +0000
66@@ -17,15 +17,15 @@
67 #ifndef UNITY_DBUSGREETER_H
68 #define UNITY_DBUSGREETER_H
69
70+#include "unitydbusobject.h"
71 #include <QDBusConnection>
72-#include <QObject>
73
74 class Greeter;
75
76 /** This is an internal class used to talk with the indicators.
77 */
78
79-class DBusGreeter : public QObject
80+class DBusGreeter : public UnityDBusObject
81 {
82 Q_OBJECT
83 Q_CLASSINFO("D-Bus Interface", "com.canonical.UnityGreeter")
84@@ -33,7 +33,7 @@
85 Q_PROPERTY(bool IsActive READ isActive NOTIFY isActiveChanged) // since 14.10
86
87 public:
88- explicit DBusGreeter(Greeter *greeter, const QDBusConnection &connection, const QString &path);
89+ explicit DBusGreeter(Greeter *greeter, const QString &path);
90
91 bool isActive() const;
92 Q_SCRIPTABLE void ShowGreeter(); // temporary, until we split the greeter again
93@@ -44,12 +44,9 @@
94
95 private Q_SLOTS:
96 void isActiveChangedHandler();
97- void notifyPropertyChanged(const QString &propertyName, const QVariant &value);
98
99 private:
100 Greeter *m_greeter;
101- QDBusConnection m_connection;
102- QString m_path;
103 };
104
105 #endif
106
107=== modified file 'plugins/LightDM/DBusGreeterList.cpp'
108--- plugins/LightDM/DBusGreeterList.cpp 2013-12-03 16:55:03 +0000
109+++ plugins/LightDM/DBusGreeterList.cpp 2014-10-15 17:55:21 +0000
110@@ -20,11 +20,9 @@
111 #include <QDBusMessage>
112 #include <QStringList>
113
114-DBusGreeterList::DBusGreeterList(Greeter *greeter, const QDBusConnection &connection, const QString &path)
115- : QObject(greeter),
116- m_greeter(greeter),
117- m_connection(connection),
118- m_path(path)
119+DBusGreeterList::DBusGreeterList(Greeter *greeter, const QString &path)
120+ : UnityDBusObject(path, "com.canonical.UnityGreeter", true, greeter),
121+ m_greeter(greeter)
122 {
123 connect(m_greeter, SIGNAL(authenticationUserChanged(const QString &)), this, SLOT(authenticationUserChangedHandler(const QString &)));
124 connect(m_greeter, SIGNAL(promptlessChanged()), this, SLOT(promptlessChangedHandler()));
125@@ -56,22 +54,3 @@
126 notifyPropertyChanged("EntryIsLocked", entryIsLocked());
127 Q_EMIT entryIsLockedChanged();
128 }
129-
130-// Manually emit a PropertiesChanged signal over DBus, because QtDBus
131-// doesn't do it for us on Q_PROPERTIES, oddly enough.
132-void DBusGreeterList::notifyPropertyChanged(const QString& propertyName, const QVariant &value)
133-{
134- QDBusMessage message;
135- QVariantMap changedProps;
136-
137- changedProps.insert(propertyName, value);
138-
139- message = QDBusMessage::createSignal(m_path,
140- "org.freedesktop.DBus.Properties",
141- "PropertiesChanged");
142- message << "com.canonical.UnityGreeter.List";
143- message << changedProps;
144- message << QStringList();
145-
146- m_connection.send(message);
147-}
148
149=== modified file 'plugins/LightDM/DBusGreeterList.h'
150--- plugins/LightDM/DBusGreeterList.h 2014-09-05 11:57:35 +0000
151+++ plugins/LightDM/DBusGreeterList.h 2014-10-15 17:55:21 +0000
152@@ -17,15 +17,15 @@
153 #ifndef UNITY_DBUSGREETERLIST_H
154 #define UNITY_DBUSGREETERLIST_H
155
156+#include "unitydbusobject.h"
157 #include <QDBusConnection>
158-#include <QObject>
159
160 class Greeter;
161
162 /** This is an internal class used to talk with the indicators.
163 */
164
165-class DBusGreeterList : public QObject
166+class DBusGreeterList : public UnityDBusObject
167 {
168 Q_OBJECT
169 Q_CLASSINFO("D-Bus Interface", "com.canonical.UnityGreeter.List")
170@@ -34,7 +34,7 @@
171 Q_PROPERTY(bool EntryIsLocked READ entryIsLocked NOTIFY entryIsLockedChanged) // since 14.04
172
173 public:
174- explicit DBusGreeterList(Greeter *greeter, const QDBusConnection &connection, const QString &path);
175+ explicit DBusGreeterList(Greeter *greeter, const QString &path);
176
177 Q_SCRIPTABLE void SetActiveEntry(const QString &entry); // since 13.04
178 Q_SCRIPTABLE QString GetActiveEntry() const; // since 13.10
179@@ -49,12 +49,9 @@
180 private Q_SLOTS:
181 void authenticationUserChangedHandler(const QString &user);
182 void promptlessChangedHandler();
183- void notifyPropertyChanged(const QString &propertyName, const QVariant &value);
184
185 private:
186 Greeter *m_greeter;
187- QDBusConnection m_connection;
188- QString m_path;
189 };
190
191 #endif
192
193=== modified file 'plugins/LightDM/plugin.cpp'
194--- plugins/LightDM/plugin.cpp 2014-07-01 20:23:44 +0000
195+++ plugins/LightDM/plugin.cpp 2014-10-15 17:55:21 +0000
196@@ -30,21 +30,14 @@
197 #include <QDBusConnection>
198 #include <QtQml/qqml.h>
199
200-static const char* GREETER_LIST_DBUS_PATH = "/list";
201-static const char* GREETER_DBUS_SERVICE = "com.canonical.UnityGreeter";
202-
203 static QObject *greeter_provider(QQmlEngine *engine, QJSEngine *scriptEngine)
204 {
205 Q_UNUSED(engine)
206 Q_UNUSED(scriptEngine)
207
208 Greeter *greeter = new Greeter();
209- QDBusConnection connection = QDBusConnection::sessionBus();
210- DBusGreeter *root = new DBusGreeter(greeter, connection, "/");
211- connection.registerObject("/", root, QDBusConnection::ExportScriptableContents);
212- DBusGreeterList *list = new DBusGreeterList(greeter, connection, GREETER_LIST_DBUS_PATH);
213- connection.registerObject(GREETER_LIST_DBUS_PATH, list, QDBusConnection::ExportScriptableContents);
214- connection.registerService(GREETER_DBUS_SERVICE);
215+ new DBusGreeter(greeter, "/");
216+ new DBusGreeterList(greeter, "/list");
217
218 return greeter;
219 }
220
221=== modified file 'plugins/Unity/DashCommunicator/dbusdashcommunicatorservice.cpp'
222--- plugins/Unity/DashCommunicator/dbusdashcommunicatorservice.cpp 2014-09-02 10:14:23 +0000
223+++ plugins/Unity/DashCommunicator/dbusdashcommunicatorservice.cpp 2014-10-15 17:55:21 +0000
224@@ -21,12 +21,8 @@
225 #include <QDebug>
226
227 DBusDashCommunicatorService::DBusDashCommunicatorService(QObject *parent):
228- QObject(parent)
229+ UnityDBusObject("/com/canonical/UnityDash", "com.canonical.UnityDash", false, parent)
230 {
231- QDBusConnection connection = QDBusConnection::sessionBus();
232-
233- connection.registerService("com.canonical.UnityDash");
234- connection.registerObject("/com/canonical/UnityDash", this, QDBusConnection::ExportScriptableSlots);
235 }
236
237 DBusDashCommunicatorService::~DBusDashCommunicatorService()
238
239=== modified file 'plugins/Unity/DashCommunicator/dbusdashcommunicatorservice.h'
240--- plugins/Unity/DashCommunicator/dbusdashcommunicatorservice.h 2014-09-02 10:14:23 +0000
241+++ plugins/Unity/DashCommunicator/dbusdashcommunicatorservice.h 2014-10-15 17:55:21 +0000
242@@ -17,9 +17,9 @@
243 #ifndef DBUSDASHCOMMUNICATORSERVICE_H
244 #define DBUSDASHCOMMUNICATORSERVICE_H
245
246-#include <QObject>
247+#include "unitydbusobject.h"
248
249-class DBusDashCommunicatorService: public QObject
250+class DBusDashCommunicatorService: public UnityDBusObject
251 {
252 Q_OBJECT
253 Q_CLASSINFO("D-Bus Interface", "com.canonical.Unity.DashCommunicator")
254
255=== modified file 'plugins/Unity/Launcher/CMakeLists.txt'
256--- plugins/Unity/Launcher/CMakeLists.txt 2014-09-29 09:43:18 +0000
257+++ plugins/Unity/Launcher/CMakeLists.txt 2014-10-15 17:55:21 +0000
258@@ -9,6 +9,7 @@
259 ${CMAKE_CURRENT_SOURCE_DIR}
260 ${CMAKE_SOURCE_DIR}/plugins/AccountsService
261 ${GSETTINGS_QT_INCLUDE_DIRS}
262+ ${libunity8-private_SOURCE_DIR}
263 )
264
265 set(QMLLAUNCHERPLUGIN_SRC
266@@ -30,7 +31,10 @@
267 ${QMLLAUNCHERPLUGIN_SRC}
268 )
269
270-target_link_libraries(UnityLauncher-qml ${GSETTINGS_QT_LDFLAGS})
271+target_link_libraries(UnityLauncher-qml
272+ unity8-private
273+ ${GSETTINGS_QT_LDFLAGS}
274+ )
275
276 qt5_use_modules(UnityLauncher-qml DBus Qml Gui)
277
278
279=== modified file 'plugins/Unity/Launcher/dbusinterface.cpp'
280--- plugins/Unity/Launcher/dbusinterface.cpp 2014-08-29 14:24:45 +0000
281+++ plugins/Unity/Launcher/dbusinterface.cpp 2014-10-15 17:55:21 +0000
282@@ -27,25 +27,13 @@
283 #include <QDebug>
284
285 DBusInterface::DBusInterface(LauncherModel *parent):
286- QDBusVirtualObject(parent),
287+ UnityDBusVirtualObject("/com/canonical/Unity/Launcher", "com.canonical.Unity.Launcher", true, parent),
288 m_launcherModel(parent)
289 {
290- /* Set up ourselves on DBus */
291- QDBusConnection con = QDBusConnection::sessionBus();
292- if (!con.registerService("com.canonical.Unity.Launcher")) {
293- qWarning() << "Unable to register launcher name";
294- }
295- if (!con.registerVirtualObject("/com/canonical/Unity/Launcher", this, QDBusConnection::VirtualObjectRegisterOption::SubPath)) {
296- qWarning() << "Unable to register launcher object";
297- }
298 }
299
300 DBusInterface::~DBusInterface()
301 {
302- /* Remove oursevles from DBus */
303- QDBusConnection con = QDBusConnection::sessionBus();
304- con.unregisterService("com.canonical.Unity.Launcher");
305- con.unregisterObject("/com/canonical/Unity/Launcher");
306 }
307
308 QString DBusInterface::introspect(const QString &path) const
309@@ -186,13 +174,13 @@
310 int newCount = message.arguments()[2].toInt();
311 if (!item || newCount != item->count()) {
312 Q_EMIT countChanged(appid, newCount);
313- emitPropChangedDbus(appid, "count", QVariant(newCount));
314+ notifyPropertyChanged("com.canonical.Unity.Launcher.Item", encodeAppId(appid), "count", QVariant(newCount));
315 }
316 } else if (message.arguments()[1].toString() == "countVisible") {
317 bool newVisible = message.arguments()[2].toBool();
318 if (!item || newVisible != item->countVisible()) {
319 Q_EMIT countVisibleChanged(appid, newVisible);
320- emitPropChangedDbus(appid, "countVisible", newVisible);
321+ notifyPropertyChanged("com.canonical.Unity.Launcher.Item", encodeAppId(appid), "countVisible", newVisible);
322 }
323 }
324 } else if (message.member() == "GetAll") {
325@@ -209,24 +197,3 @@
326 QDBusMessage reply = message.createReply(retval);
327 return connection.send(reply);
328 }
329-
330-void DBusInterface::emitPropChangedDbus(const QString& appId, const QString& property, const QVariant &value)
331-{
332- QString path("/com/canonical/Unity/Launcher/");
333- path.append(encodeAppId(appId));
334-
335- QDBusMessage message = QDBusMessage::createSignal(path, "org.freedesktop.DBus.Properties", "PropertiesChanged");
336-
337- QList<QVariant> arguments;
338- QVariantHash changedprops;
339- changedprops[property] = QVariant::fromValue(QDBusVariant(value));
340- QVariantList deletedprops;
341-
342- arguments.append(changedprops);
343- arguments.append(deletedprops);
344-
345- message.setArguments(arguments);
346-
347- QDBusConnection con = QDBusConnection::sessionBus();
348- con.send(message);
349-}
350
351=== modified file 'plugins/Unity/Launcher/dbusinterface.h'
352--- plugins/Unity/Launcher/dbusinterface.h 2014-08-29 12:59:15 +0000
353+++ plugins/Unity/Launcher/dbusinterface.h 2014-10-15 17:55:21 +0000
354@@ -18,12 +18,11 @@
355 */
356
357 #include "launcheritem.h"
358-
359-#include <QDBusVirtualObject>
360+#include "unitydbusvirtualobject.h"
361
362 class LauncherModel;
363
364-class DBusInterface: public QDBusVirtualObject
365+class DBusInterface: public UnityDBusVirtualObject
366 {
367 Q_OBJECT
368 public:
369@@ -43,8 +42,6 @@
370 static QString decodeAppId(const QString& path);
371 static QString encodeAppId(const QString& appId);
372
373- void emitPropChangedDbus(const QString& appId, const QString& property, const QVariant &value);
374-
375 LauncherModel *m_launcherModel;
376
377 };
378
379=== modified file 'plugins/Unity/Session/CMakeLists.txt'
380--- plugins/Unity/Session/CMakeLists.txt 2014-08-26 23:33:17 +0000
381+++ plugins/Unity/Session/CMakeLists.txt 2014-10-15 17:55:21 +0000
382@@ -5,6 +5,7 @@
383 include_directories(
384 ${CMAKE_CURRENT_SOURCE_DIR}
385 ${GIO_INCLUDE_DIRS}
386+ ${libunity8-private_SOURCE_DIR}
387 )
388
389 set(QMLSESSIONPLUGIN_SRC
390@@ -18,7 +19,10 @@
391 )
392
393 qt5_use_modules(UnitySession-qml DBus Qml)
394-target_link_libraries(UnitySession-qml ${GIO_LDFLAGS})
395+target_link_libraries(UnitySession-qml
396+ unity8-private
397+ ${GIO_LDFLAGS}
398+ )
399
400 # export the qmldir and qmltypes files
401 add_unity8_plugin(Unity.Session 0.1 Unity/Session TARGETS UnitySession-qml)
402
403=== modified file 'plugins/Unity/Session/dbusunitysessionservice.cpp'
404--- plugins/Unity/Session/dbusunitysessionservice.cpp 2014-07-09 12:22:19 +0000
405+++ plugins/Unity/Session/dbusunitysessionservice.cpp 2014-10-15 17:55:21 +0000
406@@ -21,15 +21,9 @@
407 #include <QDBusConnection>
408 #include <QDBusInterface>
409
410-DBusUnitySessionService::DBusUnitySessionService() : QObject()
411+DBusUnitySessionService::DBusUnitySessionService()
412+ : UnityDBusObject("/com/canonical/Unity/Session", "com.canonical.Unity")
413 {
414- QDBusConnection connection = QDBusConnection::sessionBus();
415-
416- connection.registerService("com.canonical.Unity");
417- connection.registerObject("/com/canonical/Unity/Session", this,
418- QDBusConnection::ExportScriptableSignals
419- | QDBusConnection::ExportScriptableSlots
420- | QDBusConnection::ExportScriptableInvokables);
421 }
422
423 DBusUnitySessionService::~DBusUnitySessionService()
424
425=== modified file 'plugins/Unity/Session/dbusunitysessionservice.h'
426--- plugins/Unity/Session/dbusunitysessionservice.h 2014-05-27 15:43:52 +0000
427+++ plugins/Unity/Session/dbusunitysessionservice.h 2014-10-15 17:55:21 +0000
428@@ -17,7 +17,7 @@
429 #ifndef DBUSUNITYSESSIONSERVICE_H
430 #define DBUSUNITYSESSIONSERVICE_H
431
432-#include <QObject>
433+#include "unitydbusobject.h"
434
435 /**
436 * DBusUnitySessionService provides com.canonical.Unity.Session dbus
437@@ -26,7 +26,7 @@
438 * com.canonical.Unity.Session interface provides public methods
439 * and signals to handle Logout/Reboot/Shutdown.
440 */
441-class DBusUnitySessionService : public QObject
442+class DBusUnitySessionService : public UnityDBusObject
443 {
444 Q_OBJECT
445 Q_CLASSINFO("D-Bus Interface", "com.canonical.Unity.Session")
446
447=== modified file 'qml/Shell.qml'
448--- qml/Shell.qml 2014-10-13 15:41:29 +0000
449+++ qml/Shell.qml 2014-10-15 17:55:21 +0000
450@@ -370,6 +370,10 @@
451 onHideGreeter: greeter.login()
452
453 onShowPrompt: {
454+ shell.enabled = true;
455+ if (!LightDM.Greeter.active) {
456+ return; // could happen if hideGreeter() comes in before we prompt
457+ }
458 if (greeter.narrowMode) {
459 if (isDefaultPrompt) {
460 if (lockscreen.alphaNumeric) {
461@@ -390,6 +394,9 @@
462 }
463
464 onPromptlessChanged: {
465+ if (!LightDM.Greeter.active) {
466+ return; // could happen if hideGreeter() comes in before we prompt
467+ }
468 if (greeter.narrowMode) {
469 if (LightDM.Greeter.promptless && LightDM.Greeter.authenticated) {
470 lockscreen.hide()
471@@ -401,6 +408,7 @@
472 }
473
474 onAuthenticationComplete: {
475+ shell.enabled = true;
476 if (LightDM.Greeter.authenticated) {
477 AccountsService.failedLogins = 0
478 }
479@@ -537,6 +545,11 @@
480
481 onShownChanged: {
482 if (shown) {
483+ // Disable everything so that user can't swipe greeter or
484+ // launcher until we get first prompt/authenticate, which
485+ // will re-enable the shell.
486+ shell.enabled = false;
487+
488 if (greeter.narrowMode) {
489 LightDM.Greeter.authenticate(LightDM.Users.data(0, LightDM.UserRoles.NameRole));
490 } else {
491
492=== modified file 'src/libunity8-private/CMakeLists.txt'
493--- src/libunity8-private/CMakeLists.txt 2014-09-05 11:57:01 +0000
494+++ src/libunity8-private/CMakeLists.txt 2014-10-15 17:55:21 +0000
495@@ -6,6 +6,8 @@
496
497 set(lib${LIB_NAME}_SRCS
498 abstractdbusservicemonitor.cpp
499+ unitydbusobject.cpp
500+ unitydbusvirtualobject.cpp
501 )
502
503 add_library(${LIB_NAME} SHARED
504
505=== added file 'src/libunity8-private/unitydbusobject.cpp'
506--- src/libunity8-private/unitydbusobject.cpp 1970-01-01 00:00:00 +0000
507+++ src/libunity8-private/unitydbusobject.cpp 2014-10-15 17:55:21 +0000
508@@ -0,0 +1,86 @@
509+/*
510+ * Copyright (C) 2014 Canonical, Ltd.
511+ *
512+ * This program is free software; you can redistribute it and/or modify
513+ * it under the terms of the GNU General Public License as published by
514+ * the Free Software Foundation; version 3.
515+ *
516+ * This program is distributed in the hope that it will be useful,
517+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
518+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
519+ * GNU General Public License for more details.
520+ *
521+ * You should have received a copy of the GNU General Public License
522+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
523+ */
524+
525+#include "unitydbusobject.h"
526+
527+#include <QDebug>
528+#include <QDBusMessage>
529+#include <QMetaClassInfo>
530+#include <QTimer>
531+
532+UnityDBusObject::UnityDBusObject(const QString &path, const QString &service, bool async, QObject *parent)
533+ : QObject(parent)
534+ , m_connection(QDBusConnection::sessionBus())
535+ , m_path(path)
536+ , m_service(service)
537+{
538+ if (async) {
539+ // Use a zero-timer to let Qml finish loading before we announce on DBus
540+ QTimer::singleShot(0, this, SLOT(registerObject()));
541+ } else {
542+ registerObject();
543+ }
544+}
545+
546+UnityDBusObject::~UnityDBusObject()
547+{
548+ // Leave service in place because multiple objects may be registered with
549+ // the same service. But we know we own the object path and can unregister it.
550+ m_connection.unregisterObject(path());
551+}
552+
553+QDBusConnection UnityDBusObject::connection() const
554+{
555+ return m_connection;
556+}
557+
558+QString UnityDBusObject::path() const
559+{
560+ return m_path;
561+}
562+
563+// Manually emit a PropertiesChanged signal over DBus, because QtDBus
564+// doesn't do it for us on Q_PROPERTIES, oddly enough.
565+void UnityDBusObject::notifyPropertyChanged(const QString& propertyName, const QVariant &value)
566+{
567+ QDBusMessage message;
568+ QString interface;
569+ QVariantMap changedProps;
570+
571+ interface = metaObject()->classInfo(metaObject()->indexOfClassInfo("D-Bus Interface")).value();
572+ changedProps.insert(propertyName, value);
573+
574+ message = QDBusMessage::createSignal(path(),
575+ "org.freedesktop.DBus.Properties",
576+ "PropertiesChanged");
577+ message << interface;
578+ message << changedProps;
579+ message << QStringList();
580+
581+ connection().send(message);
582+}
583+
584+void UnityDBusObject::registerObject()
585+{
586+ if (!m_connection.registerObject(m_path, this, QDBusConnection::ExportScriptableContents)) {
587+ qWarning() << "Unable to register DBus object" << m_path;
588+ }
589+ if (!m_service.isEmpty()) {
590+ if (!m_connection.registerService(m_service)) {
591+ qWarning() << "Unable to register DBus service" << m_service;
592+ }
593+ }
594+}
595
596=== added file 'src/libunity8-private/unitydbusobject.h'
597--- src/libunity8-private/unitydbusobject.h 1970-01-01 00:00:00 +0000
598+++ src/libunity8-private/unitydbusobject.h 2014-10-15 17:55:21 +0000
599@@ -0,0 +1,46 @@
600+/*
601+ * Copyright (C) 2014 Canonical, Ltd.
602+ *
603+ * This program is free software; you can redistribute it and/or modify
604+ * it under the terms of the GNU General Public License as published by
605+ * the Free Software Foundation; version 3.
606+ *
607+ * This program is distributed in the hope that it will be useful,
608+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
609+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
610+ * GNU General Public License for more details.
611+ *
612+ * You should have received a copy of the GNU General Public License
613+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
614+ */
615+
616+#ifndef UNITYDBUSOBJECT_H
617+#define UNITYDBUSOBJECT_H
618+
619+#include <QDBusConnection>
620+#include <QObject>
621+
622+class Q_DECL_EXPORT UnityDBusObject : public QObject
623+{
624+ Q_OBJECT
625+
626+public:
627+ explicit UnityDBusObject(const QString &path, const QString &service = QString(), bool async = true, QObject *parent = 0);
628+ ~UnityDBusObject();
629+
630+ QDBusConnection connection() const;
631+ QString path() const;
632+
633+protected:
634+ void notifyPropertyChanged(const QString& propertyName, const QVariant &value);
635+
636+private Q_SLOTS:
637+ void registerObject();
638+
639+private:
640+ QDBusConnection m_connection;
641+ QString m_path;
642+ QString m_service;
643+};
644+
645+#endif // UNITYDBUSOBJECT_H
646
647=== added file 'src/libunity8-private/unitydbusvirtualobject.cpp'
648--- src/libunity8-private/unitydbusvirtualobject.cpp 1970-01-01 00:00:00 +0000
649+++ src/libunity8-private/unitydbusvirtualobject.cpp 2014-10-15 17:55:21 +0000
650@@ -0,0 +1,83 @@
651+/*
652+ * Copyright (C) 2014 Canonical, Ltd.
653+ *
654+ * This program is free software; you can redistribute it and/or modify
655+ * it under the terms of the GNU General Public License as published by
656+ * the Free Software Foundation; version 3.
657+ *
658+ * This program is distributed in the hope that it will be useful,
659+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
660+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
661+ * GNU General Public License for more details.
662+ *
663+ * You should have received a copy of the GNU General Public License
664+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
665+ */
666+
667+#include "unitydbusvirtualobject.h"
668+
669+#include <QDebug>
670+#include <QDBusMessage>
671+#include <QTimer>
672+
673+UnityDBusVirtualObject::UnityDBusVirtualObject(const QString &path, const QString &service, bool async, QObject *parent)
674+ : QDBusVirtualObject(parent)
675+ , m_connection(QDBusConnection::sessionBus())
676+ , m_path(path)
677+ , m_service(service)
678+{
679+ if (async) {
680+ // Use a zero-timer to let Qml finish loading before we announce on DBus
681+ QTimer::singleShot(0, this, SLOT(registerObject()));
682+ } else {
683+ registerObject();
684+ }
685+}
686+
687+UnityDBusVirtualObject::~UnityDBusVirtualObject()
688+{
689+ // Leave service in place because multiple objects may be registered with
690+ // the same service. But we know we own the object path and can unregister it.
691+ m_connection.unregisterObject(path());
692+}
693+
694+QDBusConnection UnityDBusVirtualObject::connection() const
695+{
696+ return m_connection;
697+}
698+
699+QString UnityDBusVirtualObject::path() const
700+{
701+ return m_path;
702+}
703+
704+// Manually emit a PropertiesChanged signal over DBus, because QtDBus
705+// doesn't do it for us on Q_PROPERTIES, oddly enough.
706+void UnityDBusVirtualObject::notifyPropertyChanged(const QString& interface, const QString& node, const QString& propertyName, const QVariant &value)
707+{
708+ QDBusMessage message;
709+ QVariantMap changedProps;
710+
711+ changedProps.insert(propertyName, value);
712+
713+ message = QDBusMessage::createSignal(path() + "/" + node,
714+ "org.freedesktop.DBus.Properties",
715+ "PropertiesChanged");
716+ message << interface;
717+ message << changedProps;
718+ message << QStringList();
719+
720+ connection().send(message);
721+}
722+
723+void UnityDBusVirtualObject::registerObject()
724+{
725+ if (!m_connection.registerVirtualObject(m_path, this, QDBusConnection::VirtualObjectRegisterOption::SubPath)) {
726+ qWarning() << "Unable to register DBus object" << m_path;
727+ }
728+ if (!m_service.isEmpty()) {
729+ if (!m_connection.registerService(m_service)) {
730+ qWarning() << "Unable to register DBus service" << m_service;
731+ }
732+ }
733+}
734
735=== added file 'src/libunity8-private/unitydbusvirtualobject.h'
736--- src/libunity8-private/unitydbusvirtualobject.h 1970-01-01 00:00:00 +0000
737+++ src/libunity8-private/unitydbusvirtualobject.h 2014-10-15 17:55:21 +0000
738@@ -0,0 +1,47 @@
739+/*
740+ * Copyright (C) 2014 Canonical, Ltd.
741+ *
742+ * This program is free software; you can redistribute it and/or modify
743+ * it under the terms of the GNU General Public License as published by
744+ * the Free Software Foundation; version 3.
745+ *
746+ * This program is distributed in the hope that it will be useful,
747+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
748+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
749+ * GNU General Public License for more details.
750+ *
751+ * You should have received a copy of the GNU General Public License
752+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
753+ */
754+
755+#ifndef UNITYDBUSVIRTUALOBJECT_H
756+#define UNITYDBUSVIRTUALOBJECT_H
757+
758+#include <QDBusConnection>
759+#include <QDBusVirtualObject>
760+#include <QObject>
761+
762+class Q_DECL_EXPORT UnityDBusVirtualObject : public QDBusVirtualObject
763+{
764+ Q_OBJECT
765+
766+public:
767+ explicit UnityDBusVirtualObject(const QString &path, const QString &service = QString(), bool async = true, QObject *parent = 0);
768+ ~UnityDBusVirtualObject();
769+
770+ QDBusConnection connection() const;
771+ QString path() const;
772+
773+protected:
774+ void notifyPropertyChanged(const QString& interface, const QString& node, const QString& propertyName, const QVariant &value);
775+
776+private Q_SLOTS:
777+ void registerObject();
778+
779+private:
780+ QDBusConnection m_connection;
781+ QString m_path;
782+ QString m_service;
783+};
784+
785+#endif // UNITYDBUSVIRTUALOBJECT_H
786
787=== modified file 'tests/mocks/LightDM/CMakeLists.txt'
788--- tests/mocks/LightDM/CMakeLists.txt 2014-06-18 17:26:30 +0000
789+++ tests/mocks/LightDM/CMakeLists.txt 2014-10-15 17:55:21 +0000
790@@ -15,6 +15,7 @@
791 ${CMAKE_CURRENT_BINARY_DIR}
792 ${CMAKE_SOURCE_DIR}/plugins/Utils
793 ${CMAKE_SOURCE_DIR}/tests/mocks/libusermetrics
794+ ${libunity8-private_SOURCE_DIR}
795 )
796
797 set(QMLPLUGIN_SRC
798@@ -40,6 +41,7 @@
799 -llightdm-qt5-2
800 -L${CMAKE_BINARY_DIR}/tests/mocks/libusermetrics
801 -lusermetricsoutput
802+ unity8-private
803 )
804
805 qt5_use_modules(MockLightDM-qml DBus Gui Qml)
806
807=== modified file 'tests/plugins/Unity/Launcher/CMakeLists.txt'
808--- tests/plugins/Unity/Launcher/CMakeLists.txt 2014-09-29 09:43:18 +0000
809+++ tests/plugins/Unity/Launcher/CMakeLists.txt 2014-10-15 17:55:21 +0000
810@@ -7,6 +7,7 @@
811 ${CMAKE_SOURCE_DIR}/plugins/AccountsService
812 ${CMAKE_SOURCE_DIR}/plugins/Unity/Launcher
813 ${GSETTINGS_QT_INCLUDE_DIRS}
814+ ${libunity8-private_SOURCE_DIR}
815 )
816
817 add_definitions(-DSM_BUSNAME=sessionBus)
818@@ -34,7 +35,10 @@
819 ${APPLICATION_API_INCLUDEDIR}/unity/shell/application/ApplicationManagerInterface.h
820 ${LAUNCHER_API_INCLUDEDIR}/unity/shell/application/ApplicationInfoInterface.h
821 )
822-target_link_libraries(launchermodeltestExec ${GSETTINGS_QT_LDFLAGS})
823+target_link_libraries(launchermodeltestExec
824+ unity8-private
825+ ${GSETTINGS_QT_LDFLAGS}
826+ )
827 qt5_use_modules(launchermodeltestExec Test Core DBus Xml Gui)
828
829 # copy .desktop files into build directory for shadow builds
830
831=== modified file 'tests/plugins/Unity/Launcher/launchermodeltest.cpp'
832--- tests/plugins/Unity/Launcher/launchermodeltest.cpp 2014-09-29 14:26:56 +0000
833+++ tests/plugins/Unity/Launcher/launchermodeltest.cpp 2014-10-15 17:55:21 +0000
834@@ -124,6 +124,7 @@
835
836 void initTestCase() {
837 launcherModel = new LauncherModel(this);
838+ QCoreApplication::processEvents(); // to let the model register on DBus
839 QCOMPARE(launcherModel->rowCount(QModelIndex()), 0);
840
841 appManager = new MockAppManager(this);
842
843=== modified file 'tests/plugins/Unity/Session/CMakeLists.txt'
844--- tests/plugins/Unity/Session/CMakeLists.txt 2014-06-13 12:45:12 +0000
845+++ tests/plugins/Unity/Session/CMakeLists.txt 2014-10-15 17:55:21 +0000
846@@ -1,6 +1,7 @@
847 include_directories(
848 ${CMAKE_CURRENT_BINARY_DIR}
849 ${CMAKE_SOURCE_DIR}/plugins/Unity/Session
850+ ${libunity8-private_SOURCE_DIR}
851 )
852
853 add_definitions(-DSM_BUSNAME=sessionBus)
854@@ -17,4 +18,7 @@
855 sessionbackendtest.cpp
856 ${CMAKE_SOURCE_DIR}/plugins/Unity/Session/dbusunitysessionservice.cpp
857 )
858+target_link_libraries(sessionbackendtestExec
859+ unity8-private
860+ )
861 qt5_use_modules(sessionbackendtestExec Test Core Qml DBus)
862
863=== modified file 'tests/plugins/Unity/Session/sessionbackendtest.cpp'
864--- tests/plugins/Unity/Session/sessionbackendtest.cpp 2014-05-29 08:23:37 +0000
865+++ tests/plugins/Unity/Session/sessionbackendtest.cpp 2014-10-15 17:55:21 +0000
866@@ -45,6 +45,7 @@
867 QFETCH(QString, method);
868
869 DBusUnitySessionService dbusUnitySessionService;
870+ QCoreApplication::processEvents(); // to let the service register on DBus
871
872 QDBusConnection con = QDBusConnection::sessionBus();
873 QDBusInterface interface ("com.canonical.Unity",
874
875=== modified file 'tests/qmltests/tst_Shell.qml'
876--- tests/qmltests/tst_Shell.qml 2014-10-13 15:42:40 +0000
877+++ tests/qmltests/tst_Shell.qml 2014-10-15 17:55:21 +0000
878@@ -132,6 +132,8 @@
879 property Item shell: shellLoader.status === Loader.Ready ? shellLoader.item : null
880
881 function init() {
882+ tryCompare(shell, "enabled", true); // enabled by greeter when ready
883+
884 swipeAwayGreeter();
885
886 sessionSpy.target = findChild(shell, "greeter")
887@@ -142,6 +144,7 @@
888 }
889
890 function cleanup() {
891+ tryCompare(shell, "enabled", true); // make sure greeter didn't leave us in disabled state
892 launcherShowDashHomeSpy.target = null;
893
894 shellLoader.itemDestroyed = false;
895
896=== modified file 'tests/qmltests/tst_ShellWithPin.qml'
897--- tests/qmltests/tst_ShellWithPin.qml 2014-10-13 15:42:15 +0000
898+++ tests/qmltests/tst_ShellWithPin.qml 2014-10-15 17:55:21 +0000
899@@ -127,6 +127,7 @@
900 property Item shell: shellLoader.status === Loader.Ready ? shellLoader.item : null
901
902 function init() {
903+ tryCompare(shell, "enabled", true); // will be enabled when greeter is all ready
904 sessionSpy.target = findChild(shell, "greeter")
905 swipeAwayGreeter()
906 waitForLockscreen()
907@@ -135,6 +136,8 @@
908 }
909
910 function cleanup() {
911+ tryCompare(shell, "enabled", true); // make sure greeter didn't leave us in disabled state
912+
913 shellLoader.itemDestroyed = false
914
915 shellLoader.active = false
916
917=== modified file 'tests/qmltests/tst_TabletShell.qml'
918--- tests/qmltests/tst_TabletShell.qml 2014-10-09 13:05:21 +0000
919+++ tests/qmltests/tst_TabletShell.qml 2014-10-15 17:55:21 +0000
920@@ -119,12 +119,15 @@
921 property Item shell: shellLoader.status === Loader.Ready ? shellLoader.item : null
922
923 function init() {
924+ tryCompare(shell, "enabled", true); // will be enabled when greeter is all ready
925 sessionSpy.clear()
926 sessionSpy.target = findChild(shell, "greeter")
927 dashCommunicatorSpy.target = findInvisibleChild(shell, "dashCommunicator")
928 }
929
930 function cleanup() {
931+ tryCompare(shell, "enabled", true); // make sure greeter didn't leave us in disabled state
932+
933 shellLoader.itemDestroyed = false
934
935 shellLoader.active = false

Subscribers

People subscribed via source and target branches