Merge into trunk : shell_chrome : Code : Unity 8

Reviewer	Review Type	Date Requested	Status
Daniel d'Andrada (community)			Needs Information on 2016-03-17
Gerry Boland (community)			Approve on 2016-03-16
Unity8 CI Bot	continuous-integration	2016-03-11	Needs Fixing on 2016-03-15
PS Jenkins bot	continuous-integration	2016-03-11	Pending
Michał Sawicz			Pending
Review via email: mp+288841@code.launchpad.net

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-02-17: Posted in a previous version of this proposal

#

FAILED: Continuous integration, rev:2207
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/403/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/543/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/566
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/585
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/585
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/581/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/581/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/581/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/581/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/581/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/581/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-02-17: Posted in a previous version of this proposal

#

FAILED: Continuous integration, rev:2208
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/406/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/549/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/572
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/590
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/590
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/586/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/586/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/586/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/586/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/586/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/586/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-17: Posted in a previous version of this proposal

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-02-17: Posted in a previous version of this proposal

#

FAILED: Continuous integration, rev:2209
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/414/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/560/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/583
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/601
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/601
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/597/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/597/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/597/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/597/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/597/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/597/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-17: Posted in a previous version of this proposal

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-17: Posted in a previous version of this proposal

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Michał Sawicz (saviq) wrote on 2016-02-18: Posted in a previous version of this proposal

#

Staged mode, app fullscreen, unsets flag → remains fullscreen

When app is fullscreen and unsets flag, we should restore it.

review: Needs Fixing

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-02-18: Posted in a previous version of this proposal

#

FAILED: Continuous integration, rev:2210
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/419/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/568/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/591
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/609
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/609
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/605/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/605/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/605/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/605/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/605/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/605/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-18: Posted in a previous version of this proposal

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-02-18: Posted in a previous version of this proposal

#

FAILED: Continuous integration, rev:2211
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/423/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/572/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/595
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/613
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/613
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/609/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/609/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/609/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/609/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/609/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/609/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-18: Posted in a previous version of this proposal

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Gerry Boland (gerboland) wrote on 2016-03-10: Posted in a previous version of this proposal

#

+++ qml/Shell.qml
+ property string qmlComponent: {
could be readonly.

+++ qml/Stages/AbstractStage.qml
+ signal stageUnloaded
Why can you not depend on the already-existing Component.onDestruction signal? Is it happening too late?

Also when this is fired, the stage as not yet been unloaded. stageAboutToBeUnloaded would be a more accurate name :)

So I see you use it here:
+ property bool saveStateOnDestruction: true
+ Connections {
+ target: root
+ onStageUnloaded: {
+ resizeArea.saveWindowState();
+ resizeArea.saveStateOnDestruction = false;
+ fullscreenPolicy.active = false;
+ }
+ }
+ Component.onDestruction: {
+ if (saveStateOnDestruction) {
+ saveWindowState();
+ }
+ }
this is strange code. It looks like you are waiting for 2 events to save the windowState, and save on the first event. This is really strange looking, you need to justify it, and why Component.onDestruction is not enough. I do know that object destruction order is not guaranteed, is that it? If so, then why use it at all?

+++ qml/Stages/DesktopFullscreenPolicy.qml
this doesn't need to be an item, it doesn't contain anything visual. This can just be a single QtObject. You might be able to import QtQml instead of QtQuick too. lastSurface can be readonly too. Indent of the 'if" statement not clear.

I don't understand why it doesn't apply on the first surface. Could you also write a comment to explain the policy?

+++ qml/Stages/PhoneFullscreenPolicy.qml
I also suspect this could be a QtObject (am not sure if Connections will work inside QtObject though). It is odd to see "PhoneFullscreenPolicy" mentioned in TabletStage - would there be a better name? I'd also like a textual comment explaining what the policy is doing.

Why does this not have the first surface branch the Desktop policy has?

=== modified file 'qml/Stages/DesktopStage.qml'
- property alias requestedWidth: decoratedWindow.requestedWidth
- property alias requestedHeight: decoratedWindow.requestedHeight
+ property int requestedWidth: -1
+ property int requestedHeight: -1
why? You overriding the requested width/height? Or just because you animate them?

+++ tests/qmltests/tst_Shell.qml
just to confirm you mean to return the bool result of the comparison:
+ controls.focusedSurface.state === Mir.FullscreenState
Do we all know that JS returns the result of the last statement, if nothing is explicitly returned?

+++ qml/Shell.qml
+ property string qmlComponent: {
could be readonly.

+++ qml/Stages/AbstractStage.qml
+ signal stageUnloaded
Why can you not depend on the already-existing Component.onDestruction signal? Is it happening too late?

Also when this is fired, the stage as not yet been unloaded. stageAboutToBeUnloaded would be a more accurate name :)

So I see you use it here:
+ property bool saveStateOnDestruction: true
+ Connections {
+     target: root
+     onStageUnloaded: {
+          resizeArea.saveWindowState();
+          resizeArea.saveStateOnDestruction = false;
+          fullscreenPolicy.active = false;
+     }
+ }
+ Component.onDestruction: {
+     if (saveStateOnDestruction) {
+          saveWindowState();
+     }
+ }
this is strange code. It looks like you are waiting for 2 events to save the windowState, and save on the first event. This is really strange looking, you need to justify it, and why Component.onDestruction is not enough. I do know that object destruction order is not guaranteed, is that it? If so, then why use it at all?

+++ qml/Stages/DesktopFullscreenPolicy.qml
this doesn't need to be an item, it doesn't contain anything visual. This can just be a single QtObject. You might be able to import QtQml instead of QtQuick too. lastSurface can be readonly too. Indent of the 'if" statement not clear.

I don't understand why it doesn't apply on the first surface. Could you also write a comment to explain the policy?

+++ qml/Stages/PhoneFullscreenPolicy.qml
I also suspect this could be a QtObject (am not sure if Connections will work inside QtObject though). It is odd to see "PhoneFullscreenPolicy" mentioned in TabletStage - would there be a better name? I'd also like a textual comment explaining what the policy is doing.

Why does this not have the first surface branch the Desktop policy has?

