Unity 8

Merge lp:~mterry/unity8/fix-wakelocks into lp:unity8

  1. fix-wakelocks
  2. Merge into trunk
Proposed by Michael Terry on 2015-12-03
Status: Superseded
Proposed branch: lp:~mterry/unity8/fix-wakelocks
Merge into: lp:unity8
Prerequisite: lp:~dandrader/unity8/surfaceItemFillMode
Diff against target: 216 lines (+61/-20)
8 files modified
qml/Stages/PhoneStage.qml (+6/-4)
qml/Stages/TabletStage.qml (+6/-4)
tests/mocks/Unity/Application/ApplicationInfo.cpp (+16/-0)
tests/mocks/Unity/Application/ApplicationInfo.h (+4/-0)
tests/mocks/Unity/Application/ApplicationManager.cpp (+2/-0)
tests/mocks/Unity/Application/ApplicationManager.h (+1/-1)
tests/plugins/Unity/Launcher/launchermodeltest.cpp (+2/-0)
tests/qmltests/tst_Shell.qml (+24/-11)
To merge this branch: bzr merge lp:~mterry/unity8/fix-wakelocks
Reviewer Review Type Date Requested Status
Daniel d'Andrada (community) 2015-12-03 Approve on 2015-12-04
PS Jenkins bot continuous-integration 2015-12-03 Needs Fixing on 2015-12-03
Review via email: mp+279502@code.launchpad.net

This proposal supersedes a proposal from 2015-12-03.

This proposal has been superseded by a proposal from 2015-12-07.

Commit Message

Let qtmir know which apps are exempt from the lifecycle management.
This way, it can manage its own wakelocks better (and stop preventing the system from deep sleeping).

Description of the Change

Let qtmir know which apps are exempt from the lifecycle management, so it can manage its own wakelocks better (and stop preventing the system from deep sleeping).

 * Are there any related MPs required for this MP to build/function as expected? Please list.
 https://code.launchpad.net/~mterry/unity-api/fix-wakelocks/+merge/279476
 https://code.launchpad.net/~mterry/qtmir/fix-wakelocks/+merge/279481

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

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

 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
 I'm on that team

 * If you changed the UI, has there been a design review?
 NA

To post a comment you must log in.
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:2078
http://jenkins.qa.ubuntu.com/job/unity8-ci/6880/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/5590/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-xenial-touch/295/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/1591/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-qmluitest-xenial-amd64/294/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/1486/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/1486/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-xenial-amd64-ci/293/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-xenial-i386-ci/292/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5604/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/294/console

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

review: Needs Fixing (continuous-integration)
Daniel d'Andrada (dandrader) wrote :

In test_lifecyclePolicyExemption:

I don't see the point in changing app2 from dialer-app to gallery-app

review: Needs Fixing
Daniel d'Andrada (dandrader) wrote :

Likewise in test_lifecyclePolicyForNonTouchApp

Michael Terry (mterry) wrote :

> I don't see the point in changing app2 from dialer-app to gallery-app

I realized when making this change that the test was flawed. dialer-app is a sidestage app, so in Tablet mode, it wouldn't actually trigger a Suspend for the main stage app anyway! :(

So I switched to two main stage apps.

PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:2079
http://jenkins.qa.ubuntu.com/job/unity8-ci/6881/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/5592/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-xenial-touch/296/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/1592/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-qmluitest-xenial-amd64/295/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/1487/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/1487/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-xenial-amd64-ci/294/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-xenial-i386-ci/293/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5606/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/295/console

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

review: Needs Fixing (continuous-integration)
Daniel d'Andrada (dandrader) wrote :

On 03/12/2015 18:17, Michael Terry wrote:
>> I don't see the point in changing app2 from dialer-app to gallery-app
> I realized when making this change that the test was flawed. dialer-app is a sidestage app, so in Tablet mode, it wouldn't actually trigger a Suspend for the main stage app anyway! :(
>
> So I switched to two main stage apps.

