Merge lp:~mterry/unity8/greeter-profiles into lp:unity8

Proposed by Michael Terry on 2014-10-04
Status: Merged
Approved by: Michael Terry on 2015-02-10
Approved revision: 1338
Merged at revision: 1607
Proposed branch: lp:~mterry/unity8/greeter-profiles
Merge into: lp:unity8
Diff against target: 630 lines (+201/-43)
17 files modified
plugins/Unity/Indicators/indicator.cpp (+23/-8)
plugins/Unity/Indicators/indicator.h (+5/-1)
plugins/Unity/Indicators/indicatorsmanager.cpp (+17/-3)
plugins/Unity/Indicators/indicatorsmanager.h (+6/-1)
plugins/Unity/Indicators/indicatorsmodel.cpp (+25/-2)
plugins/Unity/Indicators/indicatorsmodel.h (+6/-1)
plugins/Unity/Indicators/modelactionrootstate.cpp (+4/-2)
plugins/Unity/Indicators/unitymenumodelcache.cpp (+8/-11)
plugins/Unity/Indicators/unitymenumodelcache.h (+2/-2)
qml/Shell.qml (+3/-1)
tests/mocks/Unity/Indicators/fakeindicatorsmodel.cpp (+13/-1)
tests/mocks/Unity/Indicators/fakeindicatorsmodel.h (+6/-0)
tests/plugins/Unity/Indicators/indicatorsmanagertest.cpp (+30/-5)
tests/plugins/Unity/Indicators/indicatorsmodeltest.cpp (+6/-3)
tests/plugins/Unity/Indicators/sharedunitymenumodeltest.cpp (+2/-2)
tests/qmltests/tst_Shell.qml (+29/-0)
tests/qmltests/tst_ShellWithPin.qml (+16/-0)
To merge this branch: bzr merge lp:~mterry/unity8/greeter-profiles
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve on 2015-02-10
Nick Dedekind (community) 2014-10-04 Approve on 2015-02-10
Michał Sawicz Needs Information on 2014-12-10
Albert Astals Cid (community) Abstain on 2014-11-20
Olga Kemmet 2014-12-02 Pending
Review via email: mp+237155@code.launchpad.net

Commit Message

Support switching the indicator profile on the fly, as the greeter appears and disappears. (But don't use it yet, not until the settings panel to control this is in place.)

These changes are mostly two types: adding support for switching profiles at all after startup and caching previous results for a moment while we do the switching.

Description of the Change

Support switching the indicator profile on the fly, as the greeter appears and disappears. (But don't use it yet, not until the settings panel to control this is in place.)

These changes are mostly two types: adding support for switching profiles at all after startup and caching previous results for a moment while we do the switching.

== 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.
Nick Dedekind (nick-dedekind) wrote :

Comments attached.
Haven't tested yet.

review: Needs Fixing
Nick Dedekind (nick-dedekind) wrote :

Tested with silo13.
Works!

Nick Dedekind (nick-dedekind) wrote :

attached a few more comments to above.

review: Needs Fixing
Michael Terry (mterry) wrote :

OK, I've addressed your semicolon and setProfile remarks. I asked you on IRC to see if you could live with async: true or not. Let's see how your testing goes.

Nick Dedekind (nick-dedekind) wrote :

> OK, I've addressed your semicolon and setProfile remarks. I asked you on IRC
> to see if you could live with async: true or not. Let's see how your testing
> goes.

You dissapeared on irc :)

<dednick> mterry: hm. ok, there doesnt really seem to be that much lag to me, but it's subjective to how much of a hurry you're in :) The lag introduced in sync mode is going to be a problem though. I was asked to put it back awhile ago due to the lag (although this was whenI wasn't keeping the menu un memory all the time).

<dednick> I think the lag with async will be reduced when my new indicator branch lands. The menu content is actually at 0 height when you drag it, so I think the list items are being loaded "only as visible". The new panel page is always the same height so the items will be loaded immediately.

I can try merge your branch into mine to see if it helps.

Michael Terry (mterry) wrote :

My internet was awful and I couldn't respond:

<mterry> dednick, OK can you try your branch merged with mine and see how it is? I'd like to top-approve this branch by end of day. I can do that with async still on if I have to, but would like to be confident it won't be awful for users

Nick Dedekind (nick-dedekind) wrote :

LGTM.

 * Did you perform an exploratory manual test run of the code change and any related functionality?
Yes

 * Did CI run pass? If not, please explain why.
Not related.

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

review: Approve
Albert Astals Cid (aacid) wrote :

Text conflict in tests/qmltests/tst_ShellWithPin.qml
1 conflicts encountered.

review: Needs Fixing
Michael Terry (mterry) wrote :

Merged from trunk

Albert Astals Cid (aacid) wrote :

Text conflict in tests/qmltests/tst_ShellWithPin.qml
1 conflicts encountered.

review: Needs Fixing
Nick Dedekind (nick-dedekind) wrote :

I've been talking with Albert, and apparently making the delegate sync is not much of a problem as only the current viewport is loaded synchronously.
So you can change it back if you want :). Or i'll put it in my new panel branch.

