Merge lp://qastaging/~nick-dedekind/unity-mir/trusted-sessions into lp://qastaging/unity-mir

FAILED: Continuous integration, rev:178
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/294/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-amd64-ci/157/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-armhf-ci/158/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-i386-ci/157/console

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

FAILED: Continuous integration, rev:184
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/325/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-amd64-ci/188/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-armhf-ci/189/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-i386-ci/188/console

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

FAILED: Continuous integration, rev:187
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/345/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-amd64-ci/9/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-armhf-ci/9/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-i386-ci/9/console

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

FAILED: Continuous integration, rev:189
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/347/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-amd64-ci/11/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-armhf-ci/11/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-i386-ci/11/console

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

FAILED: Continuous integration, rev:190
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/348/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-amd64-ci/12/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-armhf-ci/12/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-i386-ci/12/console

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

FAILED: Continuous integration, rev:192
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/354/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-amd64-ci/18/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-armhf-ci/18/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-i386-ci/18/console

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

FAILED: Continuous integration, rev:193
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/355/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-amd64-ci/19/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-armhf-ci/19/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-i386-ci/19/console

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

FAILED: Continuous integration, rev:194
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/356/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-amd64-ci/20/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-armhf-ci/20/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-i386-ci/20/console

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

FAILED: Continuous integration, rev:196
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/358/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-amd64-ci/22/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-armhf-ci/22/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-i386-ci/22/console

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

FAILED: Continuous integration, rev:197
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/364/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-amd64-ci/28/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-armhf-ci/28/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-i386-ci/28/console

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

FAILED: Continuous integration, rev:199
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/365/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-amd64-ci/29/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-armhf-ci/29/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-i386-ci/29/console

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

FAILED: Continuous integration, rev:200
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/369/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-amd64-ci/33/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-armhf-ci/33/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-i386-ci/33/console

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

FAILED: Continuous integration, rev:201
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/371/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-amd64-ci/35/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-armhf-ci/35/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-i386-ci/35/console

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

FAILED: Continuous integration, rev:202
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/372/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-amd64-ci/36/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-armhf-ci/36/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-utopic-i386-ci/36/console

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

Is this actually ready for review? If not, please mark as Work in Progress

FAILED: Continuous integration, rev:205
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~nick-dedekind/unity-mir/trusted-sessions/+merge/223432/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/unity-mir-devel-ci/16/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-devel-utopic-amd64-ci/16/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-devel-utopic-armhf-ci/16/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-devel-utopic-i386-ci/16/console

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

FAILED: Continuous integration, rev:206
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~nick-dedekind/unity-mir/trusted-sessions/+merge/223432/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/unity-mir-devel-ci/17/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-devel-utopic-amd64-ci/17/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-devel-utopic-armhf-ci/17/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-devel-utopic-i386-ci/17/console

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

FAILED: Continuous integration, rev:207
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~nick-dedekind/unity-mir/trusted-sessions/+merge/223432/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/unity-mir-devel-ci/18/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-devel-utopic-amd64-ci/18
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-devel-utopic-armhf-ci/18
        deb: http://jenkins.qa.ubuntu.com/job/unity-mir-devel-utopic-armhf-ci/18/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-devel-utopic-i386-ci/18

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

FAILED: Continuous integration, rev:209
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~nick-dedekind/unity-mir/trusted-sessions/+merge/223432/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/unity-mir-devel-ci/19/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-devel-utopic-amd64-ci/19
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-devel-utopic-armhf-ci/19
        deb: http://jenkins.qa.ubuntu.com/job/unity-mir-devel-utopic-armhf-ci/19/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-devel-utopic-i386-ci/19

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

FAILED: Continuous integration, rev:210
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~nick-dedekind/unity-mir/trusted-sessions/+merge/223432/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/unity-mir-devel-ci/20/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-devel-utopic-amd64-ci/20
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-devel-utopic-armhf-ci/20
        deb: http://jenkins.qa.ubuntu.com/job/unity-mir-devel-utopic-armhf-ci/20/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-devel-utopic-i386-ci/20

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

Commit message please. Checklist too.

- std::shared_ptr<::mir::scene::Session> m_session;
+ mutable mir::scene::Session* m_lastTopSession;

So now Application doesn't have a shared_ptr to a Mir Session object. Reason unity-mir held onto that was so that the Session object was only deleted when both Mir and unity-mir had released it, as it may take time for the Application to be removed from the app list, and QML could be asking for properties of that Session after Mir was done with it.

=== modified file 'debian/control'
not necessary change for this MR, please revert

Taking the smaller bits first

=== added file 'src/modules/Unity/Application/promptsession.h'
+ // Qt
+#include <QSharedPointer>
not needed in header