So worth adding a check in the test that ensures both apps are in the
main stage

Michael Terry (mterry) wrote :

> So worth adding a check in the test that ensures both apps are in the
> main stage

I could, but that'd be a bit redundant with this check:
compare(app1.requestedState, ApplicationInfoInterface.RequestedSuspended);

Which is what we're really interested in. The test was poor before, because the checked requested state was the same as the running state, and so it would need to look for secondary characteristics to confirm the expected state but didn't.

But now that we compare for the suspended state, I don't think it's necessary to add further checks. But they'd be harmless to add if you like.

Daniel d'Andrada (dandrader) wrote :

Otherwise code looks ok. Didn't test yet.

review: Approve (code)
Michael Terry (mterry) wrote :

I've updated the tests to have more comments and sanity checking.

lp:~mterry/unity8/fix-wakelocks updated on 2015-12-03
2080. By Michael Terry on 2015-12-03

Add more comments and sanity checking to the exempt tests

PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:2080
http://jenkins.qa.ubuntu.com/job/unity8-ci/6882/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/5593/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-xenial-touch/297/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/1593/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-qmluitest-xenial-amd64/296/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/1488/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/1488/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-xenial-amd64-ci/295/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-xenial-i386-ci/294/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5607/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/296/console

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

review: Needs Fixing (continuous-integration)
Daniel d'Andrada (dandrader) wrote :

And fixes the bug

