Merge lp:~gerboland/qtmir/fix-prompt-session-crash into lp:qtmir
- fix-prompt-session-crash
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Daniel d'Andrada |
Approved revision: | 621 |
Merged at revision: | 621 |
Proposed branch: | lp:~gerboland/qtmir/fix-prompt-session-crash |
Merge into: | lp:qtmir |
Prerequisite: | lp:~alan-griffiths/qtmir/tidy-mirserver-deps |
Diff against target: |
969 lines (+675/-84) 13 files modified
src/modules/Unity/Application/application_manager.h (+4/-2) src/modules/Unity/Application/mirsurface.cpp (+0/-14) src/modules/Unity/Application/mirsurface.h (+0/-1) src/modules/Unity/Application/sessionmap_interface.h (+38/-0) src/modules/Unity/Application/surfacemanager.cpp (+29/-4) src/modules/Unity/Application/surfacemanager.h (+9/-2) tests/framework/CMakeLists.txt (+3/-0) tests/framework/mock_sessionmap.h (+32/-0) tests/framework/mock_window_controller.h (+43/-0) tests/modules/CMakeLists.txt (+2/-1) tests/modules/SurfaceManager/CMakeLists.txt (+40/-0) tests/modules/SurfaceManager/surface_manager_test.cpp (+475/-0) tests/modules/WindowManager/mirsurface_test.cpp (+0/-60) |
To merge this branch: | bzr merge lp:~gerboland/qtmir/fix-prompt-session-crash |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Unity8 CI Bot (community) | continuous-integration | Approve | |
Daniel d'Andrada (community) | Approve | ||
Albert Astals Cid (community) | Abstain | ||
Review via email: mp+318759@code.launchpad.net |
Commit message
Stop MirSurface deleting itself, ensure SurfaceManager alone manages MirSurface lifetimes. Add SurfaceManager test suite.
This fixes bugs where a MirSurface would call deleteLater on itself, but SurfaceManager would have no idea and keep a pointer to that MirSurface in its internal list.
Instead SurfaceManager listens for signals from both miral and the MirSurface itself to decide when to delete it.
Add a test suite to verify MirSurface lifetimes.
Description of the change
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:619
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Daniel d'Andrada (dandrader) wrote : | # |
$ bzr branch lp:qtmir foo && cd foo
$ bzr merge lp:~alan-griffiths/qtmir/tidy-mirserver-deps && bzr commit -m"tidy-
$ bzr merge lp:~gerboland/qtmir/fix-prompt-session-crash
Text conflict in src/modules/
Probably because your prerequisite branch has changed in the meantime
Daniel d'Andrada (dandrader) wrote : | # |
In src/modules/
"""
@@ -710,14 +706,11 @@
{
m_
DEBUG_MSG << "(" << viewId << ")" << " after=" << m_views.count() << " live=" << m_live;
+ updateExposure();
+ setViewActiveFo
if (m_views.count() == 0) {
Q_EMIT isBeingDisplaye
- if (m_session.isNull() || !m_live) {
- deleteLater();
- }
}
- updateExposure();
- setViewActiveFo
}
void MirSurface:
"""
Why did you move updateExposure() and setViewActiveFo
Daniel d'Andrada (dandrader) wrote : | # |
You should rebase on top of https:/
Daniel d'Andrada (dandrader) wrote : | # |
But I like the core idea of this MP.
Gerry Boland (gerboland) wrote : | # |
> In src/modules/
>
> """
> @@ -710,14 +706,11 @@
> {
> m_views.
> DEBUG_MSG << "(" << viewId << ")" << " after=" << m_views.count() << "
> live=" << m_live;
> + updateExposure();
> + setViewActiveFo
> if (m_views.count() == 0) {
> Q_EMIT isBeingDisplaye
> - if (m_session.isNull() || !m_live) {
> - deleteLater();
> - }
> }
> - updateExposure();
> - setViewActiveFo
> }
>
> void MirSurface:
> """
>
> Why did you move updateExposure() and setViewActiveFo
Exactly something I was a little worried about. The slot connected to isBeingDisplaye
I suspect a deleteLater is safer. But would you have any better idea?
Daniel d'Andrada (dandrader) wrote : | # |
On 03/03/2017 08:40, Gerry Boland wrote:
>> In src/modules/
>>
>> """
>> @@ -710,14 +706,11 @@
>> {
>> m_views.
>> DEBUG_MSG << "(" << viewId << ")" << " after=" << m_views.count() << "
>> live=" << m_live;
>> + updateExposure();
>> + setViewActiveFo
>> if (m_views.count() == 0) {
>> Q_EMIT isBeingDisplaye
>> - if (m_session.isNull() || !m_live) {
>> - deleteLater();
>> - }
>> }
>> - updateExposure();
>> - setViewActiveFo
>> }
>>
>> void MirSurface:
>> """
>>
>> Why did you move updateExposure() and setViewActiveFo
> Exactly something I was a little worried about. The slot connected to isBeingDisplaye
>
> I suspect a deleteLater is safer. But would you have any better idea?
Ah, ok.
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:620
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:623
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Daniel d'Andrada (dandrader) wrote : | # |
* 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.
Yes
Albert Astals Cid (aacid) wrote : | # |
Text conflict in src/modules/
Contents conflict in src/modules/
Contents conflict in src/modules/
Text conflict in src/modules/
Text conflict in tests/modules/
5 conflicts encountered.
Was already top approved.
- 616. By Alan Griffiths
-
merge :parent
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:624
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:625
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:625
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Daniel d'Andrada (dandrader) wrote : | # |
There's still a tests/framework
Albert Astals Cid (aacid) : | # |
- 617. By Alan Griffiths
-
Move MirBuffer to miral namespace
- 618. By Alan Griffiths
-
Better naming
- 619. By Alan Griffiths
-
A move towards MirAL style
Gerry Boland (gerboland) wrote : | # |
MockSessionManager removed, good catch
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:626
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
Daniel d'Andrada (dandrader) wrote : | # |
In the test code:
"""
struct TestableSurface
{
// just exports the test-only protected constructor
TestableSur
: SurfaceManager(
MirSurface* find(const miral::WindowInfo &needle) const
{
return SurfaceManager:
}
};
"""
Why don't you just make this constructor and method public in surface manager?
- 620. By Gerry Boland
-
Stop MirSurface deleting itself, ensure SurfaceManager alone manages MirSurface lifetimes. Add SurfaceManager test suite.
This fixes bugs where a MirSurface would call deleteLater on itself, but SurfaceManager would have no idea and keep a pointer to that MirSurface in its internal list.
Instead SurfaceManager listens for signals from both miral and the MirSurface itself to decide when to delete it.
Add a test suite to verify MirSurface lifetimes.
- 621. By Gerry Boland
-
Actually include SurfaceManager tests. SM relies on AppMan singleton, which I had to abstract out to a SessionMap in order to make it testable.
Daniel d'Andrada (dandrader) wrote : | # |
* 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.
Passed locally
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:621
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:621
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Preview Diff
1 | === modified file 'src/modules/Unity/Application/application_manager.h' |
2 | --- src/modules/Unity/Application/application_manager.h 2017-02-21 18:46:30 +0000 |
3 | +++ src/modules/Unity/Application/application_manager.h 2017-03-20 15:56:36 +0000 |
4 | @@ -26,6 +26,7 @@ |
5 | |
6 | // local |
7 | #include "application.h" |
8 | +#include "sessionmap_interface.h" |
9 | #include "taskcontroller.h" |
10 | |
11 | // Unity API |
12 | @@ -55,7 +56,8 @@ |
13 | class SharedWakelock; |
14 | class SettingsInterface; |
15 | |
16 | -class ApplicationManager : public unity::shell::application::ApplicationManagerInterface |
17 | +class ApplicationManager : public unity::shell::application::ApplicationManagerInterface, |
18 | + public SessionMapInterface |
19 | { |
20 | Q_OBJECT |
21 | |
22 | @@ -94,7 +96,7 @@ |
23 | const QList<Application*> &list() const { return m_applications; } |
24 | qtmir::Application* findApplicationWithPid(const pid_t pid) const; |
25 | |
26 | - SessionInterface *findSession(const mir::scene::Session* session) const; |
27 | + SessionInterface *findSession(const mir::scene::Session* session) const override; |
28 | |
29 | public Q_SLOTS: |
30 | void authorizeSession(const pid_t pid, bool &authorized); |
31 | |
32 | === modified file 'src/modules/Unity/Application/mirsurface.cpp' |
33 | --- src/modules/Unity/Application/mirsurface.cpp 2017-03-07 23:42:05 +0000 |
34 | +++ src/modules/Unity/Application/mirsurface.cpp 2017-03-20 15:56:36 +0000 |
35 | @@ -168,7 +168,6 @@ |
36 | connect(m_surfaceObserver.get(), &SurfaceObserver::confinesMousePointerChanged, this, &MirSurface::confinesMousePointerChanged); |
37 | m_surfaceObserver->setListener(this); |
38 | |
39 | - //connect(session, &QObject::destroyed, this, &MirSurface::onSessionDestroyed); // TODO try using Shared pointer for lifecycle |
40 | connect(session, &SessionInterface::stateChanged, this, [this]() { |
41 | if (clientIsRunning() && m_pendingResize.isValid()) { |
42 | resize(m_pendingResize.width(), m_pendingResize.height()); |
43 | @@ -584,9 +583,6 @@ |
44 | INFO_MSG << "(" << value << ")"; |
45 | m_live = value; |
46 | Q_EMIT liveChanged(value); |
47 | - if (m_views.isEmpty() && !m_live) { |
48 | - deleteLater(); |
49 | - } |
50 | } |
51 | } |
52 | |
53 | @@ -711,9 +707,6 @@ |
54 | INFO_MSG << "(" << viewId << ")" << " after=" << m_views.count() << " live=" << m_live; |
55 | if (m_views.count() == 0) { |
56 | Q_EMIT isBeingDisplayedChanged(); |
57 | - if (m_session.isNull() || !m_live) { |
58 | - deleteLater(); |
59 | - } |
60 | } |
61 | updateExposure(); |
62 | setViewActiveFocus(viewId, false); |
63 | @@ -758,13 +751,6 @@ |
64 | return m_currentFrameNumber; |
65 | } |
66 | |
67 | -void MirSurface::onSessionDestroyed() |
68 | -{ |
69 | - if (m_views.isEmpty()) { |
70 | - deleteLater(); |
71 | - } |
72 | -} |
73 | - |
74 | void MirSurface::emitSizeChanged() |
75 | { |
76 | Q_EMIT sizeChanged(m_size); |
77 | |
78 | === modified file 'src/modules/Unity/Application/mirsurface.h' |
79 | --- src/modules/Unity/Application/mirsurface.h 2017-02-02 09:17:48 +0000 |
80 | +++ src/modules/Unity/Application/mirsurface.h 2017-03-20 15:56:36 +0000 |
81 | @@ -188,7 +188,6 @@ |
82 | void dropPendingBuffer(); |
83 | void onAttributeChanged(const MirWindowAttrib, const int); |
84 | void onFramesPostedObserved(); |
85 | - void onSessionDestroyed(); |
86 | void emitSizeChanged(); |
87 | void setCursor(const QCursor &cursor); |
88 | void onCloseTimedOut(); |
89 | |
90 | === added file 'src/modules/Unity/Application/sessionmap_interface.h' |
91 | --- src/modules/Unity/Application/sessionmap_interface.h 1970-01-01 00:00:00 +0000 |
92 | +++ src/modules/Unity/Application/sessionmap_interface.h 2017-03-20 15:56:36 +0000 |
93 | @@ -0,0 +1,38 @@ |
94 | +/* |
95 | + * Copyright (C) 2017 Canonical, Ltd. |
96 | + * |
97 | + * This program is free software: you can redistribute it and/or modify it under |
98 | + * the terms of the GNU Lesser General Public License version 3, as published by |
99 | + * the Free Software Foundation. |
100 | + * |
101 | + * This program is distributed in the hope that it will be useful, but WITHOUT |
102 | + * ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, |
103 | + * SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU |
104 | + * Lesser General Public License for more details. |
105 | + * |
106 | + * You should have received a copy of the GNU Lesser General Public License |
107 | + * along with this program. If not, see <http://www.gnu.org/licenses/>. |
108 | + */ |
109 | + |
110 | +#ifndef SESSIONMAP_INTERFACE_H |
111 | +#define SESSIONMAP_INTERFACE_H |
112 | + |
113 | +namespace mir { namespace scene { class Session; }} |
114 | + |
115 | +namespace qtmir { |
116 | + |
117 | +class SessionInterface; |
118 | + |
119 | + |
120 | +class SessionMapInterface |
121 | +{ |
122 | +public: |
123 | + SessionMapInterface() = default; |
124 | + virtual ~SessionMapInterface() = default; |
125 | + |
126 | + virtual SessionInterface *findSession(const mir::scene::Session* session) const = 0; |
127 | +}; |
128 | + |
129 | +} // namespace qtmir |
130 | + |
131 | +#endif // SESSIONMAP_INTERFACE_H |
132 | |
133 | === modified file 'src/modules/Unity/Application/surfacemanager.cpp' |
134 | --- src/modules/Unity/Application/surfacemanager.cpp 2017-03-20 15:56:36 +0000 |
135 | +++ src/modules/Unity/Application/surfacemanager.cpp 2017-03-20 15:56:36 +0000 |
136 | @@ -38,7 +38,8 @@ |
137 | using namespace qtmir; |
138 | namespace unityapi = unity::shell::application; |
139 | |
140 | -SurfaceManager::SurfaceManager(QObject *) |
141 | + |
142 | +SurfaceManager::SurfaceManager() |
143 | { |
144 | DEBUG_MSG << "()"; |
145 | |
146 | @@ -48,12 +49,23 @@ |
147 | qFatal("ERROR: Unity.Application QML plugin requires use of the 'mirserver' QPA plugin"); |
148 | } |
149 | |
150 | + m_sessionMap = ApplicationManager::singleton(); |
151 | m_windowController = static_cast<WindowControllerInterface*>(nativeInterface->nativeResourceForIntegration("WindowController")); |
152 | |
153 | auto windowModel = static_cast<WindowModelNotifier*>(nativeInterface->nativeResourceForIntegration("WindowModelNotifier")); |
154 | connectToWindowModelNotifier(windowModel); |
155 | } |
156 | |
157 | +SurfaceManager::SurfaceManager(WindowControllerInterface *windowController, |
158 | + WindowModelNotifier *windowModel, |
159 | + SessionMapInterface *sessionMap) |
160 | + : m_windowController(windowController) |
161 | + , m_sessionMap(sessionMap) |
162 | +{ |
163 | + DEBUG_MSG << "()"; |
164 | + connectToWindowModelNotifier(windowModel); |
165 | +} |
166 | + |
167 | void SurfaceManager::connectToWindowModelNotifier(WindowModelNotifier *notifier) |
168 | { |
169 | connect(notifier, &WindowModelNotifier::windowAdded, this, &SurfaceManager::onWindowAdded, Qt::QueuedConnection); |
170 | @@ -95,12 +107,21 @@ |
171 | } |
172 | |
173 | auto mirSession = windowInfo.window().application(); |
174 | - SessionInterface* session = ApplicationManager::singleton()->findSession(mirSession.get()); |
175 | + SessionInterface* session = m_sessionMap->findSession(mirSession.get()); |
176 | |
177 | const auto parentSurface = find(windowInfo.parent()); |
178 | const auto surface = new MirSurface(window, m_windowController, session, parentSurface); |
179 | rememberMirSurface(surface); |
180 | |
181 | + connect(surface, &MirSurface::isBeingDisplayedChanged, this, [this, surface]() { |
182 | + if ((!surface->live() || !surface->session()) |
183 | + && !surface->isBeingDisplayed()) { |
184 | + forgetMirSurface(static_cast<MirSurface*>(surface)->window()); |
185 | + surface->deleteLater(); // don't delete immediately, slot may be directly connected |
186 | + tracepoint(qtmir, surfaceDestroyed); |
187 | + } |
188 | + }); |
189 | + |
190 | if (parentSurface) { |
191 | static_cast<MirSurfaceListModel*>(parentSurface->childSurfaceList())->prependSurface(surface); |
192 | } |
193 | @@ -117,8 +138,12 @@ |
194 | DEBUG_MSG << "()"; |
195 | MirSurface *surface = find(windowInfo); |
196 | forgetMirSurface(windowInfo.window()); |
197 | - surface->setLive(false); |
198 | - tracepoint(qtmir, surfaceDestroyed); |
199 | + if (surface->isBeingDisplayed()) { |
200 | + surface->setLive(false); |
201 | + } else { |
202 | + delete surface; |
203 | + tracepoint(qtmir, surfaceDestroyed); |
204 | + } |
205 | } |
206 | |
207 | MirSurface *SurfaceManager::find(const miral::WindowInfo &needle) const |
208 | |
209 | === modified file 'src/modules/Unity/Application/surfacemanager.h' |
210 | --- src/modules/Unity/Application/surfacemanager.h 2017-03-20 15:56:36 +0000 |
211 | +++ src/modules/Unity/Application/surfacemanager.h 2017-03-20 15:56:36 +0000 |
212 | @@ -31,6 +31,7 @@ |
213 | namespace qtmir { |
214 | |
215 | class MirSurface; |
216 | +class SessionMapInterface; |
217 | class WindowControllerInterface; |
218 | |
219 | class SurfaceManager : public unity::shell::application::SurfaceManagerInterface |
220 | @@ -38,12 +39,18 @@ |
221 | Q_OBJECT |
222 | |
223 | public: |
224 | - explicit SurfaceManager(QObject *parent = 0); |
225 | + explicit SurfaceManager(); |
226 | + SurfaceManager(WindowControllerInterface *windowController, |
227 | + WindowModelNotifier *windowModel, |
228 | + SessionMapInterface *sessionMap); |
229 | virtual ~SurfaceManager() {} |
230 | |
231 | void raise(unity::shell::application::MirSurfaceInterface *surface) override; |
232 | void activate(unity::shell::application::MirSurfaceInterface *surface) override; |
233 | |
234 | + // mainly for test usage |
235 | + MirSurface* find(const miral::WindowInfo &needle) const; |
236 | + |
237 | private Q_SLOTS: |
238 | void onWindowAdded(const qtmir::NewWindow &windowInfo); |
239 | void onWindowRemoved(const miral::WindowInfo &windowInfo); |
240 | @@ -58,12 +65,12 @@ |
241 | void connectToWindowModelNotifier(WindowModelNotifier *notifier); |
242 | void rememberMirSurface(MirSurface *surface); |
243 | void forgetMirSurface(const miral::Window &window); |
244 | - MirSurface* find(const miral::WindowInfo &needle) const; |
245 | MirSurface* find(const miral::Window &needle) const; |
246 | |
247 | QVector<MirSurface*> m_allSurfaces; |
248 | |
249 | WindowControllerInterface *m_windowController; |
250 | + SessionMapInterface *m_sessionMap; |
251 | }; |
252 | |
253 | } // namespace qtmir |
254 | |
255 | === modified file 'tests/framework/CMakeLists.txt' |
256 | --- tests/framework/CMakeLists.txt 2016-12-02 16:22:45 +0000 |
257 | +++ tests/framework/CMakeLists.txt 2017-03-20 15:56:36 +0000 |
258 | @@ -28,6 +28,7 @@ |
259 | mock_prompt_session_manager.cpp |
260 | mock_renderable.cpp |
261 | mock_session.cpp |
262 | + mock_sessionmap.h |
263 | mock_settings.cpp |
264 | mock_shared_wakelock.cpp |
265 | mock_surface.cpp |
266 | @@ -46,6 +47,8 @@ |
267 | target_link_libraries( |
268 | qtmir-test-framework-static |
269 | |
270 | + unityapplicationplugin |
271 | + |
272 | ${GTEST_BOTH_LIBRARIES} |
273 | ${GMOCK_LIBRARIES} |
274 | ) |
275 | |
276 | === added file 'tests/framework/mock_sessionmap.h' |
277 | --- tests/framework/mock_sessionmap.h 1970-01-01 00:00:00 +0000 |
278 | +++ tests/framework/mock_sessionmap.h 2017-03-20 15:56:36 +0000 |
279 | @@ -0,0 +1,32 @@ |
280 | +/* |
281 | + * Copyright (C) 2017 Canonical, Ltd. |
282 | + * |
283 | + * This program is free software: you can redistribute it and/or modify it under |
284 | + * the terms of the GNU Lesser General Public License version 3, as published by |
285 | + * the Free Software Foundation. |
286 | + * |
287 | + * This program is distributed in the hope that it will be useful, but WITHOUT |
288 | + * ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, |
289 | + * SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU |
290 | + * Lesser General Public License for more details. |
291 | + * |
292 | + * You should have received a copy of the GNU Lesser General Public License |
293 | + * along with this program. If not, see <http://www.gnu.org/licenses/>. |
294 | + */ |
295 | + |
296 | +#ifndef MOCK_SESSIONMAP_H |
297 | +#define MOCK_SESSIONMAP_H |
298 | + |
299 | +#include <gmock/gmock.h> |
300 | + |
301 | +#include <Unity/Application/sessionmap_interface.h> |
302 | + |
303 | +struct MockSessionMap : public qtmir::SessionMapInterface |
304 | +{ |
305 | + MockSessionMap() {} |
306 | + virtual ~MockSessionMap() {} |
307 | + |
308 | + MOCK_CONST_METHOD1(findSession, SessionInterface*(const mir::scene::Session*)); |
309 | +}; |
310 | + |
311 | +#endif // MOCK_SESSIONMAP_H |
312 | |
313 | === added file 'tests/framework/mock_window_controller.h' |
314 | --- tests/framework/mock_window_controller.h 1970-01-01 00:00:00 +0000 |
315 | +++ tests/framework/mock_window_controller.h 2017-03-20 15:56:36 +0000 |
316 | @@ -0,0 +1,43 @@ |
317 | +/* |
318 | + * Copyright (C) 2017 Canonical, Ltd. |
319 | + * |
320 | + * This program is free software: you can redistribute it and/or modify it under |
321 | + * the terms of the GNU Lesser General Public License version 3, as published by |
322 | + * the Free Software Foundation. |
323 | + * |
324 | + * This program is distributed in the hope that it will be useful, but WITHOUT |
325 | + * ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, |
326 | + * SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU |
327 | + * Lesser General Public License for more details. |
328 | + * |
329 | + * You should have received a copy of the GNU Lesser General Public License |
330 | + * along with this program. If not, see <http://www.gnu.org/licenses/>. |
331 | + */ |
332 | + |
333 | +#ifndef MOCK_WINDOW_CONTROLLER_H |
334 | +#define MOCK_WINDOW_CONTROLLER_H |
335 | + |
336 | +#include "windowcontrollerinterface.h" |
337 | + |
338 | +#include <gmock/gmock.h> |
339 | + |
340 | +class MockWindowController : public qtmir::WindowControllerInterface |
341 | +{ |
342 | +public: |
343 | + MOCK_METHOD1(activate, void(const miral::Window &)); |
344 | + MOCK_METHOD1(raise, void(const miral::Window &)); |
345 | + |
346 | + MOCK_METHOD2(resize, void(const miral::Window &, const QSize &)); |
347 | + MOCK_METHOD2(move, void(const miral::Window &, const QPoint &)); |
348 | + |
349 | + MOCK_METHOD1(requestClose, void(const miral::Window &)); |
350 | + MOCK_METHOD1(forceClose, void(const miral::Window &)); |
351 | + |
352 | + MOCK_METHOD2(requestState, void(const miral::Window &, const Mir::State)); |
353 | + |
354 | + MOCK_METHOD2(deliverKeyboardEvent, void(const miral::Window &, const MirKeyboardEvent *)); |
355 | + MOCK_METHOD2(deliverTouchEvent, void(const miral::Window &, const MirTouchEvent *)); |
356 | + MOCK_METHOD2(deliverPointerEvent, void(const miral::Window &, const MirPointerEvent *)); |
357 | +}; |
358 | + |
359 | +#endif // MOCK_WINDOW_CONTROLLER_H |
360 | |
361 | === modified file 'tests/modules/CMakeLists.txt' |
362 | --- tests/modules/CMakeLists.txt 2017-02-21 18:46:30 +0000 |
363 | +++ tests/modules/CMakeLists.txt 2017-03-20 15:56:36 +0000 |
364 | @@ -1,6 +1,7 @@ |
365 | add_subdirectory(Application) |
366 | add_subdirectory(ApplicationManager) |
367 | add_subdirectory(General) |
368 | +add_subdirectory(Session) |
369 | add_subdirectory(SharedWakelock) |
370 | -add_subdirectory(Session) |
371 | +add_subdirectory(SurfaceManager) |
372 | add_subdirectory(WindowManager) |
373 | |
374 | === added directory 'tests/modules/SurfaceManager' |
375 | === added file 'tests/modules/SurfaceManager/CMakeLists.txt' |
376 | --- tests/modules/SurfaceManager/CMakeLists.txt 1970-01-01 00:00:00 +0000 |
377 | +++ tests/modules/SurfaceManager/CMakeLists.txt 2017-03-20 15:56:36 +0000 |
378 | @@ -0,0 +1,40 @@ |
379 | +set( |
380 | + SURFACE_MANAGER_TEST_SOURCES |
381 | + surface_manager_test.cpp |
382 | + ${CMAKE_SOURCE_DIR}/src/common/debughelpers.cpp |
383 | +) |
384 | + |
385 | +include_directories( |
386 | + ${CMAKE_SOURCE_DIR}/src/common |
387 | + ${CMAKE_SOURCE_DIR}/src/platforms/mirserver |
388 | + ${CMAKE_SOURCE_DIR}/src/modules |
389 | + ${CMAKE_SOURCE_DIR}/tests/framework |
390 | +) |
391 | + |
392 | +include_directories( |
393 | + SYSTEM |
394 | + ${APPLICATION_API_INCLUDE_DIRS} |
395 | + ${MIRAL_INCLUDE_DIRS} |
396 | + ${MIRTEST_INCLUDE_DIRS} |
397 | +) |
398 | + |
399 | +add_executable(surfacemanager_test ${SURFACE_MANAGER_TEST_SOURCES}) |
400 | + |
401 | +add_dependencies(surfacemanager_test qtmir-test-framework-static) |
402 | + |
403 | +target_link_libraries( |
404 | + surfacemanager_test |
405 | + |
406 | + unityapplicationplugin |
407 | + |
408 | + Qt5::Test |
409 | + |
410 | + -L${CMAKE_BINARY_DIR}/tests/framework |
411 | + qtmir-test-framework-static |
412 | + |
413 | + ${MIRTEST_LDFLAGS} |
414 | + ${GTEST_BOTH_LIBRARIES} |
415 | + ${GMOCK_LIBRARIES} |
416 | +) |
417 | + |
418 | +add_test(SurfaceManager, surfacemanager_test) |
419 | |
420 | === added file 'tests/modules/SurfaceManager/surface_manager_test.cpp' |
421 | --- tests/modules/SurfaceManager/surface_manager_test.cpp 1970-01-01 00:00:00 +0000 |
422 | +++ tests/modules/SurfaceManager/surface_manager_test.cpp 2017-03-20 15:56:36 +0000 |
423 | @@ -0,0 +1,475 @@ |
424 | +/* |
425 | + * Copyright (C) 2017 Canonical, Ltd. |
426 | + * |
427 | + * This program is free software: you can redistribute it and/or modify it under |
428 | + * the terms of the GNU Lesser General Public License version 3, as published by |
429 | + * the Free Software Foundation. |
430 | + * |
431 | + * This program is distributed in the hope that it will be useful, but WITHOUT |
432 | + * ANY WARRANTY; without even the implied warranties of MERCHANTABILITY, |
433 | + * SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE. See the GNU |
434 | + * Lesser General Public License for more details. |
435 | + * |
436 | + * You should have received a copy of the GNU Lesser General Public License |
437 | + * along with this program. If not, see <http://www.gnu.org/licenses/>. |
438 | + */ |
439 | + |
440 | +#include <QLoggingCategory> |
441 | +#include <QSignalSpy> |
442 | + |
443 | +// the test subject |
444 | +#include <Unity/Application/surfacemanager.h> |
445 | +#include <Unity/Application/mirsurface.h> |
446 | + |
447 | +// miral |
448 | +#include <miral/window.h> |
449 | +#include <miral/window_info.h> |
450 | + |
451 | +// mirtest |
452 | +#include <mir/test/doubles/stub_session.h> |
453 | +#include <mir/test/doubles/stub_surface.h> |
454 | + |
455 | +// local |
456 | +#include "qtmir_test.h" |
457 | +#include "fake_session.h" |
458 | +#include "mock_sessionmap.h" |
459 | +#include "mock_window_controller.h" |
460 | + |
461 | +using namespace qtmir; |
462 | +using StubSurface = mir::test::doubles::StubSurface; |
463 | +using StubSession = mir::test::doubles::StubSession; |
464 | + |
465 | + |
466 | +class SurfaceManagerTests : public ::testing::Test |
467 | +{ |
468 | +public: |
469 | + SurfaceManagerTests() |
470 | + { |
471 | + // We don't want the logging spam cluttering the test results |
472 | + QLoggingCategory::setFilterRules(QStringLiteral("qtmir.*=false")); |
473 | + |
474 | + qRegisterMetaType<unity::shell::application::MirSurfaceInterface*>(); |
475 | + qRegisterMetaType<QVector<unity::shell::application::MirSurfaceInterface*>>(); |
476 | + } |
477 | + |
478 | + testing::NiceMock<MockWindowController> wmController; |
479 | + WindowModelNotifier wmNotifier; |
480 | + MockSessionMap sessionMap; |
481 | + QScopedPointer<SurfaceManager> surfaceManager; |
482 | + QScopedPointer<QCoreApplication> qtApp; // need to spin event loop for queued connections |
483 | + |
484 | + // Needed to create miral::WindowInfo |
485 | + const std::shared_ptr<StubSession> stubSession{std::make_shared<StubSession>()}; |
486 | + const std::shared_ptr<StubSurface> stubSurface{std::make_shared<StubSurface>()}; |
487 | + const miral::Window window{stubSession, stubSurface}; |
488 | + const ms::SurfaceCreationParameters spec; |
489 | + const miral::WindowInfo windowInfo{window, spec}; |
490 | + FakeSession fakeSession; |
491 | + |
492 | +protected: |
493 | + void SetUp() override |
494 | + { |
495 | + int argc = 0; |
496 | + char* argv[0]; |
497 | + qtApp.reset(new QCoreApplication(argc, argv)); |
498 | + |
499 | + using namespace ::testing; |
500 | + ON_CALL(sessionMap,findSession(_)) |
501 | + .WillByDefault(Return(&fakeSession)); |
502 | + |
503 | + surfaceManager.reset(new SurfaceManager(&wmController, &wmNotifier, &sessionMap)); |
504 | + } |
505 | +}; |
506 | + |
507 | +/* |
508 | + * Test if MirAL notifies that a window was created, SurfaceManager emits the surfaceCreated |
509 | + * signal |
510 | + */ |
511 | +TEST_F(SurfaceManagerTests, miralWindowCreationCausesSignalEmission) |
512 | +{ |
513 | + QSignalSpy newMirSurfaceSpy(surfaceManager.data(), &SurfaceManager::surfaceCreated); |
514 | + |
515 | + // Test |
516 | + Q_EMIT wmNotifier.windowAdded(windowInfo); |
517 | + qtApp->sendPostedEvents(); |
518 | + |
519 | + // Check result |
520 | + EXPECT_EQ(1, newMirSurfaceSpy.count()); |
521 | +} |
522 | + |
523 | + |
524 | +/* |
525 | + * Test if MirAL notifies that a window was created, SurfaceManager emits the surfaceCreated |
526 | + * signal with a corresponding MirSurface |
527 | + */ |
528 | +TEST_F(SurfaceManagerTests, miralWindowCreationCausesMirSurfaceCreation) |
529 | +{ |
530 | + QSignalSpy newMirSurfaceSpy(surfaceManager.data(), &SurfaceManager::surfaceCreated); |
531 | + |
532 | + // Test |
533 | + Q_EMIT wmNotifier.windowAdded(windowInfo); |
534 | + qtApp->sendPostedEvents(); |
535 | + |
536 | + // Check result |
537 | + auto mirSurface = qvariant_cast<MirSurface*>(newMirSurfaceSpy.takeFirst().at(0)); |
538 | + ASSERT_TRUE(mirSurface); |
539 | + EXPECT_EQ(window, mirSurface->window()); |
540 | +} |
541 | + |
542 | +/* |
543 | + * Test if MirAL notifies that a window was created, SurfaceManager adds the corresponding |
544 | + * MirSurface to its internal list |
545 | + */ |
546 | +TEST_F(SurfaceManagerTests, miralWindowCreationAddsMirSurfaceToItsInternalList) |
547 | +{ |
548 | + // Test |
549 | + Q_EMIT wmNotifier.windowAdded(windowInfo); |
550 | + qtApp->sendPostedEvents(); |
551 | + |
552 | + // Check result |
553 | + auto mirSurface = surfaceManager->find(windowInfo); |
554 | + ASSERT_TRUE(mirSurface); |
555 | + EXPECT_EQ(window, mirSurface->window()); |
556 | +} |
557 | + |
558 | +/* |
559 | + * Test SurfaceManager creates a MirSurface with a Session associated |
560 | + */ |
561 | +TEST_F(SurfaceManagerTests, createdMirSurfaceHasSessionSet) |
562 | +{ |
563 | + // Setup |
564 | + using namespace ::testing; |
565 | + EXPECT_CALL(sessionMap,findSession(_)) |
566 | + .WillOnce(Return(&fakeSession)); |
567 | + |
568 | + // Test |
569 | + Q_EMIT wmNotifier.windowAdded(windowInfo); |
570 | + qtApp->sendPostedEvents(); |
571 | + |
572 | + // Check result |
573 | + auto mirSurface = surfaceManager->find(windowInfo); |
574 | + ASSERT_TRUE(mirSurface); |
575 | + EXPECT_EQ(&fakeSession, mirSurface->session()); |
576 | +} |
577 | + |
578 | +/* |
579 | + * Test when MirAL creates a surface with a parent, SurfaceManager correctly associates |
580 | + * the parent MirSurface to the MirSurface |
581 | + */ |
582 | +TEST_F(SurfaceManagerTests, parentedMiralWindowGeneratesMirSurfaceWithCorrectParent) |
583 | +{ |
584 | + // Setup |
585 | + miral::Window parentWindow(stubSession, stubSurface); |
586 | + miral::Window childWindow(stubSession, stubSurface); |
587 | + miral::WindowInfo parentWindowInfo(parentWindow, spec); |
588 | + miral::WindowInfo childWindowInfo(childWindow, spec); |
589 | + childWindowInfo.parent(parentWindow); |
590 | + |
591 | + // Test |
592 | + Q_EMIT wmNotifier.windowAdded(parentWindowInfo); |
593 | + Q_EMIT wmNotifier.windowAdded(childWindowInfo); |
594 | + qtApp->sendPostedEvents(); |
595 | + |
596 | + // Check result |
597 | + auto childMirSurface = surfaceManager->find(childWindowInfo); |
598 | + auto parentMirSurface = surfaceManager->find(parentWindowInfo); |
599 | + ASSERT_TRUE(childMirSurface); |
600 | + ASSERT_TRUE(parentMirSurface); |
601 | + |
602 | + EXPECT_EQ(parentMirSurface, childMirSurface->parentSurface()); |
603 | +} |
604 | + |
605 | +/* |
606 | + * Test when MirAL creates a surface with a parent, SurfaceManager correctly associates |
607 | + * the child MirSurface to the parent MirSurface |
608 | + */ |
609 | +TEST_F(SurfaceManagerTests, miralWindowWithChildHasMirSurfaceWithCorrectChild) |
610 | +{ |
611 | + // Setup |
612 | + miral::Window parentWindow(stubSession, stubSurface); |
613 | + miral::Window childWindow(stubSession, stubSurface); |
614 | + miral::WindowInfo parentWindowInfo(parentWindow, spec); |
615 | + miral::WindowInfo childWindowInfo(childWindow, spec); |
616 | + childWindowInfo.parent(parentWindow); |
617 | + |
618 | + // Test |
619 | + Q_EMIT wmNotifier.windowAdded(parentWindowInfo); |
620 | + Q_EMIT wmNotifier.windowAdded(childWindowInfo); |
621 | + qtApp->sendPostedEvents(); |
622 | + |
623 | + // Check result |
624 | + auto childMirSurface = surfaceManager->find(childWindowInfo); |
625 | + auto parentMirSurface = surfaceManager->find(parentWindowInfo); |
626 | + ASSERT_TRUE(childMirSurface); |
627 | + ASSERT_TRUE(parentMirSurface); |
628 | + |
629 | + ASSERT_EQ(1, parentMirSurface->childSurfaceList()->count()); |
630 | + EXPECT_EQ(childMirSurface, parentMirSurface->childSurfaceList()->first()); |
631 | +} |
632 | + |
633 | +/* |
634 | + * Test if MirAL notifies that a window is ready, SurfaceManager updates the corresponding |
635 | + * MirSurface causing it to emit a ready() signal |
636 | + */ |
637 | +TEST_F(SurfaceManagerTests, miralWindowReadyUpdatesMirSurfaceState) |
638 | +{ |
639 | + // Setup: add window and get corresponding MirSurface |
640 | + Q_EMIT wmNotifier.windowAdded(windowInfo); |
641 | + qtApp->sendPostedEvents(); |
642 | + auto mirSurface = surfaceManager->find(windowInfo); |
643 | + ASSERT_TRUE(mirSurface); |
644 | + |
645 | + QSignalSpy mirSurfaceReadySpy(mirSurface, &MirSurface::ready); |
646 | + |
647 | + // Test |
648 | + Q_EMIT wmNotifier.windowReady(windowInfo); |
649 | + qtApp->sendPostedEvents(); |
650 | + |
651 | + // Check result |
652 | + EXPECT_EQ(1, mirSurfaceReadySpy.count()); |
653 | +} |
654 | + |
655 | +/* |
656 | + * Test if MirAL notifies that a window is moved, SurfaceManager updates the corresponding |
657 | + * MirSurface position |
658 | + */ |
659 | +TEST_F(SurfaceManagerTests, miralWindowMoveUpdatesMirSurfacePosition) |
660 | +{ |
661 | + QPoint newPosition(222,333); |
662 | + |
663 | + // Setup: add window and get corresponding MirSurface |
664 | + Q_EMIT wmNotifier.windowAdded(windowInfo); |
665 | + qtApp->sendPostedEvents(); |
666 | + auto mirSurface = surfaceManager->find(windowInfo); |
667 | + ASSERT_TRUE(mirSurface); |
668 | + |
669 | + QSignalSpy mirSurfacePositionSpy(mirSurface, &MirSurface::positionChanged); |
670 | + |
671 | + // Test |
672 | + Q_EMIT wmNotifier.windowMoved(windowInfo, newPosition); |
673 | + qtApp->sendPostedEvents(); |
674 | + |
675 | + // Check result |
676 | + EXPECT_EQ(1, mirSurfacePositionSpy.count()); |
677 | + EXPECT_EQ(mirSurface->position(), newPosition); |
678 | +} |
679 | + |
680 | +/* |
681 | + * Test if MirAL notifies that a window's focus state changes, SurfaceManager updates the corresponding |
682 | + * MirSurface focus state |
683 | + */ |
684 | +TEST_F(SurfaceManagerTests, miralWindowFocusChangeUpdatesMirSurfaceFocus) |
685 | +{ |
686 | + // Setup: add window and get corresponding MirSurface |
687 | + Q_EMIT wmNotifier.windowAdded(windowInfo); |
688 | + qtApp->sendPostedEvents(); |
689 | + auto mirSurface = surfaceManager->find(windowInfo); |
690 | + ASSERT_TRUE(mirSurface); |
691 | + ASSERT_FALSE(mirSurface->focused()); // false must be the initial state |
692 | + |
693 | + QSignalSpy mirSurfaceFocusSpy(mirSurface, &MirSurface::focusedChanged); |
694 | + |
695 | + // Test |
696 | + Q_EMIT wmNotifier.windowFocusChanged(windowInfo, true); |
697 | + qtApp->sendPostedEvents(); |
698 | + |
699 | + // Check result |
700 | + EXPECT_EQ(1, mirSurfaceFocusSpy.count()); |
701 | + EXPECT_EQ(mirSurface->focused(), true); |
702 | +} |
703 | + |
704 | +/* |
705 | + * Test if MirAL notifies that a window's state changes, SurfaceManager updates the corresponding |
706 | + * MirSurface state (just testing a single state, see no value in testing all possible states here) |
707 | + */ |
708 | +TEST_F(SurfaceManagerTests, miralWindowStateChangeUpdatesMirSurfaceState) |
709 | +{ |
710 | + auto newState = Mir::FullscreenState; |
711 | + |
712 | + // Setup: add window and get corresponding MirSurface |
713 | + Q_EMIT wmNotifier.windowAdded(windowInfo); |
714 | + qtApp->sendPostedEvents(); |
715 | + auto mirSurface = surfaceManager->find(windowInfo); |
716 | + ASSERT_TRUE(mirSurface); |
717 | + |
718 | + QSignalSpy mirSurfaceStateSpy(mirSurface, &MirSurface::stateChanged); |
719 | + |
720 | + // Test |
721 | + Q_EMIT wmNotifier.windowStateChanged(windowInfo, newState); |
722 | + qtApp->sendPostedEvents(); |
723 | + |
724 | + // Check result |
725 | + EXPECT_EQ(1, mirSurfaceStateSpy.count()); |
726 | + EXPECT_EQ(mirSurface->state(), newState); |
727 | +} |
728 | + |
729 | +/* |
730 | + * Test when miral raises a list of surfaces, the raise signal is fired by SurfaceManager with |
731 | + * a list of MirSurfaces in the matching order |
732 | + */ |
733 | +TEST_F(SurfaceManagerTests, miralsRaiseWindowListTransformedToVectorOfMirSurfaces) |
734 | +{ |
735 | + // Setup |
736 | + miral::Window window1(stubSession, stubSurface); |
737 | + miral::Window window2(stubSession, stubSurface); |
738 | + miral::WindowInfo windowInfo1(window1, spec); |
739 | + miral::WindowInfo windowInfo2(window2, spec); |
740 | + |
741 | + // Setup: add 2 windows and get their MirSurfaces |
742 | + Q_EMIT wmNotifier.windowAdded(windowInfo1); |
743 | + Q_EMIT wmNotifier.windowAdded(windowInfo2); |
744 | + qtApp->sendPostedEvents(); |
745 | + auto mirSurface1 = surfaceManager->find(windowInfo1); |
746 | + auto mirSurface2 = surfaceManager->find(windowInfo2); |
747 | + ASSERT_TRUE(mirSurface1); |
748 | + ASSERT_TRUE(mirSurface2); |
749 | + |
750 | + QSignalSpy mirSurfacesRaisedSpy(surfaceManager.data(), &SurfaceManager::surfacesRaised); |
751 | + |
752 | + // Test |
753 | + std::vector<miral::Window> raiseWindowList{window2, window1}; |
754 | + Q_EMIT wmNotifier.windowsRaised(raiseWindowList); |
755 | + qtApp->sendPostedEvents(); |
756 | + |
757 | + // Check results |
758 | + ASSERT_EQ(1, mirSurfacesRaisedSpy.count()); |
759 | + auto raiseMirSurfaceList = qvariant_cast<QVector<unity::shell::application::MirSurfaceInterface*>>( |
760 | + mirSurfacesRaisedSpy.takeFirst().at(0)); // first argument of signal |
761 | + ASSERT_EQ(2, raiseMirSurfaceList.count()); |
762 | + EXPECT_EQ(mirSurface1, raiseMirSurfaceList.at(1)); |
763 | + EXPECT_EQ(mirSurface2, raiseMirSurfaceList.at(0)); |
764 | +} |
765 | + |
766 | +/* |
767 | + * Test focus requests fire focusRequested signal of the MirSurface |
768 | + */ |
769 | +TEST_F(SurfaceManagerTests, focusRequestCausesMirSurfaceToFireFocusRequestedSignal) |
770 | +{ |
771 | + // Setup: add window and get corresponding MirSurface |
772 | + Q_EMIT wmNotifier.windowAdded(windowInfo); |
773 | + qtApp->sendPostedEvents(); |
774 | + auto mirSurface = surfaceManager->find(windowInfo); |
775 | + ASSERT_TRUE(mirSurface); |
776 | + |
777 | + QSignalSpy mirSurfaceFocusRequestedSpy(mirSurface, &MirSurface::focusRequested); |
778 | + |
779 | + // Test |
780 | + Q_EMIT wmNotifier.windowRequestedRaise(windowInfo); |
781 | + qtApp->sendPostedEvents(); |
782 | + |
783 | + // Check result |
784 | + EXPECT_EQ(1, mirSurfaceFocusRequestedSpy.count()); |
785 | +} |
786 | + |
787 | +/* |
788 | + * If MirAL notifies that a window was removed, and its corresponding MirSurface is not |
789 | + * being displayed, test that SurfaceManager removes the corresponding MirSurface from |
790 | + * its internal list and deletes the MirSurface |
791 | + */ |
792 | +TEST_F(SurfaceManagerTests, miralWindowRemovedDeletesSurfaceManagerInternalEntryAndMirSurface) |
793 | +{ |
794 | + // Setup: add window and get corresponding MirSurface |
795 | + Q_EMIT wmNotifier.windowAdded(windowInfo); |
796 | + qtApp->sendPostedEvents(); |
797 | + auto mirSurface = surfaceManager->find(windowInfo); |
798 | + ASSERT_TRUE(mirSurface); |
799 | + |
800 | + QSignalSpy mirSurfaceDestroyedSpy(mirSurface, &QObject::destroyed); |
801 | + |
802 | + // Test |
803 | + Q_EMIT wmNotifier.windowRemoved(windowInfo); |
804 | + qtApp->sendPostedEvents(); |
805 | + |
806 | + // Check result |
807 | + ASSERT_EQ(2, mirSurfaceDestroyedSpy.count()); //FIXME - should be 1 |
808 | + EXPECT_FALSE(surfaceManager->find(windowInfo)); |
809 | +} |
810 | + |
811 | +/* |
812 | + * If MirAL notifies that a window was removed, and its corresponding MirSurface *is* |
813 | + * being displayed, test that SurfaceManager removes the corresponding MirSurface from |
814 | + * its internal list but does *not* delete the MirSurface, but sets it as not "live" |
815 | + */ |
816 | +TEST_F(SurfaceManagerTests, miralWindowRemovedDeletesSurfaceManagerInternalEntryButNotMirSurface) |
817 | +{ |
818 | + // Setup: add window and get corresponding MirSurface |
819 | + Q_EMIT wmNotifier.windowAdded(windowInfo); |
820 | + qtApp->sendPostedEvents(); |
821 | + auto mirSurface = surfaceManager->find(windowInfo); |
822 | + ASSERT_TRUE(mirSurface); |
823 | + |
824 | + QSignalSpy mirSurfaceDestroyedSpy(mirSurface, &QObject::destroyed); |
825 | + |
826 | + mirSurface->registerView(1); |
827 | + |
828 | + // Test |
829 | + Q_EMIT wmNotifier.windowRemoved(windowInfo); |
830 | + qtApp->sendPostedEvents(); |
831 | + |
832 | + // Check result |
833 | + ASSERT_EQ(0, mirSurfaceDestroyedSpy.count()); |
834 | + EXPECT_FALSE(surfaceManager->find(windowInfo)); |
835 | + EXPECT_FALSE(mirSurface->live()); |
836 | +} |
837 | + |
838 | +/* |
839 | + * If MirAL notifies that a window was removed, and its corresponding MirSurface *is* |
840 | + * being displayed, SurfaceManager does *not* delete the MirSurface. Later when that |
841 | + * MirSurface is not being displayed any more, SurfaceManager should delete it. |
842 | + */ |
843 | +TEST_F(SurfaceManagerTests, miralWindowRemovedSurfaceManagerDeletesMirSurfaceWhenItDoneWith) |
844 | +{ |
845 | + int viewId = 99; |
846 | + |
847 | + // Setup: add window and get corresponding MirSurface |
848 | + Q_EMIT wmNotifier.windowAdded(windowInfo); |
849 | + qtApp->sendPostedEvents(); |
850 | + auto mirSurface = surfaceManager->find(windowInfo); |
851 | + ASSERT_TRUE(mirSurface); |
852 | + |
853 | + QSignalSpy mirSurfaceDestroyedSpy(mirSurface, &QObject::destroyed); |
854 | + |
855 | + // Setup: indicate MirSurface is being displayed |
856 | + mirSurface->registerView(viewId); |
857 | + |
858 | + // Setup: notify that window removed |
859 | + Q_EMIT wmNotifier.windowRemoved(windowInfo); |
860 | + qtApp->sendPostedEvents(); |
861 | + |
862 | + // Test |
863 | + mirSurface->unregisterView(viewId); |
864 | + // MirSurface is deleteLater()ed by SurfaceManager, so need to spin event loop. |
865 | + // But DeferredDelete is special: likes to be called out specifically or it won't come out |
866 | + qtApp->sendPostedEvents(mirSurface, QEvent::DeferredDelete); |
867 | + qtApp->sendPostedEvents(); |
868 | + |
869 | + // Check result |
870 | + ASSERT_EQ(2, mirSurfaceDestroyedSpy.count()); //FIXME - should be 1 |
871 | +} |
872 | + |
873 | +/* |
874 | + * Test that if a MirSurface is live, and stops being displayed, SurfaceManager does *not* |
875 | + * delete the MirSurface. |
876 | + */ |
877 | +TEST_F(SurfaceManagerTests, surfaceManagerDoesNotDeleteLiveMirSurfaceWhenStopsBeingDisplayed) |
878 | +{ |
879 | + int viewId = 99; |
880 | + |
881 | + // Setup: add window and get corresponding MirSurface |
882 | + Q_EMIT wmNotifier.windowAdded(windowInfo); |
883 | + qtApp->sendPostedEvents(); |
884 | + auto mirSurface = surfaceManager->find(windowInfo); |
885 | + ASSERT_TRUE(mirSurface); |
886 | + ASSERT_TRUE(mirSurface->live()); |
887 | + |
888 | + QSignalSpy mirSurfaceDestroyedSpy(mirSurface, &QObject::destroyed); |
889 | + |
890 | + // Setup: indicate MirSurface is being displayed |
891 | + mirSurface->registerView(viewId); |
892 | + |
893 | + // Test |
894 | + mirSurface->unregisterView(viewId); |
895 | + |
896 | + // Check result |
897 | + ASSERT_EQ(0, mirSurfaceDestroyedSpy.count()); |
898 | +} |
899 | |
900 | === modified file 'tests/modules/WindowManager/mirsurface_test.cpp' |
901 | --- tests/modules/WindowManager/mirsurface_test.cpp 2017-02-02 09:29:46 +0000 |
902 | +++ tests/modules/WindowManager/mirsurface_test.cpp 2017-03-20 15:56:36 +0000 |
903 | @@ -110,66 +110,6 @@ |
904 | ASSERT_TRUE(spyFrameDropped.count() > 0); |
905 | } |
906 | |
907 | - |
908 | -TEST_F(MirSurfaceTest, DeleteMirSurfaceOnLastNonLiveUnregisterView) |
909 | -{ |
910 | - int argc = 0; |
911 | - char* argv[0]; |
912 | - QCoreApplication qtApp(argc, argv); // app for deleteLater event |
913 | - |
914 | - miral::Window mockWindow(stubSession, stubSurface); |
915 | - ms::SurfaceCreationParameters spec; |
916 | - miral::WindowInfo mockWindowInfo(mockWindow, spec); |
917 | - |
918 | - auto surface = new MirSurface(mockWindowInfo, nullptr); // lives on the heap, as it will delete itself |
919 | - |
920 | - bool surfaceDeleted = false; |
921 | - QObject::connect(surface, &QObject::destroyed, [&surfaceDeleted](){ surfaceDeleted = true; }); |
922 | - |
923 | - qintptr view1 = (qintptr)1; |
924 | - qintptr view2 = (qintptr)2; |
925 | - |
926 | - surface->registerView(view1); |
927 | - surface->registerView(view2); |
928 | - surface->setLive(false); |
929 | - EXPECT_FALSE(surfaceDeleted); |
930 | - |
931 | - surface->unregisterView(view1); |
932 | - surface->unregisterView(view2); |
933 | - |
934 | - QCoreApplication::sendPostedEvents(0, QEvent::DeferredDelete); |
935 | - EXPECT_TRUE(surfaceDeleted); |
936 | -} |
937 | - |
938 | - |
939 | -TEST_F(MirSurfaceTest, DISABLED_DoNotDeleteMirSurfaceOnLastLiveUnregisterView) |
940 | -{ |
941 | - int argc = 0; |
942 | - char* argv[0]; |
943 | - QCoreApplication qtApp(argc, argv); // app for deleteLater event |
944 | - |
945 | - miral::Window mockWindow(stubSession, stubSurface); |
946 | - ms::SurfaceCreationParameters spec; |
947 | - miral::WindowInfo mockWindowInfo(mockWindow, spec); |
948 | - |
949 | - auto surface = new MirSurface(mockWindowInfo, nullptr); // lives on the heap, as it may delete itself |
950 | - |
951 | - bool surfaceDeleted = false; |
952 | - QObject::connect(surface, &QObject::destroyed, [&surfaceDeleted](){ surfaceDeleted = true; }); |
953 | - |
954 | - qintptr view1 = (qintptr)1; |
955 | - qintptr view2 = (qintptr)2; |
956 | - |
957 | - surface->registerView(view1); |
958 | - surface->registerView(view2); |
959 | - |
960 | - surface->unregisterView(view1); |
961 | - surface->unregisterView(view2); |
962 | - |
963 | - QCoreApplication::sendPostedEvents(0, QEvent::DeferredDelete); |
964 | - EXPECT_FALSE(surfaceDeleted); |
965 | -} |
966 | - |
967 | /* |
968 | * Test that MirSurface.visible is recalculated after the client swaps the first frame. |
969 | * A surface is not considered visible unless it has a non-hidden & non-minimized state, and |
PASSED: Continuous integration, rev:618 /unity8- jenkins. ubuntu. com/job/ lp-qtmir- ci/545/ /unity8- jenkins. ubuntu. com/job/ build/4283 /unity8- jenkins. ubuntu. com/job/ build-0- fetch/4311 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= xenial+ overlay/ 4145 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= xenial+ overlay/ 4145/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= zesty/4145 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= zesty/4145/ artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= xenial+ overlay/ 4145 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= xenial+ overlay/ 4145/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= zesty/4145 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= zesty/4145/ artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= xenial+ overlay/ 4145 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= xenial+ overlay/ 4145/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= zesty/4145 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= zesty/4145/ artifact/ output/ *zip*/output. zip
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild: /unity8- jenkins. ubuntu. com/job/ lp-qtmir- ci/545/ rebuild
https:/