unity-mir

Merge lp://qastaging/~mterry/unity-mir/dont-hide-focused-apps into lp://qastaging/unity-mir

dont-hide-focused-apps
Merge into trunk

Proposed by Michael Terry on 2014-05-19

Status:	Merged
Approved by:	Gerry Boland on 2014-05-26
Approved revision:	226
Merged at revision:	224
Proposed branch:	lp://qastaging/~mterry/unity-mir/dont-hide-focused-apps
Merge into:	lp://qastaging/unity-mir
Diff against target:	188 lines (+88/-10) 4 files modified src/modules/Unity/Application/application.cpp (+0/-4) src/modules/Unity/Application/application_manager.cpp (+12/-5) src/modules/Unity/Application/application_manager.h (+1/-1) tests/application_manager_test.cpp (+75/-0)
To merge this branch:	bzr merge lp://qastaging/~mterry/unity-mir/dont-hide-focused-apps
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Gerry Boland (community)		2014-05-19	Approve on 2014-05-26
PS Jenkins bot (community)	continuous-integration		Approve on 2014-05-26
Review via email: mp+220130@code.qastaging.launchpad.net

Commit message

Separate suspend logic from visibility logic, to allow top app to remain visible even when suspended.

Description of the change

Separate suspend logic from visibility logic, to allow top app to remain visible even when suspended.

When testing the split greeter, Saviq noticed that the user session would go black if an app was open while the greeter was up (sliding the greeter lets you peek at user session).

This is because unity-mir hides() app sessions when they are suspended to optimized compositing. But the top app should remain visible for cross-session compositing like the greeter wants to do.

So don't automatically hide() when suspending, but instead only hide() if we are unfocusing an app to bring another app above it.

(We still automatically show() when resuming because we want to ensure that the app is visible now.)

== Checklist ==
* Are there any related MPs required for this MP to build/function as expected? Please list.
- No

* 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?
- NA

lp://qastaging/~mterry/unity-mir/dont-hide-focused-apps updated on 2014-05-19

222. By Michael Terry on 2014-05-19: Separate suspend logic from visibility logic, to allow top app to remain visible even when suspended.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-19:

PASSED: Continuous integration, rev:222
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/344/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-amd64-ci/8
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-armhf-ci/8
        deb: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-armhf-ci/8/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-i386-ci/8

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

review: Approve (continuous-integration)

Revision history for this message

Gerry Boland (gerboland) wrote on 2014-05-20:

It reads strange that you call setVisible(true) in Application, but setVisible(false) in ApplicationManager. Could you make this more consistent please?

I'd appreciate you adding a test or two, to verify nothing breaks.

review: Needs Fixing

lp://qastaging/~mterry/unity-mir/dont-hide-focused-apps updated on 2014-05-23

223. By Michael Terry on 2014-05-22: Move setVisible call to application manager

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-23:

PASSED: Continuous integration, rev:223
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/351/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-amd64-ci/15
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-armhf-ci/15
        deb: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-armhf-ci/15/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-i386-ci/15

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

review: Approve (continuous-integration)

lp://qastaging/~mterry/unity-mir/dont-hide-focused-apps updated on 2014-05-23

224. By Michael Terry on 2014-05-23: Add some tests

Revision history for this message

Michael Terry (mterry) wrote on 2014-05-23:

OK, setVisible calls moved out, and a couple tests added.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-23:

PASSED: Continuous integration, rev:224
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/352/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-amd64-ci/16
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-armhf-ci/16
        deb: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-armhf-ci/16/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-i386-ci/16

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

review: Approve (continuous-integration)

Revision history for this message

Gerry Boland (gerboland) wrote on 2014-05-26:

+ std::shared_ptr<mir::scene::Session> first_session = std::make_shared<MockSession>("Oo", a_procId);
The "Oo" not needed, jut an empty string will do

+ EXPECT_EQ(true, the_app->visible());
Could you please switch the arguments around?

Am testing now

lp://qastaging/~mterry/unity-mir/dont-hide-focused-apps updated on 2014-05-26

225. By Michael Terry on 2014-05-26: Simplify test

Revision history for this message

Michael Terry (mterry) wrote on 2014-05-26:

> The "Oo" not needed, jut an empty string will do

Ah, OK. Fixed. Just copy and paste junk.

> Could you please switch the arguments around?

Really? Looking at the other tests, it seems common to have a "static value" on left and the interesting call on right.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-26:

PASSED: Continuous integration, rev:225
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/360/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-amd64-ci/24
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-armhf-ci/24
        deb: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-armhf-ci/24/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-i386-ci/24

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

review: Approve (continuous-integration)

Revision history for this message

Gerry Boland (gerboland) wrote on 2014-05-26:

> > Could you please switch the arguments around?
>
> Really? Looking at the other tests, it seems common to have a "static value"
> on left and the interesting call on right.
Yeah sorry I know, I want to standardize things to the "value to check" and "correct value" order - see the newer tests I added recently.

lp://qastaging/~mterry/unity-mir/dont-hide-focused-apps updated on 2014-05-26