review: Approve

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'qml/Stages/PhoneStage.qml'
2--- qml/Stages/PhoneStage.qml 2015-11-04 14:58:05 +0000
3+++ qml/Stages/PhoneStage.qml 2015-12-03 20:52:33 +0000
4@@ -426,14 +426,16 @@
5
6 readonly property bool isDash: model.appId == "unity8-dash"
7
8- readonly property bool canSuspend: model.isTouchApp
9- && !isExemptFromLifecycle(model.appId)
10+ Binding {
11+ target: appDelegate.application
12+ property: "exemptFromLifecycle"
13+ value: !model.isTouchApp || isExemptFromLifecycle(model.appId)
14+ }
15
16 Binding {
17 target: appDelegate.application
18 property: "requestedState"
19- value: !canSuspend
20- || (isDash && root.keepDashRunning)
21+ value: (isDash && root.keepDashRunning)
22 || (!root.suspended && appDelegate.focus)
23 ? ApplicationInfoInterface.RequestedRunning
24 : ApplicationInfoInterface.RequestedSuspended
25
26=== modified file 'qml/Stages/TabletStage.qml'
27--- qml/Stages/TabletStage.qml 2015-11-04 14:58:05 +0000
28+++ qml/Stages/TabletStage.qml 2015-12-03 20:52:33 +0000
29@@ -612,14 +612,16 @@
30
31 readonly property bool isDash: model.appId == "unity8-dash"
32
33- readonly property bool canSuspend: model.isTouchApp
34- && !isExemptFromLifecycle(model.appId)
35+ Binding {
36+ target: spreadTile.application
37+ property: "exemptFromLifecycle"
38+ value: !model.isTouchApp || isExemptFromLifecycle(model.appId)
39+ }
40
41 Binding {
42 target: spreadTile.application
43 property: "requestedState"
44- value: !canSuspend
45- || (isDash && root.keepDashRunning)
46+ value: (isDash && root.keepDashRunning)
47 || (!root.suspended && (model.appId == priv.mainStageAppId
48 || model.appId == priv.sideStageAppId))
49 ? ApplicationInfoInterface.RequestedRunning
50
51=== modified file 'tests/mocks/Unity/Application/ApplicationInfo.cpp'
52--- tests/mocks/Unity/Application/ApplicationInfo.cpp 2015-11-06 09:11:53 +0000
53+++ tests/mocks/Unity/Application/ApplicationInfo.cpp 2015-12-03 20:52:33 +0000
54@@ -42,6 +42,7 @@
55 , m_rotatesWindowContents(false)
56 , m_requestedState(RequestedRunning)
57 , m_isTouchApp(true)
58+ , m_exemptFromLifecycle(false)
59 , m_manualSurfaceCreation(false)
60 {
61 }
62@@ -60,6 +61,7 @@
63 , m_rotatesWindowContents(false)
64 , m_requestedState(RequestedRunning)
65 , m_isTouchApp(true)
66+ , m_exemptFromLifecycle(false)
67 , m_manualSurfaceCreation(false)
68 {
69 }
70@@ -253,6 +255,20 @@
71 m_isTouchApp = isTouchApp;
72 }
73
74+bool ApplicationInfo::exemptFromLifecycle() const
75+{
76+ return m_exemptFromLifecycle;
77+}
78+
79+void ApplicationInfo::setExemptFromLifecycle(bool exemptFromLifecycle)
80+{
81+ if (m_exemptFromLifecycle != exemptFromLifecycle)
82+ {
83+ m_exemptFromLifecycle = exemptFromLifecycle;
84+ Q_EMIT exemptFromLifecycleChanged(m_exemptFromLifecycle);
85+ }
86+}
87+
88 void ApplicationInfo::onSessionSurfaceChanged(MirSurface* surface)
89 {
90 if (surface != nullptr && m_state == Starting) {
91
92=== modified file 'tests/mocks/Unity/Application/ApplicationInfo.h'
93--- tests/mocks/Unity/Application/ApplicationInfo.h 2015-10-01 17:43:10 +0000
94+++ tests/mocks/Unity/Application/ApplicationInfo.h 2015-12-03 20:52:33 +0000
95@@ -92,6 +92,9 @@
96 bool isTouchApp() const override;
97 void setIsTouchApp(bool isTouchApp); // only in mock
98
99+ bool exemptFromLifecycle() const override;
100+ void setExemptFromLifecycle(bool) override;
101+
102 public:
103 void setSession(Session* session);
104 Session* session() const { return m_session; }
105@@ -125,6 +128,7 @@
106 bool m_rotatesWindowContents;
107 RequestedState m_requestedState;
108 bool m_isTouchApp;
109+ bool m_exemptFromLifecycle;
110
111 bool m_manualSurfaceCreation;
112 };
113
114=== modified file 'tests/mocks/Unity/Application/ApplicationManager.cpp'
115--- tests/mocks/Unity/Application/ApplicationManager.cpp 2015-11-05 14:04:32 +0000
116+++ tests/mocks/Unity/Application/ApplicationManager.cpp 2015-12-03 20:52:33 +0000
117@@ -107,6 +107,8 @@
118 return app->focused();
119 case RoleIsTouchApp:
120 return app->isTouchApp();
121+ case RoleExemptFromLifecycle:
122+ return app->exemptFromLifecycle();
123 case RoleSession:
124 return QVariant::fromValue(app->session());
125 case RoleFullscreen:
126
127=== modified file 'tests/mocks/Unity/Application/ApplicationManager.h'
128--- tests/mocks/Unity/Application/ApplicationManager.h 2015-10-01 17:43:10 +0000
129+++ tests/mocks/Unity/Application/ApplicationManager.h 2015-12-03 20:52:33 +0000
130@@ -43,7 +43,7 @@
131 static ApplicationManager *singleton();
132
133 enum MoreRoles {
134- RoleSession = RoleIsTouchApp+1,
135+ RoleSession = RoleExemptFromLifecycle+1,
136 RoleFullscreen,
137 };
138 enum Flag {
139
140=== modified file 'tests/plugins/Unity/Launcher/launchermodeltest.cpp'
141--- tests/plugins/Unity/Launcher/launchermodeltest.cpp 2015-11-06 13:27:15 +0000
142+++ tests/plugins/Unity/Launcher/launchermodeltest.cpp 2015-12-03 20:52:33 +0000
143@@ -59,6 +59,8 @@
144 Qt::ScreenOrientations supportedOrientations() const override { return Qt::PortraitOrientation; }
145 bool rotatesWindowContents() const override { return false; }
146 bool isTouchApp() const override { return true; }
147+ bool exemptFromLifecycle() const override { return false; }
148+ void setExemptFromLifecycle(bool) override {}
149
150 // Methods used for mocking (not in the interface)
151 void setFocused(bool focused) { m_focused = focused; Q_EMIT focusedChanged(focused); }
152
153=== modified file 'tests/qmltests/tst_Shell.qml'
154--- tests/qmltests/tst_Shell.qml 2015-11-24 17:44:18 +0000
155+++ tests/qmltests/tst_Shell.qml 2015-12-03 20:52:33 +0000
156@@ -1857,21 +1857,28 @@
157 loadShell(data.formFactor);
158 shell.usageScenario = data.usageScenario;
159
160+ // Add two main stage apps, the second in order to suspend the first.
161+ // LibreOffice has isTouchApp set to false by our mocks.
162 var app1 = ApplicationManager.startApplication("libreoffice");
163 waitUntilAppWindowIsFullyLoaded(app1);
164- var app2 = ApplicationManager.startApplication("dialer-app");
165+ var app2 = ApplicationManager.startApplication("gallery-app");
166 waitUntilAppWindowIsFullyLoaded(app2);
167
168- // Make sure app1 is unfocused but still running
169- compare(app1.session.surface.activeFocus, false);
170- compare(app1.isTouchApp, false); // sanity check our mock, which sets this for us
171- compare(app1.requestedState, ApplicationInfoInterface.RequestedRunning);
172+ // Sanity checking
173+ compare(app1.stage, ApplicationInfoInterface.MainStage);
174+ compare(app2.stage, ApplicationInfoInterface.MainStage);
175+ verify(!app1.isTouchApp);
176+ verify(!app1.session.surface.activeFocus);
177+
178+ // Make sure app1 is exempt with a requested suspend
179+ verify(app1.exemptFromLifecycle);
180+ compare(app1.requestedState, ApplicationInfoInterface.RequestedSuspended);
181 }
182
183 function test_lifecyclePolicyExemption_data() {
184 return [
185- {tag: "phone", formFactor: "phone", usageScenario: "phone", suspendsApps: true},
186- {tag: "tablet", formFactor: "tablet", usageScenario: "tablet", suspendsApps: true}
187+ {tag: "phone", formFactor: "phone", usageScenario: "phone"},
188+ {tag: "tablet", formFactor: "tablet", usageScenario: "tablet"}
189 ]
190 }
191
192@@ -1881,14 +1888,20 @@
193
194 GSettingsController.setLifecycleExemptAppids(["webbrowser-app"]);
195
196+ // Add two main stage apps, the second in order to suspend the first
197 var app1 = ApplicationManager.startApplication("webbrowser-app");
198 waitUntilAppWindowIsFullyLoaded(app1);
199- var app2 = ApplicationManager.startApplication("dialer-app");
200+ var app2 = ApplicationManager.startApplication("gallery-app");
201 waitUntilAppWindowIsFullyLoaded(app2);
202
203- // Make sure app1 is unfocused but still running
204- compare(app1.session.surface.activeFocus, false);
205- compare(app1.requestedState, ApplicationInfoInterface.RequestedRunning);
206+ // Sanity checking
207+ compare(app1.stage, ApplicationInfoInterface.MainStage);
208+ compare(app2.stage, ApplicationInfoInterface.MainStage);
209+ verify(!app1.session.surface.activeFocus);
210+
211+ // Make sure app1 is exempt with a requested suspend
212+ verify(app1.exemptFromLifecycle);
213+ compare(app1.requestedState, ApplicationInfoInterface.RequestedSuspended);
214 }
215
216 function test_switchToStagedForcesLegacyAppClosing_data() {

Subscribers

People subscribed via source and target branches