Unity 8

Merge lp:~unity-team/unity8/fix-wakelocks into lp:unity8

  1. fix-wakelocks
  2. Merge into trunk
Proposed by Michał Sawicz
Status: Merged
Approved by: Michał Sawicz
Approved revision: 2082
Merged at revision: 2082
Proposed branch: lp:~unity-team/unity8/fix-wakelocks
Merge into: lp:unity8
Prerequisite: lp:~dandrader/unity8/multiSurfaceApp
Diff against target: 213 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:~unity-team/unity8/fix-wakelocks
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Needs Fixing
Daniel d'Andrada Pending
Review via email: mp+279764@code.launchpad.net

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

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.
Revision history for this message
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)
Revision history for this message
Daniel d'Andrada (dandrader) wrote : Posted in a previous version of this proposal

In test_lifecyclePolicyExemption:

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

review: Needs Fixing
Revision history for this message
Daniel d'Andrada (dandrader) wrote : Posted in a previous version of this proposal

Likewise in test_lifecyclePolicyForNonTouchApp

Revision history for this message
Michael Terry (mterry) wrote : Posted in a previous version of this proposal

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

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

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)
Revision history for this message
Daniel d'Andrada (dandrader) wrote : Posted in a previous version of this proposal

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

Revision history for this message
Michael Terry (mterry) wrote : Posted in a previous version of this proposal

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

Revision history for this message
Daniel d'Andrada (dandrader) wrote : Posted in a previous version of this proposal

Otherwise code looks ok. Didn't test yet.

review: Approve (code)
Revision history for this message
Michael Terry (mterry) wrote : Posted in a previous version of this proposal

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

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

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)
Revision history for this message
Daniel d'Andrada (dandrader) wrote : Posted in a previous version of this proposal

And fixes the bug

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:2081
http://jenkins.qa.ubuntu.com/job/unity8-ci/6887/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/5616/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-xenial-touch/302/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/1598/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-qmluitest-xenial-amd64/301/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/1493/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/1493/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-xenial-amd64-ci/300/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-xenial-i386-ci/299/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5630/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/301/console

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

review: Needs Fixing (continuous-integration)
lp:~unity-team/unity8/fix-wakelocks updated
2082. By Michael Terry

Fix tests

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:2082
http://jenkins.qa.ubuntu.com/job/unity8-ci/6892/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/5640/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-xenial-touch/307/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/1603/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-qmluitest-xenial-amd64/306/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/1498/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/1498/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-xenial-amd64-ci/305/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-xenial-i386-ci/304/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5654/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/306/console

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Michał Sawicz (saviq) wrote :

