Wait for running apps to die on update installations

This was only done for removals, but it is even more important for
update installations.

Change-Id: I3cfe552908c9e8b1b130d814bac5f2d3265fe845
Pick-to: 5.15
Reviewed-by: Bernd Weimer <bernd.weimer@qt.io>

6 files changed, 90 insertions, 17 deletions

diff --git a/src/manager-lib/applicationmanager.cpp b/src/manager-lib/applicationmanager.cpp
index b329b67c..49ae29f7 100644
--- a/src/manager-lib/applicationmanager.cpp
+++ b/src/manager-lib/applicationmanager.cpp
@@ -785,7 +785,7 @@ bool ApplicationManager::startApplicationInternal(const QString &appId, const QS
         return false;
     }
 
-    connect(runtime, &AbstractRuntime::stateChanged, this, [this, app](Am::RunState newRuntimeState) {
+    connect(runtime, &AbstractRuntime::stateChanged, app, [this, app](Am::RunState newRuntimeState) {
         app->setRunState(newRuntimeState);
         emit applicationRunStateChanged(app->id(), newRuntimeState);
         emitDataChanged(app, QVector<int> { IsRunning, IsStartingUp, IsShuttingDown });
diff --git a/src/manager-lib/installationtask.cpp b/src/manager-lib/installationtask.cpp
index 3ef2427b..32a18817 100644
--- a/src/manager-lib/installationtask.cpp
+++ b/src/manager-lib/installationtask.cpp
@@ -31,6 +31,7 @@
 
 #include <QTemporaryDir>
 #include <QMessageAuthenticationCode>
+#include <QPointer>
 
 #include "logging.h"
 #include "packagemanager_p.h"
@@ -339,13 +340,23 @@ void InstallationTask::checkExtractedFile(const QString &file) Q_DECL_NOEXCEPT_E
         // this will also exclusively lock the application for us
         // m_package ownership is transferred to the ApplicationManager
         QString packageId = m_package->id(); // m_package is gone after the invoke
-        QMetaObject::invokeMethod(PackageManager::instance(), [this]()
-            { m_managerApproval = PackageManager::instance()->startingPackageInstallation(m_package.take()); },
+        QPointer<Package> newPackage;
+        QMetaObject::invokeMethod(PackageManager::instance(), [this, &newPackage]()
+            { newPackage = PackageManager::instance()->startingPackageInstallation(m_package.take()); },
             Qt::BlockingQueuedConnection);
+        m_managerApproval = !newPackage.isNull();
 
         if (!m_managerApproval)
             throw Exception("PackageManager declined the installation of %1").arg(packageId);
 
+        // if any of the apps in the package were running before, we now need to wait until all of
+        // them have actually stopped
+        while (!m_canceled && newPackage && !newPackage->areAllApplicationsStoppedDueToBlock())
+            QThread::msleep(30);
+
+        if (m_canceled || newPackage.isNull())
+            throw Exception(Error::Canceled, "canceled");
+
         // we're not interested in any other files from here on...
         m_extractor->setFileExtractedCallback(nullptr);
     }
diff --git a/src/manager-lib/package.cpp b/src/manager-lib/package.cpp
index 4dc94ec9..e7226cb6 100644
--- a/src/manager-lib/package.cpp
+++ b/src/manager-lib/package.cpp
@@ -159,7 +159,11 @@ QT_BEGIN_NAMESPACE_AM
 Package::Package(PackageInfo *packageInfo, State initialState)
     : m_info(packageInfo)
     , m_state(initialState)
-{ }
+{
+    // calling block() would lead to the AM waiting for the not-yet installed apps to quit
+    if (initialState == BeingInstalled)
+        m_blocked = 1;
+}
 
 QString Package::id() const
 {
diff --git a/src/manager-lib/packagemanager.cpp b/src/manager-lib/packagemanager.cpp
index 2546f9ee..61e41546 100644
--- a/src/manager-lib/packagemanager.cpp
+++ b/src/manager-lib/packagemanager.cpp
@@ -365,8 +365,8 @@ void PackageManager::registerPackages()
         registerPackage(it.value().first, it.value().second);
 }
 
-void PackageManager::registerPackage(PackageInfo *packageInfo, PackageInfo *updatedPackageInfo,
-                                     bool currentlyBeingInstalled)
+Package *PackageManager::registerPackage(PackageInfo *packageInfo, PackageInfo *updatedPackageInfo,
+                                         bool currentlyBeingInstalled)
 {
     auto *package = new Package(packageInfo, currentlyBeingInstalled ? Package::BeingInstalled
                                                                      : Package::Installed);
@@ -376,8 +376,7 @@ void PackageManager::registerPackage(PackageInfo *packageInfo, PackageInfo *upda
     QQmlEngine::setObjectOwnership(package, QQmlEngine::CppOwnership);
 
     if (currentlyBeingInstalled) {
-        bool blocked = package->block();
-        Q_ASSERT(blocked);
+        Q_ASSERT(package->isBlocked());
 
         beginInsertRows(QModelIndex(), d->packages.count(), d->packages.count());
         qCDebug(LogSystem) << "Installing package:";
@@ -397,6 +396,8 @@ void PackageManager::registerPackage(PackageInfo *packageInfo, PackageInfo *upda
 
     if (!currentlyBeingInstalled)
         registerApplicationsAndIntentsOfPackage(package);
+
+    return package;
 }
 
 void PackageManager::registerApplicationsAndIntentsOfPackage(Package *package)
@@ -1304,19 +1305,19 @@ void PackageManager::handleFailure(AsynchronousTask *task)
 #endif // !defined(AM_DISABLE_INSTALLER)
 
 
-bool PackageManager::startingPackageInstallation(PackageInfo *info)
+Package *PackageManager::startingPackageInstallation(PackageInfo *info)
 {
     // ownership of info is transferred to PackageManager
     QScopedPointer<PackageInfo> newInfo(info);
 
     if (!newInfo || newInfo->id().isEmpty())
-        return false;
+        return nullptr;
 
     Package *package = fromId(newInfo->id());
 
     if (package) { // update
         if (!package->block())
-            return false;
+            return nullptr;
 
         // do not overwrite the base-info / update-info yet - only after a successful installation
         d->pendingPackageInfoUpdates.insert(package, newInfo.take());
@@ -1324,11 +1325,12 @@ bool PackageManager::startingPackageInstallation(PackageInfo *info)
         package->setState(Package::BeingUpdated);
         package->setProgress(0);
         emitDataChanged(package);
+        return package;
+
     } else { // installation
         // add a new package to the model and block it
-        registerPackage(newInfo.take(), nullptr, true);
+        return registerPackage(newInfo.take(), nullptr, true);
     }
-    return true;
 }
 
 bool PackageManager::startingPackageRemoval(const QString &id)
diff --git a/src/manager-lib/packagemanager.h b/src/manager-lib/packagemanager.h
index 2e931985..08a045c3 100644
--- a/src/manager-lib/packagemanager.h
+++ b/src/manager-lib/packagemanager.h
@@ -186,8 +186,7 @@ signals:
     Q_SCRIPTABLE void taskBlockingUntilInstallationAcknowledge(const QString &taskId);
 
 protected:
-
-    bool startingPackageInstallation(PackageInfo *info);
+    Package *startingPackageInstallation(PackageInfo *info);
     bool startingPackageRemoval(const QString &id);
     bool finishedPackageInstall(const QString &id);
     bool canceledPackageInstall(const QString &id);
@@ -202,8 +201,8 @@ private:
 
 private:
     void emitDataChanged(Package *package, const QVector<int> &roles = QVector<int>());
-    void registerPackage(PackageInfo *packageInfo, PackageInfo *updatedPackageInfo,
-                         bool currentlyBeingInstalled = false);
+    Package *registerPackage(PackageInfo *packageInfo, PackageInfo *updatedPackageInfo,
+                             bool currentlyBeingInstalled = false);
     void registerApplicationsAndIntentsOfPackage(Package *package);
     void unregisterApplicationsAndIntentsOfPackage(Package *package);
     static void registerQmlTypes();
diff --git a/tests/auto/qml/installer/tst_installer.qml b/tests/auto/qml/installer/tst_installer.qml
index d3443ee7..701b7242 100644
--- a/tests/auto/qml/installer/tst_installer.qml
+++ b/tests/auto/qml/installer/tst_installer.qml
@@ -65,11 +65,23 @@ TestCase {
     }
 
     SignalSpy {
+        id: taskBlockingUntilInstallationAcknowledgeSpy
+        target: PackageManager
+        signalName: "taskBlockingUntilInstallationAcknowledge"
+    }
+
+    SignalSpy {
         id: applicationChangedSpy
         target: ApplicationManager
         signalName: "applicationChanged"
     }
 
+    SignalSpy {
+        id: applicationRunStateChangedSpy
+        target: ApplicationManager
+        signalName: "applicationRunStateChanged"
+    }
+
 
     function init() {
         // Remove previous installations
@@ -249,4 +261,49 @@ TestCase {
         verify(!pkg.blocked)
         compare(pkg.version, "v1");
     }
+
+    function test_5stop_on_update() {
+        taskStateChangedSpy.clear()
+        taskBlockingUntilInstallationAcknowledgeSpy.clear()
+        applicationRunStateChangedSpy.clear()
+
+        // start the app
+        var app = ApplicationManager.application("hello-world.red")
+        verify(app)
+        verify(app.start())
+        applicationRunStateChangedSpy.wait(spyTimeout);
+        compare(applicationRunStateChangedSpy.count, 1);
+        compare(applicationRunStateChangedSpy.signalArguments[0][0], "hello-world.red")
+        compare(applicationRunStateChangedSpy.signalArguments[0][1], Am.StartingUp)
+        applicationRunStateChangedSpy.clear()
+        applicationRunStateChangedSpy.wait(spyTimeout);
+        compare(applicationRunStateChangedSpy.count, 1);
+        compare(applicationRunStateChangedSpy.signalArguments[0][0], "hello-world.red")
+        compare(applicationRunStateChangedSpy.signalArguments[0][1], Am.Running)
+        applicationRunStateChangedSpy.clear()
+
+        // now install the update
+        var id = PackageManager.startPackageInstallation(ApplicationManager.systemProperties.AM_TESTDATA_DIR
+                                                         + "/packages/hello-world.red.appkg")
+        taskBlockingUntilInstallationAcknowledgeSpy.wait(spyTimeout);
+        compare(taskBlockingUntilInstallationAcknowledgeSpy.count, 1);
+        compare(taskBlockingUntilInstallationAcknowledgeSpy.signalArguments[0][0], id);
+        taskBlockingUntilInstallationAcknowledgeSpy.clear();
+
+        // make sure the app gets shut down during the update
+        compare(applicationRunStateChangedSpy.count, 2);
+        compare(applicationRunStateChangedSpy.signalArguments[0][0], "hello-world.red")
+        compare(applicationRunStateChangedSpy.signalArguments[0][1], Am.ShuttingDown)
+        compare(applicationRunStateChangedSpy.signalArguments[1][0], "hello-world.red")
+        compare(applicationRunStateChangedSpy.signalArguments[1][1], Am.NotRunning)
+        applicationRunStateChangedSpy.clear()
+
+        PackageManager.acknowledgePackageInstallation(id);
+
+        taskFinishedSpy.wait(spyTimeout);
+        var pkg = PackageManager.package("hello-world.red")
+        compare(pkg.version, "red");
+        taskFinishedSpy.clear();
+        applicationChangedSpy.clear();
+    }
 }