Merge lp:~lukas-kde/unity8/fix-rotation-lp1614070 into lp:unity8

[Lukáš Tinkl]

Reset topmostIsFullscreen to correctly rotate the shell when dismissing SIM PIN screen

Reset topmostIsFullscreen to correctly rotate the shell when dismissing SIM PIN screen

Fixes lp:1614070

* 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
 * If you changed the UI, has there been a design review?
N/A

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

FAILED: Continuous integration, rev:2597
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1983/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/2602/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/2630/console

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

review: Needs Fixing (continuous-integration)

[Michał Sawicz (saviq)]

Hey, this is not enough, the value of topmostIsFullscreen is not getting reset after you have a fullscreen one, we've decided to revert this commit whole, will need to bring it back, fixed, with a test for rotation ideally.

review: Needs Resubmitting

[Michał Sawicz (saviq)]

OK after reading your comment on the other MP you're right, we shouldn't land it as is, but we still need a proper fix.

review: Needs Fixing

[lp:~lukas-kde/unity8/fix-rotation-lp1614070]

2598. By Lukáš Tinkl on 2016-08-19

reset when there's no more notifications around

2599. By Lukáš Tinkl on 2016-08-19

don't let all the notifications fight for topmostIsFullscreen

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

FAILED: Continuous integration, rev:2598
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1985/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/2604
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/1419
    FAILURE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/1419/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=yakkety,testname=qmluitests.sh/1419
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/2632
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2511
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2511
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2511
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2505
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2505/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2505
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2505/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2505
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2505/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2505
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2505/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2505
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2505/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2505
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2505/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2505
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2505/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2505
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2505/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2505
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2505/artifact/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)

[lp:~lukas-kde/unity8/fix-rotation-lp1614070]

2600. By Lukáš Tinkl on 2016-08-19

add a mock and test for the fullscreen notifications

2601. By Lukáš Tinkl on 2016-08-19

correctly cleanup

2602. By Lukáš Tinkl on 2016-08-19

guard another unconditional

2603. By Lukáš Tinkl on 2016-08-19

remove stray empty line

[Albert Astals Cid (aacid)]

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

 * Did CI run pass? If not, please explain why.
Let's wait before top approval.

review: Approve

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

FAILED: Continuous integration, rev:2599
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1987/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/2606
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/1421
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/1421
    FAILURE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=yakkety,testname=qmluitests.sh/1421/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/2634
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2513
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2513
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2513
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2507
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2507/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2507
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2507/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2507
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2507/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2507
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2507/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2507
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2507/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2507
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2507/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2507
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2507/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2507
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2507/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2507
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2507/artifact/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)

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

FAILED: Continuous integration, rev:2603
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1991/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/2610
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/1425
    FAILURE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/1425/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=yakkety,testname=qmluitests.sh/1425/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/2638
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2517
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2517
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2517
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2511
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2511/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2511
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2511/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2511
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2511/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2511
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2511/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2511
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2511/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2511
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2511/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2511
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2511/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2511
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2511/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2511
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2511/artifact/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)

=== modified file 'qml/Notifications/Notification.qml'
--- qml/Notifications/Notification.qml	2016-08-02 11:33:03 +0000
+++ qml/Notifications/Notification.qml	2016-08-19 13:41:38 +0000
@@ -37,7 +37,8 @@
     property var hints
     property var notification
     property color color: theme.palette.normal.background
-    property bool fullscreen: false
+    property bool fullscreen: notification.notification && typeof notification.notification.fullscreen != "undefined" ?
+                                  notification.notification.fullscreen : false // fullscreen prop only exists in the mock
     property int maxHeight
     property int margins: units.gu(1)
 
@@ -123,7 +124,9 @@
         if (fullscreen) {
             notification.notification.urgency = Notification.Critical;
         }
