Reset last_active_client when a ShellClient is removed
ClosedPublic

Authored by graesslin on Jul 23 2017, 2:21 PM.

Details

Summary

The last_active_client is set when an AbstractClient gets activated. For
the X11 case the last_active_client was getting reset to nullptr when
the last_active_client gets destroyed. But for the ShellClient that did
not yet happen. This could result in a crash.

This change addresses the problem and adds a test case which triggered
the crash. The condition of the crash are difficult to generate though -
it took me about an hour to write the test for the crash.

  1. Wayland client must be active
  2. Explicit focus to null (no active client)
  3. destroy Wayland window
  4. X11 client which sets focus on itself without interaction with window manager
Test Plan

test case no longer crashes

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 created this revision.Jul 23 2017, 2:21 PM
Restricted Application added a project: KWin. · View Herald TranscriptJul 23 2017, 2:21 PM
Restricted Application added subscribers: kwin, plasma-devel. · View Herald Transcript
davidedmundson accepted this revision.Jul 23 2017, 2:41 PM
davidedmundson added a subscriber: davidedmundson.

there's 3 other differences between removeClient that I think should also be here:


should_get_focus.removeAll(c);

(this looks like it could lead to a similar crash)


if (c == most_recently_raised)
    most_recently_raised = 0;

#ifdef KWIN_BUILD_TABBOX

if (tabBox->isDisplayed())
    tabBox->reset(true);

#endif

This revision is now accepted and ready to land.Jul 23 2017, 2:41 PM

there's 3 other differences between removeClient that I think should also be here:

yep I know, guess what I will work on next ;-)

should_get_focus is btw. currently not yet a problem as a ShellClient never gets added to that list.

This revision was automatically updated to reflect the committed changes.