=== modified file 'qml/Stages/DesktopStage.qml'
-                property alias requestedWidth: decoratedWindow.requestedWidth
-                property alias requestedHeight: decoratedWindow.requestedHeight
+                property int requestedWidth: -1
+ property int requestedHeight: -1
why? You overriding the requested width/height? Or just because you animate them?

+++ tests/qmltests/tst_Shell.qml
just to confirm you mean to return the bool result of the comparison:
+ controls.focusedSurface.state === Mir.FullscreenState
Do we all know that JS returns the result of the last statement, if nothing is explicitly returned?

Revision history for this message

Gerry Boland (gerboland) wrote on 2016-03-10: Posted in a previous version of this proposal

#

I still need to test on devices

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-11:

#

FAILED: Continuous integration, rev:2211
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/694/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/913
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/929
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/929
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/927/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/927/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/927/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/927/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/927/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/927/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Gerry Boland (gerboland) wrote on 2016-03-15:

#

Just did a functionality test, works as I'd expect.

lp:~unity-team/unity8/shell_chrome updated on 2016-03-15

2212. By Nick Dedekind on 2016-03-15: review fixes

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-15:

#

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

review: Needs Fixing (continuous-integration)

lp:~unity-team/unity8/shell_chrome updated on 2016-03-15

2213. By Nick Dedekind on 2016-03-15: comment

Revision history for this message

Nick Dedekind (nick-dedekind) wrote on 2016-03-15:

#

Download full text (3.6 KiB)

>> +++ qml/Shell.qml
>> + property string qmlComponent: {
>> could be readonly.

Fixed.

>> +++ qml/Stages/AbstractStage.qml
>> + signal stageUnloaded
>> Why can you not depend on the already-existing Component.onDestruction signal? Is it happening >> too late?

onDestruction is called after the stage which replaces the destroyed stage is created. We want to perform operations before it gets replaced, or states can be changed before we save.

>> Also when this is fired, the stage as not yet been unloaded. stageAboutToBeUnloaded would be a >> more accurate name :)

Done.

>> So I see you use it here:
>> + property bool saveStateOnDestruction: true
>> + Connections {
>> + target: root
>> + onStageUnloaded: {
>> + resizeArea.saveWindowState();
>> + resizeArea.saveStateOnDestruction = false;
>> + fullscreenPolicy.active = false;
>> + }
>> + }
>> + Component.onDestruction: {
>> + if (saveStateOnDestruction) {
>> + saveWindowState();
>> + }
>> + }

>> this is strange code. It looks like you are waiting for 2 events to save the windowState, and >> save on the first event. This is really strange looking, you need to justify it, and why
>> Component.onDestruction is not enough. I do know that object destruction order is not
>> guaranteed, is that it? If so, then why use it at all?

Component.onDestruction handles the normal closing of app.
The signal handles stage replacement. And when the stage is replaced, both will be called so we guard against the latter.

>> +++ qml/Stages/DesktopFullscreenPolicy.qml
>> this doesn't need to be an item, it doesn't contain anything visual. This can just be a single
>> QtObject. You might be able to import QtQml instead of QtQuick too. lastSurface can be
>> readonly too. Indent of the 'if" statement not clear.

Done.

>> I don't understand why it doesn't apply on the first surface. Could you also write a comment
>> to explain the policy?

At the moment, the "last surface" is the one that's displayed. This will apply to all surfaces once we have surface based managament.

>> +++ qml/Stages/PhoneFullscreenPolicy.qml
>> I also suspect this could be a QtObject (am not sure if Connections will work inside QtObject
>> though). It is odd to see "PhoneFullscreenPolicy" mentioned in TabletStage - would there be a
>> better name? I'd also like a textual comment explaining what the policy is doing.

Well I would have liked to call it Staged/WindowedFullscreenPolicy. but Stage kinda means something different in gsettings & u8 :/
But I've renamed them anyway. Because that's how I roll.

>> Why does this not have the first surface branch the Desktop policy has?

The desktop policy is only applied when the surface first appears (for a switch to desktop mode).
Phone policy is always applied.

>> === modified file 'qml/Stages/DesktopStage.qml'
>> - property alias requestedWidth: decoratedWindow.requestedWidth
>> - property alias requestedHeight: decoratedWindow.requestedHeight
>> + property int requestedWidth: -1
>> + property int requestedHeight: -1
>> why? You overriding the requested width/height? Or just because you animate them?

It's a fix for sizing the window when switching stages. It previously started off at -1 through th...

>> +++ qml/Shell.qml
>> + property string qmlComponent: {
>> could be readonly.

Fixed.

>> +++ qml/Stages/AbstractStage.qml
>> + signal stageUnloaded
>> Why can you not depend on the already-existing Component.onDestruction signal? Is it happening >> too late?

onDestruction is called after the stage which replaces the destroyed stage is created. We want to perform operations before it gets replaced, or states can be changed before we save.

>> Also when this is fired, the stage as not yet been unloaded. stageAboutToBeUnloaded would be a >> more accurate name :)

Done.

>> So I see you use it here:
>> + property bool saveStateOnDestruction: true
>> + Connections {
>> + target: root
>> + onStageUnloaded: {
>> + resizeArea.saveWindowState();
>> + resizeArea.saveStateOnDestruction = false;
>> + fullscreenPolicy.active = false;
>> + }
>> + }
>> + Component.onDestruction: {
>> + if (saveStateOnDestruction) {
>> + saveWindowState();
>> + }
>> + }

>> this is strange code. It looks like you are waiting for 2 events to save the windowState, and >> save on the first event. This is really strange looking, you need to justify it, and why
>> Component.onDestruction is not enough. I do know that object destruction order is not
>> guaranteed, is that it? If so, then why use it at all?

Component.onDestruction handles the normal closing of app.
The signal handles stage replacement. And when the stage is replaced, both will be called so we guard against the latter.

>> +++ qml/Stages/DesktopFullscreenPolicy.qml
>> this doesn't need to be an item, it doesn't contain anything visual. This can just be a single
>> QtObject. You might be able to import QtQml instead of QtQuick too. lastSurface can be
>> readonly too. Indent of the 'if" statement not clear.

Done.

>> I don't understand why it doesn't apply on the first surface. Could you also write a comment
>> to explain the policy?

At the moment, the "last surface" is the one that's displayed. This will apply to all surfaces once we have surface based managament.

