Destroy DebugConsole on hide of QWindow
ClosedPublic

Authored by graesslin on Oct 4 2016, 12:09 PM.

Details

Summary

The quit button in the DebugConsole is connected to deleteLater on the
DebugConsole. This is to clean up resources like the input event filter
which is rather expensive to have running.

When closed through the window decoration the DebugConsole window only
got hidden but not destroyed. Resulting in the input filter to continue
processing.

This change ensures that the DebugConsole gets properly destroyed once
the window gets hidden.

BUG: 369858
FIXED-IN: 5.8.1

Diff Detail

Repository
R108 KWin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
graesslin updated this revision to Diff 7083.Oct 4 2016, 12:09 PM
graesslin retitled this revision from to Destroy DebugConsole on hide of QWindow.
graesslin updated this object.
graesslin edited the test plan for this revision. (Show Details)
graesslin added reviewers: KWin, Plasma on Wayland.
Restricted Application added projects: Plasma on Wayland, KWin. · View Herald TranscriptOct 4 2016, 12:09 PM
Restricted Application added subscribers: kwin, plasma-devel. · View Herald Transcript
sebas added a subscriber: sebas.Oct 4 2016, 2:20 PM
sebas added inline comments.
debug_console.cpp
547

return?

luebking added inline comments.
debug_console.cpp
547

Why not reimplment the hideEvent?

graesslin added inline comments.Oct 4 2016, 2:30 PM
debug_console.cpp
547

I tried implementing the hideEvent, but for whatever reason it didn't get triggered.

luebking added inline comments.Oct 4 2016, 2:37 PM
debug_console.cpp
547

What about closeEvent?

graesslin updated this revision to Diff 7091.Oct 4 2016, 2:42 PM

Added missing return

graesslin added inline comments.Oct 4 2016, 2:45 PM
debug_console.cpp
547

closeEvent is also not invoked.

luebking added inline comments.Oct 4 2016, 2:45 PM
debug_console.cpp
546

if (!visible) deleteLater(); ... ;-P

547

iow QWidget is bitrot. Maybe better use QML ... :-(

graesslin added inline comments.Oct 4 2016, 3:24 PM
debug_console.cpp
547

It might be a bug in KWin's QPA, so I wouldn't blame Qt here (yet).

sebas accepted this revision.Oct 4 2016, 3:28 PM
sebas added a reviewer: sebas.

With Thomas' comment about, if you also think that's better.

This revision is now accepted and ready to land.Oct 4 2016, 3:28 PM
This revision was automatically updated to reflect the committed changes.
luebking added inline comments.Oct 16 2016, 4:36 PM
debug_console.cpp
547

Wouldn't that prevent the QWindow signal just as well?

graesslin added inline comments.Oct 17 2016, 5:29 AM
debug_console.cpp
547

Maybe, maybe not. That's the point I haven't investigated it and it might be that an additional call is required for QWidget to be supported.