Merge lp:~gerboland/unity-mir/use-upstart-app-launch2 into lp:unity-mir

[Gerry Boland]

Use upstart-app-launch for app start/stop

Use upstart-app-launch for app start/stop

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[lp:~gerboland/unity-mir/use-upstart-app-launch2]

97. By Gerry Boland on 2013-09-26

If process ends not by hand of shell, clean up after it properly

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:97
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/75/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-amd64-ci/17
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-armhf-ci/75/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-i386-ci/75

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

review: Needs Fixing (continuous-integration)

[Daniel d'Andrada (dandrader)]

+ DLOG("Application::Application (this=%p, appId=%p, state=%d", this, qPrintable(appId), static_cast<int>(state));

s/appId=%p/appId=%s

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:97
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/76/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-amd64-ci/18
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-armhf-ci/76
        deb: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-armhf-ci/76/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-i386-ci/76

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

review: Approve (continuous-integration)

[lp:~gerboland/unity-mir/use-upstart-app-launch2]

98. By Gerry Boland on 2013-09-26

s/appId=%p/appId=%s - well spotted Daniel

[Daniel d'Andrada (dandrader)]

"""
|@@ -354,12 +271,14 @@ void ApplicationManager::authorizeSessio
| authorized = false; //to be proven wrong
|
| DLOG("ApplicationManager::authorizeSession (this=%p, pid=%lld)", this, pid);
|- Application* application = findApplicationWithPid(pid);
|- if (application) {
""""

Speaking of log messages, I know this code is not touched by your patch but for extra bonus points it would be nice if
s/pid=%lld)"/pid=%"PRIu64")" for architecture independence (it's also an unsigned type, so lld is is wrong in more than one way).

[Daniel d'Andrada (dandrader)]

"""
@@ -428,7 +347,10 @@ void ApplicationManager::authorizeSessio

     QString argStr(command.data());
     QStringList arguments(argStr.split(' '));
- application = new Application(desktopData, pid, stage, Application::Starting, arguments, m_taskController);
+ application = new Application(desktopData->appId(), Application::Starting, arguments, this);
+ delete desktopData;
+ application->setPid(pid);
+ application->setStage(stage);
"""

It's rather wasteful to parse a desktop file twice in this situation. Can't we easily avoid it by having a second constructor that takes a DesktopFileReader instead of a appId as an argument?

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:98
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/77/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-amd64-ci/19
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-armhf-ci/77
        deb: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-armhf-ci/77/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-i386-ci/77

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

review: Approve (continuous-integration)

[lp:~gerboland/unity-mir/use-upstart-app-launch2]

99. By Gerry Boland on 2013-09-26

Support respawning apps again

[Daniel d'Andrada (dandrader)]

I wonder if we can fix the mess that a pid is represented as a pid_t, a qint64 and a quint64.

[Daniel d'Andrada (dandrader)]

Code-wise, my only complaint is the unnecessary recreation of the DesktopFileReader I explained above.

review: Needs Fixing

[lp:~gerboland/unity-mir/use-upstart-app-launch2]

100. By Gerry Boland on 2013-09-26

Some handy extra debug info

101. By Gerry Boland on 2013-09-26

Fix for quick focus->unfocus->focus caused app to still suspend

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:99
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/78/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-amd64-ci/20
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-armhf-ci/78
        deb: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-armhf-ci/78/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-i386-ci/78

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

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Gerry Boland (gerboland)]

> I wonder if we can fix the mess that a pid is represented as a pid_t, a qint64
> and a quint64.
Yep, it's a mess, and on my TODO list to fix in another MR.

[Gerry Boland (gerboland)]

> It's rather wasteful to parse a desktop file twice in this situation. Can't we
> easily avoid it by having a second constructor that takes a DesktopFileReader
> instead of a appId as an argument?
You raise a fair point. I wanted to avoid having to parse desktop files in ApplicationManager at all, delegating it to Application alone, but the authorizer forced my hand.

I did the duplicate as I hope much of that authorizer code will go away soon. But I'll try to improve it now.

[lp:~gerboland/unity-mir/use-upstart-app-launch2]

102. By Gerry Boland on 2013-09-27

Application constructor can take both appId or DesktopFileReader

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[lp:~gerboland/unity-mir/use-upstart-app-launch2]

103. By Gerry Boland on 2013-09-27

Add todo for screenshot

[Michał Sawicz (saviq)]

Code looks good, didn't test though.

review: Approve (code)

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Daniel d'Andrada (dandrader)]

Looks good now.

review: Approve

=== modified file 'debian/control'
--- debian/control	2013-09-25 21:06:22 +0000
+++ debian/control	2013-09-27 15:13:07 +0000
@@ -8,6 +8,7 @@
                libmirserver-dev (>= 0.0.12),
                libmirclient-dev (>= 0.0.12),
                libunity-api-dev,
+               libupstart-app-launch1-dev,
                qt5-default,
                qtbase5-dev,
                qtdeclarative5-dev,
@@ -39,6 +40,7 @@
 Depends: ${misc:Depends},
          ${shlibs:Depends},
          libunity-mir1 (= ${binary:Version}),
+         libupstart-app-launch1-dev,
          libmirserver-dev (>= 0.0.11),
          libmirclient-dev (>= 0.0.11),
          libplatform-api1-dev,
 
=== modified file 'src/modules/Unity/Application/Application.pro'
--- src/modules/Unity/Application/Application.pro	2013-09-11 13:20:08 +0000
+++ src/modules/Unity/Application/Application.pro	2013-09-27 15:13:07 +0000
@@ -9,7 +9,7 @@
 QMAKE_CXXFLAGS_RELEASE += -Werror     # so no stop on warning in debug builds
 QMAKE_LFLAGS = -std=c++11 -Wl,-no-undefined
 
-PKGCONFIG += mircommon mirserver ubuntu-platform-api
+PKGCONFIG += mircommon mirserver ubuntu-platform-api glib-2.0 upstart-app-launch-1
 
 INCLUDEPATH += ../../../unity-mir
 LIBS += -L../../../unity-mir -lunity-mir \
 
=== modified file 'src/modules/Unity/Application/application.cpp'
--- src/modules/Unity/Application/application.cpp	2013-09-12 17:29:51 +0000
+++ src/modules/Unity/Application/application.cpp	2013-09-27 15:13:07 +0000
@@ -18,6 +18,7 @@
 #include "application.h"
 #include "application_manager.h"
 #include "desktopfilereader.h"
+#include "taskcontroller.h"
 
 // unity-mir
 #include "logging.h"
@@ -25,24 +26,26 @@
 // mir
 #include <mir/shell/application_session.h>
 
-Application::Application(DesktopFileReader* desktopData, qint64 pid,
-                         Application::Stage stage, Application::State state,
-                         const QStringList& arguments, TaskController* taskController,
-                         QObject* parent)
-    : ApplicationInfoInterface(desktopData->appId(), parent)
-    , m_desktopData(desktopData)
-    , m_pid(pid)
-    , m_stage(stage)
+Application::Application(const QString &appId, Application::State state,
+                         const QStringList &arguments, QObject *parent)
+    : Application(new DesktopFileReader(appId), state, arguments, parent)
+{
+}
+
+Application::Application(DesktopFileReader *desktopFileReader, State state,
+                         const QStringList &arguments, QObject *parent)
+    : ApplicationInfoInterface(desktopFileReader->appId(), parent)
+    , m_desktopData(desktopFileReader)
+    , m_pid(0)
+    , m_stage((m_desktopData->stageHint() == "SideStage") ? Application::SideStage : Application::MainStage)
     , m_state(state)
     , m_focused(false)
     , m_fullscreen(false)
     , m_arguments(arguments)
-    , m_taskController(taskController)
     , m_suspendTimer(new QTimer(this))
 {
-    DASSERT(desktopData != NULL);
-    DLOG("Application::Application (this=%p, desktopData=%p, pid=%lld, stage=%d, state=%d",
-         this, desktopData, pid, static_cast<int>(stage), static_cast<int>(state));
+    DLOG("Application::Application (this=%p, appId=%s, state=%d", this, qPrintable(desktopFileReader->appId()),
+         static_cast<int>(state));
 
     m_suspendTimer->setSingleShot(true);
     connect(m_suspendTimer, SIGNAL(timeout()), this, SLOT(suspend()));
@@ -54,6 +57,11 @@
     delete m_desktopData;
 }
 
+bool Application::isValid() const
+{
+    return m_desktopData->loaded();
+}
+
 QString Application::desktopFile() const
 {
     return m_desktopData->file();
@@ -130,6 +138,8 @@
 
 void Application::setSession(const std::shared_ptr<mir::shell::ApplicationSession>& session)
 {
+    DLOG("Application::setSession (this=%p, session=%p)", this, session.get());
+
     // TODO(greyback) what if called with new surface?
     m_session = session;
 }
@@ -166,6 +176,9 @@
             }
             break;
         case Application::Running:
+            if (m_suspendTimer->isActive())
+                m_suspendTimer->stop();
+
             if (m_state == Application::Suspended) {
                 resume();
                 session()->set_lifecycle_state(mir_lifecycle_state_resumed);
@@ -207,20 +220,17 @@
 void Application::suspend()
 {
     DLOG("Application::suspend (this=%p)", this);
-
-    m_taskController->do_suspend(this);
+    TaskController::singleton()->suspend(appId());
 }
 
 void Application::resume()
 {
     DLOG("Application::resume (this=%p)", this);
-
-    m_taskController->do_resume(this);
+    TaskController::singleton()->resume(appId());
 }
 
 void Application::respawn()
 {
     DLOG("Application::respawn (this=%p)", this);
-
-    m_taskController->do_respawn(this);
+    TaskController::singleton()->start(appId(), m_arguments);
 }
 
=== modified file 'src/modules/Unity/Application/application.h'
--- src/modules/Unity/Application/application.h	2013-09-11 13:20:08 +0000
+++ src/modules/Unity/Application/application.h	2013-09-27 15:13:07 +0000
@@ -42,9 +42,8 @@
     Q_PROPERTY(bool fullscreen READ fullscreen NOTIFY fullscreenChanged)
 
 public:
-    Application(DesktopFileReader* desktopData, qint64 pid, Stage stage, State state,
-                const QStringList& arguments, TaskController* taskController,
-                QObject* parent = 0);
+    Application(const QString &appId, State state, const QStringList &arguments, QObject *parent = 0);
+    Application(DesktopFileReader *desktopFileReader, State state, const QStringList &arguments, QObject *parent = 0);
     virtual ~Application();
 
     // ApplicationInfoInterface
@@ -56,6 +55,7 @@
     State state() const override;
     bool focused() const override;
 
+    bool isValid() const;
     QString desktopFile() const;
     QString exec() const;
     bool fullscreen() const;
@@ -88,10 +88,8 @@
     std::shared_ptr<mir::shell::ApplicationSession> m_session;
     QString m_sessionName;
     QStringList m_arguments;
-    TaskController* m_taskController;
     QTimer* m_suspendTimer;
 
-    friend class TaskController;
     friend class ApplicationManager;
     friend class ApplicationListModel;
     friend class MirSurfaceManager;
 
=== modified file 'src/modules/Unity/Application/application_manager.cpp'
--- src/modules/Unity/Application/application_manager.cpp	2013-09-12 17:27:47 +0000
+++ src/modules/Unity/Application/application_manager.cpp	2013-09-27 15:13:07 +0000
@@ -25,6 +25,7 @@
 #include "shellserverconfiguration.h"
 #include "sessionlistener.h"
 #include "sessionauthorizer.h"
+#include "taskcontroller.h"
 #include "logging.h"
 
 // mir
@@ -33,13 +34,6 @@
 
 // Qt
 #include <QCoreApplication>
-#include <QProcess>
-
-#define USE_UPSTART 0
-
-#if !USE_UPSTART
-  #include <pwd.h>
-#endif
 
 namespace msh = mir::shell;
 
@@ -58,6 +52,7 @@
 ApplicationManager::ApplicationManager(QObject *parent)
 :   ApplicationManagerInterface(parent)
 ,   m_focusedApplication(NULL)
+,   m_taskController(TaskController::singleton())
 {
     DLOG("ApplicationManager::ApplicationManager (this=%p)", this);
 
@@ -82,11 +77,12 @@
     QObject::connect(m_mirServer->sessionAuthorizer(), &SessionAuthorizer::requestAuthorizationForSession,
                      this, &ApplicationManager::authorizeSession, Qt::BlockingQueuedConnection);
 
-    // we use random numbers, so seed the generator
-    qsrand(QTime::currentTime().msec());
+    QObject::connect(m_taskController.data(), &TaskController::processStarted,
+                     this, &ApplicationManager::onProcessStarted);
+    QObject::connect(m_taskController.data(), &TaskController::processStopped,
+                     this, &ApplicationManager::onProcessStopped);
 
     m_dbusWindowStack = new DBusWindowStack(this);
-    m_taskController = new TaskController(this);
 }
 
 ApplicationManager::~ApplicationManager()
@@ -152,10 +148,23 @@
 
 bool ApplicationManager::focusApplication(const QString &appId)
 {
-    DLOG("ApplicationManager::focusApplication (this=%p, appId=%s)", this, appId.toLatin1().data());
+    DLOG("ApplicationManager::focusApplication (this=%p, appId=%s)", this, qPrintable(appId));
     Application *application = findApplication(appId);
 
-    m_mirServer->the_session_manager()->set_focus_to(application->session());
+    if (!application) {
+        DLOG("No such running application '%s'", qPrintable(appId));
+        return false;
+    }
+
+    if (application->state() == Application::Stopped) {
+        // Respawning this app, move to end of application list so onSessionStarting works ok
+        // FIXME: this happens pretty late, shell could request respawn earlier
+        application->setState(Application::Running);
+        int from = m_applications.indexOf(application);
+        move(from, m_applications.length()-1);
+    } else {
+        m_mirServer->the_session_manager()->set_focus_to(application->session());
+    }
 
     // FIXME(dandrader): lying here. The operation is async. So we will only know whether
     // the focusing was successful once the server replies. Maybe the API in unity-api should
@@ -170,40 +179,6 @@
     m_mirServer->the_session_manager()->set_focus_to(NULL); //FIXME(greyback)
 }
 
-Application* ApplicationManager::respawnApplication(Application* application)
-{
-    DLOG("ApplicationManager::respawnApplication(this=%p, application=%p)", this, application);
-
-    // Start process - set correct environment
-    setenv("QT_QPA_PLATFORM", "ubuntumirclient", 1);
-
-    bool result;
-    qint64 pid = 0;
-    struct passwd* passwd = getpwuid(getuid());
-    DLOG("current working directory: '%s' - args='%s'", passwd ? passwd->pw_dir : "/", application->m_arguments.join(' ').toLatin1().data());
-    QProcess builder;
-    builder.setProcessChannelMode(QProcess::ForwardedChannels);
-    QString exec(application->m_arguments[0]);
-    application->m_arguments.removeFirst();
-    result = builder.startDetached(exec, application->m_arguments, QString(passwd ? passwd->pw_dir : "/"), &pid);
-    application->m_arguments.prepend(exec);
-    DLOG_IF(result == false, "process failed to start");
-    if (result) {
-        // Set existing application's pid to new instance
-        application->setPid(pid);
-
-        // Push to front
-        if (m_applications.count() > 1)
-            move(m_applications.indexOf(application), 0);
-
-        DLOG("builder '%s' respawned with pid %lld", application->name().toLatin1().data(), pid);
-        return application;
-    } else {
-        DLOG("builder '%s' failed to respawn", application->name().toLatin1().data());
-        return NULL;
-    }
-}
-
 Application* ApplicationManager::startApplication(const QString &appId,
                                                   const QStringList &arguments)
 {
@@ -211,102 +186,41 @@
 }
 
 Application *ApplicationManager::startApplication(const QString &appId, ExecFlags flags,
-                                                  const QStringList &constArgs)
-{
-    DLOG("ApplicationManager::startApplication (this=%p, appId=%s)", this, appId.toLatin1().data());
-
-    QStringList arguments = constArgs;
-
-    // Load desktop file.
-    DesktopFileReader* desktopData = new DesktopFileReader(appId);
-    if (!desktopData->loaded()) {
-        delete desktopData;
-        return NULL;
-    }
-
-#if USE_UPSTART
-    /* launch via upstart */
-    arguments.prepend("APP_ID=" + desktopData->appId());
-    arguments.prepend("application");
-
-    // Start process.
-    //FIXME(greyback) use upstart via DBUS
-    QProcess process;
-    process.start("start", arguments);
-    //FIXME(greyback) this is blocking, instead listen for finished() signal
-    process.waitForFinished(1000);
-
-    DLOG_IF(process.error(), "process 'start application' failed to start with error: %s", process.errorString().toLatin1().data());
-    if (!process.error()) {
-        // fetch pid
-        QString output = process.readAllStandardOutput();
-        int lastSpace = output.lastIndexOf(' ');
-        qint64 pid = output.remove(0, lastSpace).toInt();
-#else
-    /* Launch manually */
-
-    // Format arguments.
-    // FIXME(loicm) Special field codes are simply ignored for now.
-    //     http://standards.freedesktop.org/desktop-entry-spec/latest/ar01s06.html
-    QStringList execArguments = desktopData->exec().split(" ", QString::SkipEmptyParts);
-    DASSERT(execArguments.size() > 0);
-    QString exec(execArguments[0]);
-    const int kSize = execArguments.size();
-    for (int i = kSize - 1; i > 0; i--) {
-        if ((execArguments[i].size() == 2) && (execArguments[i][0].toLatin1() == '%')) {
-            const char kChar = execArguments[i][1].toLatin1();
-            if (kChar == 'F' || kChar == 'u' || kChar == 'U' || kChar == 'd' || kChar == 'D'
-                    || kChar == 'n' || kChar == 'N' || kChar == 'i' || kChar == 'c' || kChar == 'k'
-                    || kChar == 'v' || kChar == 'm') {
-                continue;
-            }
+                                                  const QStringList &arguments)
+{
+    DLOG("ApplicationManager::startApplication (this=%p, appId=%s)", this, qPrintable(appId));
+
+    if (!m_taskController->start(appId, arguments)) {
+        LOG("Asking Upstart to start application '%s' failed", qPrintable(appId));
+        return nullptr;
+    }
+
+    Application* application = new Application(appId, Application::Starting, arguments, this);
+    if (!application->isValid()) {
+        DLOG("Unable to instantiate application with appId '%s'", qPrintable(appId));
+        return nullptr;
+    }
+
+    // override stage if necessary
+    if (application->stage() == Application::SideStage && flags.testFlag(ApplicationManager::ForceMainStage)) {
+        application->setStage(Application::MainStage);
+    }
+
+    add(application);
+    return application;
+}
+
+void ApplicationManager::onProcessStarted(const QString &appId)
+{
+    Application *application = findApplication(appId);
+
+    if (!application) { // if shell did not start this application, but upstart did
+        application = new Application(appId, Application::Starting, QStringList(), this);
+        if (!application->isValid()) {
+            DLOG("Unable to instantiate application with appId '%s'", qPrintable(appId));
+            return;
         }
-        arguments.prepend(execArguments[i]);
-    }
-    arguments.append(QString("--desktop_file_hint=") + desktopData->file());
-    if (flags.testFlag(ApplicationManager::ForceMainStage))
-        arguments.append(QString("--stage_hint=main_stage"));
-    else if (desktopData->stageHint() == "SideStage")
-        arguments.append(QString("--stage_hint=side_stage"));
-
-    // Start process - set correct environment
-    setenv("QT_QPA_PLATFORM", "ubuntumirclient", 1);
-
-    bool result;
-    qint64 pid = 0;
-    QString path = "/";
-    // respect Path from .desktop file
-    if (desktopData->path() != "") {
-        path = desktopData->path();
-    } else {
-        struct passwd* passwd = getpwuid(getuid());
-        if (passwd)
-            path = passwd->pw_dir;
-    }
-    DLOG("current working directory: '%s'", path.toLatin1().data());
-        QProcess builder;
-    builder.setProcessChannelMode(QProcess::ForwardedChannels);
-    result = builder.startDetached(exec, arguments, path, &pid);
-
-    DLOG_IF(result == false, "process failed to start");
-    if (result) {
-#endif
-        DLOG("process started with pid %lld, adding '%s' to application lists", pid, desktopData->name().toLatin1().data());
-        arguments.prepend(exec);
-        Application* application = new Application(
-                    desktopData, pid,
-                    (flags.testFlag(ApplicationManager::ForceMainStage)
-                        || desktopData->stageHint() != "SideStage")
-                        ? Application::MainStage : Application::SideStage,
-                    Application::Starting, //FIXME(greyback): assuming running immediately
-                    arguments,
-                    m_taskController
-                    );
-
         add(application);
-        return application;
-    } else {
-        return NULL;
     }
 }
 
@@ -316,35 +230,62 @@
 
     Application *application = findApplication(appId);
 
-    if (application != NULL) {
-        if (application == m_focusedApplication) {
-            // TODO(greyback) What to do?? Focus next app, or unfocus everything??
-            m_focusedApplication = NULL;
-            Q_EMIT focusedApplicationIdChanged();
-        }
-        remove(application);
-        m_dbusWindowStack->WindowDestroyed(0, application->name());
-
-        // Start process.
-        bool result;
-
-#if USE_UPSTART
-        result = QProcess::startDetached("stop", QStringList() << "application"
-                                         << ("APP_ID=" + application->appId()));
-        DLOG_IF(result == false, "process 'stop application' failed to execute");
-#else
-        result = QProcess::startDetached("/bin/kill", QStringList() << "-9" << QString::number(application->m_pid));
-        DLOG_IF(result == false, "process '/bin/kill -9 %lld' failed to execute", application->m_pid);
-#endif
-        LOG_IF(!result, "ApplicationManager: Unable to stop process '%s'",
-               application->name().toLatin1().constData());
-        delete application;
-    }
+    if (!application) {
+        DLOG("No such running application '%s'", qPrintable(appId));
+        return false;
+    }
+
+    if (application == m_focusedApplication) {
+        // TODO(greyback) What to do?? Focus next app, or unfocus everything??
+        m_focusedApplication = NULL;
+        Q_EMIT focusedApplicationIdChanged();
+    }
+
+    remove(application);
+    m_dbusWindowStack->WindowDestroyed(0, application->name());
+
+    bool result = m_taskController->stop(application->appId());
+
+    LOG_IF(result == false, "FAILED to ask Upstart to stop application '%s'", qPrintable(application->appId()));
+    delete application;
 
     // FIXME(dandrader): lying here. The operation is async. So we will only know whether
     // the focusing was successful once the server replies. Maybe the API in unity-api should
     // reflect that?
-    return true;
+    return result;
+}
+
+void ApplicationManager::onProcessStopped(const QString &appId)
+{
+    Application *application = findApplication(appId);
+
+    // if shell did not stop the application, but upstart says it died, we assume the process has been
+    // killed, so it can be respawned later. Only exception is if that application is focused or running
+    // as then it most likely crashed. Update this logic when upstart gives some failure info.
+    if (application) {
+        bool removeApplication = false;
+
+        if (application == m_focusedApplication) {
+            // Very bad case where focused application dies. Remove from list. Should give error message
+            m_focusedApplication = nullptr;
+            Q_EMIT focusedApplicationIdChanged();
+            removeApplication = true;
+        }
+
+        if (application->state() == Application::Running) {
+            // Application probably crashed, else OOM killer struck. Either way state wasn't saved
+            // so just remove application
+            removeApplication = true;
+        } else if (application->state() == Application::Suspended) {
+            application->setState(Application::Stopped);
+        }
+
+        if (removeApplication) {
+            remove(application);
+            m_dbusWindowStack->WindowDestroyed(0, application->name());
+            delete application;
+        }
+    }
 }
 
 /************************************* Mir-side methods *************************************/
@@ -354,12 +295,14 @@
     authorized = false; //to be proven wrong
 
     DLOG("ApplicationManager::authorizeSession (this=%p, pid=%lld)", this, pid);
-    Application* application = findApplicationWithPid(pid);
-    if (application) {
-        QString name = application->appId() + QString::number(qrand());
-        application->setSessionName(name);
-        authorized = true;
-        return;
+
+    for (Application *app : m_applications) {
+        if (app->state() == Application::Starting
+                && m_taskController->appIdHasProcessId(app->appId(), pid)) {
+            app->setPid(pid);
+            authorized = true;
+            return;
+        }
     }
 
     /*
@@ -403,13 +346,13 @@
 
     // some naughty applications use a script to launch the actual application. Check for the
     // case where shell actually launched the script.
-    application = findApplication(desktopData->appId());
+    Application *application = findApplication(desktopData->appId());
     if (application && application->state() == Application::Starting) {
         DLOG("Process with pid %lld appeared, attached to existing entry '%s' in application lists",
              pid, application->appId().toLatin1().data());
         delete desktopData;
-        QString name = application->appId() + QString::number(qrand());
-        application->setSessionName(name);
+        application->setSessionName(application->appId());
+        application->setPid(pid);
         authorized = true;
         return;
     }
@@ -428,7 +371,9 @@
 
     QString argStr(command.data());
     QStringList arguments(argStr.split(' '));
-    application = new Application(desktopData, pid, stage, Application::Starting, arguments, m_taskController);
+    application = new Application(desktopData, Application::Starting, arguments, this);
+    application->setPid(pid);
+    application->setStage(stage);
     add(application);
     authorized = true;
 }
@@ -538,8 +483,11 @@
     return nullptr;
 }
 
-Application* ApplicationManager::findApplicationWithPid(const int pid)
+Application* ApplicationManager::findApplicationWithPid(const qint64 pid)
 {
+    if (pid <= 0)
+        return nullptr;
+
     for (Application *app : m_applications) {
         if (app->m_pid == pid) {
             return app;
 
=== modified file 'src/modules/Unity/Application/application_manager.h'
--- src/modules/Unity/Application/application_manager.h	2013-09-11 13:20:08 +0000
+++ src/modules/Unity/Application/application_manager.h	2013-09-27 15:13:07 +0000
@@ -27,12 +27,13 @@
 // Unity API
 #include <unity/shell/application/ApplicationManagerInterface.h>
 
-#include "taskcontroller.h"
+// local
+#include "application.h"
 
-class Application;
 class ShellServerConfiguration;
 class DBusWindowStack;
 class MirSurfaceManager;
+class TaskController;
 
 namespace mir {
     namespace shell {
@@ -82,7 +83,7 @@
     Q_INVOKABLE void move(int from, int to);
 
     const QList<Application*> &list() const { return m_applications; }
-    Application* findApplicationWithPid(const int pid);
+    Application* findApplicationWithPid(const qint64 pid);
 
 public Q_SLOTS:
     void authorizeSession(const quint64 pid, bool &authorized);
@@ -94,6 +95,9 @@
 
     void onSessionCreatedSurface(mir::shell::ApplicationSession const*, std::shared_ptr<mir::shell::Surface> const&);
 
+    void onProcessStarted(const QString& appId);
+    void onProcessStopped(const QString& appId);
+
 Q_SIGNALS:
     void focusRequested(FavoriteApplication favoriteApplication);
 
@@ -101,7 +105,6 @@
     void setFocused(Application *application);
     void add(Application *application);
     void remove(Application* application);
-    Application* respawnApplication(Application* application);
     Application* findApplicationWithSession(const std::shared_ptr<mir::shell::Session> &session);
     Application* findApplicationWithSession(const mir::shell::Session *session);
     Application* findLastExecutedApplication();
@@ -111,11 +114,10 @@
     Application* m_focusedApplication; // remove as Mir has API for this
     ShellServerConfiguration* m_mirServer;
     DBusWindowStack* m_dbusWindowStack;
-    TaskController* m_taskController;
+    QScopedPointer<TaskController> m_taskController;
     static ApplicationManager* the_application_manager;
 
     friend class DBusWindowStack;
-    friend class TaskController;
     friend class MirSurfaceManager;
 };
 
 
=== modified file 'src/modules/Unity/Application/applicationscreenshotprovider.cpp'
--- src/modules/Unity/Application/applicationscreenshotprovider.cpp	2013-09-24 18:48:11 +0000
+++ src/modules/Unity/Application/applicationscreenshotprovider.cpp	2013-09-27 15:13:07 +0000
@@ -67,11 +67,18 @@
         return QImage();
     }
 
+    // TODO: if app not ready, return an app-provided splash image. If app has been stopped with saved state
+    // return the screenshot that was saved to disk.
     if (!app->session() || !app->session()->default_surface()) {
         LOG("ApplicationScreenshotProvider - app session not found - taking screenshot too early");
         return QImage();
     }
 
+    if (app->state() == Application::Stopped || app->state() == Application::Starting) {
+        LOG("ApplicationScreenshotProvider -  unable to take screenshot of stopped/not-ready applications");
+        return QImage();
+    }
+
     /* Workaround for bug https://bugs.launchpad.net/qtubuntu/+bug/1209216 - currently all qtubuntu
      * based applications are allocated a fullscreen Mir surface, but draw in a geometry excluding
      * the panel's rectangle. Mir snapshots thus have a white rectangle which the panel hides.
 
=== modified file 'src/modules/Unity/Application/desktopfilereader.cpp'
--- src/modules/Unity/Application/desktopfilereader.cpp	2013-09-17 17:52:54 +0000
+++ src/modules/Unity/Application/desktopfilereader.cpp	2013-09-27 15:13:07 +0000
@@ -98,7 +98,8 @@
     const unsigned int kAllEntriesMask =
             (1 << DesktopFileReader::kNameIndex) | (1 << DesktopFileReader::kCommentIndex)
             | (1 << DesktopFileReader::kIconIndex) | (1 << DesktopFileReader::kExecIndex)
-            | (1 << DesktopFileReader::kPathIndex) | (1 << DesktopFileReader::kStageHintIndex);    const unsigned int kMandatoryEntriesMask =
+            | (1 << DesktopFileReader::kPathIndex) | (1 << DesktopFileReader::kStageHintIndex);
+    const unsigned int kMandatoryEntriesMask =
             (1 << DesktopFileReader::kNameIndex) | (1 << DesktopFileReader::kIconIndex)
             | (1 << DesktopFileReader::kExecIndex);
     const int kEntriesCount = ARRAY_SIZE(kEntryNames);
 
=== modified file 'src/modules/Unity/Application/taskcontroller.cpp'
--- src/modules/Unity/Application/taskcontroller.cpp	2013-08-27 13:31:20 +0000
+++ src/modules/Unity/Application/taskcontroller.cpp	2013-09-27 15:13:07 +0000
@@ -16,42 +16,116 @@
  * Authored by: Ricardo Mendoza <[e-mail]>
  */
 
-#include "logging.h"
+// local
 #include "taskcontroller.h"
-#include "application_manager.h"
+
+// unity-mir
+#include <logging.h>
+
+// Qt
+#include <QStringList>
+
+// glib
+#include <glib.h>
+
+// std
 #include <signal.h>
 #include <unistd.h>
 
-TaskController::TaskController(ApplicationManager *parent) :
+
+TaskController* TaskController::m_theTaskController = nullptr;
+
+TaskController* TaskController::singleton()
+{
+    if (!m_theTaskController) {
+        m_theTaskController = new TaskController();
+    }
+    return m_theTaskController;
+}
+
+TaskController::TaskController(QObject *parent) :
     QObject(parent)
 {
-    DLOG("TaskController::TaskController (this=%p)", this);
+    startCallback = [](const gchar * appId, gpointer userData) {
+        Q_UNUSED(userData)
+        Q_EMIT TaskController::singleton()->processStarted(QString(appId));
+    };
+
+    stopCallback = [](const gchar * appId, gpointer userData) {
+        Q_UNUSED(userData)
+        Q_EMIT TaskController::singleton()->processStopped(QString(appId));
+    };
+
+    upstart_app_launch_observer_add_app_start(startCallback, nullptr);
+    upstart_app_launch_observer_add_app_stop(stopCallback, nullptr);
 }
 
 TaskController::~TaskController()
 {
-    DLOG("TaskController::~TaskController (this=%p)", this);
-}
-
-void TaskController::do_suspend(Application* application)
-{
-    DLOG("TaskController::do_suspend (this=%p, application=%p)", this, application);
-    if (application->state() == Application::Suspended)
-        kill(application->pid(), SIGSTOP);
-}
-
-void TaskController::do_resume(Application* application)
-{
-    DLOG("TaskController::do_resume (this=%p, application=%p)", this, application);
-    if (application->state() == Application::Suspended)
-        kill(application->pid(), SIGCONT);
-}
-
-void TaskController::do_respawn(Application* application)
-{
-    DLOG("TaskController::do_respwn (this=%p, application=%p)", this, application);
-    if (application->state() == Application::Stopped) {
-        ApplicationManager *appMgr = static_cast<ApplicationManager*>(parent());
-        appMgr->respawnApplication(application);
+    upstart_app_launch_observer_delete_app_start(startCallback, nullptr);
+    upstart_app_launch_observer_delete_app_stop(stopCallback, nullptr);
+}
+
+bool TaskController::start(const QString& appId, const QStringList& arguments)
+{
+    DLOG("TaskController::start appId='%s'", qPrintable(appId));
+    gchar ** upstartArgs = nullptr;
+    bool result = false;
+
+    // Convert arguments QStringList into format suitable for upstart-app-launch
+    upstartArgs = g_new0(gchar *, arguments.length());
+
+    for (int i=0; i<arguments.length(); i++) {
+        upstartArgs[i] = arguments.at(i).toLatin1().data();
+    }
+
+    result = upstart_app_launch_start_application(appId.toLatin1().constData(),
+                                                  static_cast<const gchar * const *>(upstartArgs));
+    g_free(upstartArgs);
+
+    DLOG_IF(!result, "TaskController::startApplication appId='%s' FAILED", qPrintable(appId));
+    return result;
+}
+
+bool TaskController::stop(const QString& appId)
+{
+    DLOG("TaskController::stop appId='%s'", qPrintable(appId));
+    bool result = false;
+
+    result = upstart_app_launch_stop_application(appId.toLatin1().constData());
+
+    DLOG_IF(!result, "TaskController::stopApplication appId='%s' FAILED", qPrintable(appId));
+    return result;
+}
+
+bool TaskController::appIdHasProcessId(const QString& appId, const quint64 pid)
+{
+    DLOG("TaskController::isApplicationPid appId='%s', pid=%lld", qPrintable(appId), pid);
+    return upstart_app_launch_pid_in_app_id(pid, appId.toLatin1().constData());
+}
+
+bool TaskController::suspend(const QString& appId)
+{
+    DLOG("TaskController::suspend (this=%p, application=%p)", this, qPrintable(appId));
+    pid_t pid = upstart_app_launch_get_primary_pid(appId.toLatin1().constData());
+
+    if (pid) {
+        kill(pid, SIGSTOP);
+        return true;
+    } else {
+        return false;
+    }
+}
+
+bool TaskController::resume(const QString& appId)
+{
+    DLOG("TaskController::resume (this=%p, application=%p)", this, qPrintable(appId));
+    pid_t pid = upstart_app_launch_get_primary_pid(appId.toLatin1().constData());
+
+    if (pid) {
+        kill(pid, SIGCONT);
+        return true;
+    } else {
+        return false;
     }
 }
 
=== modified file 'src/modules/Unity/Application/taskcontroller.h'
--- src/modules/Unity/Application/taskcontroller.h	2013-08-27 13:31:20 +0000
+++ src/modules/Unity/Application/taskcontroller.h	2013-09-27 15:13:07 +0000
@@ -23,17 +23,35 @@
 
 #include "application.h"
 
-class ApplicationManager;
+// upstart
+extern "C" {
+    #include "upstart-app-launch.h"
+}
+
 class TaskController : public QObject
 {
     Q_OBJECT
 public:
-    explicit TaskController(ApplicationManager* parent);
+    static TaskController* singleton();
     ~TaskController();
 
-    void do_resume(Application* application);
-    void do_suspend(Application* application);
-    void do_respawn(Application* application);
+    bool start(const QString& appId, const QStringList& args);
+    bool stop(const QString& appId);
+
+    bool suspend(const QString& appId);
+    bool resume(const QString& appId);
+
+    bool appIdHasProcessId(const QString& appId, const quint64 pid);
+
+Q_SIGNALS:
+    void processStarted(const QString& appId);
+    void processStopped(const QString& appId);
+
+private:
+    TaskController(QObject *parent = 0);
+
+    static TaskController* m_theTaskController;
+    upstart_app_launch_app_observer_t startCallback, stopCallback;
 };
 
 #endif // TASKCONTROLLER_H