Merge lp:~dandrader/qtmir/appRestart-lp1527737 into lp:qtmir

[Daniel d'Andrada]

Handle case where TaskController requests an application that is already closing

Happens when you quickly close an application and then tap on its icon in the dash

* Are there any related MPs required for this MP to build/function as expected? Please list.
Not strictly needed but I haven't tried without it:
https://code.launchpad.net/~dandrader/qtubuntu/offscreenSurface-lp1527737/+merge/281638

 * Did you perform an exploratory manual test run of your code change and any related functionality?
Yes, on phone and desktop.

 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
Not applicable.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:436
http://jenkins.qa.ubuntu.com/job/qtmir-ci/628/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/322
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/324
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/324/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/204
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/359/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/361/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/204/console

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:437
http://jenkins.qa.ubuntu.com/job/qtmir-ci/629/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/323
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/325
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/325/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/205
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/360/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/362/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/205/console

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

review: Needs Fixing (continuous-integration)

[Gerry Boland (gerboland)]

=== modified file 'src/modules/Unity/Application/application_manager.cpp'
+ // We interpret this as a focus request for a suspended app.
+ // Shell will have this app resumed if it complies with the focus request
     QObject::connect(controller, &TaskController::resumeRequested,
- manager, &ApplicationManager::onResumeRequested);
+ manager, &ApplicationManager::onFocusRequested);

The original purpose of the resumeRequested signal from UAL was to tell AppMan to un-suspend a suspended app, since AppMan used to manage that. But since UAL does suspend/resume now itself, this signal is worthless. Suspect we should remove completely.

Does not the focusRequested signal always get fired if url-dispatcher wants to focus an app? In which case, is this interpretation even necessary? If we can, I think we should remove it.

+ << "ApplicationManager::onProcessStarting(appId="<<appId<<") - User wants to start"
+ " a new instance of an application that is still closing. Queuing its start.";
we should investigate how to launch new instance immediately, instead of waiting for the closing one to quit. I'd be worried if the closing app crashes during close, as it won't be considered closed until apport has completed collecting the crash dump, causing the new instance a long delay. Not for this MP, but perhaps a TODO?

Also nitpick: could you line up the qCDebug strings, in both spots in this method?

+ // TaskController doesn't give us any PID so we have no way of telling apart two instances
+ // of an application.
we should get that fixed. UAL must have way of dealing with multiple instances of the same App, and distinguishing them. If not, we'll need to add it.

+1 on the header for the test. We should do that from now on.

+ // DeferredDelete is special: likes to be called out specifically or it won't come out
weird?? Event loop blocked somehow?

Nice work

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:438
http://jenkins.qa.ubuntu.com/job/qtmir-ci/632/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/326
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/328
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/328/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/208
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/363/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/365/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/208/console

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:440
http://jenkins.qa.ubuntu.com/job/qtmir-ci/635/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/329
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/331
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/331/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/211
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/366/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/368/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/211/console

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

review: Needs Fixing (continuous-integration)

[Daniel d'Andrada (dandrader)]

On 07/01/2016 14:39, Gerry Boland wrote:
> Review: Needs Fixing
>
> === modified file 'src/modules/Unity/Application/application_manager.cpp'
> + // We interpret this as a focus request for a suspended app.
> + // Shell will have this app resumed if it complies with the focus request
> QObject::connect(controller, &TaskController::resumeRequested,
> - manager, &ApplicationManager::onResumeRequested);
> + manager, &ApplicationManager::onFocusRequested);
>
>
> The original purpose of the resumeRequested signal from UAL was to tell AppMan to un-suspend a suspended app, since AppMan used to manage that. But since UAL does suspend/resume now itself, this signal is worthless. Suspect we should remove completely.
>
> Does not the focusRequested signal always get fired if url-dispatcher wants to focus an app? In which case, is this interpretation even necessary? If we can, I think we should remove it.

When you tap on a suspended app in the dash you immediately get a
appStarting followed by a resumeRequest. If you don't respond to it (ie,
resume&focus the app), ages later you get a focusRequested.

So if I remove resumeRequested I would have to interpret appStarting for
a already running application as a focus request, which actually
wouldn't be wrong.

Btw, I find it odd that ubuntu_app_launch work that way, always sending
an appStarting signal even though the app is already running, but that's
the way it is...

> + << "ApplicationManager::onProcessStarting(appId="<<appId<<") - User wants to start"
> + " a new instance of an application that is still closing. Queuing its start.";
> we should investigate how to launch new instance immediately, instead of waiting for the closing one to quit. I'd be worried if the closing app crashes during close, as it won't be considered closed until apport has completed collecting the crash dump, causing the new instance a long delay. Not for this MP, but perhaps a TODO?

Agreed. The delay is noticeable. Added the TODO comment.

>
> Also nitpick: could you line up the qCDebug strings, in both spots in this method?

Wrapped the qCDebug boilerplate in a macro.

>
>
> + // TaskController doesn't give us any PID so we have no way of telling apart two instances
> + // of an application.
> we should get that fixed. UAL must have way of dealing with multiple instances of the same App, and distinguishing them. If not, we'll need to add it.

As far as I understand it ubuntu-app-launch (and the upstart service?)
have the concept that you can have only one running instance of an
application (like a service, that you start/stop using upstart).

This sounds like a bigger architectural change, out of scope for this MP.

> + // DeferredDelete is special: likes to be called out specifically or it won't come out
> weird?? Event loop blocked somehow?
>

QObject::deleteLater() generates a DeferredDelete event which is handled
in a special way in QCoreApplication etc, unlike any other QEvent.
There's code in there specifically for leaving it out of forced event
processing (like sendPostedEvents) unless you explicitly ask for that
particular event type.

[Gerry Boland (gerboland)]

> On 07/01/2016 14:39, Gerry Boland wrote:
> > Review: Needs Fixing
> >
> > === modified file 'src/modules/Unity/Application/application_manager.cpp'
> > + // We interpret this as a focus request for a suspended app.
> > + // Shell will have this app resumed if it complies with the focus
> request
> > QObject::connect(controller, &TaskController::resumeRequested,
> > - manager, &ApplicationManager::onResumeRequested);
> > + manager, &ApplicationManager::onFocusRequested);
> >
> >
> > The original purpose of the resumeRequested signal from UAL was to tell
> AppMan to un-suspend a suspended app, since AppMan used to manage that. But
> since UAL does suspend/resume now itself, this signal is worthless. Suspect we
> should remove completely.
> >
> > Does not the focusRequested signal always get fired if url-dispatcher wants
> to focus an app? In which case, is this interpretation even necessary? If we
> can, I think we should remove it.
>
> When you tap on a suspended app in the dash you immediately get a
> appStarting followed by a resumeRequest. If you don't respond to it (ie,
> resume&focus the app), ages later you get a focusRequested.
>
> So if I remove resumeRequested I would have to interpret appStarting for
> a already running application as a focus request, which actually
> wouldn't be wrong.
>
> Btw, I find it odd that ubuntu_app_launch work that way, always sending
> an appStarting signal even though the app is already running, but that's
> the way it is...

Fair enough. This will do for now.

>

> >
> >
> > + // TaskController doesn't give us any PID so we have no way of telling
> apart two instances
> > + // of an application.
> > we should get that fixed. UAL must have way of dealing with multiple
> instances of the same App, and distinguishing them. If not, we'll need to add
> it.
>
> As far as I understand it ubuntu-app-launch (and the upstart service?)
> have the concept that you can have only one running instance of an
> application (like a service, that you start/stop using upstart).
>
> This sounds like a bigger architectural change, out of scope for this MP.

