QtMir

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

  1. appRestart-lp1527737
  2. Merge into trunk
Proposed by Daniel d'Andrada on 2016-01-05
Status: Superseded
Proposed branch: lp:~dandrader/qtmir/appRestart-lp1527737
Merge into: lp:qtmir
Prerequisite: lp:~dandrader/qtmir/removeUselessClass
Diff against target: 809 lines (+375/-68)
16 files modified
CMakeLists.txt (+1/-1)
debian/control (+2/-2)
src/modules/Unity/Application/application.cpp (+16/-0)
src/modules/Unity/Application/application.h (+3/-0)
src/modules/Unity/Application/application_manager.cpp (+112/-49)
src/modules/Unity/Application/application_manager.h (+2/-1)
src/modules/Unity/Application/taskcontroller.h (+4/-0)
src/modules/Unity/Application/upstart/taskcontroller.cpp (+8/-0)
src/modules/Unity/Application/upstart/taskcontroller.h (+2/-0)
src/platforms/mirserver/mirserver.cpp (+8/-1)
src/platforms/mirserver/mirserver.h (+3/-1)
src/platforms/mirserver/mirwindowmanager.cpp (+20/-10)
src/platforms/mirserver/mirwindowmanager.h (+7/-2)
tests/framework/mock_task_controller.h (+1/-0)
tests/mirserver/WindowManager/window_manager.cpp (+1/-1)
tests/modules/ApplicationManager/application_manager_test.cpp (+185/-0)
To merge this branch: bzr merge lp:~dandrader/qtmir/appRestart-lp1527737
Reviewer Review Type Date Requested Status
Michał Sawicz Needs Fixing on 2016-01-27
PS Jenkins bot continuous-integration Needs Fixing on 2016-01-13
Unity8 CI Bot continuous-integration Needs Fixing on 2016-01-13
Gerry Boland 2016-01-05 Approve on 2016-01-08
Review via email: mp+281701@code.launchpad.net

This proposal has been superseded by a proposal from 2016-02-01.

Commit Message

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

Description of the Change

* 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.

To post a comment you must log in.
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Gerry Boland (gerboland) wrote :

=== 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) wrote :
review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Daniel d'Andrada (dandrader) wrote :

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) wrote :

> 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) wrote :

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

review: Approve
Michał Sawicz (saviq) wrote :

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) wrote :

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) wrote :

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) wrote :

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) wrote :

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) wrote :

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) wrote :

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) wrote :

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) wrote :
review: Needs Fixing (continuous-integration)
lp:~dandrader/qtmir/appRestart-lp1527737 updated on 2016-01-22
435. By Nick Dedekind on 2016-01-22

Fixed issue where stopping the session while suspending was causing app close to stall. Fixes: #1536133

436. By CI Train Bot Account on 2016-01-22

Releasing 0.4.7+16.04.20160122-0ubuntu1

Michał Sawicz (saviq) wrote :

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

review: Needs Fixing
lp:~dandrader/qtmir/appRestart-lp1527737 updated on 2016-06-28
437. By Brandon Schaefer on 2016-01-27

Changes for mir 0.19.

438. By CI Train Bot Account on 2016-01-27