>> +++ qml/Stages/PhoneFullscreenPolicy.qml
>> I also suspect this could be a QtObject (am not sure if Connections will work inside QtObject
>> though). It is odd to see "PhoneFullscreenPolicy" mentioned in TabletStage - would there be a
>> better name? I'd also like a textual comment explaining what the policy is doing.

Well I would have liked to call it Staged/WindowedFullscreenPolicy. but Stage kinda means something different in gsettings & u8 :/
But I've renamed them anyway. Because that's how I roll.

>> Why does this not have the first surface branch the Desktop policy has?

The desktop policy is only applied when the surface first appears (for a switch to desktop mode).
Phone policy is always applied.

>> === modified file 'qml/Stages/DesktopStage.qml'
>> - property alias requestedWidth: decoratedWindow.requestedWidth
>> - property alias requestedHeight: decoratedWindow.requestedHeight
>> + property int requestedWidth: -1
>> + property int requestedHeight: -1
>> why? You overriding the requested width/height? Or just because you animate them?

It's a fix for sizing the window when switching stages. It previously started off at -1 through the alias, but there were some binding issues with the animations which was causing the window to appear small.

>> +++ tests/qmltests/tst_Shell.qml
>> just to confirm you mean to return the bool result of the comparison:
>> + controls.focusedSurface.state === Mir.FullscreenState
>> Do we all know that JS returns the result of the last statement, if nothing is explicitly returned?

I think JS actually does do it, but I've made it explicit.

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-15:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-15:

#

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

review: Needs Fixing (continuous-integration)

lp:~unity-team/unity8/shell_chrome updated on 2016-03-15

2214. By Michael Zanetti on 2016-03-15: merge with prereq
2215. By Michael Zanetti on 2016-03-15: fix whitespace

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-15:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Gerry Boland (gerboland) wrote on 2016-03-16:

#

Download full text (3.8 KiB)

> >> +++ qml/Stages/AbstractStage.qml
> >> + signal stageUnloaded
> >> Why can you not depend on the already-existing Component.onDestruction
> signal? Is it happening >> too late?
>
> onDestruction is called after the stage which replaces the destroyed stage is
> created. We want to perform operations before it gets replaced, or states can
> be changed before we save.
>
> >> Also when this is fired, the stage as not yet been unloaded.
> stageAboutToBeUnloaded would be a >> more accurate name :)
>
> Done.

Thanks.

>
> >> So I see you use it here:
> >> + property bool saveStateOnDestruction: true
> >> + Connections {
> >> + target: root
> >> + onStageUnloaded: {
> >> + resizeArea.saveWindowState();
> >> + resizeArea.saveStateOnDestruction = false;
> >> + fullscreenPolicy.active = false;
> >> + }
> >> + }
> >> + Component.onDestruction: {
> >> + if (saveStateOnDestruction) {
> >> + saveWindowState();
> >> + }
> >> + }
>
> >> this is strange code. It looks like you are waiting for 2 events to save
> the windowState, and >> save on the first event. This is really strange
> looking, you need to justify it, and why
> >> Component.onDestruction is not enough. I do know that object destruction
> order is not
> >> guaranteed, is that it? If so, then why use it at all?
>
> Component.onDestruction handles the normal closing of app.
> The signal handles stage replacement. And when the stage is replaced, both
> will be called so we guard against the latter.

I see the reasoning, but I think it sensible in a later refactoring to have a single signal we can rely on, instead of these 2.

>
> >> I don't understand why it doesn't apply on the first surface. Could you
> also write a comment
> >> to explain the policy?
>
> At the moment, the "last surface" is the one that's displayed. This will apply
> to all surfaces once we have surface based managament.

Oh I see.

>
> >> +++ qml/Stages/PhoneFullscreenPolicy.qml
> >> I also suspect this could be a QtObject (am not sure if Connections will
> work inside QtObject
> >> though). It is odd to see "PhoneFullscreenPolicy" mentioned in TabletStage
> - would there be a
> >> better name? I'd also like a textual comment explaining what the policy is
> doing.
>
> Well I would have liked to call it Staged/WindowedFullscreenPolicy. but Stage
> kinda means something different in gsettings & u8 :/
> But I've renamed them anyway. Because that's how I roll.

:)

>
> >> Why does this not have the first surface branch the Desktop policy has?
>
> The desktop policy is only applied when the surface first appears (for a
> switch to desktop mode).
> Phone policy is always applied.

Got you. That statement is nice to have in comments.

> >> === modified file 'qml/Stages/DesktopStage.qml'
> >> - property alias requestedWidth: decoratedWindow.requestedWidth
> >> - property alias requestedHeight: decoratedWindow.requestedHeight
> >> + property int requestedWidth: -1
> >> + property int requestedHeight: -1
> >> why? You overriding the requested width/height? Or just because you animate
> them?
>
> It's a fix for sizing the window when switching stages. It previously started
> off at -1 through the alias, but there were som...

Unity 8

Merge lp:~unity-team/unity8/shell_chrome into lp:unity8

Commit message

Description of the change

Preview Diff

Subscribers

 === modified file 'qml/Components/InputMethod.qml'
 --- qml/Components/InputMethod.qml	2016-02-15 17:26:51 +0000
 +++ qml/Components/InputMethod.qml	2016-03-16 12:31:28 +0000