Oh of course. Was just observation.

> > + // DeferredDelete is special: likes to be called out specifically or it
> won't come out
> > weird?? Event loop blocked somehow?
> >
>
> QObject::deleteLater() generates a DeferredDelete event which is handled
> in a special way in QCoreApplication etc, unlike any other QEvent.
> There's code in there specifically for leaving it out of forced event
> processing (like sendPostedEvents) unless you explicitly ask for that
> particular event type.

Interesting, I didn't know that.

[Gerry Boland (gerboland)]

Just ran UITK AP tests to ensure sanity, no issues found. Happy to approve

review: Approve

[Michał Sawicz (saviq)]

QA found a problem with this and I can confirm on devel-proposed/mako.

Trying to follow steps from Victor [1] I got into a state where unity8 rejects the camera app:

qtmir.applications: ApplicationManager::onProcessStarting(appId="com.ubuntu.camera_camera") - User wants to start a new instance of an application that is still closing and is already queued to start later.
qtmir.mir: SessionAuthorizer::connection_is_allowed - this= SessionAuthorizer(0xb16defe4) pid= 22192
qtmir.applications: ApplicationManager::authorizeSession - pid= 22192
ApplicationManager REJECTED connection from app with pid 22192 as it was not launched by upstart, and no desktop_file_hint is specified
qtmir.applications: ApplicationManager::onProcessFailed - appId= "com.ubuntu.camera_camera"
qtmir.applications: ApplicationManager::onProcessStopped - appId= "com.ubuntu.camera_camera"

Victor can confirm the same happened to him with camera and settings apps.

[1] https://trello.com/c/nAccJ8Qd/2639-854-ubuntu-landing-030-gsettings-qt-unity8-qtmir-qtubuntu-saviq

review: Needs Fixing

