Merge lp:~dandrader/qtmir/improveSessionDebugLogging into lp:qtmir

Proposed by Daniel d'Andrada
Status: Merged
Approved by: Nick Dedekind
Approved revision: no longer in the source branch.
Merged at revision: 517
Proposed branch: lp:~dandrader/qtmir/improveSessionDebugLogging
Merge into: lp:qtmir
Diff against target: 233 lines (+44/-22)
3 files modified
src/modules/Unity/Application/mirsurfacelistmodel.cpp (+21/-0)
src/modules/Unity/Application/mirsurfacelistmodel.h (+2/-0)
src/modules/Unity/Application/session.cpp (+21/-22)
To merge this branch: bzr merge lp:~dandrader/qtmir/improveSessionDebugLogging
Reviewer Review Type Date Requested Status
Nick Dedekind (community) Approve
Unity8 CI Bot (community) continuous-integration Approve
Review via email: mp+296199@code.launchpad.net

Commit message

Improve Session debug logging

Description of the change

* Are there any related MPs required for this MP to build/function as expected? Please list.
No, but it goes well along with https://code.launchpad.net/~dandrader/qtubuntu/betterSessionName/+merge/296198

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

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

To post a comment you must log in.
Revision history for this message
Nick Dedekind (nick-dedekind) :
review: Needs Information
Revision history for this message
Daniel d'Andrada (dandrader) wrote :

On 01/06/2016 09:42, Nick Dedekind wrote:
>> +QDebug operator<<(QDebug dbg, const unityapp::MirSurfaceListInterface &surfaceListConst)
> I don't see this actually being used anywhere.
>
It is not. This is just a facility to help with debugging when you add
ad-hoc, temporary, qDebug() entries to investigate something.

So you can just do "qDebug() << someMirSurfaceList" and its contents
will be nicely printed.

Similar to what most Qt classes have (like QPoint, QRect etc).

Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:497
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/244/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/1760
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1786
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1733
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1733
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/1733
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1726
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1726/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1726
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1726/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1726
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1726/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1726
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1726/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1726
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1726/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1726
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1726/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1726
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1726/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1726
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1726/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1726
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1726/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/244/rebuild

review: Approve (continuous-integration)
Revision history for this message
Daniel d'Andrada (dandrader) wrote :

