Merge lp:~lukas-kde/unity8/fix-shell-chrome into lp:unity8

[Lukáš Tinkl]

Shell chrome fixes

Correctly save and load the window state from storage, do not restore our QML window right away but queue the state, filter it with the shell chrome policy (when in windowed mode) later on, and then apply it at altogether, once the surface creation is settled.

For a more elegant longterm fix, we should probably introduce "initialSurfaceState" in qtmir, much like the recent "initialSurfaceSize" so that the window/surface gets the correct state right from the beginning.

Shell chrome fixes

[Daniel d'Andrada (dandrader)]

"""
interval: 1000
"""

Why 1000 and not 500 or 2000. Do you know what are you waiting for more exactly?

[Daniel d'Andrada (dandrader)]

would be great if you could split the timer hack from the other, uncontroversial, proper fixes.

[lp:~lukas-kde/unity8/fix-shell-chrome]

2835. By Lukáš Tinkl on 2017-02-22

shell chrome fixes

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

FAILED: Continuous integration, rev:2835
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3182/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/4178
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/2461
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=zesty,testname=qmluitests.sh/2461
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/4206
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4043
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/4043/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4043
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/4043/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4043
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/4043/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4043
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/4043/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4043
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/4043/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4043
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/4043/artifact/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)

[lp:~lukas-kde/unity8/fix-shell-chrome]

2836. By Lukáš Tinkl on 2017-02-22

fix some tests now that camera-app is no longer fullscreen in windowed mode ;)

2837. By Lukáš Tinkl on 2017-02-23

more test fixes

2838. By Lukáš Tinkl on 2017-02-23

load/init the values in every case

2839. By Lukáš Tinkl on 2017-02-24

w/o timer

2840. By Lukáš Tinkl on 2017-02-24

cascade, then onWindowReady load and apply the state

2841. By Lukáš Tinkl on 2017-02-24

fix typo

2842. By Lukáš Tinkl on 2017-02-24

simplify

2843. By Lukáš Tinkl on 2017-02-24

fix the staged shell chrome policy

2844. By Lukáš Tinkl on 2017-02-24

revert initialSurfaceSize condition

2845. By Lukáš Tinkl on 2017-02-24

merge trunk

2846. By Lukáš Tinkl on 2017-02-24

merge lp:~dandrader/unity8/fix-shell-chrome

saves one appDelegate property

2847. By Lukáš Tinkl on 2017-02-24

fix the test assumption that mock camera-app will always be fullscreen

2848. By Lukáš Tinkl on 2017-02-24

stabilize test

2849. By Lukáš Tinkl on 2017-02-27

always save the size/state, fixes closing app in staged mode and then restarting in windowed mode

2850. By Lukáš Tinkl on 2017-02-27

fix warning

2851. By Lukáš Tinkl on 2017-03-02

try to stabilize Shell::test_spreadDisabled()

Unmerged revisions

=== modified file 'qml/Stage/Stage.qml'
--- qml/Stage/Stage.qml	2017-02-16 13:44:45 +0000
+++ qml/Stage/Stage.qml	2017-02-22 17:48:13 +0000
@@ -470,7 +470,7 @@
             extend: "stagedRightEdge"
             PropertyChanges {
                 target: sideStage
-                opacity: priv.sideStageDelegate.x === sideStage.x ? 1 : 0
+                opacity: priv.sideStageDelegate && priv.sideStageDelegate.x === sideStage.x ? 1 : 0
                 visible: true
             }
         },
@@ -723,6 +723,7 @@
                     // miral doesn't know about our window decorations. So we have to deduct them
                     value: Qt.point(appDelegate.requestedX + appDelegate.clientAreaItem.x,
                                     appDelegate.requestedY + appDelegate.clientAreaItem.y)
+                    when: root.mode == "windowed"
                 }
 
                 // In those are for windowed mode. Those values basically store the window's properties
