Merge lp:~gerboland/qtmir/url-dispatcher-respawns-so-appMan-should-not into lp:qtmir

[Gerry Boland]

AppMan: url-dispatcher launches focusRequested apps itself, so ensure AppMan sets the app state appropriately

Let app1 be lifecycle killed, but still in the app list. app2 uses url-dispatcher to focus app1. This causes url-dispatcher to launch a new instance of app1, and then notify AppMan to request focus for app1.

However it is possible if shell acted upon the requestFocus signal, and called focusApplication(app1) - before the onProcessStarting(app1) notification from upstart was received, AppMan would respawn app1 itself. So 2 instances of app1 would be executing, and later AppMan would then reject one of them.

AppMan: url-dispatcher launches focusRequested apps itself, so ensure AppMan sets the app state appropriately

Let app1 be lifecycle killed, but still in the app list. app2 uses url-dispatcher to focus app1. This causes url-dispatcher to launch a new instance of app1, and then notify AppMan to request focus for app1.

However it is possible if shell acted upon the requestFocus signal, and called focusApplication(app1) - before the onProcessStarting(app1) notification from upstart was received, AppMan would respawn app1 itself. So 2 instances of app1 would be executing, and later AppMan would then reject one of them.

[lp:~gerboland/qtmir/url-dispatcher-respawns-so-appMan-should-not]

275. By Gerry Boland on 2014-10-14

Update comments to explain new situation

276. By Gerry Boland on 2014-12-10

Merge trunk

277. By Gerry Boland on 2014-12-10

Remove unnecessary include

Unmerged revisions

277. By Gerry Boland on 2014-12-10

Remove unnecessary include

276. By Gerry Boland on 2014-12-10

Merge trunk

275. By Gerry Boland on 2014-10-14

Update comments to explain new situation

274. By Gerry Boland on 2014-10-14

AppMan: url-dispatcher launches focusRequested apps itself, so ensure AppMan sets the app state appropriately

Let app1 be lifecycle killed, but still in the app list. app2 uses url-dispatcher to focus app1. This causes url-dispatcher to launch a new instance of app1, and then notify AppMan to request focus for app1.

However it is possible if shell acted upon the requestFocus signal, and called focusApplication(app1) - before the onProcessStarting(app1) notification from upstart was received, AppMan would respawn app1 itself. So 2 instances of app1 would be executing, and later AppMan would then reject one of them.

=== modified file 'src/modules/Unity/Application/application_manager.cpp'
--- src/modules/Unity/Application/application_manager.cpp	2014-10-30 11:55:15 +0000
+++ src/modules/Unity/Application/application_manager.cpp	2014-12-10 12:46:03 +0000
@@ -280,6 +280,13 @@
         return false;
     }
 
+    // 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 << "was resumed by url-dispatcher";
+        application->setState(Application::Starting);
+    }
+
     Q_EMIT focusRequested(appId);
     return true;
 }
@@ -531,15 +538,17 @@
         Q_EMIT focusRequested(appId);
     }
     else {
-        // 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).
+        // It is highly unlikely an app whose process was killed (usually by OOM) could be respawned by means other
+        // than url-dispatcher or AppMan itself - maybe an app developer is testing lifecycle stuff?? In any case,
+        // let's just be agreeable, accept the newly spawned process and focus it immediately.
         if (application->state() == Application::Stopped) {
-            qCDebug(QTMIR_APPLICATIONS) << "Stopped application appId=" << appId << "is being resumed externally";
+            qCDebug(QTMIR_APPLICATIONS) << "Stopped application appId=" << appId
+                                        << "has been resumed in an unexpected way";
             application->setState(Application::Starting);
             Q_EMIT focusRequested(appId);
         } else {
             qCDebug(QTMIR_APPLICATIONS) << "ApplicationManager::onProcessStarting application already found with appId"
-                                        << appId;
+                                        << appId << "- is probably ok";
         }
     }
 }

=== modified file 'tests/modules/ApplicationManager/application_manager_test.cpp'
--- tests/modules/ApplicationManager/application_manager_test.cpp	2014-11-13 16:26:03 +0000
+++ tests/modules/ApplicationManager/application_manager_test.cpp	2014-12-10 12:46:03 +0000
@@ -2036,6 +2036,29 @@
 }
 
 /*
+ * Test if url-dispatcher requests focus for an app which was lifecycle killed, then
+ * AppMan assumes that url-dispatcher respawned the app (and AppMan doesn't try to respawn
+ * the app itself)
+ */
+TEST_F(ApplicationManagerTests,urlDispatcherRespawnsStoppedAppsSoWeDoNotHaveTo)
+{
+    using namespace ::testing;
+    quint64 procId1 = 5551;
+    const QString appId("testAppId");
+
+    auto app = startApplication(procId1, appId);
+    app->setState(Application::Stopped);
+
+    EXPECT_CALL(appController, startApplicationWithAppIdAndArgs(appId, _))
+        .Times(0)
+        .WillOnce(Return(true));
+
+    applicationManager.requestFocusApplication(appId);
+
+    EXPECT_EQ(app->state(), Application::Starting);
+}
+
+/*
  * Test that screenshotting callback works cross thread.
  */
 TEST_F(ApplicationManagerTests, threadedScreenshot)