-        ListView.view.topmostIsFullscreen = fullscreen;
+        if (index == 0) {
+            ListView.view.topmostIsFullscreen = fullscreen;
+        }
     }
 
     Behavior on implicitHeight {

=== modified file 'qml/Notifications/Notifications.qml'
--- qml/Notifications/Notifications.qml	2016-08-02 11:33:03 +0000
+++ qml/Notifications/Notifications.qml	2016-08-19 13:41:38 +0000
@@ -69,7 +69,9 @@
 
         property int theIndex: index
         onTheIndexChanged: {
-            ListView.view.topmostIsFullscreen = fullscreen; // when we get pushed down by e.g. volume notification
+            if (theIndex == 0) {
+                ListView.view.topmostIsFullscreen = fullscreen; // when we get pushed down by e.g. volume notification
+            }
         }
 
         // make sure there's no opacity-difference between the several
@@ -78,6 +80,8 @@
         //layer.enabled: add.running || remove.running || populate.running
     }
 
+    onCountChanged: if (count == 0) topmostIsFullscreen = false; // reset
+
     // FIXME: disabled all transitions because of LP: #1354406 workaround
     /*populate: Transition {
         UbuntuNumberAnimation {

=== modified file 'tests/mocks/Unity/Notifications/MockNotification.cpp'
--- tests/mocks/Unity/Notifications/MockNotification.cpp	2015-09-18 13:58:21 +0000
+++ tests/mocks/Unity/Notifications/MockNotification.cpp	2016-08-19 13:41:38 +0000
@@ -32,6 +32,7 @@
     QStringList actions;
     ActionModel* actionsModel;
     QVariantMap hints;
+    bool fullscreen = false;
 };
 
 MockNotification::MockNotification(QObject *parent) : QObject(parent), p(new MockNotificationPrivate()) {
@@ -181,3 +182,17 @@
 void MockNotification::close() {
     Q_EMIT completed(p->id);
 }
+
+bool MockNotification::fullscreen() const
+{
+    return p->fullscreen;
+}
+
+void MockNotification::setFullscreen(bool fullscreen)
+{
+    if (p->fullscreen == fullscreen)
+        return;
+
+    p->fullscreen = fullscreen;
+    Q_EMIT fullscreenChanged(fullscreen);
+}

=== modified file 'tests/mocks/Unity/Notifications/MockNotification.h'
--- tests/mocks/Unity/Notifications/MockNotification.h	2015-09-18 13:58:21 +0000
+++ tests/mocks/Unity/Notifications/MockNotification.h	2016-08-19 13:41:38 +0000
@@ -41,6 +41,7 @@
     Q_PROPERTY(QStringList rawActions READ rawActions WRITE setActions)
     Q_PROPERTY(ActionModel* actions READ getActions NOTIFY actionsChanged)
     Q_PROPERTY(QVariantMap hints READ getHints WRITE setHints NOTIFY hintsChanged)
+    Q_PROPERTY(bool fullscreen READ fullscreen WRITE setFullscreen NOTIFY fullscreenChanged)  // only in mock
 
 private:
     QScopedPointer<MockNotificationPrivate> p;
@@ -64,6 +65,8 @@
     void completed(int nid);
     void actionInvoked(const QString &action);
 
+    void fullscreenChanged(bool fullscreen);
+
 public:
     MockNotification(QObject *parent=nullptr);
     virtual ~MockNotification();
@@ -87,6 +90,8 @@
     void setActions(const QStringList &actions);
     QVariantMap getHints() const;
     void setHints(const QVariantMap& hints);
+    bool fullscreen() const;
+    void setFullscreen(bool fullscreen);
 
     Q_INVOKABLE void invokeAction(const QString &action);
     Q_INVOKABLE void close();

=== modified file 'tests/qmltests/Notifications/Notification.qml'
--- tests/qmltests/Notifications/Notification.qml	2015-01-26 12:03:19 +0000
+++ tests/qmltests/Notifications/Notification.qml	2016-08-19 13:41:38 +0000
@@ -28,4 +28,5 @@
     secondaryIcon: ""
     value: 0
     rawActions: []
+    fullscreen: false
 }

=== modified file 'tests/qmltests/Notifications/tst_Notifications.qml'
--- tests/qmltests/Notifications/tst_Notifications.qml	2016-08-02 10:28:28 +0000
+++ tests/qmltests/Notifications/tst_Notifications.qml	2016-08-19 13:41:38 +0000
@@ -141,6 +141,18 @@
             mockModel.append(n)
         }
 
+        function addFullscreenNotification() {
+            var component = Qt.createComponent("Notification.qml")
+            var n = component.createObject("notification", {"nid": index++,
+                                                            "type": Notification.SnapDecision,
+                                                            "hints": {"x-canonical-private-affirmative-tint": "true"},
+                                                            "summary": "SIM PIN screen",
+                                                            "body": "Enter your PIN to unlock the SIM",
+                                                            "fullscreen": true})
+            n.completed.connect(mockModel.onCompleted)
+            mockModel.append(n)
+        }
+
         function clearNotifications() {
             while(mockModel.count > 0) {
                 remove1stNotification();
@@ -230,6 +242,12 @@
 
                 Button {
                     width: parent.width
+                    text: "add a fullscreen notification"
+                    onClicked: rootRow.addFullscreenNotification()
+                }
+
+                Button {
+                    width: parent.width
                     text: "remove 1st notification"
                     onClicked: rootRow.remove1stNotification()
                 }
@@ -371,6 +389,14 @@
                     rawActions: ["ok_id",     "Ok",
                                  "snooze_id", "Snooze",
                                  "view_id",   "View"]
+                },
+                Notification {
+                    nid: 10
+                    type: Notification.SnapDecision
+                    hints: {"x-canonical-private-affirmative-tint": "true"}
+                    summary: "SIM PIN screen"
+                    body: "Enter your PIN to unlock the SIM"
+                    fullscreen: true
                 }
             ]
 
@@ -538,15 +564,12 @@
                 signalName: "actionInvoked"
             }
 
-            function init() {
-                while (mockModel.count > 0) {
-                    mockModel.remove(0);
-                }
-            }
-
             function cleanup() {
                 clickThroughSpy.clear()
                 actionSpy.clear()
+                while (mockModel.count > 0) {
+                    mockModel.removeFirst();
+                }
             }
 
             function test_NotificationRenderer(data) {
@@ -797,6 +820,50 @@
                 tryCompareFunction(function() { return notification2.expanded; }, undefined);
             }
 
+            function topmostIsFullscreen_data() {
+                return [{
+                            tag: "Confirmation notification with value",
+                            n: nlist[6]
+                        },
+                        {
+                            tag: "SIM PIN fullscreen notification",
+                            n: nlist[9]
+                        },
+                        {
+                            tag: "Snap Decision without secondary icon and no button-tint",
+                            n: nlist[3]
+                        }]
+            }
+
+            function test_topmostIsFullscreen() {
+                var data = topmostIsFullscreen_data();
+
+                // fill the model
+                data.forEach(function(notification) {
+                    mockModel.append(notification.n);
+                    notification.n.completed.connect(mockModel.onCompleted);
+                })
+
+                // make sure the view is properly updated before going on
+                notifications.forceLayout();
+                waitForRendering(notifications);
+
+                // initially, topmost is not fullscreen
+                verify(!notifications.topmostIsFullscreen)
+
+                // close the 1st one, 2nd one should be fullscreen
+                var notification0 = findChild(notifications, "notification0")
+                notification0.closeNotification();
+                waitForRendering(notifications);
+                verify(notifications.topmostIsFullscreen)
+
+                // close the 1st one (SIM PIN) again, topmost should no longer be fullscreen
+                notification0 = findChild(notifications, "notification0")
+                notification0.closeNotification();
+                waitForRendering(notifications);
+                verify(!notifications.topmostIsFullscreen)
+            }
+
             function cleanupTestCase() {
                 notifications.hasMouse = false;
             }