@@ -51,7 +51,10 @@
+     }
      state: {
--        if (surfaceItem.surface && surfaceItem.surfaceState != Mir.MinimizedState && root.enabled) {
++        if (surfaceItem.surface &&
++              surfaceItem.surfaceState != Mir.HiddenState &&
++              surfaceItem.surfaceState != Mir.MinimizedState &&
++              root.enabled) {
              return "shown";
          } else {
              return "hidden";
 === modified file 'qml/Shell.qml'
 --- qml/Shell.qml	2016-03-16 12:31:27 +0000
 +++ qml/Shell.qml	2016-03-16 12:31:28 +0000
@@ -246,7 +246,7 @@
              property string usageScenario: shell.usageScenario === "phone" || greeter.hasLockedApp
                                             ? "phone"
                                             : shell.usageScenario
--            source: {
++            readonly property string qmlComponent: {
                  if(shell.mode === "greeter") {
                      return "Stages/ShimStage.qml"
                  } else if (applicationsDisplayLoader.usageScenario === "phone") {
@@ -257,6 +257,10 @@
                      return "Stages/DesktopStage.qml";
+                 }
+             }
++            onQmlComponentChanged: {
++                if (item) item.stageAboutToBeUnloaded();
++                source = qmlComponent;
++            }
              property bool interactive: tutorial.spreadEnabled
                      && (!greeter || !greeter.shown)
 === modified file 'qml/Stages/AbstractStage.qml'
 --- qml/Stages/AbstractStage.qml	2016-03-16 12:31:27 +0000
 +++ qml/Stages/AbstractStage.qml	2016-03-16 12:31:28 +0000
@@ -51,6 +51,8 @@
                                        | Qt.InvertedPortraitOrientation
                                        | Qt.InvertedLandscapeOrientation
++    signal stageAboutToBeUnloaded
++
      // Shared code for use in stage implementations
      GSettings {
          id: lifecycleExceptions
 === modified file 'qml/Stages/DesktopStage.qml'
 --- qml/Stages/DesktopStage.qml	2016-03-10 22:37:38 +0000
 +++ qml/Stages/DesktopStage.qml	2016-03-16 12:31:28 +0000
@@ -255,8 +255,8 @@
                  focus: appId === priv.focusedAppId
                  width: decoratedWindow.width
                  height: decoratedWindow.height
--                property alias requestedWidth: decoratedWindow.requestedWidth
--                property alias requestedHeight: decoratedWindow.requestedHeight
++                property int requestedWidth: -1
++                property int requestedHeight: -1
                  property alias minimumWidth: decoratedWindow.minimumWidth
                  property alias minimumHeight: decoratedWindow.minimumHeight
                  property alias maximumWidth: decoratedWindow.maximumWidth
@@ -369,8 +369,10 @@
                          name: "fullscreen"; when: decoratedWindow.fullscreen
                          PropertyChanges {
                              target: appDelegate;
--                            x: 0; y: -PanelState.panelHeight
--                            requestedWidth: appContainer.width; requestedHeight: appContainer.height;
++                            x: 0;
++                            y: -PanelState.panelHeight
++                            requestedWidth: appContainer.width;
++                            requestedHeight: appContainer.height;
+                         }
                      },
                      State {
@@ -387,21 +389,42 @@
                          name: "maximized"; when: appDelegate.maximized && !appDelegate.minimized
                          PropertyChanges {
                              target: appDelegate;
--                            x: root.leftMargin; y: 0;
--                            requestedWidth: appContainer.width - root.leftMargin; requestedHeight: appContainer.height;
++                            x: root.leftMargin;
++                            y: 0;
                              visuallyMinimized: false;
                              visuallyMaximized: true
+                         }
++                        PropertyChanges {
++                            target: decoratedWindow
++                            requestedWidth: appContainer.width - root.leftMargin;
++                            requestedHeight: appContainer.height;
++                        }
                      },
                      State {
                          name: "maximizedLeft"; when: appDelegate.maximizedLeft && !appDelegate.minimized
--                        PropertyChanges { target: appDelegate; x: root.leftMargin; y: PanelState.panelHeight;
--                            requestedWidth: (appContainer.width - root.leftMargin)/2; requestedHeight: appContainer.height - PanelState.panelHeight }
++                        PropertyChanges {
++                            target: appDelegate
++                            x: root.leftMargin
++                            y: PanelState.panelHeight
++                        }
++                        PropertyChanges {
++                            target: decoratedWindow
++                            requestedWidth: (appContainer.width - root.leftMargin)/2
++                            requestedHeight: appContainer.height - PanelState.panelHeight
++                        }
                      },
                      State {
                          name: "maximizedRight"; when: appDelegate.maximizedRight && !appDelegate.minimized
--                        PropertyChanges { target: appDelegate; x: (appContainer.width + root.leftMargin)/2; y: PanelState.panelHeight;
--                            requestedWidth: (appContainer.width - root.leftMargin)/2; requestedHeight: appContainer.height - PanelState.panelHeight }
++                        PropertyChanges {
++                            target: appDelegate;
++                            x: (appContainer.width + root.leftMargin)/2
++                            y: PanelState.panelHeight
++                        }
++                        PropertyChanges {
++                            target: decoratedWindow
++                            requestedWidth: (appContainer.width - root.leftMargin)/2
++                            requestedHeight: appContainer.height - PanelState.panelHeight
++                        }
                      },
                      State {
                          name: "minimized"; when: appDelegate.minimized
@@ -421,13 +444,17 @@
                          enabled: appDelegate.animationsEnabled
                          PropertyAction { target: appDelegate; properties: "visuallyMinimized,visuallyMaximized" }
                          UbuntuNumberAnimation { target: appDelegate; properties: "x,y,opacity,requestedWidth,requestedHeight,scale"; duration: UbuntuAnimation.FastDuration }
++                        UbuntuNumberAnimation { target: decoratedWindow; properties: "requestedWidth,requestedHeight"; duration: UbuntuAnimation.FastDuration }
                      },
                      Transition {
                          to: "minimized"
                          enabled: appDelegate.animationsEnabled
                          PropertyAction { target: appDelegate; property: "visuallyMaximized" }
                          SequentialAnimation {
--                            UbuntuNumberAnimation { target: appDelegate; properties: "x,y,opacity,requestedWidth,requestedHeight,scale"; duration: UbuntuAnimation.FastDuration }
++                            ParallelAnimation {
++                                UbuntuNumberAnimation { target: appDelegate; properties: "x,y,opacity,scale"; duration: UbuntuAnimation.FastDuration }
++                                UbuntuNumberAnimation { target: decoratedWindow; properties: "requestedWidth,requestedHeight"; duration: UbuntuAnimation.FastDuration }
++                            }
                              PropertyAction { target: appDelegate; property: "visuallyMinimized" }
                              ScriptAction {
                                  script: {
@@ -443,7 +470,10 @@
                          enabled: appDelegate.animationsEnabled
                          PropertyAction { target: appDelegate; property: "visuallyMinimized" }
                          SequentialAnimation {
--                            UbuntuNumberAnimation { target: appDelegate; properties: "x,y,opacity,requestedWidth,requestedHeight,scale"; duration: UbuntuAnimation.FastDuration }
++                            ParallelAnimation {
++                                UbuntuNumberAnimation { target: appDelegate; properties: "x,y,opacity,scale"; duration: UbuntuAnimation.FastDuration }
++                                UbuntuNumberAnimation { target: decoratedWindow; properties: "requestedWidth,requestedHeight"; duration: UbuntuAnimation.FastDuration }
++                            }
                              PropertyAction { target: appDelegate; property: "visuallyMaximized" }
+                         }
+                     }
@@ -458,6 +488,7 @@
+                 }
                  WindowResizeArea {
++                    id: resizeArea
                      objectName: "windowResizeArea"
                      target: appDelegate
                      minWidth: units.gu(10)
@@ -469,6 +500,25 @@
                      leftMargin: root.leftMargin
                      onPressed: { ApplicationManager.focusApplication(model.appId) }
++
++                    Component.onCompleted: {
++                        loadWindowState();
++                    }
++
++                    property bool saveStateOnDestruction: true
++                    Connections {
++                        target: root
++                        onStageAboutToBeUnloaded: {
++                            resizeArea.saveWindowState();
++                            resizeArea.saveStateOnDestruction = false;
++                            fullscreenPolicy.active = false;
++                        }
++                    }
++                    Component.onDestruction: {
++                        if (saveStateOnDestruction) {
++                            saveWindowState();
++                        }
++                    }
+                 }
                  DecoratedWindow {
@@ -480,12 +530,21 @@
                      active: ApplicationManager.focusedApplicationId === model.appId
                      focus: true
++                    requestedWidth: appDelegate.requestedWidth
++                    requestedHeight: appDelegate.requestedHeight
++
                      onClose: ApplicationManager.stopApplication(model.appId)
                      onMaximize: appDelegate.maximized || appDelegate.maximizedLeft || appDelegate.maximizedRight
                                  ? appDelegate.restoreFromMaximized() : appDelegate.maximize()
                      onMinimize: appDelegate.minimize()
                      onDecorationPressed: { ApplicationManager.focusApplication(model.appId) }
+                 }
++
++                WindowedFullscreenPolicy {
++                    id: fullscreenPolicy
++                    active: true
++                    application: decoratedWindow.application
++                }
+             }
+         }
+     }
 === modified file 'qml/Stages/PhoneStage.qml'
 --- qml/Stages/PhoneStage.qml	2016-03-10 09:19:38 +0000
 +++ qml/Stages/PhoneStage.qml	2016-03-16 12:31:28 +0000
@@ -629,6 +629,16 @@
                          property: "focusedAppOrientationChangesEnabled"
                          value: orientationChangesEnabled
+                     }
++
++                    StagedFullscreenPolicy {
++                        id: fullscreenPolicy
++                        application: appDelegate.application
++                    }
++
++                    Connections {
++                        target: root
++                        onStageAboutToBeUnloaded: fullscreenPolicy.active = false
++                    }
+                 }
+             }
+         }
 === added file 'qml/Stages/StagedFullscreenPolicy.qml'
 --- qml/Stages/StagedFullscreenPolicy.qml	1970-01-01 00:00:00 +0000
 +++ qml/Stages/StagedFullscreenPolicy.qml	2016-03-16 12:31:28 +0000