Michael Terry (mterry) wrote :

Merged from trunk and removed async again!

Albert Astals Cid (aacid) wrote :

Ok, let's wait for CI again before top approval

review: Abstain
Albert Astals Cid (aacid) wrote :

Text conflict in plugins/Unity/Indicators/unitymenumodelcache.cpp
Conflict adding file qml/Panel/IndicatorItem.qml. Moved existing file to qml/Panel/IndicatorItem.qml.moved.
Contents conflict in qml/Panel/Indicators.qml
Contents conflict in qml/Panel/Indicators/DefaultIndicatorWidget.qml
Text conflict in qml/Panel/MenuContent.qml
Text conflict in qml/Shell.qml
Text conflict in tests/mocks/Unity/Indicators/fakeindicatorsmodel.h
Contents conflict in tests/qmltests/Panel/Indicators/tst_CachedUnityMenuModel.qml
Text conflict in tests/qmltests/tst_Shell.qml
9 conflicts encountered.

Reminder: Was already top approved, should be again after merge is fixed.

review: Needs Fixing
Michał Sawicz (saviq) wrote :

Bump?

Michael Terry (mterry) wrote :
Download full text (3.8 KiB)

I've merged from trunk, but don't want to top-approve yet. I was tracking down a weird issue I was seeing for most of the day. It seems when using the "phone_greeter" profile, *something* frequently causes unity8 to hang (not crash).

When this happens, unity8 seems to be deep within a stack similar to, but not exactly:

#0 qPremultiply (x=0) at ../../include/QtGui/../../src/gui/painting/qrgb.h:98
#1 convert_RGBA_to_ARGB_PM (dest=<optimized out>, src=0x1b46d48)
    at image/qimage_conversions.cpp:381
#2 0xb6cb1baa in QImage::convertToFormat_helper (this=this@entry=0xbee3e930,
    format=format@entry=QImage::Format_ARGB32_Premultiplied,
    flags=flags@entry=...) at image/qimage.cpp:1912
#3 0xb67ba796 in convertToFormat (flags=...,
    f=QImage::Format_ARGB32_Premultiplied, this=0xbee3e930)
    at /usr/include/arm-linux-gnueabihf/qt5/QtGui/qimage.h:177
#4 QQuickDefaultTextureFactory::QQuickDefaultTextureFactory (this=this@entry=
    0x177e5b8, image=...) at util/qquickpixmapcache.cpp:107
#5 0xb67ba7f6 in textureFactoryForImage (image=...)
    at util/qquickpixmapcache.cpp:126
#6 0xb67bc1c2 in createPixmapDataSync (ok=<synthetic pointer>,
    requestSize=..., url=..., engine=<optimized out>,
    declarativePixmap=0x1759f30) at util/qquickpixmapcache.cpp:1021
#7 QQuickPixmap::load (this=this@entry=0x1759f30, engine=<optimized out>,
    url=..., requestSize=..., options=options@entry=...)
    at util/qquickpixmapcache.cpp:1243
#8 0xb685e232 in QQuickImageBase::load (this=0x17607d8)
    at items/qquickimagebase.cpp:222
#9 0xb68e1ffe in QQuickImageBase::qt_metacall (this=this@entry=0x17607d8,
    _c=_c@entry=QMetaObject::WriteProperty, _id=5, _a=_a@entry=0xbee3eac4)
    at .moc/moc_qquickimagebase_p.cpp:268
#10 0xb68e22c0 in QQuickImage::qt_metacall (this=0x17607d8,
    _c=QMetaObject::WriteProperty, _id=<optimized out>, _a=0xbee3eac4)
    at .moc/moc_qquickimage_p.cpp:228
#11 0xb6ae0f34 in QQmlProxyMetaObject::metaCall (this=0x175a0e8,
    c=QMetaObject::WriteProperty, id=48, a=0xbee3eac4)
    at qml/qqmlproxymetaobject.cpp:121
#12 0xb644165c in QMetaObject::metacall (object=<optimized out>,
    cl=<optimized out>, idx=<optimized out>, argv=0xbee3eac4)
    at kernel/qmetaobject.cpp:306
#13 0xb6afa448 in writeProperty (this=<optimized out>, p=<optimized out>,
    flags=..., idx=<optimized out>, obj=<optimized out>)
    at qml/qqmlvaluetype_p.h:102
#14 QQmlValueTypeBase<QSize>::write (this=<optimized out>,
    obj=<optimized out>, idx=<optimized out>, flags=...)
    at qml/qqmlvaluetype_p.h:133
#15 0xb6ad9d50 in QQmlPropertyPrivate::writeValueProperty (
    object=object@entry=0x17607d8, core=..., value=...,
    context=context@entry=0x1e49708, flags=flags@entry=...)
    at qml/qqmlproperty.cpp:1242