namespace mir
+{
+namespace scene
+{
+class Session;
+class PromptSession;
+class PromptSessionManager;
please indent like in ApplicationManager

In PromptSession constructor, line 45, please line up the arguments (space missing)

+ virtual ~PromptSession();
Why virtual? Could use "= default" to have default deconstructor generated, saving you these few lines:
 +Session::~Session()
+{
+}

+ std::shared_ptr<mir::scene::Session> topProvider() const;
I find the name confusing. What is a "provider"? It returns a Session. "top" also isn't useful, unless you happen to know the internals of unity-mir::PromptSession is a stack.

+ std::shared_ptr<mir::scene::PromptSession> mirPromptSession() const;
shows that we can now easily confuse mir's PromptSession with unity-mir's PromptSession. Perhaps a slightly different class name? PromptSessionWrapper?

+Q_DECLARE_METATYPE(unitymir::PromptSession*)
Why do you need to register it as a type with MOC?

=== added file 'src/modules/Unity/Application/promptsession.cpp'
+#include <QDebug>
not used

+PromptSession::PromptSession(const std::shared_ptr<mir::scene::PromptSession>& promptSession, const std::shared_ptr<mir::scene::PromptSessionManager>& promptSessionManager)
please wrap. I'm not strict, but 120 chars is reasonable.

+ m_promptSessionManager->for_each_provider_in(m_promptSession,
+ [&child](std::shared_ptr<mir::scene::Session> const& provider) {
+ child = provider;
+ });
Really? Is there no way to identify the "top" one other than iterating the whole list/stack?!

+ auto helper = m_promptSessionManager->helper_for(m_promptSession);
I only like using auto if the type is really obvious. In this case it's not, I'd prefer you either declare the type, or use name "helperSession"

+ [&](std::shared_ptr<mir::scene::Session> const& provider) {
You only need to capture "found" here, not all locals.

+ }
please add comment to the namespace closing brace, it's a style I'm trying to maintain in this project.

=== added file 'src/modules/Unity/Application/session.h'
What's the point of this entire class? Do you need to share a mir Session with multiple Applications??

=== added file 'src/unity-mir/promptsessionlistener.cpp'
+ * Copyright (C) 2013 Canonical, Ltd.
2014

+ DLOG("PromptSessionListener::prompt_provider_added (this=%p, prompt_session=%p, prompt_provider=%p)", this, &prompt_session, (void*)prompt_provider.get());
+ DLOG("PromptSessionListener::prompt_provider_removed (this=%p, prompt_session=%p, prompt_provider=%p)", this, &prompt_session, (void*)prompt_provider.get());
please wrap

Same for the header file, prompt_provider_{added,removed} should be wrapped

=== modified file 'src/unity-mir/qmirserver.cpp'
Unnecessary change, please revert

=== modified file 'src/unity-mir/sessionauthorizer.cpp'
Unnecessary change, please revert

=== modified file 'src/modules/Unity/Application/proc_info.h'
+ virtual quint64 ppid(quint64 pid);
what does that mean? Parent process id? Does process-cpp not have a method to get that?

=== modified file 'src/mo...

=== modified file 'src/modules/Unity/Application/mirsurface.h'
- explicit MirSurface(std::shared_ptr<mir::scene::Surface> surface, Application* application, QQuickItem *parent = 0);
1069 + explicit MirSurface(std::shared_ptr<mir::scene::Surface> surface, QSharedPointer<Session> const& session, QQuickItem *parent = 0);

I object strongly, why create distinction between an Application and a Session? Why does a shell need to know about Sessions?

In my vocabulary, Applications have Surfaces, and that's all a shell needs to know to operate. Those Surfaces can have relationships and properties, which shell can use to treat them differently.

Why do you need this?

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

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

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

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

> Taking the smaller bits first
>
>
>
> === added file 'src/modules/Unity/Application/promptsession.h'
> + // Qt
> +#include <QSharedPointer>
> not needed in header
>
> namespace mir
> +{
> +namespace scene
> +{
> +class Session;
> +class PromptSession;
> +class PromptSessionManager;
> please indent like in ApplicationManager
>
> In PromptSession constructor, line 45, please line up the arguments (space
> missing)
>
> + virtual ~PromptSession();
> Why virtual? Could use "= default" to have default deconstructor generated,
> saving you these few lines:
> +Session::~Session()
> +{
> +}
>
> + std::shared_ptr<mir::scene::Session> topProvider() const;
> I find the name confusing. What is a "provider"? It returns a Session. "top"
> also isn't useful, unless you happen to know the internals of unity-
> mir::PromptSession is a stack.
>
> + std::shared_ptr<mir::scene::PromptSession> mirPromptSession() const;
> shows that we can now easily confuse mir's PromptSession with unity-mir's
> PromptSession. Perhaps a slightly different class name? PromptSessionWrapper?
>
> +Q_DECLARE_METATYPE(unitymir::PromptSession*)
> Why do you need to register it as a type with MOC?
>
>
> === added file 'src/modules/Unity/Application/promptsession.cpp'
> +#include <QDebug>
> not used
>
> +PromptSession::PromptSession(const
> std::shared_ptr<mir::scene::PromptSession>& promptSession, const
> std::shared_ptr<mir::scene::PromptSessionManager>& promptSessionManager)
> please wrap. I'm not strict, but 120 chars is reasonable.

removed unitymir::PromptSession

>
> + m_promptSessionManager->for_each_provider_in(m_promptSession,
> + [&child](std::shared_ptr<mir::scene::Session> const& provider) {
> + child = provider;
> + });
> Really? Is there no way to identify the "top" one other than iterating the
> whole list/stack?!

nope. but there won't be many (like only one/two...)

>
> + auto helper = m_promptSessionManager->helper_for(m_promptSession);
> I only like using auto if the type is really obvious. In this case it's not,
> I'd prefer you either declare the type, or use name "helperSession"
>
> + [&](std::shared_ptr<mir::scene::Session> const& provider) {
> You only need to capture "found" here, not all locals.
>
> + }
> please add comment to the namespace closing brace, it's a style I'm trying to
> maintain in this project.
>
>
>
>
> === added file 'src/modules/Unity/Application/session.h'
> What's the point of this entire class? Do you need to share a mir Session with
> multiple Applications??
>

removed unitymir::Session

>
> === added file 'src/unity-mir/promptsessionlistener.cpp'
> + * Copyright (C) 2013 Canonical, Ltd.
> 2014
>
> + DLOG("PromptSessionListener::prompt_provider_added (this=%p,
> prompt_session=%p, prompt_provider=%p)", this, &prompt_session,
> (void*)prompt_provider.get());
> + DLOG("PromptSessionListener::prompt_provider_removed (this=%p,
> prompt_session=%p, prompt_provider=%p)", this, &prompt_session,
> (void*)prompt_provider.get());
> please wrap
>
> Same for the header file, prompt_provider_{added,removed} should be wrapped
>

done

>
>
> === modified file 'src/unity-mir/qmirserver.cpp'
> Unnecessary change, please reve...

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

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

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

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

Thanks for the design changes, I'm much happier with it now. I've mostly small annoying things to request. Taking everything except AppManager and the tests (will check them last):

=== modified file 'src/modules/Unity/Application/application.h'
+ std::shared_ptr<mir::scene::PromptSessionManager> const promptSessionManager,
Could pass by reference to save a copy?

Also, you've seen that the current variable declaration syntax mess that is the "const Type& name" and "Type const& name" confusion. I need to tidy it up, but I'm slowly standardizing everything to "const Type &name" - would you hate me if I asked you to do the same, just in the bits you added? It's ok to say no :)

+ std::shared_ptr<mir::scene::Session> effectiveSession() const;
I think "foregroundSession" is a clearer name than effectiveSession.

session() is also not a great name now, but changing it everywhere would be lots more work, so we can leave it for now.

std::shared_ptr<mir::scene::PromptSession> Application::promptSession() const;
There could be multiple such prompt sessions, but this returns only the most recently created one. Maybe better name is "foremostPromptSession" or "activePromptSession"?

+ mutable mir::scene::Session* m_lastTopSession;
"Top" -> "Foreground" please.

Note that ApplicationManager is a friend of Application, so all your additional public methods can be made private, as they're not useful to shell/QML.

+ void pushPromptSession(const std::shared_ptr<mir::scene::PromptSession>& session);
+ void removePromptSession(const std::shared_ptr<mir::scene::PromptSession>& session);
Could you rename the first to "append" - if I see "push" I expect a "pop", else "add/append" and "remove" pair nicely

=== modified file 'src/modules/Unity/Application/application.cpp'
+ mir::scene::PromptSession
please add "namespace ms = mir::scene" to the top of the file and write "ms::PromptSession" everywhere, helps to hide some of Mir's namespace verbosity.

+ std::shared_ptr<mir::scene::PromptSession> prompt_session
camelCase please.

+ std::shared_ptr<mir::scene::Session> child;
'child' could be declared outside the while loop, save create/destroy on each iteration.

+ if (m_pid == pid) {
2 spaces after m_pid. Braces not really needed either for this 1-liner (which is your usual style).

+void Application::pushPromptSession(const std::shared_ptr<mir::scene::PromptSession>& prompt_session)
camelCase please

+void Application::removePromptSession
A warning if no matching prompt session was found would not hurt, and might save future debugging pain.

=== modified file 'src/modules/Unity/Application/mirsurface.h'
Whitespace change, not needed in this MR.

Ok, last need to read AppMan, the tests, and do functional testing.

=== added file 'src/unity-mir/promptsessionlistener.cpp'
+#include <QThread>
needed?

+void Application::removePromptSession
A warning if no matching prompt session was found would not hurt, and might save future debugging pain

It calls this on applications which may not necessarily have the prompt session (loops through all apps). More efficient than checking, then removing.

Fixed others.

ApplicationManager:

=== modified file 'src/modules/Unity/Application/application_manager.h'
+ std::shared_ptr<mir::shell::FocusController> const& focusController,
+ const std::shared_ptr<mir::scene::PromptSessionManager>& promptSessionManager,
Consistency please

+ Application* findApplicationWithPid(const qint64 pid, bool checkSessions)
+ Application* findApplicationWithSession(const std::shared_ptr<mir::scene::Session> &session, bool checkSessions);
+ Application* findApplicationWithSession(const mir::scene::Session *session, bool checkSessions);
"checkSessions" - better name would be more like "includeChildSessions" or something like that, WDYT?

+ void onApplicationEffectiveSessionChanged();
s/Effective/Foreground/

+ if (info->contains("trust-session-demo-trusted-helper") ||
+ info->contains("unity8") ||
+ (ppid != 0 && m_procInfo->commandLine(ppid)->contains("trust-session-demo-trusted-helper"))) {
That's pretty messy. Why considering an app with "unity8" in the process name as a trusted helper? So you're allowing any process directly spawned by the trusted helper. I guess that's all we can do, until our internal discussions conclude.

+ info->asStringList()[0].toLatin1().data());
qPrintable(info->asStringList().first()) an alternative, up to you

+ if (m_fencePIDs.contains(pid)) {
It looks like you've changed m_fencePIDs contains the PIDs of all whitelisted and trusted helpers while they are running. That's ok, but "fence" is not a good name now - perhaps "ignored" or "hidden" ?

+ if (!session) { return NULL; }
2 lines, no braces please. "nullptr" too.

Ok, just tests to go

Tests
=== modified file 'tests/application_manager_test.cpp'

+ Application* startApplication(quint64 procId, QString const& appId)
Not a bad idea.

+ auto session1 = std::make_shared<MockSession>(appId.toStdString(), procId);
+ auto promptSession = std::make_shared<MockPromptSession>();

I don't see these being used anywhere though.

TEST_F(ApplicationManagerTests,onceAppAddedToApplicationLists_mirSurfaceCreatedEventHandled)
+ std::shared_ptr<mir::scene::Session> session = std::make_shared<MockSession>("", procId);
+++
why did you move these lines before the focusSpy?

+ std::shared_ptr<mir::scene::Surface> providerSurface((mir::scene::Surface*)__LINE__, [](mir::scene::Surface*) {}); // use mock!
What does that do? Is that comment a TODO?

+ {
+ return 0;
strange indent

Ah, can you re-propose against lp:unity-mir please?

lp:unity-mir/devel really only for Mir devel compatibility changes

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

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

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

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

> Tests
> === modified file 'tests/application_manager_test.cpp'
>
> + Application* startApplication(quint64 procId, QString const& appId)
> Not a bad idea.
>
> + auto session1 = std::make_shared<MockSession>(appId.toStdString(), procId);
> + auto promptSession = std::make_shared<MockPromptSession>();
>
> I don't see these being used anywhere though.
>

Moved AppMan.onSessionstart(session1) to startApplication.
Removed other.

>
>
> TEST_F(ApplicationManagerTests,onceAppAddedToApplicationLists_mirSurfaceCreate
> dEventHandled)
> + std::shared_ptr<mir::scene::Session> session =
> std::make_shared<MockSession>("", procId);
> +++
> why did you move these lines before the focusSpy?
>

This was because of the foregroundSessionChanged signal causing a screenshot dataChanged. However, there was a bit of code duplication in the AppMan which I removed. So I've changed it back.

>
> + std::shared_ptr<mir::scene::Surface>
> providerSurface((mir::scene::Surface*)__LINE__, [](mir::scene::Surface*) {});
> // use mock!
> What does that do? Is that comment a TODO?

Made comment a TODO
mmmm. that's a shared_ptr with a custom deleter (an empty deleter in this case. Ever seen mir::test::fake_shared?).
It's using the line number as a pointer address so that calling Session::default_surface does not return nullptr, which is a condition for a Application::foregroundSession sourced by a PromptSession.
Not ideal, but that's why the TODO is there.

>
> + {
> + return 0;
> strange indent

Done