Approving per superseded.

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-08 14:40:30 +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-08 14:40:30 +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-12-08 14:40:30 +0000
53+++ tests/mocks/Unity/Application/ApplicationInfo.cpp 2015-12-08 14:40:30 +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@@ -263,3 +265,17 @@
71 }
72 }
73 }
74+
75+bool ApplicationInfo::exemptFromLifecycle() const
76+{
77+ return m_exemptFromLifecycle;
78+}
79+
80+void ApplicationInfo::setExemptFromLifecycle(bool exemptFromLifecycle)
81+{
82+ if (m_exemptFromLifecycle != exemptFromLifecycle)
83+ {
84+ m_exemptFromLifecycle = exemptFromLifecycle;
85+ Q_EMIT exemptFromLifecycleChanged(m_exemptFromLifecycle);
86+ }
87+}
88
89=== modified file 'tests/mocks/Unity/Application/ApplicationInfo.h'
90--- tests/mocks/Unity/Application/ApplicationInfo.h 2015-12-08 14:40:30 +0000
91+++ tests/mocks/Unity/Application/ApplicationInfo.h 2015-12-08 14:40:30 +0000
92@@ -92,6 +92,9 @@
93 bool isTouchApp() const override;
94 void setIsTouchApp(bool isTouchApp); // only in mock
95
96+ bool exemptFromLifecycle() const override;
97+ void setExemptFromLifecycle(bool) override;
98+
99 public:
100 void setSession(Session* session);
101 Session* session() const { return m_session; }
102@@ -125,6 +128,7 @@
103 bool m_rotatesWindowContents;
104 RequestedState m_requestedState;
105 bool m_isTouchApp;
106+ bool m_exemptFromLifecycle;
107
108 bool m_manualSurfaceCreation;
109 };
110
111=== modified file 'tests/mocks/Unity/Application/ApplicationManager.cpp'
112--- tests/mocks/Unity/Application/ApplicationManager.cpp 2015-11-05 14:04:32 +0000
113+++ tests/mocks/Unity/Application/ApplicationManager.cpp 2015-12-08 14:40:30 +0000
114@@ -107,6 +107,8 @@
115 return app->focused();
116 case RoleIsTouchApp:
117 return app->isTouchApp();
118+ case RoleExemptFromLifecycle:
119+ return app->exemptFromLifecycle();
120 case RoleSession:
121 return QVariant::fromValue(app->session());
122 case RoleFullscreen:
123
124=== modified file 'tests/mocks/Unity/Application/ApplicationManager.h'
125--- tests/mocks/Unity/Application/ApplicationManager.h 2015-10-01 17:43:10 +0000
126+++ tests/mocks/Unity/Application/ApplicationManager.h 2015-12-08 14:40:30 +0000
127@@ -43,7 +43,7 @@
128 static ApplicationManager *singleton();
129
130 enum MoreRoles {
131- RoleSession = RoleIsTouchApp+1,
132+ RoleSession = RoleExemptFromLifecycle+1,
133 RoleFullscreen,
134 };
135 enum Flag {
136
137=== modified file 'tests/plugins/Unity/Launcher/launchermodeltest.cpp'
138--- tests/plugins/Unity/Launcher/launchermodeltest.cpp 2015-11-06 13:27:15 +0000
139+++ tests/plugins/Unity/Launcher/launchermodeltest.cpp 2015-12-08 14:40:30 +0000
140@@ -59,6 +59,8 @@
141 Qt::ScreenOrientations supportedOrientations() const override { return Qt::PortraitOrientation; }
142 bool rotatesWindowContents() const override { return false; }
143 bool isTouchApp() const override { return true; }
144+ bool exemptFromLifecycle() const override { return false; }
145+ void setExemptFromLifecycle(bool) override {}
146
147 // Methods used for mocking (not in the interface)
148 void setFocused(bool focused) { m_focused = focused; Q_EMIT focusedChanged(focused); }
149
150=== modified file 'tests/qmltests/tst_Shell.qml'
151--- tests/qmltests/tst_Shell.qml 2015-12-08 14:40:30 +0000
152+++ tests/qmltests/tst_Shell.qml 2015-12-08 14:40:30 +0000
153@@ -1857,21 +1857,28 @@
154 loadShell(data.formFactor);
155 shell.usageScenario = data.usageScenario;
156
157+ // Add two main stage apps, the second in order to suspend the first.
158+ // LibreOffice has isTouchApp set to false by our mocks.
159 var app1 = ApplicationManager.startApplication("libreoffice");
160 waitUntilAppWindowIsFullyLoaded(app1);
161- var app2 = ApplicationManager.startApplication("dialer-app");
162+ var app2 = ApplicationManager.startApplication("gallery-app");
163 waitUntilAppWindowIsFullyLoaded(app2);
164
165- // Make sure app1 is unfocused but still running
166- compare(app1.session.lastSurface.activeFocus, false);
167- compare(app1.isTouchApp, false); // sanity check our mock, which sets this for us
168- compare(app1.requestedState, ApplicationInfoInterface.RequestedRunning);
169+ // Sanity checking
170+ compare(app1.stage, ApplicationInfoInterface.MainStage);
171+ compare(app2.stage, ApplicationInfoInterface.MainStage);
172+ verify(!app1.isTouchApp);
173+ verify(!app1.session.lastSurface.activeFocus);
174+
175+ // Make sure app1 is exempt with a requested suspend
176+ verify(app1.exemptFromLifecycle);
177+ compare(app1.requestedState, ApplicationInfoInterface.RequestedSuspended);
178 }
179
180 function test_lifecyclePolicyExemption_data() {
181 return [
182- {tag: "phone", formFactor: "phone", usageScenario: "phone", suspendsApps: true},
183- {tag: "tablet", formFactor: "tablet", usageScenario: "tablet", suspendsApps: true}
184+ {tag: "phone", formFactor: "phone", usageScenario: "phone"},
185+ {tag: "tablet", formFactor: "tablet", usageScenario: "tablet"}
186 ]
187 }
188
189@@ -1881,14 +1888,20 @@
190
191 GSettingsController.setLifecycleExemptAppids(["webbrowser-app"]);
192
193+ // Add two main stage apps, the second in order to suspend the first
194 var app1 = ApplicationManager.startApplication("webbrowser-app");
195 waitUntilAppWindowIsFullyLoaded(app1);
196- var app2 = ApplicationManager.startApplication("dialer-app");
197+ var app2 = ApplicationManager.startApplication("gallery-app");
198 waitUntilAppWindowIsFullyLoaded(app2);
199
200- // Make sure app1 is unfocused but still running
201- compare(app1.session.lastSurface.activeFocus, false);
202- compare(app1.requestedState, ApplicationInfoInterface.RequestedRunning);
203+ // Sanity checking
204+ compare(app1.stage, ApplicationInfoInterface.MainStage);
205+ compare(app2.stage, ApplicationInfoInterface.MainStage);
206+ verify(!app1.session.lastSurface.activeFocus);
207+
208+ // Make sure app1 is exempt with a requested suspend
209+ verify(app1.exemptFromLifecycle);
210+ compare(app1.requestedState, ApplicationInfoInterface.RequestedSuspended);
211 }
212
213 function test_switchToStagedForcesLegacyAppClosing_data() {

Subscribers

People subscribed via source and target branches