#16 0xb6ada166 in QQmlPropertyPrivate::writeBinding (
    object=object@entry=0x17607d8, core=..., context=<optimized out>,
    expression=expression@entry=0x175cd80, result=result@entry=...,
    isUndefined=false, flags=...) at qml/qqmlproperty.cpp:1578
#17 0xb6b24146 in QQmlBinding::update (this=0x175cd70, flags=...)
    at qml/qqmlbinding.cpp:266
#18 0xb6b2461c in update (this=<optimized out>) at qml/qqmlbinding_p.h:105
#19 ...

Read more...

Michael Terry (mterry) wrote :

Oh also, I never saw this on the previous versions of this MP. The indicators have been adding a lot of support for phone_greeter and trunk has also changed a lot, so I believe this is new.

Michael Terry (mterry) wrote :

Changing indicator-power's profile file to read:

[phone_greeter]
ObjectPath=/com/canonical/indicator/power/phone
Position=25

intead of:

[phone_greeter]
ObjectPath=/com/canonical/indicator/power/desktop_greeter
Position=25

avoids the issue. I'll talk with Ted about what's happening with desktop_greeter...

Albert Astals Cid (aacid) wrote :

Ok, merge problem fixed, changing my vote to abstain

review: Abstain
Michael Terry (mterry) wrote :

OK, design reviewed and said it was good. Marking approved again, but note that https://code.launchpad.net/~charlesk/indicator-power/show-phone-menu-in-phone-greeter/+merge/243473 should land in the same silo.

Michał Sawicz (saviq) wrote :

FAIL! : qmltestrunner::Shell::test_greeterChangesIndicatorProfile() property profile
   Actual (): phone
   Expected (): phone_greeter
   Loc: [/home/michal/Pobrane/unity8-8.02+15.04.20141208/tests/qmltests/tst_Shell.qml(548)]

review: Needs Fixing
Michael Terry (mterry) wrote :

Guh! That's what I get for making last minute changes. Design asked me to tweak things so that when the phone doesn't have a passphrase/passcode, the indicators don't change. It was a simple one-liner, but of course I forgot the tests.

Updated tests to work now.

Michał Sawicz (saviq) wrote :

I believe the behavior of "no indicators" vs. "full indicators" should change with that branch to "limited indicators" vs. "full indicators", can you please confirm whether that's the case and make the necessary changes?

review: Needs Information
Michael Terry (mterry) wrote :

(I asked Design, waiting for final feedback)

Albert Astals Cid (aacid) wrote :

Text conflict in plugins/Unity/Indicators/rootstateparser.cpp
Text conflict in qml/Panel/Indicators/VisibleIndicators.qml
2 conflicts encountered.

Michael Terry (mterry) wrote :

OK, this is ready for review again. I merged from trunk and disabled the dynamic-greeter-profile switching. That will remain disabled until Design finishes with how the settings panel for that should look (there are three options they want, whereas we only currently support two -- and will continue to only support two with this branch)

lp:~mterry/unity8/greeter-profiles updated on 2015-02-10
1338. By Michael Terry on 2015-02-10

Add a C++ test for switching profiles

Nick Dedekind (nick-dedekind) wrote :

