Merge lp:~gerboland/qtmir/upstart-respawning-app into lp:qtmir

[Gerry Boland]

Fix AppMan handling Upstart resuming a Stopped application

AppMan did not expect apps to be restarted by anyone other than itself. However url-dispatcher will launch an instance of an application if it needs to. Should that app be in a Stopped state, AppMan would reject it. This patch has AppMan accept that new process and fire a focus event to the shell.

Fix AppMan handling Upstart resuming a Stopped application

 * Are there any related MPs required for this MP to build/function as expected? Please list.
N
 * Did you perform an exploratory manual test run of your code change and any related functionality?
Y
 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

[lp:~gerboland/qtmir/upstart-respawning-app]

256. By Gerry Boland on 2014-09-24

qCDebug

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:256
http://jenkins.qa.ubuntu.com/job/qtmir-ci/87/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-utopic-amd64-ci/87
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-utopic-armhf-ci/87
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-utopic-armhf-ci/87/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Josh Arenson (josharenson)]

Reproduced the issue without the patch, and CNR with the patch. All tests pass. LGTM.

review: Approve

=== modified file 'src/modules/Unity/Application/application_manager.cpp'
--- src/modules/Unity/Application/application_manager.cpp	2014-09-18 12:00:38 +0000
+++ src/modules/Unity/Application/application_manager.cpp	2014-09-24 13:20:32 +0000
@@ -493,7 +493,16 @@
         Q_EMIT focusRequested(appId);
     }
     else {
-        qWarning() << "ApplicationManager::onProcessStarting application already found with appId" << appId;
+        // url-dispatcher can relaunch apps which have been OOM-killed - AppMan must accept the newly spawned
+        // application and focus it immediately (as user expects app to still be running).
+        if (application->state() == Application::Stopped) {
+            qCDebug(QTMIR_APPLICATIONS) << "Stopped application appId=" << appId << "is being resumed externally";
+            application->setState(Application::Starting);
+            Q_EMIT focusRequested(appId);
+        } else {
+            qCDebug(QTMIR_APPLICATIONS) << "ApplicationManager::onProcessStarting application already found with appId"
+                                        << appId;
+        }
     }
 }
 

=== modified file 'tests/modules/ApplicationManager/application_manager_test.cpp'
--- tests/modules/ApplicationManager/application_manager_test.cpp	2014-09-18 12:00:38 +0000
+++ tests/modules/ApplicationManager/application_manager_test.cpp	2014-09-24 13:20:32 +0000
@@ -1787,6 +1787,60 @@
 }
 
 /*
+ * Test for when a background application that has been OOM killed is relaunched by upstart.
+ * AppMan will have the application in the app lists, in a Stopped state. Upstart will notify of
+ * the app launching (like any normal app). Need to set the old Application instance to Starting
+ * state and emit focusRequested to shell - authorizeSession will then associate new process with
+ * the Application as normal.
+ */
+TEST_F(ApplicationManagerTests,stoppedBackgroundAppRelaunchedByUpstart)
+{
+    using namespace ::testing;
+    const QString appId("testAppId");
+    quint64 procId = 5551;
+
+    // Set up Mocks & signal watcher
+    auto mockDesktopFileReader = new NiceMock<MockDesktopFileReader>(appId, QFileInfo());
+    ON_CALL(*mockDesktopFileReader, loaded()).WillByDefault(Return(true));
+    ON_CALL(*mockDesktopFileReader, appId()).WillByDefault(Return(appId));
+
+    ON_CALL(desktopFileReaderFactory, createInstance(appId, _)).WillByDefault(Return(mockDesktopFileReader));
+
+    EXPECT_CALL(appController, startApplicationWithAppIdAndArgs(appId, _))
+        .Times(1)
+        .WillOnce(Return(true));
+
+    applicationManager.startApplication(appId, ApplicationManager::NoFlag);
+    applicationManager.onProcessStarting(appId);
+    std::shared_ptr<mir::scene::Session> session = std::make_shared<MockSession>("", procId);
+    bool authed = true;
+    applicationManager.authorizeSession(procId, authed);
+    onSessionStarting(session);
+
+    // App creates surface, focuses it, puts it in background, then is OOM killed.
+    std::shared_ptr<mir::scene::Surface> surface(nullptr);
+    applicationManager.onSessionCreatedSurface(session.get(), surface);
+    applicationManager.focusApplication(appId);
+    applicationManager.unfocusCurrentApplication();
+
+    onSessionStopping(session);
+    applicationManager.onProcessFailed(appId, false);
+    applicationManager.onProcessStopped(appId);
+
+    Application *app = applicationManager.findApplication(appId);
+    EXPECT_EQ(app->state(), Application::Stopped);
+
+    QSignalSpy focusRequestSpy(&applicationManager, SIGNAL(focusRequested(const QString &)));
+
+    // Upstart re-launches app
+    applicationManager.onProcessStarting(appId);
+
+    EXPECT_EQ(app->state(), Application::Starting);
+    EXPECT_EQ(focusRequestSpy.count(), 1);
+    EXPECT_EQ(applicationManager.count(), 1);
+}
+
+/*
  * Test that screenshotting callback works cross thread.
  */
 TEST_F(ApplicationManagerTests, threadedScreenshot)