Merge lp:~gerboland/unity-mir/add-fail-and-resume-focus-support into lp:unity-mir

[Gerry Boland]

Add support for upstart-app-launch to request application resume & focus. This revealed extra work: prevent unexpected focus events propagating to shell, fix lifecycle bug and add workaround for handling of non-application sessions (like QtWebProcess and maliit).

To be tested in conjunction with this: https://code.launchpad.net/~gerboland/unity8/unity8-with-WM-fixes/+merge/188946

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:100
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/81/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-amd64-ci/23/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-armhf-ci/81/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-i386-ci/81/console

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

review: Needs Fixing (continuous-integration)

[Michał Sawicz (saviq)]

How does upstart know if an app stop was expected or not? Exit code?

77 + // be notified of that through the onProcessStarted slot. Else resume.

No such slot anymore.

review: Needs Information

[Gerry Boland (gerboland)]

> How does upstart know if an app stop was expected or not? Exit code?
An excellent question that I cannot answer.

> 77 + // be notified of that through the onProcessStarted slot. Else
> resume.
>
> No such slot anymore.
Fixed

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:101
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/83/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-amd64-ci/25/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-armhf-ci/83/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-saucy-i386-ci/83/console

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Gerry Boland (gerboland)]

Oops, pushed bad stuff here. Will fix

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Gerry Boland (gerboland)]

Ok, ready for final review

[lp:~gerboland/unity-mir/add-fail-and-resume-focus-support]

107. By Gerry Boland on 2013-10-02

Message a bit noisy, make it debug only

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Ricardo Mendoza (ricmm)]

Tried and tested on Nexus4, perfecto.

review: Approve

[Michał Sawicz (saviq)]

Code-wise looks good, will need someone else to have a look functionally.

review: Approve (code)

[Michał Sawicz (saviq)]

OK, worky!

review: Approve

