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

Proposed by Lukáš Tinkl on 2017-02-22
Status: Superseded
Proposed branch: lp:~lukas-kde/unity8/fix-shell-chrome
Merge into: lp:unity8
Prerequisite: lp:~ci-train-bot/unity8/unity8-ubuntu-zesty-2481
Diff against target: 276 lines (+55/-40)
5 files modified
qml/Stage/Stage.qml (+35/-13)
qml/Stage/StagedFullscreenPolicy.qml (+7/-5)
qml/Stage/WindowStateSaver.qml (+2/-1)
qml/Stage/WindowedFullscreenPolicy.qml (+5/-11)
tests/qmltests/tst_Shell.qml (+6/-10)
To merge this branch: bzr merge lp:~lukas-kde/unity8/fix-shell-chrome
Reviewer Review Type Date Requested Status
Unity8 CI Bot continuous-integration Needs Fixing on 2017-02-22
Unity Team 2017-02-22 Pending
Review via email: mp+318010@code.launchpad.net

This proposal has been superseded by a proposal from 2017-02-22.

Commit message

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.

Description of the change

Shell chrome fixes

To post a comment you must log in.
Daniel d'Andrada (dandrader) wrote :

"""
interval: 1000
"""

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

Daniel d'Andrada (dandrader) wrote :

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

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

shell chrome fixes