@@ -0,0 +1,58 @@
++/*
++ * Copyright (C) 2016 Canonical, Ltd.
++ *
++ * This program is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; version 3.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ */
++
++import QtQuick 2.4
++import Unity.Application 0.1
++
++// This component will change the state of the surface based on the surface
++// state and shell chrome.
++//
++// Chrome changed to LowChrome -> server sets client window state to "fullscreen"
++// Chrome changed to NormalChrome -> server sets client window to "restored" state.
++// Chrome set and state change to restored -> server RESETS client window state to "fullscreen"
++// Chrome not set and state change to restored -> client window stays "restored"
++// Chrome not set and state change to fulscreen -> client window stays "fullscreen"
++QtObject {
++    property bool active: true
++    property QtObject application: null
++
++    readonly property var lastSurface: application && application.session ?
++                              application.session.lastSurface : null
++    onLastSurfaceChanged: {
++        if (!active || !lastSurface) return;
++        if (lastSurface.shellChrome === Mir.LowChrome) {
++            lastSurface.state = Mir.FullscreenState;
++        }
++    }
++
++    property var _connections: Connections {
++        target: lastSurface
++        onShellChromeChanged: {
++            if (!active || !lastSurface) return;
++            if (lastSurface.shellChrome === Mir.LowChrome) {
++                lastSurface.state = Mir.FullscreenState;
++            } else {
++                lastSurface.state = Mir.RestoredState;
++            }
++        }
++        onStateChanged: {
++            if (!active) return;
++            if (lastSurface.state === Mir.RestoredState && lastSurface.shellChrome === Mir.LowChrome) {
++                lastSurface.state = Mir.FullscreenState;
++            }
++        }
++    }
++}
 === modified file 'qml/Stages/TabletStage.qml'
 --- qml/Stages/TabletStage.qml	2016-03-16 12:31:27 +0000
 +++ qml/Stages/TabletStage.qml	2016-03-16 12:31:28 +0000
@@ -947,6 +947,15 @@
                          period: (spreadView.positionMarker2 - spreadView.positionMarker1) / 3
                          progress: spreadTile.progress - spreadView.positionMarker1
+                     }
++
++                    StagedFullscreenPolicy {
++                        id: fullscreenPolicy
++                        application: spreadTile.application
++                    }
++                    Connections {
++                        target: root
++                        onStageAboutToBeUnloaded: fullscreenPolicy.active = false
++                    }
+                 }
+             }
+         }
 === modified file 'qml/Stages/WindowResizeArea.qml'
 --- qml/Stages/WindowResizeArea.qml	2016-03-10 22:43:31 +0000
 +++ qml/Stages/WindowResizeArea.qml	2016-03-16 12:31:28 +0000
@@ -69,7 +69,7 @@
          onHeightChanged: priv.updateNormalGeometry();