On 01/06/2016 09:42, Nick Dedekind wrote:
>> +{
>> >+ auto surfaceList = const_cast<unityapp::MirSurfaceListInterface*>(&surfaceListConst);
> It seems strange to have to const cast, since we shouldn't be modifying the list.
> I notice you have a const get in the qtmir::MirSurfaceModel.
>

unity::shell::application::MirSurfaceListInterface::get is not const.
And that's the class/interface used here, not qtmir::MirSurfaceModel.

That way it works with both ProxySurfaceListModel and MirSurfaceListModel.

But yes, it would be good to have a const version of get() in
unity::shell::application::MirSurfaceListInterface so this cast would
not be needed.

503. By Kevin DuBois

rebuild for mir 0.23

504. By CI Train Bot Account

Releasing 0.4.8+16.10.20160602-0ubuntu1

Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

LGTM

review: Approve
505. By Daniel d'Andrada

Improve Session debug logging

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/modules/Unity/Application/mirsurfacelistmodel.cpp'
--- src/modules/Unity/Application/mirsurfacelistmodel.cpp 2016-05-17 19:57:52 +0000
+++ src/modules/Unity/Application/mirsurfacelistmodel.cpp 2016-06-09 19:21:39 +0000
@@ -20,6 +20,9 @@
2020
21#include <paths.h>21#include <paths.h>
2222
23#include <QDebug>
24#include <QDebugStateSaver>
25
23namespace unityapp = unity::shell::application;26namespace unityapp = unity::shell::application;
24using namespace qtmir;27using namespace qtmir;
2528
@@ -284,3 +287,21 @@
284287
285 return m_sourceList->data(index, role);288 return m_sourceList->data(index, role);
286}289}
290
291QDebug operator<<(QDebug dbg, const unityapp::MirSurfaceListInterface &surfaceListConst)
292{
293 auto surfaceList = const_cast<unityapp::MirSurfaceListInterface*>(&surfaceListConst);
294
295 QDebugStateSaver saver(dbg);
296 dbg.nospace();
297 dbg << "MirSurfaceList(";
298 for (int i = 0; i < surfaceList->count(); ++i) {
299 if (i > 0) {
300 dbg << ", ";
301 }
302 auto surface = surfaceList->get(i);
303 dbg << (void*)surface;
304 }
305 dbg << ')';
306 return dbg;
307}
287308
=== modified file 'src/modules/Unity/Application/mirsurfacelistmodel.h'
--- src/modules/Unity/Application/mirsurfacelistmodel.h 2016-05-10 21:06:21 +0000
+++ src/modules/Unity/Application/mirsurfacelistmodel.h 2016-06-09 19:21:39 +0000
@@ -94,4 +94,6 @@
9494
95} // namespace qtmir95} // namespace qtmir
9696
97QDebug operator<<(QDebug, const unity::shell::application::MirSurfaceListInterface &);
98
97#endif // QTMIR_MIRSURFACELISTMODEL_H99#endif // QTMIR_MIRSURFACELISTMODEL_H
98100
=== modified file 'src/modules/Unity/Application/session.cpp'
--- src/modules/Unity/Application/session.cpp 2016-05-17 19:18:44 +0000
+++ src/modules/Unity/Application/session.cpp 2016-06-09 19:21:39 +0000
@@ -38,6 +38,8 @@
3838
39using unity::shell::application::ApplicationInfoInterface;39using unity::shell::application::ApplicationInfoInterface;
4040
41#define DEBUG_MSG qCDebug(QTMIR_SURFACES).nospace() << "Session[" << (void*)this << ",name=" << name() << "]::" << __func__
42
41namespace qtmir43namespace qtmir
42{44{
4345
@@ -75,7 +77,7 @@
75 , m_live(true)77 , m_live(true)
76 , m_promptSessionManager(promptSessionManager)78 , m_promptSessionManager(promptSessionManager)
77{79{
78 qCDebug(QTMIR_SESSIONS) << "Session::Session() " << this->name();80 DEBUG_MSG << "()";
7981
80 setSuspendTimer(new Timer);82 setSuspendTimer(new Timer);
8183
@@ -84,7 +86,7 @@
8486
85Session::~Session()87Session::~Session()
86{88{
87 qCDebug(QTMIR_SESSIONS) << "Session::~Session() " << name();89 DEBUG_MSG << "()";
88 stopPromptSessions();90 stopPromptSessions();
8991
90 QList<SessionInterface*> children(m_children->list());92 QList<SessionInterface*> children(m_children->list());
@@ -107,7 +109,7 @@
107 Q_ASSERT(m_state == Session::Suspending);109 Q_ASSERT(m_state == Session::Suspending);
108110
109 if (m_surfaceList.count() == 0) {111 if (m_surfaceList.count() == 0) {
110 qCDebug(QTMIR_SESSIONS) << "Application::suspend - no surface to call stopFrameDropper() on!";112 DEBUG_MSG << " no surface to call stopFrameDropper() on!";
111 } else {113 } else {
112 for (int i = 0; i < m_surfaceList.count(); ++i) {114 for (int i = 0; i < m_surfaceList.count(); ++i) {
113 auto surface = static_cast<MirSurfaceInterface*>(m_surfaceList.get(i));115 auto surface = static_cast<MirSurfaceInterface*>(m_surfaceList.get(i));
@@ -153,8 +155,7 @@
153 return;155 return;
154 }156 }
155157
156 qCDebug(QTMIR_SESSIONS) << "Session::setState - session=" << name()158 DEBUG_MSG << "(state=" << sessionStateToString(state) << ")";
157 << "state=" << sessionStateToString(state);
158159
159 if (m_state == Suspending) {160 if (m_state == Suspending) {
160 m_suspendTimer->stop();161 m_suspendTimer->stop();
@@ -198,7 +199,7 @@
198199
199void Session::registerSurface(MirSurfaceInterface *newSurface)200void Session::registerSurface(MirSurfaceInterface *newSurface)
200{201{
201 qCDebug(QTMIR_SESSIONS) << "Session::resgisterSurface - session=" << name() << "surface=" << newSurface;202 DEBUG_MSG << "(surface=" << newSurface << ")";
202203
203 // Only notify QML of surface creation once it has drawn its first frame.204 // Only notify QML of surface creation once it has drawn its first frame.
204 if (newSurface->isFirstFrameDrawn()) {205 if (newSurface->isFirstFrameDrawn()) {
@@ -214,7 +215,7 @@
214215
215void Session::prependSurface(MirSurfaceInterface *newSurface)216void Session::prependSurface(MirSurfaceInterface *newSurface)
216{217{
217 qCDebug(QTMIR_SESSIONS) << "Session::prependSurface - session=" << name() << "surface=" << newSurface;218 DEBUG_MSG << "(surface=" << newSurface << ")";
218219
219 connect(newSurface, &MirSurfaceInterface::stateChanged,220 connect(newSurface, &MirSurfaceInterface::stateChanged,
220 this, &Session::updateFullscreenProperty);221 this, &Session::updateFullscreenProperty);
@@ -246,7 +247,7 @@
246247
247void Session::removeSurface(MirSurfaceInterface* surface)248void Session::removeSurface(MirSurfaceInterface* surface)
248{249{
249 qCDebug(QTMIR_SESSIONS) << "Session::removeSurface - session=" << name() << "surface=" << surface;250 DEBUG_MSG << "(surface=" << surface << ")";
250251
251 surface->disconnect(this);252 surface->disconnect(this);
252253
@@ -277,8 +278,8 @@
277278
278void Session::setFullscreen(bool fullscreen)279void Session::setFullscreen(bool fullscreen)
279{280{
280 qCDebug(QTMIR_SESSIONS) << "Session::setFullscreen - session=" << this << "fullscreen=" << fullscreen;
281 if (m_fullscreen != fullscreen) {281 if (m_fullscreen != fullscreen) {
282 DEBUG_MSG << "(" << fullscreen << ")";
282 m_fullscreen = fullscreen;283 m_fullscreen = fullscreen;
283 Q_EMIT fullscreenChanged(m_fullscreen);284 Q_EMIT fullscreenChanged(m_fullscreen);
284 }285 }
@@ -286,7 +287,7 @@
286287
287void Session::suspend()288void Session::suspend()
288{289{
289 qCDebug(QTMIR_SESSIONS) << "Session::suspend - session=" << this << "state=" << sessionStateToString(m_state);290 DEBUG_MSG << " state=" << sessionStateToString(m_state);
290 if (m_state == Running) {291 if (m_state == Running) {
291 session()->set_lifecycle_state(mir_lifecycle_state_will_suspend);292 session()->set_lifecycle_state(mir_lifecycle_state_will_suspend);
292 m_suspendTimer->start();293 m_suspendTimer->start();
@@ -305,7 +306,7 @@
305306
306void Session::resume()307void Session::resume()
307{308{
308 qCDebug(QTMIR_SESSIONS) << "Session::resume - session=" << this << "state=" << sessionStateToString(m_state);309 DEBUG_MSG << " state=" << sessionStateToString(m_state);
309310
310 if (m_state == Suspending || m_state == Suspended) {311 if (m_state == Suspending || m_state == Suspended) {
311 doResume();312 doResume();
@@ -336,7 +337,7 @@
336337
337void Session::close()338void Session::close()
338{339{
339 qCDebug(QTMIR_SESSIONS) << "Session::close - " << name();340 DEBUG_MSG << "()";
340341
341 if (m_state == Stopped) return;342 if (m_state == Stopped) return;
342343
@@ -348,7 +349,7 @@
348349
349void Session::stop()350void Session::stop()
350{351{
351 qCDebug(QTMIR_SESSIONS) << "Session::stop - " << name();352 DEBUG_MSG << "()";
352353
353 if (m_state != Stopped) {354 if (m_state != Stopped) {
354355
@@ -372,7 +373,7 @@
372void Session::setLive(const bool live)373void Session::setLive(const bool live)
373{374{
374 if (m_live != live) {375 if (m_live != live) {
375 qCDebug(QTMIR_SESSIONS) << "Session::setLive - " << name() << "live=" << live;376 DEBUG_MSG << "(" << live << ")";
376377
377 m_live = live;378 m_live = live;
378 Q_EMIT liveChanged(m_live);379 Q_EMIT liveChanged(m_live);
@@ -396,7 +397,7 @@
396397
397void Session::insertChildSession(uint index, SessionInterface* session)398void Session::insertChildSession(uint index, SessionInterface* session)
398{399{
399 qCDebug(QTMIR_SESSIONS) << "Session::insertChildSession - " << session->name() << " to " << name() << " @ " << index;400 DEBUG_MSG << "(index=" << index << ", Session[" << (void*)session << ",name=" << session->name() << "])";
400 Q_ASSERT(!m_children->contains(session));401 Q_ASSERT(!m_children->contains(session));
401402
402 m_children->insert(index, session);403 m_children->insert(index, session);
@@ -424,7 +425,7 @@
424425
425void Session::removeChildSession(SessionInterface* session)426void Session::removeChildSession(SessionInterface* session)
426{427{
427 qCDebug(QTMIR_SESSIONS) << "Session::removeChildSession - " << session->name() << " from " << name();428 DEBUG_MSG << "(Session[" << (void*)session << ",name=" << session->name() << "])";
428429
429 disconnect(session, 0, this, 0);430 disconnect(session, 0, this, 0);
430431
@@ -452,16 +453,14 @@
452453
453void Session::appendPromptSession(const std::shared_ptr<ms::PromptSession>& promptSession)454void Session::appendPromptSession(const std::shared_ptr<ms::PromptSession>& promptSession)
454{455{
455 qCDebug(QTMIR_SESSIONS) << "Session::appendPromptSession session=" << name()456 DEBUG_MSG << "(promptSession=" << (promptSession ? promptSession.get() : nullptr) << ")";
456 << "promptSession=" << (promptSession ? promptSession.get() : nullptr);
457457
458 m_promptSessions.append(promptSession);458 m_promptSessions.append(promptSession);
459}459}
460460
461void Session::removePromptSession(const std::shared_ptr<ms::PromptSession>& promptSession)461void Session::removePromptSession(const std::shared_ptr<ms::PromptSession>& promptSession)
462{462{
463 qCDebug(QTMIR_SESSIONS) << "Session::removePromptSession session=" << name()463 DEBUG_MSG << "(promptSession=" << (promptSession ? promptSession.get() : nullptr) << ")";
464 << "promptSession=" << (promptSession ? promptSession.get() : nullptr);
465464
466 m_promptSessions.removeAll(promptSession);465 m_promptSessions.removeAll(promptSession);
467}466}
@@ -477,7 +476,7 @@
477 QListIterator<std::shared_ptr<ms::PromptSession>> it(copy);476 QListIterator<std::shared_ptr<ms::PromptSession>> it(copy);
478 for ( it.toBack(); it.hasPrevious(); ) {477 for ( it.toBack(); it.hasPrevious(); ) {
479 std::shared_ptr<ms::PromptSession> promptSession = it.previous();478 std::shared_ptr<ms::PromptSession> promptSession = it.previous();
480 qCDebug(QTMIR_SESSIONS) << "Session::stopPromptSessions - promptSession=" << promptSession.get();479 DEBUG_MSG << " - promptSession=" << promptSession.get();
481480
482 m_promptSessionManager->stop_prompt_session(promptSession);481 m_promptSessionManager->stop_prompt_session(promptSession);
483 }482 }
@@ -500,7 +499,7 @@
500void Session::deleteIfZombieAndEmpty()499void Session::deleteIfZombieAndEmpty()
501{500{
502 if (!m_live && m_children->rowCount() == 0 && m_surfaceList.isEmpty()) {501 if (!m_live && m_children->rowCount() == 0 && m_surfaceList.isEmpty()) {
503 qCDebug(QTMIR_SESSIONS).nospace() << "Session::deleteIfZombieAndEmpty[" << name() << "] - deleteLater()";502 DEBUG_MSG << " - deleteLater()";
504 deleteLater();503 deleteLater();
505 }504 }
506}505}

Subscribers

People subscribed via source and target branches