@@ -768,7 +769,6 @@
                                     Math.max(0, priv.virtualKeyboardHeight - (appContainer.height - (appDelegate.requestedY + appDelegate.height))))
                     when: root.oskEnabled && appDelegate.focus && (appDelegate.state == "normal" || appDelegate.state == "restored")
                           && root.inputMethodRect.height > 0
-
                 }
 
                 Behavior on x { id: xBehavior; enabled: priv.closingIndex >= 0; UbuntuNumberAnimation { onRunningChanged: if (!running) priv.closingIndex = -1} }
@@ -933,7 +933,8 @@
                         } else if (model.window.state === Mir.MaximizedBottomRightState) {
                             appDelegate.maximizeBottomRight();
                         } else if (model.window.state === Mir.RestoredState) {
-                            if (appDelegate.fullscreen && appDelegate.prevWindowState != WindowStateStorage.WindowStateRestored) {
+                            if (appDelegate.fullscreen && appDelegate.prevWindowState != WindowStateStorage.WindowStateRestored
+                                    && appDelegate.prevWindowState != WindowStateStorage.WindowStateNormal) {
                                 model.window.requestState(WindowStateStorage.toMirState(appDelegate.prevWindowState));
                             } else {
                                 appDelegate.restore();
@@ -945,6 +946,22 @@
                     }
                 }
 
+                // queue the requests to change the window/surface state
+                Timer {
+                    id: delayedStateTimer
+                    property int windowState: -1 // Mir::State
+                    interval: 1000
+                    onTriggered: {
+                        if (delayedStateTimer.windowState != -1) {
+                            if (windowedFullscreenPolicy.active) { // need to apply the windowed shell chrome policy on top the saved window state
+                                delayedStateTimer.windowState = windowedFullscreenPolicy.applyPolicy(delayedStateTimer.windowState, appDelegate.surface.shellChrome);
+                            }
+                            appDelegate.window.requestState(delayedStateTimer.windowState);
+                            delayedStateTimer.windowState = -1;
+                        }
+                    }
+                }
+
                 Component.onCompleted: {
                     if (application && application.rotatesWindowContents) {
                         decoratedWindow.surfaceOrientationAngle = shellOrientationAngle;
@@ -955,9 +972,12 @@
                     // First, cascade the newly created window, relative to the currently/old focused window.
                     windowedX = priv.focusedAppDelegate ? priv.focusedAppDelegate.windowedX + units.gu(3) : (normalZ - 1) * units.gu(3)
                     windowedY = priv.focusedAppDelegate ? priv.focusedAppDelegate.windowedY + units.gu(3) : normalZ * units.gu(3)
-                    // Now load any saved state. This needs to happen *after* the cascading!
-                    windowStateSaver.load();
-                    model.window.requestState(WindowStateStorage.toMirState(windowState));
+
+                    if (root.mode == "windowed") {
+                        // Now load any saved state. This needs to happen *after* the cascading!
+                        delayedStateTimer.windowState = WindowStateStorage.toMirState(windowStateSaver.load());
+                        delayedStateTimer.restart();
+                    }
 
                     updateQmlFocusFromMirSurfaceFocus();
 
@@ -965,7 +985,9 @@
                     _constructing = false;
                 }
                 Component.onDestruction: {
-                    windowStateSaver.save();
+                    if (root.mode == "windowed") {
+                        windowStateSaver.save();
+                    }
 
                     if (!root.parent) {
                         // This stage is about to be destroyed. Don't mess up with the model at this point
@@ -1441,7 +1463,7 @@
                 ]
                 transitions: [
                     Transition {
-                        from: "staged,stagedWithSideStage"; to: "normal"
+                        from: "staged,stagedWithSideStage"
                         enabled: appDelegate.animationsEnabled
                         PropertyAction { target: appDelegate; properties: "visuallyMinimized,visuallyMaximized" }
                         UbuntuNumberAnimation { target: appDelegate; properties: "x,y,requestedX,requestedY,opacity,requestedWidth,requestedHeight,scale"; duration: priv.animationDuration }
@@ -1462,8 +1484,7 @@
                     },
                     Transition {
                         from: "normal,staged"; to: "stagedWithSideStage"
-                        UbuntuNumberAnimation { target: appDelegate; properties: "x,y"; duration: priv.animationDuration }
-                        UbuntuNumberAnimation { target: appDelegate; properties: "requestedWidth,requestedHeight"; duration: priv.animationDuration }
+                        UbuntuNumberAnimation { target: appDelegate; properties: "x,y,requestedWidth,requestedHeight"; duration: priv.animationDuration }
                     },
                     Transition {
                         to: "windowedRightEdge"
@@ -1503,7 +1524,6 @@
                         from: ",normal,restored,maximized,maximizedLeft,maximizedRight,maximizedTopLeft,maximizedTopRight,maximizedBottomLeft,maximizedBottomRight,maximizedHorizontally,maximizedVertically,fullscreen"
                         to: "minimized"
                         SequentialAnimation {
-                            ScriptAction { script: print("transitioning:", appDelegate.x, appDelegate.y, appDelegate.scale) }
                             ScriptAction { script: { fakeRectangle.stop(); } }
                             PropertyAction { target: appDelegate; property: "visuallyMaximized" }
                             UbuntuNumberAnimation { target: appDelegate; properties: "x,y,scale,opacity"; duration: priv.animationDuration }
@@ -1514,7 +1534,6 @@
                         from: "minimized"
                         to: ",normal,restored,maximized,maximizedLeft,maximizedRight,maximizedTopLeft,maximizedTopRight,maximizedBottomLeft,maximizedBottomRight,maximizedHorizontally,maximizedVertically,fullscreen"
                         SequentialAnimation {
-                            ScriptAction { script: print("transitioning:", appDelegate.x, appDelegate.y, appDelegate.scale) }
                             PropertyAction { target: appDelegate; property: "visuallyMinimized,z" }
                             ParallelAnimation {
                                 UbuntuNumberAnimation { target: appDelegate; properties: "x"; from: -appDelegate.width / 2; duration: priv.animationDuration }
@@ -1662,12 +1681,15 @@
                 WindowedFullscreenPolicy {
                     id: windowedFullscreenPolicy
                     active: root.mode == "windowed"
-                    surface: model.window.surface
                 }
                 StagedFullscreenPolicy {
                     id: stagedFullscreenPolicy
                     active: root.mode == "staged" || root.mode == "stagedWithSideStage"
                     surface: model.window.surface
+                    onStateRequested: {
+                        delayedStateTimer.windowState = requestedState;
+                        delayedStateTimer.restart();
+                    }
                 }
 
                 SpreadDelegateInputArea {

=== modified file 'qml/Stage/StagedFullscreenPolicy.qml'
--- qml/Stage/StagedFullscreenPolicy.qml	2016-11-30 19:24:02 +0000
+++ qml/Stage/StagedFullscreenPolicy.qml	2017-02-22 17:48:13 +0000
@@ -28,11 +28,13 @@
 QtObject {
     property bool active: true
 
+    signal stateRequested(int requestedState)
+
     property var surface: null
     onSurfaceChanged: {
         if (!active || !surface) return;
         if (surface.shellChrome === Mir.LowChrome) {
-            surface.requestState(Mir.FullscreenState);
+            stateRequested(Mir.FullscreenState);
         }
     }
 
@@ -41,15 +43,15 @@
         onShellChromeChanged: {
             if (!active || !surface) return;
             if (surface.shellChrome === Mir.LowChrome) {
-                surface.requestState(Mir.FullscreenState);
+                stateRequested(Mir.FullscreenState);
             } else {
-                surface.requestState(Mir.RestoredState);
+                stateRequested(Mir.RestoredState);
             }
         }
         onStateChanged: {
-            if (!active) return;
+            if (!active || !surface) return;
             if (surface.state === Mir.RestoredState && surface.shellChrome === Mir.LowChrome) {
-                surface.requestState(Mir.FullscreenState);
+                stateRequested(Mir.FullscreenState);
             }
         }
     }

=== modified file 'qml/Stage/WindowStateSaver.qml'
--- qml/Stage/WindowStateSaver.qml	2016-11-23 18:43:48 +0000
+++ qml/Stage/WindowStateSaver.qml	2017-02-22 17:48:13 +0000
@@ -42,13 +42,14 @@
         target.windowedY = Qt.binding(function() { return Math.max(Math.min(windowGeometry.y, screenHeight - target.windowedHeight), minimumY); });
 
         var windowState = WindowStateStorage.getState(target.appId, WindowStateStorage.WindowStateNormal)
-        target.restore(false /* animated */, windowState);
 
         target.updateNormalGeometry();
 
         // initialize the x/y to restore to
         target.restoredX = target.normalX;
         target.restoredY = target.normalY;
+
+        return windowState;
     }
 
     function save() {

=== modified file 'qml/Stage/WindowedFullscreenPolicy.qml'
--- qml/Stage/WindowedFullscreenPolicy.qml	2016-11-30 19:24:02 +0000
+++ qml/Stage/WindowedFullscreenPolicy.qml	2017-02-22 17:48:13 +0000
@@ -23,17 +23,11 @@
 // state of the window is returned to restored.
 QtObject {
     property bool active: true
-    property QtObject surface: null
-
-    property bool _firstTimeSurface: true
-
-    onSurfaceChanged: {
-        if (!active || !surface) return;
-        if (!_firstTimeSurface) return;
-        _firstTimeSurface = false;
-
-        if (surface.state === Mir.FullscreenState && surface.shellChrome === Mir.LowChrome) {
-            surface.requestState(Mir.RestoredState);
+
+    function applyPolicy(surfaceState, surfaceChrome) {
+        if (surfaceState === Mir.FullscreenState && surfaceChrome === Mir.LowChrome) {
+            return Mir.RestoredState;
         }
+        return surfaceState;
     }
 }

=== modified file 'tests/qmltests/tst_Shell.qml'
--- tests/qmltests/tst_Shell.qml	2017-02-16 13:46:11 +0000
+++ tests/qmltests/tst_Shell.qml	2017-02-22 17:48:13 +0000
@@ -334,7 +334,7 @@
 
                 Row {
                     CheckBox {
-                        id: fullscreeAppCheck
+                        id: fullscreenAppCheck
                         activeFocusOnPress: false
                         activeFocusOnTab: false
 
@@ -348,13 +348,10 @@
                         }
 
                         Binding {
-                            target: fullscreeAppCheck
+                            target: fullscreenAppCheck
                             when: topLevelSurfaceList && topLevelSurfaceList.focusedWindow
                             property: "checked"
-                            value: {
-                                if (!topLevelSurfaceList || !topLevelSurfaceList.focusedWindow) return false;
-                                return topLevelSurfaceList.focusedWindow.state === Mir.FullscreenState
-                            }
+                            value: topLevelSurfaceList.focusedWindow.state === Mir.FullscreenState
                         }
                     }
                     Label {
@@ -365,6 +362,8 @@
                 Row {
                     CheckBox {
                         id: chromeAppCheck
+                        activeFocusOnPress: false
+                        activeFocusOnTab: false
 
                         onTriggered: {
                             if (!topLevelSurfaceList.focusedWindow || !topLevelSurfaceList.focusedWindow.surface) return;
@@ -380,10 +379,7 @@
                             target: chromeAppCheck
                             when: topLevelSurfaceList && topLevelSurfaceList.focusedWindow !== null && topLevelSurfaceList.focusedWindow.surface !== null
                             property: "checked"
-                            value: {
-                                if (!topLevelSurfaceList || !topLevelSurfaceList.focusedWindow || !topLevelSurfaceList.focusedWindow.surface) return false;
-                                topLevelSurfaceList.focusedWindow.surface.shellChrome === Mir.LowChrome
-                            }
+                            value: topLevelSurfaceList.focusedWindow.surface.shellChrome === Mir.LowChrome
                         }
                     }
                     Label {