unity-scopes-shell

Merge lp:~stolowski/unity-scopes-shell/fix-temp-scopes into lp:unity-scopes-shell

  1. fix-temp-scopes
  2. Merge into trunk
Proposed by Andrea Cimitan
Status: Merged
Approved by: Albert Astals Cid
Approved revision: 196
Merged at revision: 196
Proposed branch: lp:~stolowski/unity-scopes-shell/fix-temp-scopes
Merge into: lp:unity-scopes-shell
Diff against target: 188 lines (+39/-20)
7 files modified
debian/control (+2/-1)
src/Unity/CMakeLists.txt (+1/-1)
src/Unity/scope.cpp (+6/-9)
src/Unity/scope.h (+0/-2)
src/Unity/scopes.cpp (+23/-6)
src/Unity/scopes.h (+6/-0)
tests/CMakeLists.txt (+1/-1)
To merge this branch: bzr merge lp:~stolowski/unity-scopes-shell/fix-temp-scopes
Reviewer Review Type Date Requested Status
Albert Astals Cid (community) Approve
Michał Sawicz Needs Fixing
PS Jenkins bot (community) continuous-integration Needs Fixing
Review via email: mp+248235@code.launchpad.net

Commit message

Keep temporary scopes list in the Scopes object instances and make Scopes the partent of temp scopes instances. Temp scopes can be closed (and freed) using closeScope() method of the original scope, or by calling closeScope() method of Scopes instance.

Description of the change

Keep temporary scopes list in the Scopes object instances and make Scopes the partent of temp scopes instances. Temp scopes can be closed (and freed) using closeScope() method of the original scope, or by calling closeScope() method of Scopes instance.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:190
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~stolowski/unity-scopes-shell/fix-temp-scopes/+merge/248235/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/unity-scopes-shell-ci/292/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-shell-vivid-amd64-ci/34
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-shell-vivid-armhf-ci/34
        deb: http://jenkins.qa.ubuntu.com/job/unity-scopes-shell-vivid-armhf-ci/34/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-shell-vivid-i386-ci/34

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scopes-shell-ci/292/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Albert Astals Cid (aacid) wrote :

Shouldn't
  Q_INVOKABLE void closeScope(unity::shell::scopes::ScopeInterface* scope);
go to unity::shell::scopes::ScopesInterface ?

review: Needs Information
Revision history for this message
Paweł Stołowski (stolowski) wrote :

> Shouldn't
> Q_INVOKABLE void closeScope(unity::shell::scopes::ScopeInterface* scope);
> go to unity::shell::scopes::ScopesInterface ?
Ok, added, see https://code.launchpad.net/~stolowski/unity-api/scopes-close-scope/+merge/248725

Revision history for this message
Albert Astals Cid (aacid) wrote :

 + libunity-api-dev (>= 7.95),
 libunity-api-dev (>= 7.94),

Should be a replace not an addition?

review: Needs Fixing
Revision history for this message
Albert Astals Cid (aacid) wrote :

Looks good :)

review: Approve
Revision history for this message
Michał Sawicz (saviq) wrote :

Please bump Provides:, too, it should be unity-shell-scopes-6 now.

review: Needs Fixing
Revision history for this message
Albert Astals Cid (aacid) wrote :

Can you please turn

if (!m_tempScopes.contains(scope)) {
   m_tempScopes.insert(scope);
 }

into just
   m_tempScopes.insert(scope);

It's a QSet after all, inserting won't produce a duplicate anyway

review: Needs Fixing
lp:~stolowski/unity-scopes-shell/fix-temp-scopes updated
196. By Paweł Stołowski

No need to check if scope exists in the set.

Revision history for this message
Albert Astals Cid (aacid) :
review: Approve
lp:~stolowski/unity-scopes-shell/fix-temp-scopes updated
197. By Paweł Stołowski

