Mir

Merge lp://qastaging/~afrantzis/mir/fix-1511798-alternative into lp://qastaging/mir

fix-1511798-alternative
Merge into development-branch

Proposed by Alexandros Frantzis on 2015-11-06

Status:	Rejected
Rejected by:	Alexandros Frantzis on 2015-11-12
Proposed branch:	lp://qastaging/~afrantzis/mir/fix-1511798-alternative
Merge into:	lp://qastaging/mir
Diff against target:	280 lines (+156/-18) 5 files modified src/server/scene/mediating_display_changer.cpp (+1/-1) src/server/shell/abstract_shell.cpp (+20/-11) tests/acceptance-tests/test_display_configuration.cpp (+3/-3) tests/acceptance-tests/test_nested_mir.cpp (+82/-0) tests/unit-tests/scene/test_abstract_shell.cpp (+50/-3)
To merge this branch:	bzr merge lp://qastaging/~afrantzis/mir/fix-1511798-alternative
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Alan Griffiths		2015-11-06	Needs Fixing on 2015-11-06
PS Jenkins bot (community)	continuous-integration		Needs Fixing on 2015-11-06
Review via email: mp+276852@code.qastaging.launchpad.net

Commit message

shell: fix updating of display configuration when applications and nested servers exit

Description of the change

shell: fix updating of display configuration when applications and nested servers exit

The tests for nested mir behavior were cherrypicked from Alan's original proposal.

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2015-11-06:

I'm confused.

When I try the scenario in lp:1511798 I don't see an attempt to restore the host config after exiting the client. (But that's what display_configuration_reset_when_application_exits is supposed to test.)

I need to check what is different between the automated and manual test scenarios.

review: Abstain

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-11-06:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5517/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2015-11-06:

> I'm confused.
>
> When I try the scenario in lp:1511798 I don't see an attempt to restore the
> host config after exiting the client. (But that's what
> display_configuration_reset_when_application_exits is supposed to test.)
>
> I need to check what is different between the automated and manual test
> scenarios.

I understand the difference - AbstractShell::focus_next_session() is selecting the dying surface's titlebar. (And that doesn't exist in the core WM that gets tested.)

I have to think about the right solution here.

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2015-11-06:

I think the right solution is to move the "session->destroy_surface(surface);" call from AbstractShell::destroy_surface() to <Policy>::handle_delete_surface(). (There are four of these, plus DefaultWindowManager::remove_surface() in playground.)

It seems like a PITA, but this allows the window management policy control over the point at which the surface and decorations are destroyed.

review: Needs Fixing

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2015-11-06:

Like this: lp:~alan-griffiths/mir/fix-1511798-another-alternative

Revision history for this message

Alexandros Frantzis (afrantzis) wrote on 2015-11-12:

Rejected in favor of the other alternative.