Unity8 CI Bot (unity8-ci-bot) wrote :

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)
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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'qml/Stage/Stage.qml'
2--- qml/Stage/Stage.qml 2017-02-16 13:44:45 +0000
3+++ qml/Stage/Stage.qml 2017-02-22 17:48:13 +0000
4@@ -470,7 +470,7 @@
5 extend: "stagedRightEdge"
6 PropertyChanges {
7 target: sideStage
8- opacity: priv.sideStageDelegate.x === sideStage.x ? 1 : 0
9+ opacity: priv.sideStageDelegate && priv.sideStageDelegate.x === sideStage.x ? 1 : 0
10 visible: true
11 }
12 },
13@@ -723,6 +723,7 @@
14 // miral doesn't know about our window decorations. So we have to deduct them
15 value: Qt.point(appDelegate.requestedX + appDelegate.clientAreaItem.x,
16 appDelegate.requestedY + appDelegate.clientAreaItem.y)
17+ when: root.mode == "windowed"
18 }
19
20 // In those are for windowed mode. Those values basically store the window's properties
21@@ -768,7 +769,6 @@
22 Math.max(0, priv.virtualKeyboardHeight - (appContainer.height - (appDelegate.requestedY + appDelegate.height))))
23 when: root.oskEnabled && appDelegate.focus && (appDelegate.state == "normal" || appDelegate.state == "restored")
24 && root.inputMethodRect.height > 0
25-
26 }
27
28 Behavior on x { id: xBehavior; enabled: priv.closingIndex >= 0; UbuntuNumberAnimation { onRunningChanged: if (!running) priv.closingIndex = -1} }
29@@ -933,7 +933,8 @@
30 } else if (model.window.state === Mir.MaximizedBottomRightState) {
31 appDelegate.maximizeBottomRight();
32 } else if (model.window.state === Mir.RestoredState) {
33- if (appDelegate.fullscreen && appDelegate.prevWindowState != WindowStateStorage.WindowStateRestored) {
34+ if (appDelegate.fullscreen && appDelegate.prevWindowState != WindowStateStorage.WindowStateRestored
35+ && appDelegate.prevWindowState != WindowStateStorage.WindowStateNormal) {
36 model.window.requestState(WindowStateStorage.toMirState(appDelegate.prevWindowState));
37 } else {
38 appDelegate.restore();
39@@ -945,6 +946,22 @@
40 }
41 }
42
43+ // queue the requests to change the window/surface state
44+ Timer {
45+ id: delayedStateTimer
46+ property int windowState: -1 // Mir::State
47+ interval: 1000
48+ onTriggered: {
49+ if (delayedStateTimer.windowState != -1) {
50+ if (windowedFullscreenPolicy.active) { // need to apply the windowed shell chrome policy on top the saved window state
51+ delayedStateTimer.windowState = windowedFullscreenPolicy.applyPolicy(delayedStateTimer.windowState, appDelegate.surface.shellChrome);
52+ }
53+ appDelegate.window.requestState(delayedStateTimer.windowState);
54+ delayedStateTimer.windowState = -1;
55+ }
56+ }
57+ }
58+
59 Component.onCompleted: {
60 if (application && application.rotatesWindowContents) {
61 decoratedWindow.surfaceOrientationAngle = shellOrientationAngle;
62@@ -955,9 +972,12 @@
63 // First, cascade the newly created window, relative to the currently/old focused window.
64 windowedX = priv.focusedAppDelegate ? priv.focusedAppDelegate.windowedX + units.gu(3) : (normalZ - 1) * units.gu(3)
65 windowedY = priv.focusedAppDelegate ? priv.focusedAppDelegate.windowedY + units.gu(3) : normalZ * units.gu(3)
66- // Now load any saved state. This needs to happen *after* the cascading!
67- windowStateSaver.load();
68- model.window.requestState(WindowStateStorage.toMirState(windowState));
69+
70+ if (root.mode == "windowed") {
71+ // Now load any saved state. This needs to happen *after* the cascading!
72+ delayedStateTimer.windowState = WindowStateStorage.toMirState(windowStateSaver.load());
73+ delayedStateTimer.restart();
74+ }
75
76 updateQmlFocusFromMirSurfaceFocus();
77
78@@ -965,7 +985,9 @@
79 _constructing = false;
80 }
81 Component.onDestruction: {
82- windowStateSaver.save();
83+ if (root.mode == "windowed") {
84+ windowStateSaver.save();
85+ }
86
87 if (!root.parent) {
88 // This stage is about to be destroyed. Don't mess up with the model at this point
89@@ -1441,7 +1463,7 @@
90 ]
91 transitions: [
92 Transition {
93- from: "staged,stagedWithSideStage"; to: "normal"
94+ from: "staged,stagedWithSideStage"
95 enabled: appDelegate.animationsEnabled
96 PropertyAction { target: appDelegate; properties: "visuallyMinimized,visuallyMaximized" }
97 UbuntuNumberAnimation { target: appDelegate; properties: "x,y,requestedX,requestedY,opacity,requestedWidth,requestedHeight,scale"; duration: priv.animationDuration }
98@@ -1462,8 +1484,7 @@
99 },
100 Transition {
101 from: "normal,staged"; to: "stagedWithSideStage"
102- UbuntuNumberAnimation { target: appDelegate; properties: "x,y"; duration: priv.animationDuration }
103- UbuntuNumberAnimation { target: appDelegate; properties: "requestedWidth,requestedHeight"; duration: priv.animationDuration }
104+ UbuntuNumberAnimation { target: appDelegate; properties: "x,y,requestedWidth,requestedHeight"; duration: priv.animationDuration }
105 },
106 Transition {
107 to: "windowedRightEdge"
108@@ -1503,7 +1524,6 @@
109 from: ",normal,restored,maximized,maximizedLeft,maximizedRight,maximizedTopLeft,maximizedTopRight,maximizedBottomLeft,maximizedBottomRight,maximizedHorizontally,maximizedVertically,fullscreen"
110 to: "minimized"
111 SequentialAnimation {
112- ScriptAction { script: print("transitioning:", appDelegate.x, appDelegate.y, appDelegate.scale) }
113 ScriptAction { script: { fakeRectangle.stop(); } }
114 PropertyAction { target: appDelegate; property: "visuallyMaximized" }
115 UbuntuNumberAnimation { target: appDelegate; properties: "x,y,scale,opacity"; duration: priv.animationDuration }
116@@ -1514,7 +1534,6 @@
117 from: "minimized"
118 to: ",normal,restored,maximized,maximizedLeft,maximizedRight,maximizedTopLeft,maximizedTopRight,maximizedBottomLeft,maximizedBottomRight,maximizedHorizontally,maximizedVertically,fullscreen"
119 SequentialAnimation {
120- ScriptAction { script: print("transitioning:", appDelegate.x, appDelegate.y, appDelegate.scale) }
121 PropertyAction { target: appDelegate; property: "visuallyMinimized,z" }
122 ParallelAnimation {
123 UbuntuNumberAnimation { target: appDelegate; properties: "x"; from: -appDelegate.width / 2; duration: priv.animationDuration }
124@@ -1662,12 +1681,15 @@
125 WindowedFullscreenPolicy {
126 id: windowedFullscreenPolicy
127 active: root.mode == "windowed"
128- surface: model.window.surface
129 }
130 StagedFullscreenPolicy {
131 id: stagedFullscreenPolicy
132 active: root.mode == "staged" || root.mode == "stagedWithSideStage"
133 surface: model.window.surface
134+ onStateRequested: {
135+ delayedStateTimer.windowState = requestedState;
136+ delayedStateTimer.restart();
137+ }
138 }
139
140 SpreadDelegateInputArea {
141
142=== modified file 'qml/Stage/StagedFullscreenPolicy.qml'
143--- qml/Stage/StagedFullscreenPolicy.qml 2016-11-30 19:24:02 +0000
144+++ qml/Stage/StagedFullscreenPolicy.qml 2017-02-22 17:48:13 +0000
145@@ -28,11 +28,13 @@
146 QtObject {
147 property bool active: true
148
149+ signal stateRequested(int requestedState)
150+
151 property var surface: null
152 onSurfaceChanged: {
153 if (!active || !surface) return;
154 if (surface.shellChrome === Mir.LowChrome) {
155- surface.requestState(Mir.FullscreenState);
156+ stateRequested(Mir.FullscreenState);
157 }
158 }
159
160@@ -41,15 +43,15 @@
161 onShellChromeChanged: {
162 if (!active || !surface) return;
163 if (surface.shellChrome === Mir.LowChrome) {
164- surface.requestState(Mir.FullscreenState);
165+ stateRequested(Mir.FullscreenState);
166 } else {
167- surface.requestState(Mir.RestoredState);
168+ stateRequested(Mir.RestoredState);
169 }
170 }
171 onStateChanged: {
172- if (!active) return;
173+ if (!active || !surface) return;
174 if (surface.state === Mir.RestoredState && surface.shellChrome === Mir.LowChrome) {
175- surface.requestState(Mir.FullscreenState);
176+ stateRequested(Mir.FullscreenState);
177 }
178 }
179 }
180
181=== modified file 'qml/Stage/WindowStateSaver.qml'
182--- qml/Stage/WindowStateSaver.qml 2016-11-23 18:43:48 +0000
183+++ qml/Stage/WindowStateSaver.qml 2017-02-22 17:48:13 +0000
184@@ -42,13 +42,14 @@
185 target.windowedY = Qt.binding(function() { return Math.max(Math.min(windowGeometry.y, screenHeight - target.windowedHeight), minimumY); });
186
187 var windowState = WindowStateStorage.getState(target.appId, WindowStateStorage.WindowStateNormal)
188- target.restore(false /* animated */, windowState);
189
190 target.updateNormalGeometry();
191
192 // initialize the x/y to restore to
193 target.restoredX = target.normalX;
194 target.restoredY = target.normalY;
195+
196+ return windowState;
197 }
198
199 function save() {
200
201=== modified file 'qml/Stage/WindowedFullscreenPolicy.qml'
202--- qml/Stage/WindowedFullscreenPolicy.qml 2016-11-30 19:24:02 +0000
203+++ qml/Stage/WindowedFullscreenPolicy.qml 2017-02-22 17:48:13 +0000
204@@ -23,17 +23,11 @@
205 // state of the window is returned to restored.
206 QtObject {
207 property bool active: true
208- property QtObject surface: null
209-
210- property bool _firstTimeSurface: true
211-
212- onSurfaceChanged: {
213- if (!active || !surface) return;
214- if (!_firstTimeSurface) return;
215- _firstTimeSurface = false;
216-
217- if (surface.state === Mir.FullscreenState && surface.shellChrome === Mir.LowChrome) {
218- surface.requestState(Mir.RestoredState);
219+
220+ function applyPolicy(surfaceState, surfaceChrome) {
221+ if (surfaceState === Mir.FullscreenState && surfaceChrome === Mir.LowChrome) {
222+ return Mir.RestoredState;
223 }
224+ return surfaceState;
225 }
226 }
227
228=== modified file 'tests/qmltests/tst_Shell.qml'
229--- tests/qmltests/tst_Shell.qml 2017-02-16 13:46:11 +0000
230+++ tests/qmltests/tst_Shell.qml 2017-02-22 17:48:13 +0000
231@@ -334,7 +334,7 @@
232
233 Row {
234 CheckBox {
235- id: fullscreeAppCheck
236+ id: fullscreenAppCheck
237 activeFocusOnPress: false
238 activeFocusOnTab: false
239
240@@ -348,13 +348,10 @@
241 }
242
243 Binding {
244- target: fullscreeAppCheck
245+ target: fullscreenAppCheck
246 when: topLevelSurfaceList && topLevelSurfaceList.focusedWindow
247 property: "checked"
248- value: {
249- if (!topLevelSurfaceList || !topLevelSurfaceList.focusedWindow) return false;
250- return topLevelSurfaceList.focusedWindow.state === Mir.FullscreenState
251- }
252+ value: topLevelSurfaceList.focusedWindow.state === Mir.FullscreenState
253 }
254 }
255 Label {
256@@ -365,6 +362,8 @@
257 Row {
258 CheckBox {
259 id: chromeAppCheck
260+ activeFocusOnPress: false
261+ activeFocusOnTab: false
262
263 onTriggered: {
264 if (!topLevelSurfaceList.focusedWindow || !topLevelSurfaceList.focusedWindow.surface) return;
265@@ -380,10 +379,7 @@
266 target: chromeAppCheck
267 when: topLevelSurfaceList && topLevelSurfaceList.focusedWindow !== null && topLevelSurfaceList.focusedWindow.surface !== null
268 property: "checked"
269- value: {
270- if (!topLevelSurfaceList || !topLevelSurfaceList.focusedWindow || !topLevelSurfaceList.focusedWindow.surface) return false;
271- topLevelSurfaceList.focusedWindow.surface.shellChrome === Mir.LowChrome
272- }
273+ value: topLevelSurfaceList.focusedWindow.surface.shellChrome === Mir.LowChrome
274 }
275 }
276 Label {

Subscribers

People subscribed via source and target branches