=== modified file 'src/modules/Unity/Application/ApplicationImage.qml'
--- src/modules/Unity/Application/ApplicationImage.qml	2013-08-30 16:00:46 +0000
+++ src/modules/Unity/Application/ApplicationImage.qml	2013-10-02 21:19:30 +0000
@@ -22,7 +22,6 @@
     readonly property bool ready: source && (image.status == Image.Ready)
 
     function scheduleUpdate() {
-        print("ApplicationImage::scheduleUpdate", (source) ? source.appId : "");
         image.source = "";
         if (source) {
             image.source = Qt.binding( function() {

=== modified file 'src/modules/Unity/Application/application_manager.cpp'
--- src/modules/Unity/Application/application_manager.cpp	2013-10-02 11:48:37 +0000
+++ src/modules/Unity/Application/application_manager.cpp	2013-10-02 21:19:30 +0000
@@ -51,9 +51,11 @@
 
 ApplicationManager::ApplicationManager(QObject *parent)
 :   ApplicationManagerInterface(parent)
-,   m_focusedApplication(NULL)
+,   m_focusedApplication(nullptr)
+,   m_applicationToBeFocused(nullptr)
 ,   m_lifecycleExceptions(QStringList() << "music-app")
 ,   m_taskController(TaskController::singleton())
+,   m_fenceNext(false)
 {
     DLOG("ApplicationManager::ApplicationManager (this=%p)", this);
 
@@ -78,10 +80,14 @@
     QObject::connect(m_mirServer->sessionAuthorizer(), &SessionAuthorizer::requestAuthorizationForSession,
                      this, &ApplicationManager::authorizeSession, Qt::BlockingQueuedConnection);
 
-    QObject::connect(m_taskController.data(), &TaskController::processStarted,
-                     this, &ApplicationManager::onProcessStarted);
+    QObject::connect(m_taskController.data(), &TaskController::processStartReport,
+                     this, &ApplicationManager::onProcessStartReportReceived);
     QObject::connect(m_taskController.data(), &TaskController::processStopped,
                      this, &ApplicationManager::onProcessStopped);
+    QObject::connect(m_taskController.data(), &TaskController::requestFocus,
+                     this, &ApplicationManager::onFocusRequested);
+    QObject::connect(m_taskController.data(), &TaskController::requestResume,
+                     this, &ApplicationManager::onResumeRequested);
 
     m_dbusWindowStack = new DBusWindowStack(this);
 }
@@ -157,6 +163,8 @@
         return false;
     }
 
+    m_applicationToBeFocused = application;
+
     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
@@ -177,6 +185,7 @@
 {
     DLOG("ApplicationManager::unfocusCurrentApplication (this=%p)", this);
 
+    m_applicationToBeFocused = nullptr;
     m_mirServer->the_session_manager()->set_focus_to(NULL); //FIXME(greyback)
 }
 
@@ -211,8 +220,15 @@
     return application;
 }
 
-void ApplicationManager::onProcessStarted(const QString &appId)
+void ApplicationManager::onProcessStartReportReceived(const QString &appId, const bool failure)
 {
+    DLOG("ApplicationManager::onProcessStartReportReceived (this=%p, appId=%s, failure=%c)",
+         this, qPrintable(appId), (failure) ? 'Y' : 'N');
+
+    if (failure) {
+        onProcessStopped(appId, true);
+    }
+
     Application *application = findApplication(appId);
 
     if (!application) { // if shell did not start this application, but upstart did
@@ -256,7 +272,7 @@
     return result;
 }
 
-void ApplicationManager::onProcessStopped(const QString &appId)
+void ApplicationManager::onProcessStopped(const QString &appId, const bool unexpected)
 {
     Application *application = findApplication(appId);
 
@@ -287,6 +303,36 @@
             delete application;
         }
     }
+
+    if (unexpected) {
+        // TODO: pop up a message box/notification?
+        LOG("ApplicationManager: application '%s' died unexpectedly!", qPrintable(appId));
+    }
+}
+
+void ApplicationManager::onFocusRequested(const QString& appId)
+{
+    DLOG("ApplicationManager::onFocusRequested (this=%p, appId=%s)", this, qPrintable(appId));
+
+    focusApplication(appId);
+}
+
+void ApplicationManager::onResumeRequested(const QString& appId)
+{
+    DLOG("ApplicationManager::onResumeRequested (this=%p, appId=%s)", this, qPrintable(appId));
+
+    Application *application = findApplication(appId);
+
+    if (!application) {
+        DLOG("ApplicationManager::onResumeRequested: No such running application '%s'", qPrintable(appId));
+        return;
+    }
+
+    // If app Stopped, trust that upstart-app-launch respawns it itself, and AppManager will
+    // be notified of that through the onProcessStartReportReceived slot. Else resume.
+    if (application->state() == Application::Suspended) {
+        application->setState(Application::Running);
+    }
 }
 
 /************************************* Mir-side methods *************************************/
@@ -323,6 +369,7 @@
     // FIXME: special exception for the OSK - maliit-server - not very secure
     if (command.startsWith("maliit-server") || command.startsWith("/usr/lib/arm-linux-gnueabihf/qt5/libexec/QtWebProcess")) {
         authorized = true;
+        m_fenceNext = true;
         return;
     }
 
@@ -383,11 +430,17 @@
 {
     DLOG("ApplicationManager::onSessionStarting (this=%p, application=%s)", this, session->name().c_str());
 
+    if (m_fenceNext) {
+        m_fenceNext = false;
+        return;
+    }
+
     //FIXME(greyback) Mir not supplying any identifier that we can use to link the PID to the session
     // so am assuming that the *most recently* launched application session is the one that connects
     Application* application = findLastExecutedApplication();
     if (application && application->state() != Application::Running) {
         application->setSession(session);
+        m_applicationToBeFocused = application;
     } else {
         DLOG("ApplicationManager::onSessionStarting - unauthorized application!!");
     }
@@ -417,11 +470,15 @@
     Application* application = findApplicationWithSession(session);
 
     // Don't give application focus until it has created it's surface, when it is set as state "Running"
-    if (application && application->state() != Application::Starting
+    // and only notify shell of focus changes that it actually expects
+    if (application && application->state() != Application::Starting && application == m_applicationToBeFocused
             && application != m_focusedApplication) {
         setFocused(application);
         QModelIndex appIndex = findIndex(application);
         Q_EMIT dataChanged(appIndex, appIndex, QVector<int>() << RoleFocused);
+    } else {
+        if (application == nullptr)
+            DLOG("Invalid application focused, discarding the event");
     }
 }
 
@@ -432,7 +489,8 @@
         Q_ASSERT(m_focusedApplication->focused());
         m_focusedApplication->setFocused(false);
 
-        if (!m_lifecycleExceptions.contains(m_focusedApplication->appId())) {
+        if (!m_lifecycleExceptions.contains(m_focusedApplication->appId())
+                && m_focusedApplication->state() != Application::Suspended) {
             m_focusedApplication->setState(Application::Suspended);
         }
 
@@ -466,6 +524,11 @@
     if (application == m_focusedApplication)
         return;
 
+    // set state of previously focused app to suspended
+    if (m_focusedApplication && !m_lifecycleExceptions.contains(m_focusedApplication->appId())) {
+        m_focusedApplication->setState(Application::Suspended);
+    }
+
     m_focusedApplication = application;
     m_focusedApplication->setFocused(true);
     m_focusedApplication->setState(Application::Running);

=== modified file 'src/modules/Unity/Application/application_manager.h'
--- src/modules/Unity/Application/application_manager.h	2013-09-30 16:22:31 +0000
+++ src/modules/Unity/Application/application_manager.h	2013-10-02 21:19:30 +0000
@@ -95,8 +95,10 @@
 
     void onSessionCreatedSurface(mir::shell::ApplicationSession const*, std::shared_ptr<mir::shell::Surface> const&);
 
-    void onProcessStarted(const QString& appId);
-    void onProcessStopped(const QString& appId);
+    void onProcessStartReportReceived(const QString& appId, const bool failure);
+    void onProcessStopped(const QString& appId, const bool unexpected);
+    void onFocusRequested(const QString& appId);
+    void onResumeRequested(const QString& appId);
 
 Q_SIGNALS:
     void focusRequested(FavoriteApplication favoriteApplication);
@@ -112,11 +114,13 @@
 
     QList<Application*> m_applications;
     Application* m_focusedApplication; // remove as Mir has API for this
+    Application* m_applicationToBeFocused; // a basic form of focus stealing prevention
     QStringList m_lifecycleExceptions;
     ShellServerConfiguration* m_mirServer;
     DBusWindowStack* m_dbusWindowStack;
     QScopedPointer<TaskController> m_taskController;
     static ApplicationManager* the_application_manager;
+    bool m_fenceNext;
 
     friend class DBusWindowStack;
     friend class MirSurfaceManager;

=== modified file 'src/modules/Unity/Application/taskcontroller.cpp'
--- src/modules/Unity/Application/taskcontroller.cpp	2013-09-26 10:02:15 +0000
+++ src/modules/Unity/Application/taskcontroller.cpp	2013-10-02 21:19:30 +0000
@@ -48,22 +48,50 @@
 {
     startCallback = [](const gchar * appId, gpointer userData) {
         Q_UNUSED(userData)
-        Q_EMIT TaskController::singleton()->processStarted(QString(appId));
+        Q_EMIT TaskController::singleton()->processStartReport(QString(appId), false);
     };
 
     stopCallback = [](const gchar * appId, gpointer userData) {
         Q_UNUSED(userData)
-        Q_EMIT TaskController::singleton()->processStopped(QString(appId));
+        Q_EMIT TaskController::singleton()->processStopped(QString(appId), false);
+    };
+
+    focusCallback = [](const gchar * appId, gpointer userData) {
+        Q_UNUSED(userData)
+        Q_EMIT TaskController::singleton()->requestFocus(QString(appId));
+    };
+
+    resumeCallback = [](const gchar * appId, gpointer userData) {
+        Q_UNUSED(userData)
+        Q_EMIT TaskController::singleton()->requestResume(QString(appId));
+    };
+
+    failureCallback = [](const gchar * appId, upstart_app_launch_app_failed_t failureType, gpointer userData) {
+        Q_UNUSED(userData)
+        if (failureType == UPSTART_APP_LAUNCH_APP_FAILED_CRASH) {
+            Q_EMIT TaskController::singleton()->processStopped(QString(appId), true);
+        } else if (failureType == UPSTART_APP_LAUNCH_APP_FAILED_START_FAILURE) {
+            Q_EMIT TaskController::singleton()->processStartReport(QString(appId), true);
+        } else {
+            LOG("TaskController: unknown failure type returned from upstart-app-launch");
+        }
+        Q_EMIT TaskController::singleton()->requestResume(QString(appId));
     };
 
     upstart_app_launch_observer_add_app_start(startCallback, nullptr);
     upstart_app_launch_observer_add_app_stop(stopCallback, nullptr);
+    upstart_app_launch_observer_add_app_focus(focusCallback, nullptr);
+    upstart_app_launch_observer_add_app_resume(resumeCallback, nullptr);
+    upstart_app_launch_observer_add_app_failed(failureCallback, nullptr);
 }
 
 TaskController::~TaskController()
 {
     upstart_app_launch_observer_delete_app_start(startCallback, nullptr);
     upstart_app_launch_observer_delete_app_stop(stopCallback, nullptr);
+    upstart_app_launch_observer_delete_app_focus(focusCallback, nullptr);
+    upstart_app_launch_observer_delete_app_resume(resumeCallback, nullptr);
+    upstart_app_launch_observer_delete_app_failed(failureCallback, nullptr);
 }
 
 bool TaskController::start(const QString& appId, const QStringList& arguments)

=== modified file 'src/modules/Unity/Application/taskcontroller.h'
--- src/modules/Unity/Application/taskcontroller.h	2013-09-26 10:02:15 +0000
+++ src/modules/Unity/Application/taskcontroller.h	2013-10-02 21:19:30 +0000
@@ -44,14 +44,17 @@
     bool appIdHasProcessId(const QString& appId, const quint64 pid);
 
 Q_SIGNALS:
-    void processStarted(const QString& appId);
-    void processStopped(const QString& appId);
+    void processStartReport(const QString& appId, const bool failure);
+    void processStopped(const QString& appId, const bool unexpectedly);
+    void requestFocus(const QString& appId);
+    void requestResume(const QString& appId);
 
 private:
     TaskController(QObject *parent = 0);
 
     static TaskController* m_theTaskController;
-    upstart_app_launch_app_observer_t startCallback, stopCallback;
+    upstart_app_launch_app_observer_t startCallback, stopCallback, focusCallback, resumeCallback;
+    upstart_app_launch_app_failed_observer_t failureCallback;
 };
 
 #endif // TASKCONTROLLER_H