Bumped required version of unity api.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/control'
2--- debian/control 2015-02-03 08:47:02 +0000
3+++ debian/control 2015-02-12 17:19:47 +0000
4@@ -3,7 +3,7 @@
5 Section: libs
6 Build-Depends: cmake,
7 debhelper (>= 9),
8- libunity-api-dev (>= 7.94),
9+ libunity-api-dev (>= 7.96),
10 libunity-scopes-dev (>= 0.6.12~),
11 libgsettings-qt-dev (>= 0.1),
12 libqtdbustest1-dev (>= 0.2),
13@@ -38,6 +38,7 @@
14 unity-scopes-impl-0,
15 unity-scopes-impl-1,
16 unity-scopes-impl-4,
17+ unity-scopes-impl-6,
18 Breaks: unity8-private (<< 7.84),
19 unity8 (<< 8.02)
20 Replaces: unity8-private (<< 7.84)
21
22=== modified file 'src/Unity/CMakeLists.txt'
23--- src/Unity/CMakeLists.txt 2015-02-04 15:50:47 +0000
24+++ src/Unity/CMakeLists.txt 2015-02-12 17:19:47 +0000
25@@ -2,7 +2,7 @@
26 include(Plugins)
27
28 # Dependencies
29-pkg_check_modules(SCOPES_API REQUIRED unity-shell-scopes=5)
30+pkg_check_modules(SCOPES_API REQUIRED unity-shell-scopes=6)
31 pkg_check_modules(SCOPESLIB REQUIRED libunity-scopes>=0.6.12)
32 pkg_check_modules(GSETTINGSQT REQUIRED gsettings-qt)
33 pkg_check_modules(UBUNTU_LOCATION_SERVICE REQUIRED ubuntu-location-service)
34
35=== modified file 'src/Unity/scope.cpp'
36--- src/Unity/scope.cpp 2015-02-04 04:51:44 +0000
37+++ src/Unity/scope.cpp 2015-02-12 17:19:47 +0000
38@@ -279,13 +279,13 @@
39 // create temp dash page
40 auto meta_sptr = m_scopesInstance->getCachedMetadata(scopeId);
41 if (meta_sptr) {
42- scope = new scopes_ng::Scope(this);
43+ scope = new scopes_ng::Scope(m_scopesInstance);
44 scope->setScopeData(*meta_sptr);
45 scope->setScopesInstance(m_scopesInstance);
46 scope->setCurrentNavigationId(departmentId);
47 scope->setFilterState(query.filter_state());
48 scope->setSearchQuery(searchString);
49- m_tempScopes.insert(scope);
50+ m_scopesInstance->addTempScope(scope);
51 Q_EMIT openScope(scope);
52 } else if (allowDelayedActivation) {
53 // request registry refresh to get the missing metadata
54@@ -429,13 +429,10 @@
55 }
56 }
57
58-
59 unity::shell::scopes::ScopeInterface* Scope::findTempScope(QString const& id) const
60 {
61- for (auto s: m_tempScopes) {
62- if (s->id() == id) {
63- return s;
64- }
65+ if (m_scopesInstance) {
66+ return m_scopesInstance->findTempScope(id);
67 }
68 return nullptr;
69 }
70@@ -1234,8 +1231,8 @@
71
72 void Scope::closeScope(unity::shell::scopes::ScopeInterface* scope)
73 {
74- if (m_tempScopes.remove(scope)) {
75- delete scope;
76+ if (m_scopesInstance) {
77+ m_scopesInstance->closeScope(scope);
78 }
79 }
80
81
82=== modified file 'src/Unity/scope.h'
83--- src/Unity/scope.h 2015-01-26 06:01:36 +0000
84+++ src/Unity/scope.h 2015-02-12 17:19:47 +0000
85@@ -29,7 +29,6 @@
86 #include <QNetworkConfigurationManager>
87 #include <QPointer>
88 #include <QMultiMap>
89-#include <QSet>
90 #include <QGSettings>
91 #include <QUuid>
92
93@@ -246,7 +245,6 @@
94 QTimer m_clearTimer;
95 QTimer m_invalidateTimer;
96 QList<std::shared_ptr<unity::scopes::CategorisedResult>> m_cachedResults;
97- QSet<unity::shell::scopes::ScopeInterface*> m_tempScopes;
98 QMultiMap<QString, Department*> m_departmentModels;
99 QMultiMap<QString, Department*> m_altNavModels;
100 QMap<Department*, QString> m_inverseDepartments;
101
102=== modified file 'src/Unity/scopes.cpp'
103--- src/Unity/scopes.cpp 2014-12-12 12:32:14 +0000
104+++ src/Unity/scopes.cpp 2015-02-12 17:19:47 +0000
105@@ -547,12 +547,7 @@
106 Scope* scope = getScopeById(scopeName);
107 if (scope == nullptr) {
108 // check temporary scopes
109- for (auto s: m_scopes) {
110- scope = qobject_cast<Scope*>(s->findTempScope(scopeName));
111- if (scope) {
112- break;
113- }
114- }
115+ scope = qobject_cast<Scope*>(findTempScope(scopeName));
116 }
117
118 if (scope) {
119@@ -652,6 +647,28 @@
120 }
121 }
122
123+void Scopes::addTempScope(unity::shell::scopes::ScopeInterface* scope)
124+{
125+ m_tempScopes.insert(scope);
126+}
127+
128+void Scopes::closeScope(unity::shell::scopes::ScopeInterface* scope)
129+{
130+ if (m_tempScopes.remove(scope)) {
131+ scope->deleteLater();
132+ }
133+}
134+
135+unity::shell::scopes::ScopeInterface* Scopes::findTempScope(QString const& id) const
136+{
137+ for (auto s: m_tempScopes) {
138+ if (s->id() == id) {
139+ return s;
140+ }
141+ }
142+ return nullptr;
143+}
144+
145 void Scopes::moveFavoriteTo(QString const& scopeId, int index)
146 {
147 if (m_dashSettings)
148
149=== modified file 'src/Unity/scopes.h'
150--- src/Unity/scopes.h 2014-12-02 12:19:29 +0000
151+++ src/Unity/scopes.h 2015-02-12 17:19:47 +0000
152@@ -30,6 +30,7 @@
153 #include <QTimer>
154 #include <QStringList>
155 #include <QSharedPointer>
156+#include <QSet>
157 #include <QGSettings>
158
159 #include <unity/scopes/Runtime.h>
160@@ -73,6 +74,10 @@
161 LocationService::Ptr locationService() const;
162 QString userAgentString() const;
163
164+ unity::shell::scopes::ScopeInterface* findTempScope(QString const& id) const;
165+ void addTempScope(unity::shell::scopes::ScopeInterface* scope);
166+ Q_INVOKABLE void closeScope(unity::shell::scopes::ScopeInterface* scope) override;
167+
168 Q_SIGNALS:
169 void metadataRefreshed();
170
171@@ -115,6 +120,7 @@
172 QTimer m_startupQueryTimeout;
173
174 unity::scopes::Runtime::SPtr m_scopesRuntime;
175+ QSet<unity::shell::scopes::ScopeInterface*> m_tempScopes;
176
177 std::unique_ptr<Priv> m_priv;
178 };
179
180=== modified file 'tests/CMakeLists.txt'
181--- tests/CMakeLists.txt 2014-11-28 08:52:12 +0000
182+++ tests/CMakeLists.txt 2015-02-12 17:19:47 +0000
183@@ -1,4 +1,4 @@
184-pkg_check_modules(SCOPES_API REQUIRED unity-shell-scopes=4)
185+pkg_check_modules(SCOPES_API REQUIRED unity-shell-scopes=6)
186 pkg_check_modules(SCOPESLIB REQUIRED libunity-scopes>=0.6.2)
187 pkg_check_modules(GSETTINGSQT REQUIRED gsettings-qt)
188 pkg_check_modules(QTDBUSTEST REQUIRED libqtdbustest-1>=0.2 REQUIRED)

Subscribers

People subscribed via source and target branches

to all changes: