QtMir

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

  1. improveSessionDebugLogging
  2. Merge into trunk
Proposed by Daniel d'Andrada on 2016-06-01
Status: Merged
Approved by: Nick Dedekind on 2016-06-06
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) 2016-06-01 Approve on 2016-06-06
Unity8 CI Bot continuous-integration Approve on 2016-06-01
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.
Nick Dedekind (nick-dedekind) :
review: Needs Information
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).

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)
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.

lp:~dandrader/qtmir/improveSessionDebugLogging updated on 2016-06-02
503. By Kevin DuBois on 2016-06-02

rebuild for mir 0.23

504. By CI Train Bot Account on 2016-06-02

Releasing 0.4.8+16.10.20160602-0ubuntu1

Nick Dedekind (nick-dedekind) wrote :

LGTM

review: Approve
lp:~dandrader/qtmir/improveSessionDebugLogging updated on 2016-06-09
505. By Daniel d'Andrada on 2016-06-09

Improve Session debug logging

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/modules/Unity/Application/mirsurfacelistmodel.cpp'
2--- src/modules/Unity/Application/mirsurfacelistmodel.cpp 2016-05-17 19:57:52 +0000
3+++ src/modules/Unity/Application/mirsurfacelistmodel.cpp 2016-06-09 19:21:39 +0000
4@@ -20,6 +20,9 @@
5
6 #include <paths.h>
7
8+#include <QDebug>
9+#include <QDebugStateSaver>
10+
11 namespace unityapp = unity::shell::application;
12 using namespace qtmir;
13
14@@ -284,3 +287,21 @@
15
16 return m_sourceList->data(index, role);
17 }
18+
19+QDebug operator<<(QDebug dbg, const unityapp::MirSurfaceListInterface &surfaceListConst)
20+{
21+ auto surfaceList = const_cast<unityapp::MirSurfaceListInterface*>(&surfaceListConst);
22+
23+ QDebugStateSaver saver(dbg);
24+ dbg.nospace();
25+ dbg << "MirSurfaceList(";
26+ for (int i = 0; i < surfaceList->count(); ++i) {
27+ if (i > 0) {
28+ dbg << ", ";
29+ }
30+ auto surface = surfaceList->get(i);
31+ dbg << (void*)surface;
32+ }
33+ dbg << ')';
34+ return dbg;
35+}
36
37=== modified file 'src/modules/Unity/Application/mirsurfacelistmodel.h'
38--- src/modules/Unity/Application/mirsurfacelistmodel.h 2016-05-10 21:06:21 +0000
39+++ src/modules/Unity/Application/mirsurfacelistmodel.h 2016-06-09 19:21:39 +0000
40@@ -94,4 +94,6 @@
41
42 } // namespace qtmir
43
44+QDebug operator<<(QDebug, const unity::shell::application::MirSurfaceListInterface &);
45+
46 #endif // QTMIR_MIRSURFACELISTMODEL_H
47
48=== modified file 'src/modules/Unity/Application/session.cpp'
49--- src/modules/Unity/Application/session.cpp 2016-05-17 19:18:44 +0000
50+++ src/modules/Unity/Application/session.cpp 2016-06-09 19:21:39 +0000
51@@ -38,6 +38,8 @@
52
53 using unity::shell::application::ApplicationInfoInterface;
54
55+#define DEBUG_MSG qCDebug(QTMIR_SURFACES).nospace() << "Session[" << (void*)this << ",name=" << name() << "]::" << __func__
56+
57 namespace qtmir
58 {
59
60@@ -75,7 +77,7 @@
61 , m_live(true)
62 , m_promptSessionManager(promptSessionManager)
63 {
64- qCDebug(QTMIR_SESSIONS) << "Session::Session() " << this->name();
65+ DEBUG_MSG << "()";
66
67 setSuspendTimer(new Timer);
68
69@@ -84,7 +86,7 @@
70
71 Session::~Session()
72 {
73- qCDebug(QTMIR_SESSIONS) << "Session::~Session() " << name();
74+ DEBUG_MSG << "()";
75 stopPromptSessions();
76
77 QList<SessionInterface*> children(m_children->list());
78@@ -107,7 +109,7 @@
79 Q_ASSERT(m_state == Session::Suspending);
80
81 if (m_surfaceList.count() == 0) {
82- qCDebug(QTMIR_SESSIONS) << "Application::suspend - no surface to call stopFrameDropper() on!";
83+ DEBUG_MSG << " no surface to call stopFrameDropper() on!";
84 } else {
85 for (int i = 0; i < m_surfaceList.count(); ++i) {
86 auto surface = static_cast<MirSurfaceInterface*>(m_surfaceList.get(i));
87@@ -153,8 +155,7 @@
88 return;
89 }
90
91- qCDebug(QTMIR_SESSIONS) << "Session::setState - session=" << name()
92- << "state=" << sessionStateToString(state);
93+ DEBUG_MSG << "(state=" << sessionStateToString(state) << ")";
94
95 if (m_state == Suspending) {
96 m_suspendTimer->stop();
97@@ -198,7 +199,7 @@
98
99 void Session::registerSurface(MirSurfaceInterface *newSurface)
100 {
101- qCDebug(QTMIR_SESSIONS) << "Session::resgisterSurface - session=" << name() << "surface=" << newSurface;
102+ DEBUG_MSG << "(surface=" << newSurface << ")";
103
104 // Only notify QML of surface creation once it has drawn its first frame.
105 if (newSurface->isFirstFrameDrawn()) {
106@@ -214,7 +215,7 @@
107
108 void Session::prependSurface(MirSurfaceInterface *newSurface)
109 {
110- qCDebug(QTMIR_SESSIONS) << "Session::prependSurface - session=" << name() << "surface=" << newSurface;
111+ DEBUG_MSG << "(surface=" << newSurface << ")";
112
113 connect(newSurface, &MirSurfaceInterface::stateChanged,
114 this, &Session::updateFullscreenProperty);
115@@ -246,7 +247,7 @@
116
117 void Session::removeSurface(MirSurfaceInterface* surface)
118 {
119- qCDebug(QTMIR_SESSIONS) << "Session::removeSurface - session=" << name() << "surface=" << surface;
120+ DEBUG_MSG << "(surface=" << surface << ")";
121
122 surface->disconnect(this);
123
124@@ -277,8 +278,8 @@
125
126 void Session::setFullscreen(bool fullscreen)
127 {
128- qCDebug(QTMIR_SESSIONS) << "Session::setFullscreen - session=" << this << "fullscreen=" << fullscreen;
129 if (m_fullscreen != fullscreen) {
130+ DEBUG_MSG << "(" << fullscreen << ")";
131 m_fullscreen = fullscreen;
132 Q_EMIT fullscreenChanged(m_fullscreen);
133 }
134@@ -286,7 +287,7 @@
135
136 void Session::suspend()
137 {
138- qCDebug(QTMIR_SESSIONS) << "Session::suspend - session=" << this << "state=" << sessionStateToString(m_state);
139+ DEBUG_MSG << " state=" << sessionStateToString(m_state);
140 if (m_state == Running) {
141 session()->set_lifecycle_state(mir_lifecycle_state_will_suspend);
142 m_suspendTimer->start();
143@@ -305,7 +306,7 @@
144
145 void Session::resume()
146 {
147- qCDebug(QTMIR_SESSIONS) << "Session::resume - session=" << this << "state=" << sessionStateToString(m_state);
148+ DEBUG_MSG << " state=" << sessionStateToString(m_state);
149
150 if (m_state == Suspending || m_state == Suspended) {
151 doResume();
152@@ -336,7 +337,7 @@
153
154 void Session::close()
155 {
156- qCDebug(QTMIR_SESSIONS) << "Session::close - " << name();
157+ DEBUG_MSG << "()";
158
159 if (m_state == Stopped) return;
160
161@@ -348,7 +349,7 @@
162
163 void Session::stop()
164 {
165- qCDebug(QTMIR_SESSIONS) << "Session::stop - " << name();
166+ DEBUG_MSG << "()";
167
168 if (m_state != Stopped) {
169
170@@ -372,7 +373,7 @@
171 void Session::setLive(const bool live)
172 {
173 if (m_live != live) {
174- qCDebug(QTMIR_SESSIONS) << "Session::setLive - " << name() << "live=" << live;
175+ DEBUG_MSG << "(" << live << ")";
176
177 m_live = live;
178 Q_EMIT liveChanged(m_live);
179@@ -396,7 +397,7 @@
180
181 void Session::insertChildSession(uint index, SessionInterface* session)
182 {
183- qCDebug(QTMIR_SESSIONS) << "Session::insertChildSession - " << session->name() << " to " << name() << " @ " << index;
184+ DEBUG_MSG << "(index=" << index << ", Session[" << (void*)session << ",name=" << session->name() << "])";
185 Q_ASSERT(!m_children->contains(session));
186
187 m_children->insert(index, session);
188@@ -424,7 +425,7 @@
189
190 void Session::removeChildSession(SessionInterface* session)
191 {
192- qCDebug(QTMIR_SESSIONS) << "Session::removeChildSession - " << session->name() << " from " << name();
193+ DEBUG_MSG << "(Session[" << (void*)session << ",name=" << session->name() << "])";
194
195 disconnect(session, 0, this, 0);
196
197@@ -452,16 +453,14 @@
198
199 void Session::appendPromptSession(const std::shared_ptr<ms::PromptSession>& promptSession)
200 {
201- qCDebug(QTMIR_SESSIONS) << "Session::appendPromptSession session=" << name()
202- << "promptSession=" << (promptSession ? promptSession.get() : nullptr);
203+ DEBUG_MSG << "(promptSession=" << (promptSession ? promptSession.get() : nullptr) << ")";
204
205 m_promptSessions.append(promptSession);
206 }
207
208 void Session::removePromptSession(const std::shared_ptr<ms::PromptSession>& promptSession)
209 {
210- qCDebug(QTMIR_SESSIONS) << "Session::removePromptSession session=" << name()
211- << "promptSession=" << (promptSession ? promptSession.get() : nullptr);
212+ DEBUG_MSG << "(promptSession=" << (promptSession ? promptSession.get() : nullptr) << ")";
213
214 m_promptSessions.removeAll(promptSession);
215 }
216@@ -477,7 +476,7 @@
217 QListIterator<std::shared_ptr<ms::PromptSession>> it(copy);
218 for ( it.toBack(); it.hasPrevious(); ) {
219 std::shared_ptr<ms::PromptSession> promptSession = it.previous();
220- qCDebug(QTMIR_SESSIONS) << "Session::stopPromptSessions - promptSession=" << promptSession.get();
221+ DEBUG_MSG << " - promptSession=" << promptSession.get();
222
223 m_promptSessionManager->stop_prompt_session(promptSession);
224 }
225@@ -500,7 +499,7 @@
226 void Session::deleteIfZombieAndEmpty()
227 {
228 if (!m_live && m_children->rowCount() == 0 && m_surfaceList.isEmpty()) {
229- qCDebug(QTMIR_SESSIONS).nospace() << "Session::deleteIfZombieAndEmpty[" << name() << "] - deleteLater()";
230+ DEBUG_MSG << " - deleteLater()";
231 deleteLater();
232 }
233 }

Subscribers

People subscribed via source and target branches