[Daniel d'Andrada (dandrader)]

On 12/01/2016 14:39, Michał Sawicz wrote:
> Review: Needs Fixing
>
> QA found a problem with this and I can confirm on devel-proposed/mako.
>
> Trying to follow steps from Victor [1] I got into a state where unity8 rejects the camera app:
>
> qtmir.applications: ApplicationManager::onProcessStarting(appId="com.ubuntu.camera_camera") - User wants to start a new instance of an application that is still closing and is already queued to start later.
> qtmir.mir: SessionAuthorizer::connection_is_allowed - this= SessionAuthorizer(0xb16defe4) pid= 22192
> qtmir.applications: ApplicationManager::authorizeSession - pid= 22192
> ApplicationManager REJECTED connection from app with pid 22192 as it was not launched by upstart, and no desktop_file_hint is specified
> qtmir.applications: ApplicationManager::onProcessFailed - appId= "com.ubuntu.camera_camera"
> qtmir.applications: ApplicationManager::onProcessStopped - appId= "com.ubuntu.camera_camera"
>
> Victor can confirm the same happened to him with camera and settings apps.
>
> [1] https://trello.com/c/nAccJ8Qd/2639-854-ubuntu-landing-030-gsettings-qt-unity8-qtmir-qtubuntu-saviq

I followed the steps below:

"""

  * Open messaging app.
  * Slide up to create a new message.
  * Tap on the camera icon.
  * In content hub tap on the camera app.
  * Take a photo in the camera app.
  * Tap on the big tick. Returned to the messaging app with the image in
    the text window.
  * Swipe in from the right to reveal the spread.
  * Open spread.
  * Switch to the camera app.

Expected result:

  * Camera app should display the live view.

Actual result:

  * Camera app shows a black screen

"""

A second after switching to camera-app it goes away.
Log shows that camera-app quit (closed itself). Which is expected as it
was summoned by the content hub as a helper and was no longer needed.
Different from the log you got.

In this situation there's nothing unity8 can do. If the app closes
itself shortly after it's focused, I don't think unity8 should go and
restart it.

[Daniel d'Andrada (dandrader)]

Furthermore, in this "messaging -> content-hub -> camera-app" use case, unity8 is *not* asking camera-app to close itself, unlike the use case of the bug report which is what this MP is fixing.

From unity8's point of view, camera-app is simply quitting on its own accord.

[Daniel d'Andrada (dandrader)]

it probably shouldn't even be in the spread in the first place, like if it were a surface from a prompt helper session.

[Michał Sawicz (saviq)]

Then you didn't get the issue I described. There are no reliable steps to reproduce, but after I followed those steps (and camera crashed due to bug #1524131, I couldn't start camera at all after it closed. The above messages were printed in unity8's log, not how it says that camera was not started by upstart and that's why it's rejected. But camera was, in fact, started by upstart, from the apps scope. Victor reported this same problem with settings app, so unrelated to content hub. I've not seen it happen without this change.

[Unity8 CI Bot (unity8-ci-bot)]

FAILED: Continuous integration, rev:440
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-1-ci/3/
Executed test runs:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-1-ci/3/rebuild

review: Needs Fixing (continuous-integration)

[Daniel d'Andrada (dandrader)]

I just can't reproduce it. I'm stuck. Added the handling of a special case though, where upstart would send a bogus signal.

Could you please install the latest version of this patch + lp:~dandrader/qtubuntu/offscreenSurface-lp1527737 and then send me the log of this failure? Ah, and not just the last few lines!

[Unity8 CI Bot (unity8-ci-bot)]

FAILED: Continuous integration, rev:441
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-1-ci/13/
Executed test runs:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-1-ci/13/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:441
http://jenkins.qa.ubuntu.com/job/qtmir-ci/638/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/332/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/334/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/214/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/369/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/371/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/214/console

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

review: Needs Fixing (continuous-integration)

[Michał Sawicz (saviq)]

Eh, you'll need to rebase this one, too...

review: Needs Fixing

[Unity8 CI Bot (unity8-ci-bot)]

FAILED: Continuous integration, rev:444
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-1-ci/47/
Executed test runs:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-1-ci/47/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:444
http://jenkins.qa.ubuntu.com/job/qtmir-ci/671/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/365/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/367/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/247/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/402/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/404/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/247/console

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

review: Needs Fixing (continuous-integration)

[lp:~dandrader/qtmir/appRestart-lp1527737]

439. By Nick Dedekind on 2016-02-08

Added fix and test for closing app during the StoppedResumable state. Fixes: #1541388
Approved by: Michael Zanetti

440. By CI Train Bot Account on 2016-02-08

Releasing 0.4.7+16.04.20160208-0ubuntu1

441. By Nick Dedekind on 2016-02-12

Moved test framework into a static library for quicker recompilation.
Approved by: Gerry Boland

442. By Daniel d'Andrada on 2016-02-12

Remove the useless TaskController

It was just forwarding calls between ApplicationManager and ApplicationCrontroller.
Had no logic of its own.

ApplicationCrontroller was then renamed to TaskController as the latter has a better API and it also keeps ApplicationManager code more or less untouched.

Tests have been improved a bit by better emulating TaskController behavior
Approved by: Nick Dedekind

443. By Daniel d'Andrada on 2016-02-12

Let shell decide the initial surface size Fixes: #1532974
Approved by: Nick Dedekind

444. By Daniel d'Andrada on 2016-02-12

Surface Size Hints
Approved by: Lukáš Tinkl

445. By Albert Astals Cid on 2016-02-12

Reset start time if the timestamp travels to the past
 Fixes: #1524488
Approved by: Nick Dedekind

446. By Albert Astals Cid on 2016-02-12

Provide branch prediction information to the if in compressTimestamp
Approved by: Gerry Boland

447. By Michał Sawicz on 2016-02-12

Use QStandardPaths to determine QML cache location

Also use temporary XDG_CACHE_HOME in ApplicationManagerTests so we don't pollute user's $HOME
Approved by: Unity8 CI Bot, Albert Astals Cid

448. By Daniel d'Andrada on 2016-02-12

Update mir version requirement
Approved by: Nick Dedekind

449. By Michał Sawicz on 2016-02-12

Bump unity-api dependencies
Approved by: Michael Terry

450. By CI Train Bot Account on 2016-02-12

Releasing 0.4.7+16.04.20160212-0ubuntu1

451. By Alan Griffiths on 2016-02-19

Mir 0.20 Release
Approved by: Cemil Azizoglu

452. By CI Train Bot Account on 2016-02-19

Releasing 0.4.7+16.04.20160219-0ubuntu1

[Gerry Boland (gerboland)]

/me picks this up again. Will see what light I can shed on the thing QA found

[Gerry Boland (gerboland)]

Eep, lots of conflicts with trunk

review: Needs Fixing

[Gerry Boland (gerboland)]

I fixed conflicts here: lp:~gerboland/qtmir/appRestart-lp1527737-rebased save you the trouble

[Daniel d'Andrada (dandrader)]

On 08/03/2016 12:15, Gerry Boland wrote:
> I fixed conflicts here: lp:~gerboland/qtmir/appRestart-lp1527737-rebased save you the trouble

Dednick had some hot fixes on problems similar to this one recently.
Which makes me think how relevant this MP still is (haven't spent any
time perusing it though).
And I also made a lot of changes in lifecycle code in the surface-based
window management work. If we want to proceed with this MP I would
rather put it on top of the surface WM work. It's like a whole new world
(surface-based versus application/single-surface based lifecycle)

[lp:~dandrader/qtmir/appRestart-lp1527737]

453. By Gerry Boland on 2016-03-10

Allow Mir remove command line arguments it understands, before letting Qt process them.
Approved by: Daniel d'Andrada

454. By Alan Griffiths on 2016-03-10

Copy the Window Management work-in-progress from Mir examples.
Approved by: Gerry Boland

455. By Michael Zanetti on 2016-03-10

Add a dedicated property for the inputMethodSurface

When we're hot-plugging screens, the OSK ui might only be created after surfaceCreated() is emitted and we miss that signal. Fixes: #1545286
Approved by: Daniel d'Andrada

456. By Gerry Boland on 2016-03-10

Screen: only enable orientation sensor for internal display.

It's not great, but works around orientation sensor not working after device unplug from monitor. I believe this is because QtUbuntu-Sensors and Platform-api are not dealing properly with multiple accessors of the sensors. Instead only the last accessor gets correct state, and the earlier ones get nothing. Fixes: #1545286
Approved by: Unity8 CI Bot, Michael Zanetti

457. By Daniel d'Andrada on 2016-03-10

Ensure QmlEngine doesn't delete our MirSurfaces
Approved by: Gerry Boland, Unity8 CI Bot

458. By Gerry Boland on 2016-03-10

Ensure ScreenWindow geometry correctly set and used after moving Screen

NB: this will conflict with lp:~unity-team/qtmir/set-display-config, from which this is a cherry-pick Fixes: #1545286
Approved by: Gerry Boland, Unity8 CI Bot

459. By Michael Terry on 2016-03-10

Fix some mocks to use more realistic code paths.

Specifically, MockApplicationController::doAppIdHasProcessId() was returning -1 if the appId was unknown. But that function returns a bool, so the -1 got translated into a true value. So that mocked method returned true for appIds that hadn't been registered yet.

In order to fix that, I decided to rewrite doAppIdHasProcessId to call primaryPidForAppId so that it could be overridden easier in tests. And adjusted the affected tests to all set an ON_CALL(primaryPidForAppId).

(Because otherwise the test has no way to influence the pid used for children -- why do we actually fork off a while(true) process?)

There are several other fixes here for pieces of the mocks that I found were missing while exploring solutions for the above problem:

- I'm actually not 100% clear on why this started being called, but MockSession::name() couldn't create a default value for QString and the gtest framework was bailing when it was called. So I added a default.

- Similarly, if ApplicationManager::authorizeSession() tries to examine an app's commandline, gtest will abort because MockProcInfo::command_line() doesn't have a mock return value. So I added a default.

- And if authorizeSession() lasted long enough to see that the Application object already existed, it deleted the desktop reader object it had made. But the test mocks always returned the same object, so this caused a segfault. I retooled the ApplicationManager tests to return a new object each time DesktopFileReaderFactory::createInstance was called.
Approved by: Gerry Boland, Unity8 CI Bot

460. By Alberto Aguirre on 2016-03-10

Hook MirOpenGLContext::doneCurrent to Screen::doneCurrent

Actually unbind the opengl/egl context when requested.
Approved by: Gerry Boland

461. By Michał Sawicz on 2016-03-10

Drop dummy autopkgtest

It doesn't make sense after all - DEP-8 tests are meant to test packages as-installed, not rebuild them from scratch. This meant that new Mir releases got blocked on this, because old qtmir was trying to build against new Mir, which usually fails, as needs adaptation for API changes.
Approved by: Gerry Boland

462. By CI Train Bot Account on 2016-03-10

Releasing 0.4.7+16.04.20160310.1-0ubuntu1

463. By Nick Dedekind on 2016-03-22

Added setStage for sidestage redesign and removed ExecFlags
Approved by: Daniel d'Andrada

464. By Michał Sawicz on 2016-03-22

Add support for low shell chrome. Fixes: #1535397
Approved by: Gerry Boland

465. By Michał Sawicz on 2016-03-22

Support for switching keyboard layouts Fixes: #1491340, #1524400
Approved by: Michael Terry

466. By CI Train Bot Account on 2016-03-22

Releasing 0.4.8+16.04.20160322.1-0ubuntu1

467. By Daniel van Vugt on 2016-03-29

Don't use preferred_mode_index. Some devices like TVs don't provide a
preferred mode, so preferred_mode_index may be out of range (a clumsy
way to represent 'none'). Instead use the resolution of the current display mode, which should always be in range.

Fixes LP: #1560497 Fixes: #1560497
Approved by: Gerry Boland

468. By CI Train Bot Account on 2016-03-29

Releasing 0.4.8+16.04.20160329.1-0ubuntu1

469. By Michał Sawicz on 2016-03-30

Drop leftover Xs-Testsuite header
Approved by: Unity8 CI Bot, Gerry Boland

470. By CI Train Bot Account on 2016-03-30

Releasing 0.4.8+16.04.20160330-0ubuntu1

471. By Daniel d'Andrada on 2016-04-13

Logging of Qt's OpenGL debug messages now must be enabled via CMake option
Approved by: Gerry Boland

472. By Daniel d'Andrada on 2016-04-13

Application: improve debug logging

Minimize boilerplate in logging code, as well as standardise its format.
Approved by: Gerry Boland

473. By Daniel d'Andrada on 2016-04-13

Remove application screenshot provider

It's no longer needed now that QML provides a "item-snapshot" feature.

Besides, it has no purpose in a surface-based window management.
Approved by: Lukáš Tinkl, Gerry Boland

474. By Daniel d'Andrada on 2016-04-13

MirSurface: replace keymapLayout and keymapVariant with keymap

That's easier for unity8 to use, leads to declarative code.
Approved by: Lukáš Tinkl

475. By Daniel d'Andrada on 2016-04-13

Surface-based window management

- Session is no longer exported to QML. It's now an internal qtmir concept.
Approved by: Gerry Boland

476. By Michał Sawicz on 2016-04-13

Inline -gles packaging.
Approved by: Gerry Boland

477. By CI Train Bot Account on 2016-04-13

Releasing 0.4.8+16.04.20160413-0ubuntu1

478. By Alberto Aguirre on 2016-04-26

Mir 0.22 compatibility
Approved by: Cemil Azizoglu, Kevin DuBois, Brandon Schaefer

479. By CI Train Bot Account on 2016-04-26

Releasing 0.4.8+16.04.20160426-0ubuntu1

480. By Michael Terry on 2016-04-28

Use latest UbuntuAppLaunch API which supports libertine apps.
Approved by: Nick Dedekind

481. By Lukáš Tinkl on 2016-04-28

Regression fix: restore window title handling Fixes: #1563522
Approved by: Nick Dedekind

482. By CI Train Bot Account on 2016-04-28

Releasing 0.4.8+16.04.20160428-0ubuntu1

483. By Gerry Boland on 2016-04-29

Enhance ScreenController & the DisplayConfigurationPolicy to implement dynamic grid units.

- Rename ScreenController to ScreenModel, as it just reflects current screen state, does not offer means to configure it
- ScreenController can update state of existing Screens, based on Mir DisplayConfiguration changes.
- Expand Screen to include scale & form factor properties, with getter/notifier in NativeInterface. This enables the dynamic grid units in the shell
- Add a Unity.Screens qml module to give QML better information about connected screens, and allow basic reconfiguring.
- Implement a basic display configuration policy to set suitable scale and form factor on an external display (needed for dynamic grid units) Fixes: #1573532
Approved by: Unity8 CI Bot, Daniel d'Andrada

484. By CI Train Bot Account on 2016-04-29

Releasing 0.4.8+16.04.20160429.1-0ubuntu1

485. By Daniel d'Andrada on 2016-05-11

Session: Add a blank surface to the public list if it already has child prompt surfaces

A prompt session may come in too early, before its parent session got a chance
to draw its surface's first frame.

Normally that wouldn't be a problem and we could withhold that parent surface until
it's finally drawn to. But unfortunately the application process might be blocked,
unable to draw anything, until its child prompt session gets dismissed.

Because of that we have no option but to expose this blank surface to shell so that
it can display it along with the prompt surface on top of it, so that the user can
interact with it right away and finally unblock the application. Fixes: #1578665
Approved by: Unity8 CI Bot

486. By CI Train Bot Account on 2016-05-11

Releasing 0.4.8+16.04.20160511-0ubuntu1

487. By Daniel d'Andrada on 2016-05-18

Fix ProxySurfaceListModel::countChanged signal
Approved by: Unity8 CI Bot

488. By Daniel d'Andrada on 2016-05-18

Move prompt surfaces from MirSurface to Application and emit firstChanged signal

489. By CI Train Bot Account on 2016-05-18

Releasing 0.4.8+16.04.20160518.1-0ubuntu1

490. By Gerry Boland on 2016-05-20

UAL can throw on creating Application if invalid appId, catch instead of aborting Fixes: #1578258
Approved by: Unity8 CI Bot, Michael Terry

491. By Daniel d'Andrada on 2016-05-20

Application: Don't respawn if closed while still starting up

+ refactoring of Application::onSessionStateChanged to explicitly cover every single internal state on session stopping. Fixes: #1575577
Approved by: Gerry Boland, Unity8 CI Bot, Michael Terry

492. By Alan Griffiths on 2016-05-20

Reduce coupling to MirServer - it has been misused as a context object.
Approved by: Alan Griffiths, Unity8 CI Bot, Daniel d'Andrada

493. By Michał Sawicz on 2016-05-20

Centralize logging categories

The header part is already centralized. Make the cpp the same.

Moves all logging category symbols to the mirserver lib.

Enables mirserver to use logging categories that were previously declared in the Unity.Application module.
Approved by: Unity8 CI Bot, Michael Terry

494. By Daniel d'Andrada on 2016-05-20

Also interpret the cursor names used by Mir client API Fixes: #1447839
Approved by: Unity8 CI Bot, Michał Sawicz

495. By Timo Jyrinki on 2016-05-20

Use FindQt5PlatformSupport to find platform support, other methods not available on Qt 5.6 anymore. Fixes: #1554404
Approved by: Gerry Boland

496. By CI Train Bot Account on 2016-05-20

Releasing 0.4.8+16.10.20160520.1-0ubuntu1

497. By Daniel d'Andrada on 2016-05-25

Bump unity-api versions
Approved by: Lukáš Tinkl

498. By Alan Griffiths on 2016-05-25

Drop the (unused) prototype Window Management code
Approved by: Gerry Boland, Unity8 CI Bot

499. By Daniel d'Andrada on 2016-05-25

Fix mir::scene::Surface focus attribute updates
Approved by: Unity8 CI Bot

500. By Albert Astals Cid on 2016-05-25

Enable workaround_brokenFBOReadBack on various chipsets

Gives for impressive improvements in QSGDefaultDistanceFieldGlyphCache::storeGlyphs
impressive = 788ms -> 6ms

Copied from qtubuntu
 Fixes: #1581123
Approved by: Gerry Boland, Unity8 CI Bot

501. By Alan Griffiths on 2016-05-25

Remove workaround for lp:1502200
Approved by: Unity8 CI Bot, Daniel d'Andrada

502. By CI Train Bot Account on 2016-05-25

Releasing 0.4.8+16.10.20160525.2-0ubuntu1

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

505. By Michael Zanetti on 2016-06-14

Adding ApplicationInfo::surfaceCount property

506. By Albert Astals Cid on 2016-06-14

Add support for compiler sanitizers via ECM

507. By Albert Astals Cid on 2016-06-14

Compile with -fsanitize=undefined

508. By Albert Astals Cid on 2016-06-14

Fix leak in ScreensModelTest (LP: #1585502)

509. By Albert Astals Cid on 2016-06-14

Fix leak in SessionManager test (LP: #1585498)

510. By Albert Astals Cid on 2016-06-14

Remove unused m_sessions member

511. By Albert Astals Cid on 2016-06-14

Add missing breaks

512. By Albert Astals Cid on 2016-06-14

Initialize m_lastX and m_lastY

513. By Albert Astals Cid on 2016-06-14

Initialize m_sessionId

514. By Albert Astals Cid on 2016-06-14

Give the locker a name

515. By Albert Astals Cid on 2016-06-14

Fix memory leak in QtEventFeederTest (LP: #1585503)

516. By Albert Astals Cid on 2016-06-14

Fix leaks in application_manager_test (LP: #1585501)

517. By Daniel d'Andrada on 2016-06-14

Improve Session debug logging

518. By Daniel d'Andrada on 2016-06-14

MirSurfaceListModel: prepending a surface always causes firstChanged() emission

519. By Gerry Boland on 2016-06-14

Release temporary GL context ASAP, fixes QtMir on X11. Add some hotpath branching hints (LP: #1588921)

520. By CI Train Bot Account on 2016-06-14

Releasing 0.4.8+16.10.20160614-0ubuntu1

521. By Daniel d'Andrada on 2016-06-22

Handle case where TaskController requests an application that is already closing

Happens when you quickly close an application and then tap on its icon in the dash

522. By Daniel d'Andrada on 2016-06-22

Optimize code

523. By Daniel d'Andrada on 2016-06-28

Fix bad rebase on top of latest trunk

Code was still using the old, deprecated signal.

Unmerged revisions

523. By Daniel d'Andrada on 2016-06-28

Fix bad rebase on top of latest trunk

Code was still using the old, deprecated signal.

522. By Daniel d'Andrada on 2016-06-22

Optimize code

521. By Daniel d'Andrada on 2016-06-22

Handle case where TaskController requests an application that is already closing

Happens when you quickly close an application and then tap on its icon in the dash

=== modified file 'src/modules/Unity/Application/application_manager.cpp'
--- src/modules/Unity/Application/application_manager.cpp	2016-05-06 08:41:15 +0000
+++ src/modules/Unity/Application/application_manager.cpp	2016-06-16 20:19:32 +0000
@@ -56,6 +56,8 @@
 namespace unityapi = unity::shell::application;
 
 #define DEBUG_MSG qCDebug(QTMIR_APPLICATIONS).nospace() << "ApplicationManager::" << __func__
+#define WARNING_MSG qCWarning(QTMIR_APPLICATIONS).nospace() << "ApplicationManager::" << __func__
+#define CRITICAL_MSG qCCritical(QTMIR_APPLICATIONS).nospace() << "ApplicationManager::" << __func__
 
 namespace qtmir
 {
@@ -102,8 +104,10 @@
                      manager, &ApplicationManager::onProcessFailed);
     QObject::connect(controller, &TaskController::focusRequested,
                      manager, &ApplicationManager::onFocusRequested);
+    // We interpret this as a focus request for a suspended app.
+    // Shell will have this app resumed if it complies with the focus request
     QObject::connect(controller, &TaskController::resumeRequested,
-                     manager, &ApplicationManager::onResumeRequested);
+                     manager, &ApplicationManager::onFocusRequested);
 }
 
 } // namespace
@@ -368,35 +372,61 @@
 void ApplicationManager::onProcessStarting(const QString &appId)
 {
     tracepoint(qtmir, onProcessStarting);
-    qCDebug(QTMIR_APPLICATIONS) << "ApplicationManager::onProcessStarting - appId=" << appId;
-
-    Application *application = findApplication(appId);
-    if (!application) { // then shell did not start this application, so ubuntu-app-launch must have - add to list
-        auto appInfo = m_taskController->getInfoForApp(appId);
-        if (!appInfo) {
-            qCWarning(QTMIR_APPLICATIONS) << "ApplicationManager::onProcessStarting - Unable to instantiate application with appId" << appId;
-            return;
-        }
-
-        application = new Application(
-                    m_sharedWakelock,
-                    appInfo,
-                    QStringList(),
-                    this);
-        add(application);
-        application->requestFocus();
-    }
-    else {
-        if (application->internalState() == Application::InternalState::StoppedResumable) {
-            // url-dispatcher can relaunch apps which have been OOM-killed - AppMan must accept the newly spawned
-            // application and focus it immediately (as user expects app to still be running).
-            qCDebug(QTMIR_APPLICATIONS) << "Stopped application appId=" << appId << "is being resumed externally";
-            application->requestFocus();
-        } else {
-            qCDebug(QTMIR_APPLICATIONS) << "ApplicationManager::onProcessStarting application already found with appId"
-                                        << appId;
-        }
-    }
+
+    Application *application = nullptr;
+
+    application = findClosingApplication(appId);
+    if (application) {
+        // TODO: We should create a new Application instance right away in order to give immediate
+        // feedback to the user, even though there's no actual process backing it up yet.
+        if (!m_queuedStartApplications.contains(appId)) {
+            m_queuedStartApplications.append(appId);
+            DEBUG_MSG << "(" << appId << ") User wants to start a new instance of an application that is still closing."
+                " Queuing its start.";
+            connect(application, &QObject::destroyed, this, [this, appId]() {
+                    m_queuedStartApplications.removeAll(appId);
+                    startApplication(appId, QStringList());
+                }, Qt::QueuedConnection); // Queued so that we finish the app removal before starting again.
+        } else {
+            // Just ignore it.
+            DEBUG_MSG << "(" << appId << ") User wants to start a new instance of an application that is still closing"
+                " and is already queued to start later.";
+        }
+    } else {
+        application = findApplication(appId);
+        if (!application) { // then shell did not start this application, so ubuntu-app-launch must have - add to list
+            createNewApplication(appId);
+        } else {
+            if (application->internalState() == Application::InternalState::StoppedResumable) {
+                // url-dispatcher can relaunch apps which have been OOM-killed - AppMan must accept the newly spawned
+                // application and focus it immediately (as user expects app to still be running).
+                DEBUG_MSG << "(" << appId << ") Stopped application is being resumed externally";
+                application->requestFocus();
+            } else {
+                DEBUG_MSG << "(" << appId << ") application already exists";
+            }
+            application->setProcessState(Application::ProcessRunning);
+        }
+    }
+}
+
+void ApplicationManager::createNewApplication(const QString &appId)
+{
+    DEBUG_MSG << "(" << appId << ")";
+
+    auto appInfo = m_taskController->getInfoForApp(appId);
+    if (!appInfo) {
+        WARNING_MSG << "(" << appId << ") Unable to instantiate application";
+        return;
+    }
+
+    auto application = new Application(
+                m_sharedWakelock,
+                appInfo,
+                QStringList(),
+                this);
+    add(application);
+    application->requestFocus();
     application->setProcessState(Application::ProcessRunning);
 }
 
@@ -412,7 +442,7 @@
 
     Application *application = findApplication(appId);
     if (!application) {
-        qCritical() << "No such running application with appId" << appId;
+        CRITICAL_MSG << "(" << inputAppId << ") No such running application";
         return false;
     }
 
@@ -454,14 +484,18 @@
     tracepoint(qtmir, onProcessStopped);
     qCDebug(QTMIR_APPLICATIONS) << "ApplicationManager::onProcessStopped - appId=" << appId;
 
-    Application *application = findApplication(appId);
+    // TaskController doesn't give us any PID so we have no way of telling apart two instances
+    // of an application.
+    // The only situation where we expect to have two simultaneous instances is where there's one
+    // closing and a newer one just starting, in which case we assume he's referring to the former.
+    Application *application = findClosingApplication(appId);
     if (!application) {
-        application = findClosingApplication(appId);
+        application = findApplication(appId);
     }
 
     if (!application) {
-        qDebug() << "ApplicationManager::onProcessStopped reports stop of appId=" << appId
-                 << "which AppMan is not managing, ignoring the event";
+        // Might happen if the onProcessFailed() was called for this app and it already got deleted in response.
+        DEBUG_MSG << "(" << appId << ") Not managing it, ignoring.";
         return;
     }
 
@@ -490,29 +524,15 @@
 
 void ApplicationManager::onFocusRequested(const QString& appId)
 {
-    qCDebug(QTMIR_APPLICATIONS) << "ApplicationManager::onFocusRequested - appId=" << appId;
-
-    Application *application = findApplication(appId);
-    if (application) {
-        application->requestFocus();
-    }
-}
-
-void ApplicationManager::onResumeRequested(const QString& appId)
-{
-    qCDebug(QTMIR_APPLICATIONS) << "ApplicationManager::onResumeRequested - appId=" << appId;
-
-    Application *application = findApplication(appId);
-
-    if (!application) {
-        qCritical() << "ApplicationManager::onResumeRequested: No such running application" << appId;
-        return;
-    }
-
-    // We interpret this as a focus request for a suspended app.
-    // Shell will have this app resumed if it complies with the focus request
-    if (application->state() == Application::Suspended) {
-        application->requestFocus();
+    Application *application = findApplication(appId);
+
+    if (m_queuedStartApplications.contains(appId) || findClosingApplication(appId) != nullptr) {
+        DEBUG_MSG << "(" << appId << ") Ignoring request as the application is closing and/or queued to start";
+    } else if (application) {
+        DEBUG_MSG << "(" << appId << ") Emitting focusRequested";
+        Q_EMIT focusRequested(appId);
+    } else {
+        WARNING_MSG << "(" << appId << ") No such running application";
     }
 }
 
@@ -702,9 +722,9 @@
 
     connect(application, &Application::stopProcessRequested, this, [=]() {
         if (!m_taskController->stop(appId) && application->pid() > 0) {
-            qWarning() << "FAILED to ask Upstart to stop application with appId" << appId
-                       << "Sending SIGTERM to process:" << appId;
-            kill(application->pid(), SIGTERM);
+            qCWarning(QTMIR_APPLICATIONS) << "FAILED to ask Upstart to stop application with appId" << appId
+                       << "Sending SIGTERM to process";
+            m_taskController->kill(application->pid());
             application->setProcessState(Application::ProcessStopped);
         }
     });

=== modified file 'src/modules/Unity/Application/application_manager.h'
--- src/modules/Unity/Application/application_manager.h	2016-05-05 15:02:01 +0000
+++ src/modules/Unity/Application/application_manager.h	2016-06-16 20:19:32 +0000
@@ -111,7 +111,6 @@
     void onProcessSuspended(const QString& appId);
     void onProcessFailed(const QString& appId, TaskController::Error error);
     void onFocusRequested(const QString& appId);
-    void onResumeRequested(const QString& appId);
 
 Q_SIGNALS:
     void emptyChanged();
@@ -132,6 +131,7 @@
     QModelIndex findIndex(Application* application);
     void resumeApplication(Application *application);
     QString toString() const;
+    void createNewApplication(const QString &appId);
 
     Application* findApplicationWithPromptSession(const mir::scene::PromptSession* promptSession);
     Application *findClosingApplication(const QString &inputAppId) const;

=== modified file 'src/modules/Unity/Application/mirsurfacelistmodel.cpp'
--- src/modules/Unity/Application/mirsurfacelistmodel.cpp	2016-05-17 19:57:52 +0000
+++ src/modules/Unity/Application/mirsurfacelistmodel.cpp	2016-06-16 20:19:32 +0000
@@ -20,6 +20,9 @@
 
 #include <paths.h>
 
+#include <QDebug>
+#include <QDebugStateSaver>
+
 namespace unityapp = unity::shell::application;
 using namespace qtmir;
 
@@ -284,3 +287,21 @@
 
     return m_sourceList->data(index, role);
 }
+
+QDebug operator<<(QDebug dbg, const unityapp::MirSurfaceListInterface &surfaceListConst)
+{
+    auto surfaceList = const_cast<unityapp::MirSurfaceListInterface*>(&surfaceListConst);
+
+    QDebugStateSaver saver(dbg);
+    dbg.nospace();
+    dbg << "MirSurfaceList(";
+    for (int i = 0; i < surfaceList->count(); ++i) {
+        if (i > 0) {
+            dbg << ", ";
+        }
+        auto surface = surfaceList->get(i);
+        dbg << (void*)surface;
+    }
+    dbg << ')';
+    return dbg;
+}

=== modified file 'src/modules/Unity/Application/mirsurfacelistmodel.h'
--- src/modules/Unity/Application/mirsurfacelistmodel.h	2016-05-10 21:06:21 +0000
+++ src/modules/Unity/Application/mirsurfacelistmodel.h	2016-06-16 20:19:32 +0000
@@ -94,4 +94,6 @@
 
 } // namespace qtmir
 
+QDebug operator<<(QDebug, const unity::shell::application::MirSurfaceListInterface &);
+
 #endif // QTMIR_MIRSURFACELISTMODEL_H

=== modified file 'src/modules/Unity/Application/session.cpp'
--- src/modules/Unity/Application/session.cpp	2016-05-17 19:18:44 +0000
+++ src/modules/Unity/Application/session.cpp	2016-06-16 20:19:32 +0000
@@ -38,6 +38,8 @@
 
 using unity::shell::application::ApplicationInfoInterface;
 
+#define DEBUG_MSG qCDebug(QTMIR_SURFACES).nospace() << "Session[" << (void*)this << ",name=" << name() << "]::" << __func__
+
 namespace qtmir
 {
 
@@ -75,7 +77,7 @@
     , m_live(true)
     , m_promptSessionManager(promptSessionManager)
 {
-    qCDebug(QTMIR_SESSIONS) << "Session::Session() " << this->name();
+    DEBUG_MSG << "()";
 
     setSuspendTimer(new Timer);
 
@@ -84,7 +86,7 @@
 
 Session::~Session()
 {
-    qCDebug(QTMIR_SESSIONS) << "Session::~Session() " << name();
+    DEBUG_MSG << "()";
     stopPromptSessions();
 
     QList<SessionInterface*> children(m_children->list());
@@ -107,7 +109,7 @@
     Q_ASSERT(m_state == Session::Suspending);
 
     if (m_surfaceList.count() == 0) {
-        qCDebug(QTMIR_SESSIONS) << "Application::suspend - no surface to call stopFrameDropper() on!";
+        DEBUG_MSG << " no surface to call stopFrameDropper() on!";
     } else {
         for (int i = 0; i < m_surfaceList.count(); ++i) {
             auto surface = static_cast<MirSurfaceInterface*>(m_surfaceList.get(i));
@@ -153,8 +155,7 @@
         return;
     }
 
-    qCDebug(QTMIR_SESSIONS) << "Session::setState - session=" << name()
-                            << "state=" << sessionStateToString(state);
+    DEBUG_MSG << "(state=" << sessionStateToString(state) << ")";
 
     if (m_state == Suspending) {
         m_suspendTimer->stop();
@@ -198,7 +199,7 @@
 
 void Session::registerSurface(MirSurfaceInterface *newSurface)
 {
-    qCDebug(QTMIR_SESSIONS) << "Session::resgisterSurface - session=" << name() << "surface=" << newSurface;
+    DEBUG_MSG << "(surface=" << newSurface << ")";
 
     // Only notify QML of surface creation once it has drawn its first frame.
     if (newSurface->isFirstFrameDrawn()) {
@@ -214,7 +215,7 @@
 
 void Session::prependSurface(MirSurfaceInterface *newSurface)
 {
-    qCDebug(QTMIR_SESSIONS) << "Session::prependSurface - session=" << name() << "surface=" << newSurface;
+    DEBUG_MSG << "(surface=" << newSurface << ")";
 
     connect(newSurface, &MirSurfaceInterface::stateChanged,
         this, &Session::updateFullscreenProperty);
@@ -246,7 +247,7 @@
 
 void Session::removeSurface(MirSurfaceInterface* surface)
 {
-    qCDebug(QTMIR_SESSIONS) << "Session::removeSurface - session=" << name() << "surface=" << surface;
+    DEBUG_MSG << "(surface=" << surface << ")";
 
     surface->disconnect(this);
 
@@ -277,8 +278,8 @@
 
 void Session::setFullscreen(bool fullscreen)
 {
-    qCDebug(QTMIR_SESSIONS) << "Session::setFullscreen - session=" << this << "fullscreen=" << fullscreen;
     if (m_fullscreen != fullscreen) {
+        DEBUG_MSG << "(" << fullscreen << ")";
         m_fullscreen = fullscreen;
         Q_EMIT fullscreenChanged(m_fullscreen);
     }
@@ -286,7 +287,7 @@
 
 void Session::suspend()
 {
-    qCDebug(QTMIR_SESSIONS) << "Session::suspend - session=" << this << "state=" << sessionStateToString(m_state);
+    DEBUG_MSG << " state=" << sessionStateToString(m_state);
     if (m_state == Running) {
         session()->set_lifecycle_state(mir_lifecycle_state_will_suspend);
         m_suspendTimer->start();
@@ -305,7 +306,7 @@
 
 void Session::resume()
 {
-    qCDebug(QTMIR_SESSIONS) << "Session::resume - session=" << this << "state=" << sessionStateToString(m_state);
+    DEBUG_MSG << " state=" << sessionStateToString(m_state);
 
     if (m_state == Suspending || m_state == Suspended) {
         doResume();
@@ -336,7 +337,7 @@
 
 void Session::close()
 {
-    qCDebug(QTMIR_SESSIONS) << "Session::close - " << name();
+    DEBUG_MSG << "()";
 
     if (m_state == Stopped) return;
 
@@ -348,7 +349,7 @@
 
 void Session::stop()
 {
-    qCDebug(QTMIR_SESSIONS) << "Session::stop - " << name();
+    DEBUG_MSG << "()";
 
     if (m_state != Stopped) {
 
@@ -372,7 +373,7 @@
 void Session::setLive(const bool live)
 {
     if (m_live != live) {
-        qCDebug(QTMIR_SESSIONS) << "Session::setLive - " << name() << "live=" << live;
+        DEBUG_MSG << "(" << live << ")";
 
         m_live = live;
         Q_EMIT liveChanged(m_live);
@@ -396,7 +397,7 @@
 
 void Session::insertChildSession(uint index, SessionInterface* session)
 {
-    qCDebug(QTMIR_SESSIONS) << "Session::insertChildSession - " << session->name() << " to " << name() << " @  " << index;
+    DEBUG_MSG << "(index=" << index << ", Session[" << (void*)session << ",name=" << session->name() << "])";
     Q_ASSERT(!m_children->contains(session));
 
     m_children->insert(index, session);
@@ -424,7 +425,7 @@
 
 void Session::removeChildSession(SessionInterface* session)
 {
-    qCDebug(QTMIR_SESSIONS) << "Session::removeChildSession - " << session->name() << " from " << name();
+    DEBUG_MSG << "(Session[" << (void*)session << ",name=" << session->name() << "])";
 
     disconnect(session, 0, this, 0);
 
@@ -452,16 +453,14 @@
 
 void Session::appendPromptSession(const std::shared_ptr<ms::PromptSession>& promptSession)
 {
-    qCDebug(QTMIR_SESSIONS) << "Session::appendPromptSession session=" << name()
-            << "promptSession=" << (promptSession ? promptSession.get() : nullptr);
+    DEBUG_MSG << "(promptSession=" << (promptSession ? promptSession.get() : nullptr) << ")";
 
     m_promptSessions.append(promptSession);
 }
 
 void Session::removePromptSession(const std::shared_ptr<ms::PromptSession>& promptSession)
 {
-    qCDebug(QTMIR_SESSIONS) << "Session::removePromptSession session=" << name()
-            << "promptSession=" << (promptSession ? promptSession.get() : nullptr);
+    DEBUG_MSG << "(promptSession=" << (promptSession ? promptSession.get() : nullptr) << ")";
 
     m_promptSessions.removeAll(promptSession);
 }
@@ -477,7 +476,7 @@
     QListIterator<std::shared_ptr<ms::PromptSession>> it(copy);
     for ( it.toBack(); it.hasPrevious(); ) {
         std::shared_ptr<ms::PromptSession> promptSession = it.previous();
-        qCDebug(QTMIR_SESSIONS) << "Session::stopPromptSessions - promptSession=" << promptSession.get();
+        DEBUG_MSG << " - promptSession=" << promptSession.get();
 
         m_promptSessionManager->stop_prompt_session(promptSession);
     }
@@ -500,7 +499,7 @@
 void Session::deleteIfZombieAndEmpty()
 {
     if (!m_live && m_children->rowCount() == 0 && m_surfaceList.isEmpty()) {
-        qCDebug(QTMIR_SESSIONS).nospace() << "Session::deleteIfZombieAndEmpty[" << name() << "] - deleteLater()";
+        DEBUG_MSG << " - deleteLater()";
         deleteLater();
     }
 }

=== modified file 'src/modules/Unity/Application/taskcontroller.h'
--- src/modules/Unity/Application/taskcontroller.h	2016-04-22 14:03:26 +0000
+++ src/modules/Unity/Application/taskcontroller.h	2016-06-16 20:19:32 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014-2015 Canonical, Ltd.
+ * Copyright (C) 2014-2016 Canonical, Ltd.
  *
  * This program is free software: you can redistribute it and/or modify it under
  * the terms of the GNU Lesser General Public License version 3, as published by
@@ -54,6 +54,10 @@
 
     virtual QSharedPointer<qtmir::ApplicationInfo> getInfoForApp(const QString &appId) const = 0;
 
+    // Not a perfect fit for this class but we need the kill(pid_t pid, int sig) function
+    // behind an interface so we can test it (replacing with a mock or fake)
+    virtual void kill(pid_t pid) = 0;
+
 Q_SIGNALS:
     void processStarting(const QString &appId);
     void applicationStarted(const QString &appId);

=== modified file 'src/modules/Unity/Application/upstart/taskcontroller.cpp'
--- src/modules/Unity/Application/upstart/taskcontroller.cpp	2016-05-04 16:35:50 +0000
+++ src/modules/Unity/Application/upstart/taskcontroller.cpp	2016-06-16 20:19:32 +0000
@@ -30,6 +30,9 @@
 }
 #include <ubuntu-app-launch/registry.h>
 
+// std
+#include <csignal>
+
 namespace ual = ubuntu::app_launch;
 
 namespace qtmir
@@ -243,5 +246,10 @@
     return QSharedPointer<qtmir::ApplicationInfo>(appInfo);
 }
 
+void TaskController::kill(pid_t pid)
+{
+    ::kill(pid, SIGTERM);
+}
+
 } // namespace upstart
 } // namespace qtmir

=== modified file 'src/modules/Unity/Application/upstart/taskcontroller.h'
--- src/modules/Unity/Application/upstart/taskcontroller.h	2016-04-22 14:03:26 +0000
+++ src/modules/Unity/Application/upstart/taskcontroller.h	2016-06-16 20:19:32 +0000
@@ -41,6 +41,8 @@
 
     QSharedPointer<qtmir::ApplicationInfo> getInfoForApp(const QString &appId) const override;
 
+    void kill(pid_t pid) override;
+
 private:
     struct Private;
     QScopedPointer<Private> impl;

=== modified file 'tests/framework/mock_task_controller.h'
--- tests/framework/mock_task_controller.h	2016-04-22 14:03:26 +0000
+++ tests/framework/mock_task_controller.h	2016-06-16 20:19:32 +0000
@@ -38,6 +38,7 @@
     MOCK_METHOD2(start, bool(const QString&, const QStringList&));
     MOCK_METHOD1(suspend, bool(const QString&));
     MOCK_METHOD1(resume, bool(const QString&));
+    MOCK_METHOD1(kill, void(pid_t pid));
 
     bool doAppIdHasProcessId(const QString& appId, pid_t pid);
 

=== modified file 'tests/modules/ApplicationManager/application_manager_test.cpp'
--- tests/modules/ApplicationManager/application_manager_test.cpp	2016-05-03 15:27:26 +0000
+++ tests/modules/ApplicationManager/application_manager_test.cpp	2016-06-16 20:19:32 +0000
@@ -1900,3 +1900,176 @@
 
     EXPECT_EQ(1, focusRequestedSpy.count());
 }
+
+/*
+ * Context:
+ *  - we are waiting for an application to quit following a close()
+ *
+ * Event:
+ *  - TaskController requests that application to be resumed.
+ *    Most likely because the user tapped the app icon in the Dash. From the user's
+ *    perspective the application is already closed and he wants to start a new
+ *    instance of it.
+ *    But from TaskController's perspective the application is still running and
+ *    therefore should just be resumed/focused.
+ *
+ * Expected outcome:
+ *  - ApplicationManager should wait for the closing application to finally end (or
+      be killed) and then finally launch a new instance of it.
+ *
+ * Regression test for https://bugs.launchpad.net/ubuntu/+source/qtmir/+bug/1527737
+ * "Apps do not start if restarted quickly after closing"
+ *
+ * Two test versions:
+ * _appQuits - Application complies and quits
+ * _appKilled - Application ignores close request and thus get killed
+ */
+TEST_F(ApplicationManagerTests, focusRequestedForClosingApplication_appQuits)
+{
+    using namespace ::testing;
+
+    int argc = 0;
+    char* argv[0];
+    QCoreApplication qtApp(argc, argv); // app for deleteLater event
+
+    const QString appId("testAppId");
+    quint64 procId = 5551;
+
+    ON_CALL(*taskController, appIdHasProcessId(appId, procId)).WillByDefault(Return(true));
+
+    EXPECT_CALL(*taskController, start(appId, _))
+        .Times(1)
+        .WillOnce(Return(true));
+
+    auto app = applicationManager.startApplication(appId);
+    applicationManager.onProcessStarting(appId);
+    std::shared_ptr<mir::scene::Session> session = std::make_shared<MockSession>("", procId);
+    bool authed = true;
+    applicationManager.authorizeSession(procId, authed);
+    ASSERT_TRUE(authed);
+    onSessionStarting(session);
+
+    FakeMirSurface *surface = new FakeMirSurface;
+    onSessionCreatedSurface(session.get(), surface);
+    surface->drawFirstFrame();
+
+    ASSERT_EQ(Application::InternalState::Running, app->internalState());
+
+    QSignalSpy closeRequestedSpy(surface, SIGNAL(closeRequested()));
+
+    // User swipes away the application, visually closing/removing it.
+    applicationManager.stopApplication(appId);
+
+    // But asking ApplicationManager to stop the application just makes it request its
+    // surfaces to be closed
+    EXPECT_EQ(Application::InternalState::Closing, app->internalState());
+    EXPECT_EQ(1, closeRequestedSpy.count());
+
+    // User then taps on the app icon
+    applicationManager.onProcessStarting(appId);
+    applicationManager.onFocusRequested(appId);
+
+    // But no application instance should have been created because of that
+    EXPECT_TRUE(applicationManager.findApplication(appId) == nullptr);
+
+    // ApplicationManager will ask TaskController to start the application again
+    // once the closing instance is gone
+    Mock::VerifyAndClearExpectations(taskController);
+    EXPECT_CALL(*taskController, start(appId, _))
+        .Times(1)
+        .WillOnce(Return(true));
+
+    // Simulates that the application complied to the close() request and stopped itself
+    onSessionStopping(session);
+    applicationManager.onProcessStopped(appId);
+
+    // DeferredDelete is special: likes to be called out specifically or it won't come out
+    qtApp.sendPostedEvents(app, QEvent::DeferredDelete);
+    qtApp.sendPostedEvents();
+
+    EXPECT_TRUE(applicationManager.findApplication(appId) != nullptr);
+}
+TEST_F(ApplicationManagerTests, focusRequestedForClosingApplication_appKilled)
+{
+    using namespace ::testing;
+
+    int argc = 0;
+    char* argv[0];
+    QCoreApplication qtApp(argc, argv); // app for deleteLater event
+
+    const QString appId("testAppId");
+    quint64 procId = 5551;
+
+    ON_CALL(*taskController, appIdHasProcessId(appId, procId)).WillByDefault(Return(true));
+
+    EXPECT_CALL(*taskController, start(appId, _))
+        .Times(1)
+        .WillOnce(Return(true));
+
+    auto app = applicationManager.startApplication(appId);
+    applicationManager.onProcessStarting(appId);
+    std::shared_ptr<mir::scene::Session> session = std::make_shared<MockSession>("", procId);
+    bool authed = true;
+    applicationManager.authorizeSession(procId, authed);
+    ASSERT_TRUE(authed);
+    onSessionStarting(session);
+
+    FakeMirSurface *surface = new FakeMirSurface;
+    onSessionCreatedSurface(session.get(), surface);
+    surface->drawFirstFrame();
+
+    EXPECT_EQ(Application::InternalState::Running, app->internalState());
+
+    QSharedPointer<FakeTimeSource> fakeTimeSource(new FakeTimeSource);
+    FakeTimer *fakeStopTimer = new FakeTimer(fakeTimeSource);
+    app->setStopTimer(fakeStopTimer);
+
+    QSignalSpy closeRequestedSpy(surface, SIGNAL(closeRequested()));
+
+    // User swipes away the application, visually closing/removing it.
+    applicationManager.stopApplication(appId);
+
+    // But asking ApplicationManager to stop the application just makes it request its
+    // surfaces to be closed
+    EXPECT_EQ(Application::InternalState::Closing, app->internalState());
+    EXPECT_EQ(1, closeRequestedSpy.count());
+
+    // User then taps on the app icon
+    applicationManager.onProcessStarting(appId);
+    applicationManager.onFocusRequested(appId);
+
+    // But no application instance should have been created because of that
+    EXPECT_TRUE(applicationManager.findApplication(appId) == nullptr);
+
+    // ApplicationManager will ask TaskController to start the application again
+    // once the closing instance is gone
+    Mock::VerifyAndClearExpectations(taskController);
+    EXPECT_CALL(*taskController, start(appId, _))
+        .Times(1)
+        .WillOnce(Return(true));
+
+    EXPECT_CALL(*taskController, stop(appId))
+        .Times(1)
+        .WillOnce(Return(false));
+
+    EXPECT_CALL(*taskController, kill(procId))
+        .Times(1)
+        .WillOnce(Invoke(
+            [this, session](pid_t) {
+                // simulate application dying and thus severing the mir connection
+                onSessionStopping(session);
+            }
+        ));
+
+    if (fakeStopTimer->isRunning()) {
+        // Simulate that it has timed out.
+        fakeTimeSource->m_msecsSinceReference = fakeStopTimer->nextTimeoutTime() + 1;
+        fakeStopTimer->update();
+    }
+
+    // DeferredDelete is special: likes to be called out specifically or it won't come out
+    qtApp.sendPostedEvents(app, QEvent::DeferredDelete);
+    qtApp.sendPostedEvents();
+
+    EXPECT_TRUE(applicationManager.findApplication(appId) != nullptr);
+}