Releasing 0.4.7+16.04.20160127.1-0ubuntu1

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

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'CMakeLists.txt'
2--- CMakeLists.txt 2016-02-01 16:09:52 +0000
3+++ CMakeLists.txt 2016-02-01 16:09:52 +0000
4@@ -75,7 +75,7 @@
5 pkg_check_modules(GSETTINGS_QT REQUIRED gsettings-qt)
6 pkg_check_modules(QTDBUSTEST libqtdbustest-1 REQUIRED)
7 pkg_check_modules(QTDBUSMOCK libqtdbusmock-1 REQUIRED)
8-pkg_check_modules(APPLICATION_API REQUIRED unity-shell-application=12)
9+pkg_check_modules(APPLICATION_API REQUIRED unity-shell-application=13)
10
11 include_directories(${APPLICATION_API_INCLUDE_DIRS})
12
13
14=== modified file 'debian/control'
15--- debian/control 2016-01-20 20:27:26 +0000
16+++ debian/control 2016-02-01 16:09:52 +0000
17@@ -22,7 +22,7 @@
18 libubuntu-app-launch2-dev,
19 libubuntu-application-api-dev (>= 2.1.0),
20 libudev-dev,
21- libunity-api-dev (>= 7.104),
22+ libunity-api-dev (>= 7.106),
23 liburl-dispatcher1-dev,
24 libxkbcommon-dev,
25 libxrender-dev,
26@@ -93,7 +93,7 @@
27 Conflicts: libqtmir,
28 libunity-mir1,
29 Provides: unity-application-impl,
30- unity-application-impl-12,
31+ unity-application-impl-13,
32 Description: Qt plugin for Unity specific Mir APIs
33 QtMir provides Qt/QML bindings for Mir features that are exposed through the
34 qtmir-desktop or qtmir-android QPA plugin such as Application management
35
36=== modified file 'src/modules/Unity/Application/application.cpp'
37--- src/modules/Unity/Application/application.cpp 2016-02-01 16:09:52 +0000
38+++ src/modules/Unity/Application/application.cpp 2016-02-01 16:09:52 +0000
39@@ -795,4 +795,20 @@
40 connect(m_closeTimer, &Timer::timeout, this, &Application::stop);
41 }
42
43+QSize Application::initialSurfaceSize() const
44+{
45+ return m_initialSurfaceSize;
46+}
47+
48+void Application::setInitialSurfaceSize(const QSize &size)
49+{
50+ qCDebug(QTMIR_APPLICATIONS).nospace() << "Application::setInitialSurfaceSize - appId=" << appId()
51+ << " size=" << size;
52+
53+ if (size != m_initialSurfaceSize) {
54+ m_initialSurfaceSize = size;
55+ Q_EMIT initialSurfaceSizeChanged(m_initialSurfaceSize);
56+ }
57+}
58+
59 } // namespace qtmir
60
61=== modified file 'src/modules/Unity/Application/application.h'
62--- src/modules/Unity/Application/application.h 2016-02-01 16:09:52 +0000
63+++ src/modules/Unity/Application/application.h 2016-02-01 16:09:52 +0000
64@@ -108,6 +108,8 @@
65 bool isTouchApp() const override;
66 bool exemptFromLifecycle() const override;
67 void setExemptFromLifecycle(bool) override;
68+ QSize initialSurfaceSize() const override;
69+ void setInitialSurfaceSize(const QSize &size) override;
70
71 void setStage(Stage stage);
72
73@@ -188,6 +190,7 @@
74 ProcessState m_processState;
75 AbstractTimer *m_closeTimer;
76 bool m_exemptFromLifecycle;
77+ QSize m_initialSurfaceSize;
78
79 friend class ApplicationManager;
80 friend class SessionManager;
81
82=== modified file 'src/modules/Unity/Application/application_manager.cpp'
83--- src/modules/Unity/Application/application_manager.cpp 2016-02-01 16:09:52 +0000
84+++ src/modules/Unity/Application/application_manager.cpp 2016-02-01 16:09:52 +0000
85@@ -32,6 +32,7 @@
86 #include "sessionlistener.h"
87 #include "sessionauthorizer.h"
88 #include "logging.h"
89+#include <mirwindowmanager.h>
90
91 // mir
92 #include <mir/scene/surface.h>
93@@ -49,6 +50,10 @@
94 // std
95 #include <csignal>
96
97+#define DEBUG_MSG(appIdParam) qCDebug(QTMIR_APPLICATIONS).nospace() << "ApplicationManager::" << __func__ << "(appId=" << appIdParam << ") - "
98+#define WARNING_MSG(appIdParam) qCWarning(QTMIR_APPLICATIONS).nospace() << "ApplicationManager::" << __func__ << "(appId=" << appIdParam << ") - "
99+#define CRITICAL_MSG(appIdParam) qCCritical(QTMIR_APPLICATIONS).nospace() << "ApplicationManager::" << __func__ << "(appId=" << appIdParam << ") - "
100+
101 namespace ms = mir::scene;
102
103 Q_LOGGING_CATEGORY(QTMIR_APPLICATIONS, "qtmir.applications")
104@@ -102,8 +107,10 @@
105 manager, &ApplicationManager::onProcessFailed);
106 QObject::connect(controller, &TaskController::focusRequested,
107 manager, &ApplicationManager::onFocusRequested);
108+ // We interpret this as a focus request for a suspended app.
109+ // Shell will have this app resumed if it complies with the focus request
110 QObject::connect(controller, &TaskController::resumeRequested,
111- manager, &ApplicationManager::onResumeRequested);
112+ manager, &ApplicationManager::onFocusRequested);
113 }
114
115 } // namespace
116@@ -118,7 +125,7 @@
117 return nullptr;
118 }
119
120- auto mirServer = nativeInterface->m_mirServer;
121+ auto mirServer = nativeInterface->m_mirServer.lock();
122
123 SessionListener *sessionListener = static_cast<SessionListener*>(nativeInterface->nativeResourceForIntegration("SessionListener"));
124 SessionAuthorizer *sessionAuthorizer = static_cast<SessionAuthorizer*>(nativeInterface->nativeResourceForIntegration("SessionAuthorizer"));
125@@ -146,6 +153,9 @@
126 connectToSessionListener(appManager, sessionListener);
127 connectToSessionAuthorizer(appManager, sessionAuthorizer);
128 connectToTaskController(appManager, taskController.data());
129+ connect(mirServer->windowManager(), &MirWindowManager::sessionAboutToCreateSurface,
130+ appManager, &ApplicationManager::onSessionAboutToCreateSurface,
131+ Qt::BlockingQueuedConnection);
132
133 // Emit signal to notify Upstart that Mir is ready to receive client connections
134 // see http://upstart.ubuntu.com/cookbook/#expect-stop
135@@ -348,22 +358,22 @@
136
137 Application *application = findApplication(appId);
138 if (application) {
139- qWarning() << "ApplicationManager::startApplication - application appId=" << appId << " already exists";
140+ qCDebug(QTMIR_APPLICATIONS) << "ApplicationManager::startApplication - application appId=" << appId << " already exists";
141 return nullptr;
142 }
143
144- if (m_queuedStartApplications.contains(inputAppId)) {
145- qWarning() << "ApplicationManager::startApplication - application appId=" << appId << " is queued to start";
146+ if (m_queuedStartApplications.contains(appId)) {
147+ qCDebug(QTMIR_APPLICATIONS) << "ApplicationManager::startApplication - application appId=" << appId << " is queued to start";
148 return nullptr;
149 } else {
150- application = findClosingApplication(inputAppId);
151+ application = findClosingApplication(appId);
152 if (application) {
153- m_queuedStartApplications.append(inputAppId);
154- qWarning() << "ApplicationManager::startApplication - application appId=" << appId << " is closing. Queuing start";
155- connect(application, &QObject::destroyed, this, [this, application, inputAppId, flags, arguments]() {
156- m_queuedStartApplications.removeAll(inputAppId);
157+ m_queuedStartApplications.append(appId);
158+ qCDebug(QTMIR_APPLICATIONS) << "ApplicationManager::startApplication - application appId=" << appId << " is closing. Queuing start";
159+ connect(application, &QObject::destroyed, this, [this, application, appId, flags, arguments]() {
160+ m_queuedStartApplications.removeAll(appId);
161 // start the app.
162- startApplication(inputAppId, flags, arguments);
163+ startApplication(appId, flags, arguments);
164 }, Qt::QueuedConnection); // Queued so that we finish the app removal before starting again.
165 return nullptr;
166 }
167@@ -403,10 +413,33 @@
168 void ApplicationManager::onProcessStarting(const QString &appId)
169 {
170 tracepoint(qtmir, onProcessStarting);
171- qCDebug(QTMIR_APPLICATIONS) << "ApplicationManager::onProcessStarting - appId=" << appId;
172-
173- Application *application = findApplication(appId);
174+
175+ Application *application = nullptr;
176+
177+ application = findClosingApplication(appId);
178+ if (application) {
179+ // TODO: We should create a new Application instance right away in order to give immediate
180+ // feedback to the user, even though there's no actual process backing it up yet.
181+ if (!m_queuedStartApplications.contains(appId)) {
182+ m_queuedStartApplications.append(appId);
183+ DEBUG_MSG(appId) << "User wants to start a new instance of an application that is still closing."
184+ " Queuing its start.";
185+ connect(application, &QObject::destroyed, this, [this, appId]() {
186+ m_queuedStartApplications.removeAll(appId);
187+ startApplication(appId, QStringList());
188+ }, Qt::QueuedConnection); // Queued so that we finish the app removal before starting again.
189+ } else {
190+ // Just ignore it.
191+ DEBUG_MSG(appId) << "User wants to start a new instance of an application that is still closing"
192+ " and is already queued to start later.";
193+ }
194+ return;
195+ }
196+
197+ application = findApplication(appId);
198 if (!application) { // then shell did not start this application, so ubuntu-app-launch must have - add to list
199+ DEBUG_MSG(appId) << "Creating new application";
200+
201 application = new Application(
202 m_sharedWakelock,
203 m_desktopFileReaderFactory->createInstance(appId, m_taskController->findDesktopFileForAppId(appId)),
204@@ -414,7 +447,7 @@
205 this);
206
207 if (!application->isValid()) {
208- qWarning() << "Unable to instantiate application with appId" << appId;
209+ WARNING_MSG(appId) << "Unable to instantiate application";
210 return;
211 }
212
213@@ -425,11 +458,10 @@
214 if (application->state() == Application::Stopped) {
215 // url-dispatcher can relaunch apps which have been OOM-killed - AppMan must accept the newly spawned
216 // application and focus it immediately (as user expects app to still be running).
217- qCDebug(QTMIR_APPLICATIONS) << "Stopped application appId=" << appId << "is being resumed externally";
218+ DEBUG_MSG(appId) << "Stopped application is being resumed externally";
219 Q_EMIT focusRequested(appId);
220 } else {
221- qCDebug(QTMIR_APPLICATIONS) << "ApplicationManager::onProcessStarting application already found with appId"
222- << appId;
223+ DEBUG_MSG(appId) << "application already exists";
224 }
225 }
226 application->setProcessState(Application::ProcessRunning);
227@@ -447,17 +479,19 @@
228
229 Application *application = findApplication(appId);
230 if (!application) {
231- qCritical() << "No such running application with appId" << appId;
232+ qCCritical(QTMIR_APPLICATIONS) << "No such running application with appId" << appId;
233 return false;
234 }
235
236+ remove(application);
237+
238 application->close();
239- remove(application);
240-
241- connect(application, &QObject::destroyed, this, [this, application](QObject*) {
242+ connect(application, &Application::stopped, this, [this, application]() {
243 m_closingApplications.removeAll(application);
244+ application->deleteLater();
245 });
246 m_closingApplications.append(application);
247+
248 return true;
249 }
250
251@@ -467,9 +501,13 @@
252
253 qCDebug(QTMIR_APPLICATIONS) << "ApplicationManager::onProcessFailed - appId=" << appId;
254
255- Application *application = findApplication(appId);
256- if (!application) {
257- qWarning() << "ApplicationManager::onProcessFailed - upstart reports failure of application" << appId
258+ Application *application = findClosingApplication(appId);
259+ if (!application) {
260+ application = findApplication(appId);
261+ }
262+
263+ if (!application) {
264+ qCWarning(QTMIR_APPLICATIONS) << "ApplicationManager::onProcessFailed - upstart reports failure of application" << appId
265 << "that AppManager is not managing";
266 return;
267 }
268@@ -484,13 +522,18 @@
269 tracepoint(qtmir, onProcessStopped);
270 qCDebug(QTMIR_APPLICATIONS) << "ApplicationManager::onProcessStopped - appId=" << appId;
271
272- Application *application = findApplication(appId);
273+ // TaskController doesn't give us any PID so we have no way of telling apart two instances
274+ // of an application.
275+ // The only situation where we expect to have two simultaneous instances is where there's one
276+ // closing and a newer one just starting, in which case we assume he's referring to the former.
277+ Application *application = findClosingApplication(appId);
278 if (!application) {
279- application = findClosingApplication(appId);
280+ application = findApplication(appId);
281 }
282
283 if (!application) {
284- qDebug() << "ApplicationManager::onProcessStopped reports stop of appId=" << appId
285+ // Might happen if the onProcessFailed() was called for this app and it already got deleted in response.
286+ qCDebug(QTMIR_APPLICATIONS) << "ApplicationManager::onProcessStopped reports stop of appId=" << appId
287 << "which AppMan is not managing, ignoring the event";
288 return;
289 }
290@@ -520,26 +563,15 @@
291
292 void ApplicationManager::onFocusRequested(const QString& appId)
293 {
294- qCDebug(QTMIR_APPLICATIONS) << "ApplicationManager::onFocusRequested - appId=" << appId;
295-
296- Q_EMIT focusRequested(appId);
297-}
298-
299-void ApplicationManager::onResumeRequested(const QString& appId)
300-{
301- qCDebug(QTMIR_APPLICATIONS) << "ApplicationManager::onResumeRequested - appId=" << appId;
302-
303 Application *application = findApplication(appId);
304
305- if (!application) {
306- qCritical() << "ApplicationManager::onResumeRequested: No such running application" << appId;
307- return;
308- }
309-
310- // We interpret this as a focus request for a suspended app.
311- // Shell will have this app resumed if it complies with the focus request
312- if (application->state() == Application::Suspended) {
313+ if (m_queuedStartApplications.contains(appId) || findClosingApplication(appId) != nullptr) {
314+ DEBUG_MSG(appId) << "Ignoring request as the application is closing and/or queued to start";
315+ } else if (application) {
316+ DEBUG_MSG(appId) << "Emitting focusRequested";
317 Q_EMIT focusRequested(appId);
318+ } else {
319+ CRITICAL_MSG(appId) << "No such running application";
320 }
321 }
322
323@@ -698,7 +730,16 @@
324 // FIXME: this is not desktop application friendly, but resolves issue where trust-prompt
325 // helpers take a long time to shut down, but destroys their surface quickly.
326 remove(application);
327- application->deleteLater();
328+ if (application->processState() == Application::ProcessUnknown) {
329+ // We will never get a TaskController::processStopped() signal for this application.
330+ // So remove it right away.
331+ application->deleteLater();
332+ } else {
333+ // It can't be in the ApplicationManager model and in the closing list at the same time
334+ Q_ASSERT(findClosingApplication(application->appId()) == nullptr);
335+ // Wait for a TaskController::processStopped()
336+ m_closingApplications.append(application);
337+ }
338 }
339 }
340
341@@ -752,9 +793,9 @@
342
343 connect(application, &Application::stopProcessRequested, this, [=]() {
344 if (!m_taskController->stop(application->longAppId()) && application->pid() > 0) {
345- qWarning() << "FAILED to ask Upstart to stop application with appId" << appId
346- << "Sending SIGTERM to process:" << appId;
347- kill(application->pid(), SIGTERM);
348+ qCWarning(QTMIR_APPLICATIONS).nospace() << "FAILED to ask Upstart to stop application with appId "
349+ << appId << ". Killing it with SIGTERM.";
350+ m_taskController->kill(application->pid());
351 application->setProcessState(Application::ProcessStopped);
352 }
353 });
354@@ -787,6 +828,9 @@
355 disconnect(application, &Application::focusedChanged, this, 0);
356 disconnect(application, &Application::stateChanged, this, 0);
357 disconnect(application, &Application::stageChanged, this, 0);
358+ disconnect(application, &Application::suspendProcessRequested, this, 0);
359+ disconnect(application, &Application::resumeProcessRequested, this, 0);
360+ disconnect(application, &Application::stopped, this, 0);
361
362 int i = m_applications.indexOf(application);
363 if (i != -1) {
364@@ -858,4 +902,23 @@
365 return nullptr;
366 }
367
368+void ApplicationManager::onSessionAboutToCreateSurface(
369+ const std::shared_ptr<mir::scene::Session> &session, int type, QSize &size)
370+{
371+ if (type == mir_surface_type_normal) {
372+ Application* application = findApplicationWithSession(session);
373+
374+ if (application) {
375+ qCDebug(QTMIR_APPLICATIONS).nospace() << "ApplicationManager::onSessionAboutToCreateSurface appId="
376+ << application->appId();
377+ size = application->initialSurfaceSize();
378+ } else {
379+ qCDebug(QTMIR_APPLICATIONS).nospace() << "ApplicationManager::onSessionAboutToCreateSurface unknown app";
380+ }
381+ } else {
382+ qCDebug(QTMIR_APPLICATIONS).nospace() << "ApplicationManager::onSessionAboutToCreateSurface type=" << type
383+ << " NOOP";
384+ }
385+}
386+
387 } // namespace qtmir
388
389=== modified file 'src/modules/Unity/Application/application_manager.h'
390--- src/modules/Unity/Application/application_manager.h 2016-02-01 16:09:52 +0000
391+++ src/modules/Unity/Application/application_manager.h 2016-02-01 16:09:52 +0000
392@@ -129,7 +129,6 @@
393 void onProcessSuspended(const QString& appId);
394 void onProcessFailed(const QString& appId, TaskController::Error error);
395 void onFocusRequested(const QString& appId);
396- void onResumeRequested(const QString& appId);
397
398 Q_SIGNALS:
399 void focusRequested(const QString &appId);
400@@ -137,6 +136,8 @@
401
402 private Q_SLOTS:
403 void onAppDataChanged(const int role);
404+ void onSessionAboutToCreateSurface(const std::shared_ptr<mir::scene::Session> &session,
405+ int type, QSize &size);
406
407 private:
408 void setFocused(Application *application);
409
410=== modified file 'src/modules/Unity/Application/taskcontroller.h'
411--- src/modules/Unity/Application/taskcontroller.h 2016-02-01 16:09:52 +0000
412+++ src/modules/Unity/Application/taskcontroller.h 2016-02-01 16:09:52 +0000
413@@ -53,6 +53,10 @@
414
415 virtual QFileInfo findDesktopFileForAppId(const QString &appId) const = 0;
416
417+ // Not a perfect fit for this class but we need the kill(pid_t pid, int sig) function
418+ // behind an interface so we can test it (replacing with a mock or fake)
419+ virtual void kill(pid_t pid) = 0;
420+
421 Q_SIGNALS:
422 void processStarting(const QString &appId);
423 void applicationStarted(const QString &appId);
424
425=== modified file 'src/modules/Unity/Application/upstart/taskcontroller.cpp'
426--- src/modules/Unity/Application/upstart/taskcontroller.cpp 2016-02-01 16:09:52 +0000
427+++ src/modules/Unity/Application/upstart/taskcontroller.cpp 2016-02-01 16:09:52 +0000
428@@ -28,6 +28,9 @@
429 #include "ubuntu-app-launch.h"
430 }
431
432+// std
433+#include <csignal>
434+
435 namespace qtmir
436 {
437 namespace upstart
438@@ -253,5 +256,10 @@
439 return QFileInfo();
440 }
441
442+void TaskController::kill(pid_t pid)
443+{
444+ ::kill(pid, SIGTERM);
445+}
446+
447 } // namespace upstart
448 } // namespace qtmir
449
450=== modified file 'src/modules/Unity/Application/upstart/taskcontroller.h'
451--- src/modules/Unity/Application/upstart/taskcontroller.h 2016-02-01 16:09:52 +0000
452+++ src/modules/Unity/Application/upstart/taskcontroller.h 2016-02-01 16:09:52 +0000
453@@ -42,6 +42,8 @@
454
455 QFileInfo findDesktopFileForAppId(const QString &appId) const override;
456
457+ void kill(pid_t pid) override;
458+
459 private:
460 struct Private;
461 QScopedPointer<Private> impl;
462
463=== modified file 'src/platforms/mirserver/mirserver.cpp'
464--- src/platforms/mirserver/mirserver.cpp 2015-11-10 11:07:23 +0000
465+++ src/platforms/mirserver/mirserver.cpp 2016-02-01 16:09:52 +0000
466@@ -107,7 +107,9 @@
467 override_the_window_manager_builder([this](mir::shell::FocusController* focus_controller)
468 -> std::shared_ptr<mir::shell::WindowManager>
469 {
470- return {MirWindowManager::create(focus_controller, the_shell_display_layout())};
471+ auto windowManager = MirWindowManager::create(focus_controller, the_shell_display_layout());
472+ m_windowManager = windowManager;
473+ return windowManager;
474 });
475
476 wrap_display_configuration_policy(
477@@ -193,3 +195,8 @@
478 std::weak_ptr<MirShell> m_shell = the_shell();
479 return m_shell.lock().get();
480 }
481+
482+MirWindowManager *MirServer::windowManager()
483+{
484+ return m_windowManager.lock().get();
485+}
486
487=== modified file 'src/platforms/mirserver/mirserver.h'
488--- src/platforms/mirserver/mirserver.h 2015-08-20 10:16:54 +0000
489+++ src/platforms/mirserver/mirserver.h 2016-02-01 16:09:52 +0000
490@@ -27,6 +27,7 @@
491 using MirShell = mir::shell::Shell;
492 class PromptSessionListener;
493 class ScreenController;
494+class MirWindowManager;
495
496 // We use virtual inheritance of mir::Server to facilitate derived classes (e.g. testing)
497 // calling initialization functions before MirServer is constructed.
498@@ -61,11 +62,12 @@
499 SessionAuthorizer *sessionAuthorizer();
500 SessionListener *sessionListener();
501 PromptSessionListener *promptSessionListener();
502+ MirWindowManager *windowManager();
503 MirShell *shell();
504
505 private:
506 std::weak_ptr<MirShell> m_shell;
507- std::shared_ptr<QtEventFeeder> m_qtEventFeeder;
508+ std::weak_ptr<MirWindowManager> m_windowManager;
509 const QSharedPointer<ScreenController> m_screenController;
510 };
511
512
513=== modified file 'src/platforms/mirserver/mirwindowmanager.cpp'
514--- src/platforms/mirserver/mirwindowmanager.cpp 2015-12-02 12:27:45 +0000
515+++ src/platforms/mirserver/mirwindowmanager.cpp 2016-02-01 16:09:52 +0000
516@@ -100,16 +100,26 @@
517 {
518 tracepoint(qtmirserver, surfacePlacementStart);
519
520- // TODO: Callback unity8 so that it can make a decision on that.
521- // unity8 must bear in mind that the called function will be on a Mir thread though.
522- // The QPA shouldn't be deciding for itself on such things.
523-
524+ QSize initialSize;
525+ // can be connected to via Qt::BlockingQueuedConnection to alter surface initial size
526+ {
527+ int surfaceType = requestParameters.type.is_set() ? requestParameters.type.value() : -1;
528+ Q_EMIT sessionAboutToCreateSurface(session, surfaceType, initialSize);
529+ }
530 ms::SurfaceCreationParameters placedParameters = requestParameters;
531
532- // Just make it fullscreen for now
533- mir::geometry::Rectangle rect{requestParameters.top_left, requestParameters.size};
534- m_displayLayout->size_to_output(rect);
535- placedParameters.size = rect.size;
536+ if (initialSize.isValid()) {
537+ placedParameters.size.width = mir::geometry::Width(initialSize.width());
538+ placedParameters.size.height = mir::geometry::Height(initialSize.height());
539+ } else {
540+ qCWarning(QTMIR_MIR_MESSAGES) << "MirWindowManagerImpl::add_surface(): didn't get a initial surface"
541+ " size from shell. Falling back to fullscreen placement";
542+ // This is bad. Fallback to fullscreen
543+ mir::geometry::Rectangle rect{requestParameters.top_left, requestParameters.size};
544+ m_displayLayout->size_to_output(rect);
545+ placedParameters.size = rect.size;
546+ }
547+
548
549 qCDebug(QTMIR_MIR_MESSAGES) << "MirWindowManagerImpl::add_surface(): size requested ("
550 << requestParameters.size.width.as_int() << "," << requestParameters.size.height.as_int() << ") and placed ("
551@@ -174,9 +184,9 @@
552 }
553 }
554
555-std::unique_ptr<MirWindowManager> MirWindowManager::create(
556+std::shared_ptr<MirWindowManager> MirWindowManager::create(
557 mir::shell::FocusController* /*focus_controller*/,
558 const std::shared_ptr<mir::shell::DisplayLayout> &displayLayout)
559 {
560- return std::make_unique<MirWindowManagerImpl>(displayLayout);
561+ return std::make_shared<MirWindowManagerImpl>(displayLayout);
562 }
563
564=== modified file 'src/platforms/mirserver/mirwindowmanager.h'
565--- src/platforms/mirserver/mirwindowmanager.h 2015-11-19 12:55:57 +0000
566+++ src/platforms/mirserver/mirwindowmanager.h 2016-02-01 16:09:52 +0000
567@@ -20,6 +20,7 @@
568 #include <mir/shell/window_manager.h>
569
570 #include <QObject>
571+#include <QSize>
572
573 namespace mir {
574 namespace shell {
575@@ -33,10 +34,14 @@
576 Q_OBJECT
577
578 public:
579-
580- static std::unique_ptr<MirWindowManager> create(
581+ static std::shared_ptr<MirWindowManager> create(
582 mir::shell::FocusController* focus_controller,
583 const std::shared_ptr<mir::shell::DisplayLayout> &displayLayout);
584+
585+Q_SIGNALS:
586+ // requires Qt::BlockingQueuedConnection!!
587+ void sessionAboutToCreateSurface(const std::shared_ptr<mir::scene::Session> &session,
588+ int type, QSize &size);
589 };
590
591 #endif /* QPAMIRSERVER_WINDOW_MANAGER_H */
592
593=== modified file 'tests/framework/mock_task_controller.h'
594--- tests/framework/mock_task_controller.h 2016-02-01 16:09:52 +0000
595+++ tests/framework/mock_task_controller.h 2016-02-01 16:09:52 +0000
596@@ -39,6 +39,7 @@
597 MOCK_METHOD2(start, bool(const QString&, const QStringList&));
598 MOCK_METHOD1(suspend, bool(const QString&));
599 MOCK_METHOD1(resume, bool(const QString&));
600+ MOCK_METHOD1(kill, void(pid_t pid));
601
602 pid_t doPrimaryPidForAppId(const QString& appId);
603
604
605=== modified file 'tests/mirserver/WindowManager/window_manager.cpp'
606--- tests/mirserver/WindowManager/window_manager.cpp 2016-01-22 17:51:40 +0000
607+++ tests/mirserver/WindowManager/window_manager.cpp 2016-02-01 16:09:52 +0000
608@@ -81,7 +81,7 @@
609
610 StubFocusController focus_controller;
611
612- const std::unique_ptr<MirWindowManager> window_manager =
613+ const std::shared_ptr<MirWindowManager> window_manager =
614 MirWindowManager::create(&focus_controller, mock_display_layout);
615
616 const Rectangle arbitrary_display{{0, 0}, {97, 101}};
617
618=== modified file 'tests/modules/ApplicationManager/application_manager_test.cpp'
619--- tests/modules/ApplicationManager/application_manager_test.cpp 2016-02-01 16:09:52 +0000
620+++ tests/modules/ApplicationManager/application_manager_test.cpp 2016-02-01 16:09:52 +0000
621@@ -2266,3 +2266,188 @@
622 EXPECT_EQ(Application::Stopped, application->state());
623 EXPECT_EQ(spy.count(), 1);
624 }
625+
626+/*
627+ * Context:
628+ * - we are waiting for an application to quit following a close()
629+ *
630+ * Event:
631+ * - TaskController requests that application to be resumed.
632+ * Most likely because the user tapped the app icon in the Dash. From the user's
633+ * perspective the application is already closed and he wants to start a new
634+ * instance of it.
635+ * But from TaskController's perspective the application is still running and
636+ * therefore should just be resumed/focused.
637+ *
638+ * Expected outcome:
639+ * - ApplicationManager should wait for the closing application to finally end (or
640+ be killed) and then finally launch a new instance of it.
641+ *
642+ * Regression test for https://bugs.launchpad.net/ubuntu/+source/qtmir/+bug/1527737
643+ * "Apps do not start if restarted quickly after closing"
644+ *
645+ * Two test versions:
646+ * _appQuits - Application complies and quits
647+ * _appKilled - Application ignores close request and thus get killed
648+ */
649+TEST_F(ApplicationManagerTests, focusRequestedForClosingApplication_appQuits)
650+{
651+ using namespace ::testing;
652+
653+ int argc = 0;
654+ char* argv[0];
655+ QCoreApplication qtApp(argc, argv); // app for deleteLater event
656+
657+ const QString appId("testAppId");
658+ quint64 procId = 5551;
659+
660+ ON_CALL(desktopFileReaderFactory, createInstance(appId, _))
661+ .WillByDefault(Invoke(
662+ [](const QString &appId, const QFileInfo&) { return new FakeDesktopFileReader(appId); }
663+ ));
664+
665+ EXPECT_CALL(*taskController, start(appId, _))
666+ .Times(1)
667+ .WillOnce(Return(true));
668+
669+ auto app = applicationManager.startApplication(appId, ApplicationManager::NoFlag);
670+ applicationManager.onProcessStarting(appId);
671+ std::shared_ptr<mir::scene::Session> session = std::make_shared<MockSession>("", procId);
672+ bool authed = true;
673+ applicationManager.authorizeSession(procId, authed);
674+ onSessionStarting(session);
675+
676+ FakeMirSurface *surface = new FakeMirSurface;
677+ onSessionCreatedSurface(session.get(), surface);
678+ surface->drawFirstFrame();
679+
680+ EXPECT_EQ(Application::InternalState::Running, app->internalState());
681+
682+ QSignalSpy closeRequestedSpy(surface, SIGNAL(closeRequested()));
683+
684+ // User swipes away the application, visually closing/removing it.
685+ applicationManager.stopApplication(appId);
686+
687+ // But asking ApplicationManager to stop the application just makes it request its
688+ // surfaces to be closed
689+ EXPECT_EQ(Application::InternalState::Closing, app->internalState());
690+ EXPECT_EQ(1, closeRequestedSpy.count());
691+
692+ QSignalSpy appAddedSpy(&applicationManager, SIGNAL(applicationAdded(const QString&)));
693+
694+ // User then taps on the app icon
695+ applicationManager.onProcessStarting(appId);
696+ applicationManager.onFocusRequested(appId);
697+
698+ // But no application instance should have been created because of that
699+ EXPECT_TRUE(applicationManager.findApplication(appId) == nullptr);
700+ EXPECT_EQ(0, appAddedSpy.count());
701+
702+ // ApplicationManager will ask TaskController to start the application again
703+ // once the closing instance is gone
704+ Mock::VerifyAndClearExpectations(taskController);
705+ EXPECT_CALL(*taskController, start(appId, _))
706+ .Times(1)
707+ .WillOnce(Return(true));
708+
709+ // Simulates that the application complied to the close() request and stopped itself
710+ onSessionStopping(session);
711+ applicationManager.onProcessStopped(appId);
712+
713+ // DeferredDelete is special: likes to be called out specifically or it won't come out
714+ qtApp.sendPostedEvents(app, QEvent::DeferredDelete);
715+ qtApp.sendPostedEvents();
716+
717+ EXPECT_TRUE(applicationManager.findApplication(appId) != nullptr);
718+ EXPECT_EQ(1, appAddedSpy.count());
719+}
720+TEST_F(ApplicationManagerTests, focusRequestedForClosingApplication_appKilled)
721+{
722+ using namespace ::testing;
723+
724+ int argc = 0;
725+ char* argv[0];
726+ QCoreApplication qtApp(argc, argv); // app for deleteLater event
727+
728+ const QString appId("testAppId");
729+ quint64 procId = 5551;
730+
731+ ON_CALL(desktopFileReaderFactory, createInstance(appId, _))
732+ .WillByDefault(Invoke(
733+ [](const QString &appId, const QFileInfo&) { return new FakeDesktopFileReader(appId); }
734+ ));
735+
736+ EXPECT_CALL(*taskController, start(appId, _))
737+ .Times(1)
738+ .WillOnce(Return(true));
739+
740+ auto app = applicationManager.startApplication(appId, ApplicationManager::NoFlag);
741+ applicationManager.onProcessStarting(appId);
742+ std::shared_ptr<mir::scene::Session> session = std::make_shared<MockSession>("", procId);
743+ bool authed = true;
744+ applicationManager.authorizeSession(procId, authed);
745+ onSessionStarting(session);
746+
747+ FakeMirSurface *surface = new FakeMirSurface;
748+ onSessionCreatedSurface(session.get(), surface);
749+ surface->drawFirstFrame();
750+
751+ EXPECT_EQ(Application::InternalState::Running, app->internalState());
752+
753+ QSharedPointer<FakeTimeSource> fakeTimeSource(new FakeTimeSource);
754+ FakeTimer *fakeCloseTimer = new FakeTimer(fakeTimeSource);
755+ app->setCloseTimer(fakeCloseTimer);
756+
757+ QSignalSpy closeRequestedSpy(surface, SIGNAL(closeRequested()));
758+
759+ // User swipes away the application, visually closing/removing it.
760+ applicationManager.stopApplication(appId);
761+
762+ // But asking ApplicationManager to stop the application just makes it request its
763+ // surfaces to be closed
764+ EXPECT_EQ(Application::InternalState::Closing, app->internalState());
765+ EXPECT_EQ(1, closeRequestedSpy.count());
766+
767+ QSignalSpy appAddedSpy(&applicationManager, SIGNAL(applicationAdded(const QString&)));
768+
769+ // User then taps on the app icon
770+ applicationManager.onProcessStarting(appId);
771+ applicationManager.onFocusRequested(appId);
772+
773+ // But no application instance should have been created because of that
774+ EXPECT_TRUE(applicationManager.findApplication(appId) == nullptr);
775+ EXPECT_EQ(0, appAddedSpy.count());
776+
777+ // ApplicationManager will ask TaskController to start the application again
778+ // once the closing instance is gone
779+ Mock::VerifyAndClearExpectations(taskController);
780+ EXPECT_CALL(*taskController, start(appId, _))
781+ .Times(1)
782+ .WillOnce(Return(true));
783+
784+ EXPECT_CALL(*taskController, stop(appId))
785+ .Times(1)
786+ .WillOnce(Return(false));
787+
788+ EXPECT_CALL(*taskController, kill(procId))
789+ .Times(1)
790+ .WillOnce(Invoke(
791+ [this, session](pid_t) {
792+ // simulate application dying and thus severing the mir connection
793+ onSessionStopping(session);
794+ }
795+ ));
796+
797+ if (fakeCloseTimer->isRunning()) {
798+ // Simulate that closeTimer has timed out.
799+ fakeTimeSource->m_msecsSinceReference = fakeCloseTimer->nextTimeoutTime() + 1;
800+ fakeCloseTimer->update();
801+ }
802+
803+ // DeferredDelete is special: likes to be called out specifically or it won't come out
804+ qtApp.sendPostedEvents(app, QEvent::DeferredDelete);
805+ qtApp.sendPostedEvents();
806+
807+ EXPECT_TRUE(applicationManager.findApplication(appId) != nullptr);
808+ EXPECT_EQ(1, appAddedSpy.count());
809+}

Subscribers

People subscribed via source and target branches