Looks fine to me.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/Unity/Indicators/indicator.cpp'
2--- plugins/Unity/Indicators/indicator.cpp 2013-10-10 15:55:20 +0000
3+++ plugins/Unity/Indicators/indicator.cpp 2015-02-10 16:08:40 +0000
4@@ -31,24 +31,26 @@
5 {
6 }
7
8-void Indicator::init(const QString &profile, const QString& busName, const QSettings& settings)
9+void Indicator::init(const QString& busName, const QSettings& settings)
10 {
11+ // Save all keys we care about from the QSettings object. It's annoying
12+ // that we can't just copy the object.
13+ m_settings.clear();
14+ Q_FOREACH(const QString& key, settings.allKeys()) {
15+ if (key.endsWith("/Position") || key.endsWith("/ObjectPath")) {
16+ m_settings.insert(key, settings.value(key));
17+ }
18+ }
19+
20 setId(settings.value("Indicator Service/Name").toString());
21
22- QVariant pos = settings.value(profile + "/Position");
23- if (!pos.isValid())
24- pos = settings.value("Indicator Service/Position", QVariant::fromValue(0));
25- setPosition(pos.toInt());
26-
27 QString actionObjectPath = settings.value("Indicator Service/ObjectPath").toString();
28- QString menuObjectPath = settings.value(profile + "/ObjectPath").toString();
29
30 QVariantMap properties;
31 properties.clear();
32 properties.insert("busType", 1);
33 properties.insert("busName", busName);
34 properties.insert("actionsObjectPath", actionObjectPath);
35- properties.insert("menuObjectPath", menuObjectPath);
36 setIndicatorProperties(properties);
37 }
38
39@@ -78,6 +80,19 @@
40 }
41 }
42
43+void Indicator::setProfile(const QString& profile)
44+{
45+ QVariant pos = m_settings.value(profile + "/Position");
46+ if (!pos.isValid())
47+ pos = m_settings.value("Indicator Service/Position", QVariant::fromValue(0));
48+ setPosition(pos.toInt());
49+
50+ QString menuObjectPath = m_settings.value(profile + "/ObjectPath").toString();
51+ QVariantMap map = m_properties.toMap();
52+ map.insert("menuObjectPath", menuObjectPath);
53+ setIndicatorProperties(map);
54+}
55+
56 QVariant Indicator::indicatorProperties() const
57 {
58 return m_properties;
59
60=== modified file 'plugins/Unity/Indicators/indicator.h'
61--- plugins/Unity/Indicators/indicator.h 2013-10-10 15:47:40 +0000
62+++ plugins/Unity/Indicators/indicator.h 2015-02-10 16:08:40 +0000
63@@ -35,12 +35,15 @@
64 Indicator(QObject *parent = 0);
65 virtual ~Indicator();
66
67- void init(const QString &profile, const QString& busName, const QSettings& settings);
68+ void init(const QString& busName, const QSettings& settings);
69
70 QString identifier() const;
71 int position() const;
72 QVariant indicatorProperties() const;
73
74+public Q_SLOTS:
75+ void setProfile(const QString& profile);
76+
77 Q_SIGNALS:
78 void identifierChanged(const QString &identifier);
79 void positionChanged(int position);
80@@ -55,6 +58,7 @@
81 QString m_identifier;
82 int m_position;
83 QVariant m_properties;
84+ QVariantMap m_settings;
85 };
86
87 #endif // INDICATOR_H
88
89=== modified file 'plugins/Unity/Indicators/indicatorsmanager.cpp'
90--- plugins/Unity/Indicators/indicatorsmanager.cpp 2014-04-18 17:04:59 +0000
91+++ plugins/Unity/Indicators/indicatorsmanager.cpp 2015-02-10 16:08:40 +0000
92@@ -54,10 +54,9 @@
93
94 }
95
96-void IndicatorsManager::load(const QString& profile)
97+void IndicatorsManager::load()
98 {
99 unload();
100- m_profile = profile;
101
102 QStringList xdgLocations = shellDataDirs();//QStandardPaths::standardLocations(QStandardPaths::GenericDataLocation);
103
104@@ -216,6 +215,19 @@
105 }
106 }
107
108+QString IndicatorsManager::profile() const
109+{
110+ return m_profile;
111+}
112+
113+void IndicatorsManager::setProfile(const QString& profile)
114+{
115+ if (m_profile != profile) {
116+ m_profile = profile;
117+ Q_EMIT profileChanged(m_profile);
118+ }
119+}
120+
121 void IndicatorsManager::startVerify(const QString& path)
122 {
123 QHashIterator<QString, IndicatorData*> iter(m_indicatorsData);
124@@ -268,7 +280,9 @@
125 Indicator::Ptr new_indicator(new Indicator(this));
126 data->m_indicator = new_indicator;
127 QSettings settings(data->m_fileInfo.absoluteFilePath(), QSettings::IniFormat, this);
128- new_indicator->init(m_profile, data->m_fileInfo.fileName(), settings);
129+ new_indicator->init(data->m_fileInfo.fileName(), settings);
130+ new_indicator->setProfile(m_profile);
131+ QObject::connect(this, SIGNAL(profileChanged(const QString&)), new_indicator.data(), SLOT(setProfile(const QString&)));
132 return new_indicator;
133 }
134
135
136=== modified file 'plugins/Unity/Indicators/indicatorsmanager.h'
137--- plugins/Unity/Indicators/indicatorsmanager.h 2014-04-09 14:51:09 +0000
138+++ plugins/Unity/Indicators/indicatorsmanager.h 2015-02-10 16:08:40 +0000
139@@ -32,13 +32,17 @@
140 {
141 Q_OBJECT
142 Q_PROPERTY(bool loaded READ isLoaded NOTIFY loadedChanged)
143+ Q_PROPERTY(QString profile READ profile WRITE setProfile NOTIFY profileChanged)
144 public:
145 explicit IndicatorsManager(QObject* parent = 0);
146 ~IndicatorsManager();
147
148- Q_INVOKABLE void load(const QString& profile);
149+ Q_INVOKABLE void load();
150 Q_INVOKABLE void unload();
151
152+ QString profile() const;
153+ void setProfile(const QString& profile);
154+
155 Indicator::Ptr indicator(const QString& indicator_name);
156
157 QList<Indicator::Ptr> indicators();
158@@ -47,6 +51,7 @@
159
160 Q_SIGNALS:
161 void loadedChanged(bool);
162+ void profileChanged(const QString&);
163
164 void indicatorLoaded(const QString& indicator_name);
165 void indicatorAboutToBeUnloaded(const QString& indicator_name);
166
167=== modified file 'plugins/Unity/Indicators/indicatorsmodel.cpp'
168--- plugins/Unity/Indicators/indicatorsmodel.cpp 2014-10-03 10:17:29 +0000
169+++ plugins/Unity/Indicators/indicatorsmodel.cpp 2015-02-10 16:08:40 +0000
170@@ -56,6 +56,7 @@
171 m_manager = new IndicatorsManager(this);
172 QObject::connect(m_manager, SIGNAL(indicatorLoaded(const QString&)), this, SLOT(onIndicatorLoaded(const QString&)));
173 QObject::connect(m_manager, SIGNAL(indicatorAboutToBeUnloaded(const QString&)), this, SLOT(onIndicatorAboutToBeUnloaded(const QString&)));
174+ QObject::connect(m_manager, SIGNAL(profileChanged(const QString&)), this, SIGNAL(profileChanged()));
175
176 QObject::connect(this, SIGNAL(rowsInserted(const QModelIndex &, int, int)), this, SIGNAL(countChanged()));
177 QObject::connect(this, SIGNAL(rowsRemoved(const QModelIndex &, int, int)), this, SIGNAL(countChanged()));
178@@ -81,14 +82,36 @@
179 }
180
181 /*!
182+ \qmlproperty IndicatorsModel::profile
183+ The indicator profile to use for the model.
184+
185+ \b Note: methods should only be called after the Component has completed.
186+*/
187+QString IndicatorsModel::profile() const
188+{
189+ return m_manager->profile();
190+}
191+
192+/*!
193+ \qmlproperty IndicatorsModel::setProfile
194+ Set the indicator profile to use for the model.
195+
196+ \b Note: methods should only be called after the Component has completed.
197+*/
198+void IndicatorsModel::setProfile(const QString &profile)
199+{
200+ m_manager->setProfile(profile);
201+}
202+
203+/*!
204 \qmlmethod IndicatorsModel::unload()
205
206 Load all indicators.
207 */
208-void IndicatorsModel::load(const QString& profile)
209+void IndicatorsModel::load()
210 {
211 m_indicators.clear();
212- m_manager->load(profile);
213+ m_manager->load();
214 }
215
216 /*!
217
218=== modified file 'plugins/Unity/Indicators/indicatorsmodel.h'
219--- plugins/Unity/Indicators/indicatorsmodel.h 2014-01-30 14:54:01 +0000
220+++ plugins/Unity/Indicators/indicatorsmodel.h 2015-02-10 16:08:40 +0000
221@@ -34,17 +34,21 @@
222 Q_OBJECT
223 Q_ENUMS(Roles)
224 Q_PROPERTY(int count READ count NOTIFY countChanged)
225+ Q_PROPERTY(QString profile READ profile WRITE setProfile NOTIFY profileChanged)
226
227 public:
228
229 IndicatorsModel(QObject *parent=0);
230 ~IndicatorsModel();
231
232- Q_INVOKABLE void load(const QString& profile);
233+ Q_INVOKABLE void load();
234 Q_INVOKABLE void unload();
235
236 Q_INVOKABLE QVariant data(int row, int role) const;
237
238+ QString profile() const;
239+ void setProfile(const QString& profile);
240+
241 /* QAbstractItemModel */
242 QHash<int, QByteArray> roleNames() const;
243 int columnCount(const QModelIndex &parent = QModelIndex()) const;
244@@ -55,6 +59,7 @@
245
246 Q_SIGNALS:
247 void countChanged();
248+ void profileChanged();
249 void indicatorDataChanged(const QVariant& data);
250
251 private Q_SLOTS:
252
253=== modified file 'plugins/Unity/Indicators/modelactionrootstate.cpp'
254--- plugins/Unity/Indicators/modelactionrootstate.cpp 2014-12-17 11:19:01 +0000
255+++ plugins/Unity/Indicators/modelactionrootstate.cpp 2015-02-10 16:08:40 +0000
256@@ -71,7 +71,7 @@
257
258 bool ModelActionRootState::valid() const
259 {
260- return m_menu && m_menu->rowCount() > 0;
261+ return !currentState().empty();
262 }
263
264 void ModelActionRootState::onModelRowsAdded(const QModelIndex& parent, int start, int end)
265@@ -121,7 +121,9 @@
266 m_menu->setActionStateParser(oldParser);
267
268 setCurrentState(state);
269- } else {
270+ } else if (!m_menu) {
271 setCurrentState(QVariantMap());
272 }
273+ // else if m_menu->rowCount() == 0, let's leave existing cache in place
274+ // until the new menu comes in, to avoid flashing the UI empty for a moment
275 }
276
277=== modified file 'plugins/Unity/Indicators/unitymenumodelcache.cpp'
278--- plugins/Unity/Indicators/unitymenumodelcache.cpp 2014-10-20 14:39:46 +0000
279+++ plugins/Unity/Indicators/unitymenumodelcache.cpp 2015-02-10 16:08:40 +0000
280@@ -46,17 +46,14 @@
281 QQmlEngine::setObjectOwnership(model, QQmlEngine::CppOwnership);
282
283 QSharedPointer<UnityMenuModel> menuModel(model);
284- connect(model, &QObject::destroyed, this, [this] {
285- QMutableHashIterator<QByteArray, QWeakPointer<UnityMenuModel>> iter(m_registry);
286- while(iter.hasNext()) {
287- auto keyVal = iter.next();
288- if (keyVal.value().isNull()) {
289- iter.remove();
290- break;
291- }
292- }
293- });
294- m_registry[path] = menuModel.toWeakRef();
295+
296+ // Keep a shared pointer (rather than weak pointer which would cause the
297+ // model to be deleted when all shared pointers we give out are deleted).
298+ // We want to keep all models cached because when we switch indicator
299+ // profiles, we will be switching paths often. And we want to keep the
300+ // old model around, ready to be used. Otherwise the UI might momentarily
301+ // wait as we populate the model from DBus yet again.
302+ m_registry[path] = menuModel;
303
304 menuModel->setMenuObjectPath(path);
305 return menuModel;
306
307=== modified file 'plugins/Unity/Indicators/unitymenumodelcache.h'
308--- plugins/Unity/Indicators/unitymenumodelcache.h 2014-10-20 14:39:46 +0000
309+++ plugins/Unity/Indicators/unitymenumodelcache.h 2015-02-10 16:08:40 +0000
310@@ -25,7 +25,7 @@
311 #include <QObject>
312 #include <QHash>
313 #include <QPointer>
314-#include <QWeakPointer>
315+#include <QSharedPointer>
316
317 class UnityMenuModel;
318
319@@ -43,7 +43,7 @@
320 Q_INVOKABLE virtual bool contains(const QByteArray& path);
321
322 protected:
323- QHash<QByteArray, QWeakPointer<UnityMenuModel>> m_registry;
324+ QHash<QByteArray, QSharedPointer<UnityMenuModel>> m_registry;
325 static QPointer<UnityMenuModelCache> theCache;
326 };
327
328
329=== modified file 'qml/Shell.qml'
330--- qml/Shell.qml 2015-02-05 10:29:34 +0000
331+++ qml/Shell.qml 2015-02-10 16:08:40 +0000
332@@ -750,7 +750,9 @@
333 expandedPanelHeight: units.gu(7)
334
335 indicatorsModel: Indicators.IndicatorsModel {
336- Component.onCompleted: load(indicatorProfile);
337+ // TODO: This should be sourced by device type (e.g. "desktop", "tablet", "phone"...)
338+ profile: indicatorProfile
339+ Component.onCompleted: load()
340 }
341 }
342 callHint {
343
344=== modified file 'tests/mocks/Unity/Indicators/fakeindicatorsmodel.cpp'
345--- tests/mocks/Unity/Indicators/fakeindicatorsmodel.cpp 2014-10-07 10:28:59 +0000
346+++ tests/mocks/Unity/Indicators/fakeindicatorsmodel.cpp 2015-02-10 16:08:40 +0000
347@@ -21,7 +21,8 @@
348 #include "indicators.h"
349
350 FakeIndicatorsModel::FakeIndicatorsModel(QObject *parent)
351- : QAbstractListModel(parent)
352+ : QAbstractListModel(parent),
353+ m_profile("phone")
354 {
355 QObject::connect(this, SIGNAL(rowsInserted(const QModelIndex &, int, int)), this, SIGNAL(countChanged()));
356 QObject::connect(this, SIGNAL(rowsRemoved(const QModelIndex &, int, int)), this, SIGNAL(countChanged()));
357@@ -38,6 +39,17 @@
358 return rowCount();
359 }
360
361+QString FakeIndicatorsModel::profile() const
362+{
363+ return m_profile;
364+}
365+
366+void FakeIndicatorsModel::setProfile(const QString& profile)
367+{
368+ m_profile = profile;
369+ Q_EMIT profileChanged();
370+}
371+
372 void FakeIndicatorsModel::load(const QString&)
373 {
374 }
375
376=== modified file 'tests/mocks/Unity/Indicators/fakeindicatorsmodel.h'
377--- tests/mocks/Unity/Indicators/fakeindicatorsmodel.h 2014-10-07 10:28:59 +0000
378+++ tests/mocks/Unity/Indicators/fakeindicatorsmodel.h 2015-02-10 16:08:40 +0000
379@@ -27,7 +27,9 @@
380 Q_OBJECT
381 Q_ENUMS(Roles)
382 Q_PROPERTY(int count READ count NOTIFY countChanged)
383+ Q_PROPERTY(QString profile READ profile WRITE setProfile NOTIFY profileChanged)
384 Q_PROPERTY(QVariant modelData READ modelData WRITE setModelData NOTIFY modelDataChanged)
385+
386 public:
387
388 FakeIndicatorsModel(QObject *parent=0);
389@@ -42,6 +44,8 @@
390
391 Q_INVOKABLE QVariant data(int row, int role) const;
392
393+ QString profile() const;
394+ void setProfile(const QString& profile);
395 void setModelData(const QVariant& data);
396 QVariant modelData() const { return m_modelData; }
397
398@@ -54,11 +58,13 @@
399
400 Q_SIGNALS:
401 void countChanged();
402+ void profileChanged();
403 void modelDataChanged();
404
405 private:
406 int count() const;
407
408+ QString m_profile;
409 QVariant m_modelData;
410 };
411
412
413=== modified file 'tests/plugins/Unity/Indicators/indicatorsmanagertest.cpp'
414--- tests/plugins/Unity/Indicators/indicatorsmanagertest.cpp 2014-01-30 14:54:01 +0000
415+++ tests/plugins/Unity/Indicators/indicatorsmanagertest.cpp 2015-02-10 16:08:40 +0000
416@@ -50,7 +50,8 @@
417 QVERIFY(!manager.isLoaded());
418 QCOMPARE(manager.indicators().count(), 0);
419
420- manager.load("test1");
421+ manager.setProfile("test1");
422+ manager.load();
423
424 QVERIFY(manager.isLoaded());
425 QCOMPARE(manager.indicators().count(), 4);
426@@ -67,7 +68,8 @@
427 void testPluginInterfaceProfile1()
428 {
429 IndicatorsManager manager;
430- manager.load("test1");
431+ manager.setProfile("test1");
432+ manager.load();
433
434 Indicator::Ptr indicator = manager.indicator("indicator-fake1");
435 QVERIFY(indicator ? true : false);
436@@ -90,7 +92,8 @@
437 void testPluginInterfaceProfile2()
438 {
439 IndicatorsManager manager;
440- manager.load("test2");
441+ manager.setProfile("test2");
442+ manager.load();
443
444 Indicator::Ptr indicator = manager.indicator("indicator-fake1");
445 QVERIFY(indicator ? true : false);
446@@ -108,12 +111,33 @@
447 }
448
449 /*
450+ * Test switching the indicator profile data
451+ */
452+ void testPluginInterfaceProfileSwitch()
453+ {
454+ IndicatorsManager manager;
455+ manager.setProfile("test1");
456+ manager.load();
457+
458+ Indicator::Ptr indicator = manager.indicator("indicator-fake1");
459+ QVERIFY(indicator ? true : false);
460+
461+ QVariantMap props = indicator->indicatorProperties().toMap();
462+ QCOMPARE(props["menuObjectPath"].toString(), QString("/com/canonical/indicator/fake1/test1"));
463+
464+ manager.setProfile("test2");
465+ props = indicator->indicatorProperties().toMap();
466+ QCOMPARE(props["menuObjectPath"].toString(), QString("/com/canonical/indicator/fake1/test2"));
467+ }
468+
469+ /*
470 * Test if a new plugin object is create for each different plugin
471 */
472 void testPluginInstance()
473 {
474 IndicatorsManager manager;
475- manager.load("test2");
476+ manager.setProfile("test2");
477+ manager.load();
478
479 Indicator::Ptr i0 = manager.indicator("indicator-fake1");
480 Indicator::Ptr i1 = manager.indicator("indicator-fake1");
481@@ -133,7 +157,8 @@
482 void testPluginInitAndShutdown()
483 {
484 IndicatorsManager manager;
485- manager.load("test1");
486+ manager.setProfile("test1");
487+ manager.load();
488
489 QWeakPointer<Indicator> wp0;
490 QWeakPointer<Indicator> wp1;
491
492=== modified file 'tests/plugins/Unity/Indicators/indicatorsmodeltest.cpp'
493--- tests/plugins/Unity/Indicators/indicatorsmodeltest.cpp 2014-01-30 14:54:01 +0000
494+++ tests/plugins/Unity/Indicators/indicatorsmodeltest.cpp 2015-02-10 16:08:40 +0000
495@@ -47,10 +47,11 @@
496 void testLoad()
497 {
498 IndicatorsModel model;
499+ model.setProfile("test1");
500
501 QCOMPARE(model.property("count").toInt(), 0);
502
503- model.load("test1");
504+ model.load();
505
506 QCOMPARE(model.property("count").toInt(), 4);
507
508@@ -67,7 +68,8 @@
509 {
510 // Priority order. (2, 1, 4, 3)
511 IndicatorsModel model;
512- model.load("test1");
513+ model.setProfile("test1");
514+ model.load();
515
516 // should be in order:
517 // fake3, fake4, fake1, fake2
518@@ -104,7 +106,8 @@
519 {
520 // Priority order. (2, 1, 4, 3)
521 IndicatorsModel model;
522- model.load("test2");
523+ model.setProfile("test2");
524+ model.load();
525
526 // should be in order:
527 // fake3, fake4, fake1, fake2
528
529=== modified file 'tests/plugins/Unity/Indicators/sharedunitymenumodeltest.cpp'
530--- tests/plugins/Unity/Indicators/sharedunitymenumodeltest.cpp 2014-10-14 14:18:55 +0000
531+++ tests/plugins/Unity/Indicators/sharedunitymenumodeltest.cpp 2015-02-10 16:08:40 +0000
532@@ -61,7 +61,7 @@
533 QCOMPARE(model1->model(), model2->model());
534 }
535
536- void testSharedOwnership()
537+ void testSavedData()
538 {
539 QSharedPointer<SharedUnityMenuModel> model1(createFullModel("test1"));
540 QSharedPointer<SharedUnityMenuModel> model2(createFullModel("test1"));
541@@ -70,7 +70,7 @@
542 model1.clear();
543 QCOMPARE(UnityMenuModelCache::singleton()->contains("/com/canonical/test1"), true);
544 model2.clear();
545- QCOMPARE(UnityMenuModelCache::singleton()->contains("/com/canonical/test1"), false);
546+ QCOMPARE(UnityMenuModelCache::singleton()->contains("/com/canonical/test1"), true);
547 }
548
549 // Tests that changing cached model data does not change the model path of others
550
551=== modified file 'tests/qmltests/tst_Shell.qml'
552--- tests/qmltests/tst_Shell.qml 2015-01-20 11:50:19 +0000
553+++ tests/qmltests/tst_Shell.qml 2015-02-10 16:08:40 +0000
554@@ -25,6 +25,7 @@
555 import Ubuntu.Telephony 0.1 as Telephony
556 import Unity.Application 0.1
557 import Unity.Connectivity 0.1
558+import Unity.Indicators 0.1
559 import Unity.Notifications 1.0
560 import Unity.Test 0.1 as UT
561 import Powerd 0.1
562@@ -169,6 +170,7 @@
563
564 function init() {
565 tryCompare(shell, "enabled", true); // enabled by greeter when ready
566+ shell.indicatorProfile = "phone";
567
568 swipeAwayGreeter();
569
570@@ -546,6 +548,33 @@
571 && itemRectInShell.y + itemRectInShell.height <= shell.height;
572 }
573
574+ function test_greeterDoesNotChangeIndicatorProfile() {
575+ var panel = findChild(shell, "panel");
576+ tryCompare(panel.indicators.indicatorsModel, "profile", shell.indicatorProfile);
577+
578+ LightDM.Greeter.showGreeter();
579+ tryCompare(panel.indicators.indicatorsModel, "profile", shell.indicatorProfile);
580+
581+ LightDM.Greeter.hideGreeter();
582+ tryCompare(panel.indicators.indicatorsModel, "profile", shell.indicatorProfile);
583+ }
584+
585+ function test_shellProfileChangesReachIndicators() {
586+ var panel = findChild(shell, "panel");
587+
588+ shell.indicatorProfile = "test1";
589+ for (var i = 0; i < panel.indicators.indicatorsModel.count; ++i) {
590+ var properties = panel.indicators.indicatorsModel.data(i, IndicatorsModelRole.IndicatorProperties);
591+ verify(properties["menuObjectPath"].substr(-5), "test1");
592+ }
593+
594+ shell.indicatorProfile = "test2";
595+ for (var i = 0; i < panel.indicators.indicatorsModel.count; ++i) {
596+ var properties = panel.indicators.indicatorsModel.data(i, IndicatorsModelRole.IndicatorProperties);
597+ verify(properties["menuObjectPath"].substr(-5), "test2");
598+ }
599+ }
600+
601 function test_focusRequestedHidesGreeter() {
602 var greeter = findChild(shell, "greeter");
603
604
605=== modified file 'tests/qmltests/tst_ShellWithPin.qml'
606--- tests/qmltests/tst_ShellWithPin.qml 2015-02-05 10:29:34 +0000
607+++ tests/qmltests/tst_ShellWithPin.qml 2015-02-10 16:08:40 +0000
608@@ -232,6 +232,22 @@
609 tryCompare(ApplicationManager, "focusedApplicationId", app)
610 }
611
612+ function test_greeterChangesIndicatorProfile() {
613+ skip("Not supported yet, waiting on design for new settings panel");
614+
615+ var panel = findChild(shell, "panel");
616+ tryCompare(panel.indicators.indicatorsModel, "profile", shell.indicatorProfile + "_greeter");
617+
618+ LightDM.Greeter.hideGreeter();
619+ tryCompare(panel.indicators.indicatorsModel, "profile", shell.indicatorProfile);
620+
621+ LightDM.Greeter.showGreeter();
622+ tryCompare(panel.indicators.indicatorsModel, "profile", shell.indicatorProfile + "_greeter");
623+
624+ LightDM.Greeter.hideGreeter();
625+ tryCompare(panel.indicators.indicatorsModel, "profile", shell.indicatorProfile);
626+ }
627+
628 function test_login() {
629 sessionSpy.clear()
630 tryCompare(sessionSpy, "count", 0)

Subscribers

People subscribed via source and target branches