Merge lp:~nick-dedekind/unity8/1534541.zombie-session into lp:unity8

[Nick Dedekind]

Remove zombie if they're not animated.

 * Are there any related MPs required for this MP to build/function as expected? Please list.
No

 * Did you perform an exploratory manual test run of your code change and any related functionality?
Yes

 * Did you make sure that your branch does not contain spurious tags?
Yes

 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

 * If you changed the UI, has there been a design review?
N/A

[Unity8 CI Bot (unity8-ci-bot)]

FAILED: Continuous integration, rev:2174
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/264/
Executed test runs:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/264/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2174
http://jenkins.qa.ubuntu.com/job/unity8-ci/7206/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/6310
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-xenial-touch/621/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/1911
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-qmluitest-xenial-amd64/614
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/1806
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/1806
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-xenial-amd64-ci/613
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-xenial-i386-ci/612
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-touch/4850
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/6321
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/6321/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/27271
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-xenial-touch/330/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/619
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/619/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/27269

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

review: Needs Fixing (continuous-integration)

[Albert Astals Cid (aacid)]

I tried reviewing this and got hit by https://bugs.launchpad.net/ubuntu/+source/unity-scopes-api/+bug/1543132

[Marcus Tomlinson (marcustomlinson)]

I've built this branch here: https://launchpad.net/~unity-api-team/+archive/ubuntu/dev-build-1

Tested on the phone and it works like a charm!!

I can confirm that this change fixes Bug #1534541 (together with its associated duplicates).

Nice work Nick!

review: Approve

[Albert Astals Cid (aacid)]

Code looks sane, test passes and fails without the new code.

 * Did you perform an exploratory manual test run of the code change and any related functionality?
Yes

 * Did CI run pass?
To what it can pass.

 * Did you make sure that the branch does not contain spurious tags?
Yes

review: Approve

=== modified file 'qml/Stages/SessionContainer.qml'
--- qml/Stages/SessionContainer.qml	2015-12-01 12:17:24 +0000
+++ qml/Stages/SessionContainer.qml	2016-02-03 14:09:31 +0000
@@ -152,6 +152,8 @@
             popped.completed.connect(function() { root.session.release(); } );
             popped.end();
             d.animations = tmp;
+        } else {
+            root.session.release();
         }
     }
 

=== modified file 'tests/mocks/Unity/Application/ApplicationTestInterface.cpp'
--- tests/mocks/Unity/Application/ApplicationTestInterface.cpp	2015-11-05 17:59:16 +0000
+++ tests/mocks/Unity/Application/ApplicationTestInterface.cpp	2016-02-03 14:09:31 +0000
@@ -36,7 +36,7 @@
     connection.registerObject("/com/canonical/Unity8/Mocks", parent);
 }
 
-Session* ApplicationTestInterface::addChildSession(Session* existingSession, const QString& surfaceImage)
+Session* ApplicationTestInterface::addChildSession(Session* existingSession, const QString& surfaceImage, bool addSurface)
 {
     if (!existingSession) return nullptr;
 
@@ -49,7 +49,7 @@
                              .arg(existingSession->childSessions()->count()),
         screenshotUrl);
     existingSession->addChildSession(session);
-    session->createSurface();
+    if (addSurface) session->createSurface();
 
     return session;
 }
@@ -70,7 +70,7 @@
         existingSurface->setLive(false);
 }
 
-quint32 ApplicationTestInterface::addChildSession(const QString& appId, quint32 existingSessionId, const QString& surfaceImage)
+quint32 ApplicationTestInterface::addChildSession(const QString& appId, quint32 existingSessionId, const QString& surfaceImage, bool addSurface)
 {
     qDebug() << "ApplicationTestInterface::addChildSession to " << appId;
 
@@ -100,7 +100,7 @@
                              .arg(parentSession->childSessions()->count()),
         screenshotUrl);
     parentSession->addChildSession(session);
-    session->createSurface();
+    if (addSurface) { session->createSurface(); }
     m_childSessions[sessionId] = session;
 
     return sessionId;

=== modified file 'tests/mocks/Unity/Application/ApplicationTestInterface.h'
--- tests/mocks/Unity/Application/ApplicationTestInterface.h	2015-08-03 15:00:47 +0000
+++ tests/mocks/Unity/Application/ApplicationTestInterface.h	2016-02-03 14:09:31 +0000
@@ -30,12 +30,12 @@
 public:
     ApplicationTestInterface(QObject* parent = 0);
 
-    Q_INVOKABLE Session* addChildSession(Session* existingSession, const QString& surfaceImage);
+    Q_INVOKABLE Session* addChildSession(Session* existingSession, const QString& surfaceImage, bool addSurface);
     Q_INVOKABLE void removeSession(Session* existingSession);
     Q_INVOKABLE void removeSurface(MirSurface* existingSurface);
 
 public Q_SLOTS:
-    quint32 addChildSession(const QString& appId, quint32 existingSessionId, const QString& surfaceImage);
+    quint32 addChildSession(const QString& appId, quint32 existingSessionId, const QString& surfaceImage, bool addSurface);
     void removeSession(quint32 sessionId);
 
 private:

=== modified file 'tests/qmltests/Stages/RecursingChildSessionControl.qml'
--- tests/qmltests/Stages/RecursingChildSessionControl.qml	2015-12-01 12:17:24 +0000
+++ tests/qmltests/Stages/RecursingChildSessionControl.qml	2016-02-03 14:09:31 +0000
@@ -88,7 +88,7 @@
 
             CheckBox {
                 id: _surfaceCheckbox;
-                checked: false;
+                checked: root.session.lastSurface ? true : false
                 activeFocusOnPress: false
                 enabled: root.session
                 onCheckedChanged: {
@@ -101,7 +101,7 @@
 
                 Connections {
                     target: root.session ? root.session : null
-                    onSurfaceChanged: {
+                    onLastSurfaceChanged: {
                         surfaceCheckbox.checked = root.session.lastSurface !== null
                     }
                 }
@@ -137,7 +137,7 @@
                 text: "Add Child"
                 onClicked: {
                     var screenshot = Math.round(Math.random()*100 % (screenshotIds.length-1));
-                    var session = ApplicationTest.addChildSession(root.session, root.screenshotIds[screenshot]);
+                    var session = ApplicationTest.addChildSession(root.session, root.screenshotIds[screenshot], true);
                 }
             }
         }

=== modified file 'tests/qmltests/Stages/tst_SessionContainer.qml'
--- tests/qmltests/Stages/tst_SessionContainer.qml	2015-12-01 12:17:24 +0000
+++ tests/qmltests/Stages/tst_SessionContainer.qml	2016-02-03 14:09:31 +0000
@@ -155,7 +155,8 @@
             var sessions = [];
             for (i = 0; i < data.count; i++) {
                 var session = ApplicationTest.addChildSession(sessionContainer.session,
-                                                              "gallery");
+                                                              "gallery",
+                                                              true);
                 session.createSurface();
                 sessionContainer.session.addChildSession(session);
                 compare(sessionContainer.childSessions.count(), i+1);
@@ -180,7 +181,7 @@
             // 3 sessions should cover all edge cases
             for(i = 0; i < 3; i++) {
                 var a_session = ApplicationTest.addChildSession(
-                    sessionContainer.session, "gallery"
+                    sessionContainer.session, "gallery", true
                 )
 
                 a_session.createSurface();
@@ -226,7 +227,8 @@
             var container = sessionContainer;
             for (i = 0; i < data.depth; i++) {
                 var session = ApplicationTest.addChildSession(lastSession,
-                                                              "gallery");
+                                                              "gallery",
+                                                              true);
                 session.createSurface();
                 lastSession.addChildSession(session);
                 compare(container.childSessions.count(), 1);
@@ -250,7 +252,8 @@
             var sessionContainer = sessionContainerLoader.item;
 
             var session = ApplicationTest.addChildSession(sessionContainer.session,
-                                                          "gallery");
+                                                          "gallery",
+                                                          true);
             session.createSurface();
             sessionContainer.session.addChildSession(session);
 
@@ -288,22 +291,41 @@
             var sessionContainer = sessionContainerLoader.item;
 
             var session = ApplicationTest.addChildSession(sessionContainer.session,
-                                                          "gallery");
-
-            var delegate = findChild(sessionContainer, "childDelegate0");
-            var childContainer = findChild(delegate, "sessionContainer");
-
-            // wait for animation to begin
-            tryCompareFunction(function() { return isContainerAnimating(childContainer); }, true);
-            // wait for animation to end
-            tryCompareFunction(function() { return isContainerAnimating(childContainer); }, false);
-
-            ApplicationTest.removeSession(session);
-
-            // wait for animation to begin
-            tryCompareFunction(function() { return isContainerAnimating(childContainer); }, true);
-            // wait for animation to end
-            tryCompareFunction(function() { return isContainerAnimating(childContainer); }, false);
+                                                          "gallery",
+                                                          true);
+
+            var delegate = findChild(sessionContainer, "childDelegate0");
+            var childContainer = findChild(delegate, "sessionContainer");
+
+            // wait for animation to begin
+            tryCompareFunction(function() { return isContainerAnimating(childContainer); }, true);
+            // wait for animation to end
+            tryCompareFunction(function() { return isContainerAnimating(childContainer); }, false);
+
+            ApplicationTest.removeSession(session);
+
+            // wait for animation to begin
+            tryCompareFunction(function() { return isContainerAnimating(childContainer); }, true);
+            // wait for animation to end
+            tryCompareFunction(function() { return isContainerAnimating(childContainer); }, false);
+
+            tryCompareFunction(function() { return sessionContainer.childSessions.count() }, 0);
+        }
+
+        function test_childSessionsRemovedOnStopWhenNotLive() {
+            sessionCheckbox.checked = true;
+            var sessionContainer = sessionContainerLoader.item;
+
+            var session = ApplicationTest.addChildSession(sessionContainer.session,
+                                                          "gallery",
+                                                          false);
+
+            var delegate = findChild(sessionContainer, "childDelegate0");
+            var childContainer = findChild(delegate, "sessionContainer");
+
+            ApplicationTest.removeSession(session);
+
+            tryCompareFunction(function() { return sessionContainer.childSessions.count() }, 0);
         }
     }
 }