+     }
--    Component.onCompleted: {
++    function loadWindowState() {
          var windowGeometry = windowStateStorage.getGeometry(root.windowId,
                                                              Qt.rect(target.x, target.y, defaultWidth, defaultHeight));
@@ -87,7 +87,7 @@
          priv.updateNormalGeometry();
+     }
--    Component.onDestruction: {
++    function saveWindowState() {
          windowStateStorage.saveState(root.windowId, target.state == "maximized" ? WindowStateStorage.WindowStateMaximized : WindowStateStorage.WindowStateNormal)
          windowStateStorage.saveGeometry(root.windowId, Qt.rect(priv.normalX, priv.normalY, priv.normalWidth, priv.normalHeight))
+     }
 === added file 'qml/Stages/WindowedFullscreenPolicy.qml'
 --- qml/Stages/WindowedFullscreenPolicy.qml	1970-01-01 00:00:00 +0000
 +++ qml/Stages/WindowedFullscreenPolicy.qml	2016-03-16 12:31:28 +0000
@@ -0,0 +1,41 @@
++/*
++ * Copyright (C) 2016 Canonical, Ltd.
++ *
++ * This program is free software; you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License as published by
++ * the Free Software Foundation; version 3.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ */
++
++import QtQml 2.2
++import Unity.Application 0.1
++
++// This component will change the state of the surface when the stage is loaded.
++//
++// On first surface load; if the surface is set to low chrome & fullscreen, the
++// state of the window is returned to restored.
++QtObject {
++    property bool active: true
++    property QtObject application: null
++
++    readonly property var lastSurface: application && application.session ?
++                                      application.session.lastSurface : null
++    property bool _firstTimeSurface: true
++
++    onLastSurfaceChanged: {
++        if (!active || !lastSurface) return;
++        if (!_firstTimeSurface) return;
++        _firstTimeSurface = false;
++
++        if (lastSurface.state === Mir.FullscreenState && lastSurface.shellChrome === Mir.LowChrome) {
++            lastSurface.state = Mir.RestoredState;
++        }
++    }
++}
 === modified file 'tests/mocks/Unity/Application/ApplicationInfo.cpp'
 --- tests/mocks/Unity/Application/ApplicationInfo.cpp	2016-01-22 19:44:56 +0000
 +++ tests/mocks/Unity/Application/ApplicationInfo.cpp	2016-03-16 12:31:28 +0000
@@ -44,6 +44,7 @@
      , m_isTouchApp(true)
      , m_exemptFromLifecycle(false)
      , m_manualSurfaceCreation(false)
++    , m_shellChrome(Mir::NormalChrome)
+ {
+ }
@@ -63,6 +64,7 @@
      , m_isTouchApp(true)
      , m_exemptFromLifecycle(false)
      , m_manualSurfaceCreation(false)
++    , m_shellChrome(Mir::NormalChrome)
+ {
+ }
@@ -102,9 +104,11 @@
      if (m_session) {
          m_session->setApplication(this);
          m_session->setParent(this);
++        m_session->setFullscreen(m_fullscreen);
          SessionManager::singleton()->registerSession(m_session);
          connect(m_session, &Session::surfaceAdded,
                  this, &ApplicationInfo::onSessionSurfaceAdded);
++        connect(m_session, &Session::fullscreenChanged, this, &ApplicationInfo::fullscreenChanged);
          if (!m_manualSurfaceCreation) {
              QTimer::singleShot(500, m_session, &Session::createSurface);
@@ -191,12 +195,17 @@
  void ApplicationInfo::setFullscreen(bool value)
+ {
--    if (value != m_fullscreen) {
--        m_fullscreen = value;
--        Q_EMIT fullscreenChanged(value);
++    m_fullscreen = value;
++    if (m_session) {
++        m_session->setFullscreen(value);
+     }
+ }
++bool ApplicationInfo::fullscreen() const
++{
++    return m_session ? m_session->fullscreen() : m_fullscreen;
++}
++
  void ApplicationInfo::setManualSurfaceCreation(bool value)
+ {
      if (value != m_manualSurfaceCreation) {
@@ -262,6 +271,7 @@
          } else {
              setState(Suspended);
+         }
++        surface->setShellChrome(m_shellChrome);
+     }
+ }
@@ -291,3 +301,11 @@
          Q_EMIT initialSurfaceSizeChanged(m_initialSurfaceSize);
+     }
+ }
++
++void ApplicationInfo::setShellChrome(Mir::ShellChrome shellChrome)
++{
++    m_shellChrome = shellChrome;
++    if (m_session && m_session->lastSurface()) {
++        m_session->lastSurface()->setShellChrome(shellChrome);
++    }
++}
 === modified file 'tests/mocks/Unity/Application/ApplicationInfo.h'
 --- tests/mocks/Unity/Application/ApplicationInfo.h	2016-01-19 21:41:34 +0000
 +++ tests/mocks/Unity/Application/ApplicationInfo.h	2016-03-16 12:31:28 +0000
@@ -24,6 +24,7 @@
  // unity-api
  #include <unity/shell/application/ApplicationInfoInterface.h>
++#include <unity/shell/application/Mir.h>
  using namespace unity::shell::application;
@@ -78,7 +79,7 @@
      QString screenshot() const { return m_screenshotFileName; }
      void setFullscreen(bool value);
--    bool fullscreen() const { return m_fullscreen; }
++    bool fullscreen() const;
      Qt::ScreenOrientations supportedOrientations() const override;
      void setSupportedOrientations(Qt::ScreenOrientations orientations);
@@ -97,6 +98,8 @@
      QSize initialSurfaceSize() const override;
      void setInitialSurfaceSize(const QSize &size) override;
++
++    Q_INVOKABLE void setShellChrome(Mir::ShellChrome shellChrome);
  public:
      void setSession(Session* session);
      Session* session() const { return m_session; }
@@ -134,6 +137,7 @@
      QSize m_initialSurfaceSize;
      bool m_manualSurfaceCreation;
++    Mir::ShellChrome m_shellChrome;
  };
  Q_DECLARE_METATYPE(ApplicationInfo*)
 === modified file 'tests/mocks/Unity/Application/ApplicationManager.cpp'
 --- tests/mocks/Unity/Application/ApplicationManager.cpp	2016-03-16 12:31:27 +0000
 +++ tests/mocks/Unity/Application/ApplicationManager.cpp	2016-03-16 12:31:28 +0000
@@ -328,6 +328,7 @@
      application->setName("Camera");
      application->setScreenshotId("camera");
      application->setIconId("camera");
++    application->setShellChrome(Mir::LowChrome);
      application->setFullscreen(true);
      application->setSupportedOrientations(Qt::PortraitOrientation
                                          | Qt::LandscapeOrientation
@@ -341,7 +342,8 @@
      application->setName("Gallery");
      application->setScreenshotId("gallery");
      application->setIconId("gallery");
--    application->setFullscreen(true);
++    application->setShellChrome(Mir::LowChrome);
++    application->setStage(ApplicationInfo::MainStage);
      m_availableApplications.append(application);
      application = new ApplicationInfo(this);
@@ -353,7 +355,7 @@
      application = new ApplicationInfo(this);
      application->setAppId("webbrowser-app");
--    application->setFullscreen(true);
++    application->setShellChrome(Mir::LowChrome);
      application->setName("Browser");
      application->setScreenshotId("browser");
      application->setIconId("browser");
@@ -378,7 +380,7 @@
      application->setName("GMail");
      application->setIconId("gmail");
      application->setScreenshotId("gmail-webapp.svg");
--    application->setFullscreen(false);
++    application->setStage(ApplicationInfo::MainStage);
      application->setSupportedOrientations(Qt::PortraitOrientation
                                          | Qt::LandscapeOrientation
                                          | Qt::InvertedPortraitOrientation
@@ -390,7 +392,7 @@
      application->setName("Music");
      application->setIconId("soundcloud");
      application->setScreenshotId("music");
--    application->setFullscreen(false);
++    application->setStage(ApplicationInfo::MainStage);
      application->setSupportedOrientations(Qt::PortraitOrientation
                                          | Qt::LandscapeOrientation
                                          | Qt::InvertedPortraitOrientation
 === modified file 'tests/mocks/Unity/Application/MirSurface.cpp'
 --- tests/mocks/Unity/Application/MirSurface.cpp	2016-03-10 22:39:10 +0000
 +++ tests/mocks/Unity/Application/MirSurface.cpp	2016-03-16 12:31:28 +0000
@@ -38,6 +38,7 @@
      , m_width(-1)
      , m_height(-1)
      , m_slowToResize(false)
++    , m_shellChrome(Mir::NormalChrome)
+ {
  //    qDebug() << "MirSurface::MirSurface() " << name;
      QQmlEngine::setObjectOwnership(this, QQmlEngine::CppOwnership);
@@ -134,6 +135,21 @@
+ }
++Mir::ShellChrome MirSurface::shellChrome() const
++{
++    return m_shellChrome;
++}
++
++void MirSurface::setShellChrome(Mir::ShellChrome shellChrome)
++{
++    if (shellChrome == m_shellChrome)
++        return;
++
++    m_shellChrome = shellChrome;
++    Q_EMIT shellChromeChanged(shellChrome);
++}
++
++
  void MirSurface::registerView(qintptr viewId)
+ {
 === modified file 'tests/mocks/Unity/Application/MirSurface.h'
 --- tests/mocks/Unity/Application/MirSurface.h	2016-03-01 12:56:35 +0000
 +++ tests/mocks/Unity/Application/MirSurface.h	2016-03-16 12:31:28 +0000
@@ -74,10 +74,13 @@
      int widthIncrement() const override { return m_widthIncrement; }
      int heightIncrement() const override { return m_heightIncrement; }
++    Mir::ShellChrome shellChrome() const override;
++
      ////
      // API for tests
      Q_INVOKABLE void setLive(bool live);
++    Q_INVOKABLE void setShellChrome(Mir::ShellChrome shellChrome);
      void registerView(qintptr viewId);
      void unregisterView(qintptr viewId);
@@ -109,9 +112,6 @@
      void setActiveFocus(bool);
  Q_SIGNALS:
--    void stateChanged(Mir::State);
--    void liveChanged(bool live);
--    void orientationAngleChanged(Mir::OrientationAngle angle);
      void widthChanged();
      void heightChanged();
      void slowToResizeChanged();
@@ -152,6 +152,8 @@
      QSize m_delayedResize;
      QSize m_pendingResize;
++    Mir::ShellChrome m_shellChrome;
++
      struct View {
          bool visible;
      };
 === modified file 'tests/mocks/Unity/Application/MirSurfaceItem.cpp'
 --- tests/mocks/Unity/Application/MirSurfaceItem.cpp	2016-03-16 12:31:27 +0000
 +++ tests/mocks/Unity/Application/MirSurfaceItem.cpp	2016-03-16 12:31:28 +0000
@@ -100,6 +100,15 @@
+     }
+ }
++Mir::ShellChrome MirSurfaceItem::shellChrome() const
++{
++    if (m_qmlSurface) {
++        return m_qmlSurface->shellChrome();
++    } else {
++        return Mir::NormalChrome;
++    }
++}
++
  Mir::OrientationAngle MirSurfaceItem::orientationAngle() const
+ {
      if (m_qmlSurface) {
 === modified file 'tests/mocks/Unity/Application/MirSurfaceItem.h'
 --- tests/mocks/Unity/Application/MirSurfaceItem.h	2016-03-16 12:31:27 +0000
 +++ tests/mocks/Unity/Application/MirSurfaceItem.h	2016-03-16 12:31:28 +0000
@@ -47,6 +47,7 @@
      Mir::Type type() const override;
      QString name() const override;
      bool live() const override;
++    Mir::ShellChrome shellChrome() const override;
      Mir::State surfaceState() const override;
      void setSurfaceState(Mir::State) override {}
 === modified file 'tests/mocks/Unity/Application/Session.cpp'
 --- tests/mocks/Unity/Application/Session.cpp	2015-12-01 12:17:24 +0000
 +++ tests/mocks/Unity/Application/Session.cpp	2016-03-16 12:31:28 +0000
@@ -34,6 +34,7 @@
      , m_surface(nullptr)
      , m_parentSession(nullptr)
      , m_children(new SessionModel(this))
++    , m_fullscreen(false)
+ {
  //    qDebug() << "Session::Session() " << this->name();
@@ -71,6 +72,25 @@
      deleteLater();
+ }
++void Session::updateFullscreenProperty()
++{
++    if (m_surfaces.rowCount() > 0) {
++        // TODO: Figure out something better
++        setFullscreen(lastSurface()->state() == Mir::FullscreenState);
++    } else {
++        // Keep the current value of the fullscreen property until we get a new
++        // surface
++    }
++}
++
++void Session::setFullscreen(bool fullscreen)
++{
++    if (m_fullscreen != fullscreen) {
++        m_fullscreen = fullscreen;
++        Q_EMIT fullscreenChanged(m_fullscreen);
++    }
++}
++
  void Session::setApplication(ApplicationInfo* application)
+ {
      if (m_application == application)
@@ -82,15 +102,18 @@
  void Session::appendSurface(MirSurface* surface)
+ {
--    // qDebug() << "Session::appendSurface - session=" << name() << "surface=" << surface;
++     qDebug() << "Session::appendSurface - session=" << name() << "surface=" << surface;
      m_surfaces.insert(m_surfaces.rowCount(), surface);
++    connect(surface, &MirSurfaceInterface::stateChanged, this, &Session::updateFullscreenProperty);
      connect(surface, &QObject::destroyed,
              this, [this, surface]() { this->removeSurface(surface); });
      Q_EMIT lastSurfaceChanged(surface);
      Q_EMIT surfaceAdded(surface);
++
++    updateFullscreenProperty();
+ }
  void Session::removeSurface(MirSurface* surface)
@@ -99,6 +122,8 @@
      if (m_surfaces.contains(surface)) {
          m_surfaces.remove(surface);
+     }
++
++    updateFullscreenProperty();
+ }
  void Session::setScreenshot(const QUrl& screenshot)
 === modified file 'tests/mocks/Unity/Application/Session.h'
 --- tests/mocks/Unity/Application/Session.h	2015-12-01 12:17:24 +0000
 +++ tests/mocks/Unity/Application/Session.h	2016-03-16 12:31:28 +0000
@@ -47,6 +47,7 @@
      //getters
      QString name() const { return m_name; }
      bool live() const { return m_live; }
++    bool fullscreen() const { return m_fullscreen; }
      ApplicationInfo* application() const { return m_application; }
      MirSurface *lastSurface() const;
      ObjectListModel<MirSurface>* surfaces() const;
@@ -57,6 +58,7 @@
      void removeSurface(MirSurface* surface);
      void setScreenshot(const QUrl& m_screenshot);
      void setLive(bool live);
++    void setFullscreen(bool fullscreen);
      Q_INVOKABLE void addChildSession(Session* session);
      void insertChildSession(uint index, Session* session);
@@ -68,6 +70,7 @@
      void liveChanged(bool live);
      void surfaceAdded(MirSurface *surface);
      void lastSurfaceChanged(MirSurface *surface);
++    void fullscreenChanged(bool fullscreen);
      // internal mock use
      void deregister();
@@ -75,6 +78,9 @@
  public Q_SLOTS:
      Q_INVOKABLE void createSurface();
++private Q_SLOTS:
++    void updateFullscreenProperty();
++
  private:
      SessionModel* childSessions() const;
      void setParentSession(Session* session);
@@ -87,6 +93,7 @@
      Session* m_parentSession;
      SessionModel* m_children;
      ObjectListModel<MirSurface> m_surfaces;
++    bool m_fullscreen;
      friend class ApplicationTestInterface;
  };
 === modified file 'tests/qmltests/Stages/tst_WindowResizeArea.qml'
 --- tests/qmltests/Stages/tst_WindowResizeArea.qml	2015-11-30 18:25:47 +0000
 +++ tests/qmltests/Stages/tst_WindowResizeArea.qml	2016-03-16 12:31:28 +0000
@@ -72,6 +72,13 @@
                  windowId: "test-window-id"
                  screenWidth: root.width
                  screenHeight: root.height
++
++                Component.onCompleted: {
++                    loadWindowState();
++                }
++                Component.onDestruction: {
++                    saveWindowState();
++                }
+             }
              Rectangle {
 === modified file 'tests/qmltests/tst_Shell.qml'
 --- tests/qmltests/tst_Shell.qml	2016-03-10 22:43:31 +0000
 +++ tests/qmltests/tst_Shell.qml	2016-03-16 12:31:28 +0000
@@ -131,6 +131,9 @@
          anchors.right: root.right
          width: units.gu(30)
++        property var focusedApp: ApplicationManager.findApplication(ApplicationManager.focusedApplicationId)
++        property var focusedSurface: focusedApp && focusedApp.session ? focusedApp.session.lastSurface : null
++
          Rectangle {
              id: controlRect
              anchors { left: parent.left; right: parent.right }
@@ -251,6 +254,65 @@
                          appId: modelData
+                     }
+                 }
++
++                Label { text: "Focused Application"; font.bold: true }
++
++                Row {
++                    CheckBox {
++                        id: fullscreeAppCheck
++
++                        onTriggered: {
++                            if (!controls.focusedSurface) return;
++                            if (controls.focusedSurface.state == Mir.FullscreenState) {
++                                controls.focusedSurface.state = Mir.RestoredState;
++                            } else {
++                                controls.focusedSurface.state = Mir.FullscreenState;
++                            }
++                        }
++
++                        Binding {
++                            target: fullscreeAppCheck
++                            when: controls.focusedSurface
++                            property: "checked"
++                            value: {
++                                if (!controls.focusedSurface) return false;
++                                return controls.focusedSurface.state === Mir.FullscreenState
++                            }
++                        }
++                    }
++                    Label {
++                        text: "Fullscreen"
++                    }
++                }
++
++                Row {
++                    CheckBox {
++                        id: chromeAppCheck
++
++                        onTriggered: {
++                            if (!controls.focusedSurface) return;
++                            if (controls.focusedSurface.shellChrome == Mir.LowChrome) {
++                                controls.focusedSurface.setShellChrome(Mir.NormalChrome);
++                            } else {
++                                controls.focusedSurface.setShellChrome(Mir.LowChrome);
++                            }
++                        }
++
++                        Binding {
++                            target: chromeAppCheck
++                            when: controls.focusedSurface !== null
++                            property: "checked"
++                            value: {
++                                if (!controls.focusedSurface) return false;
++                                controls.focusedSurface.shellChrome === Mir.LowChrome
++                            }
++                        }
++                    }
++                    Label {
++                        text: "Low Chrome"
++                    }
++                }
++
+             }
+         }
+     }