Merge lp:~unity-team/unity8/fix-wakelocks into lp:unity8
- fix-wakelocks
- Merge into trunk
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 | ||||
Related bugs: |
|
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:/
https:/
* 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
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
Daniel d'Andrada (dandrader) wrote : Posted in a previous version of this proposal | # |
In test_lifecycleP
I don't see the point in changing app2 from dialer-app to gallery-app
Daniel d'Andrada (dandrader) wrote : Posted in a previous version of this proposal | # |
Likewise in test_lifecycleP
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.
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:2079
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
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
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(
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 : Posted in a previous version of this proposal | # |
Otherwise code looks ok. Didn't test yet.
Michael Terry (mterry) wrote : Posted in a previous version of this proposal | # |
I've updated the tests to have more comments and sanity checking.
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:2080
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
Daniel d'Andrada (dandrader) wrote : Posted in a previous version of this proposal | # |
And fixes the bug
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:2081
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
- 2082. By Michael Terry
-
Fix tests
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:2082
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
Michał Sawicz (saviq) wrote : | # |
Approving per superseded.
Preview Diff
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 | 426 | 426 | ||
6 | 427 | readonly property bool isDash: model.appId == "unity8-dash" | 427 | readonly property bool isDash: model.appId == "unity8-dash" |
7 | 428 | 428 | ||
10 | 429 | readonly property bool canSuspend: model.isTouchApp | 429 | Binding { |
11 | 430 | && !isExemptFromLifecycle(model.appId) | 430 | target: appDelegate.application |
12 | 431 | property: "exemptFromLifecycle" | ||
13 | 432 | value: !model.isTouchApp || isExemptFromLifecycle(model.appId) | ||
14 | 433 | } | ||
15 | 431 | 434 | ||
16 | 432 | Binding { | 435 | Binding { |
17 | 433 | target: appDelegate.application | 436 | target: appDelegate.application |
18 | 434 | property: "requestedState" | 437 | property: "requestedState" |
21 | 435 | value: !canSuspend | 438 | value: (isDash && root.keepDashRunning) |
20 | 436 | || (isDash && root.keepDashRunning) | ||
22 | 437 | || (!root.suspended && appDelegate.focus) | 439 | || (!root.suspended && appDelegate.focus) |
23 | 438 | ? ApplicationInfoInterface.RequestedRunning | 440 | ? ApplicationInfoInterface.RequestedRunning |
24 | 439 | : ApplicationInfoInterface.RequestedSuspended | 441 | : ApplicationInfoInterface.RequestedSuspended |
25 | 440 | 442 | ||
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 | 612 | 612 | ||
31 | 613 | readonly property bool isDash: model.appId == "unity8-dash" | 613 | readonly property bool isDash: model.appId == "unity8-dash" |
32 | 614 | 614 | ||
35 | 615 | readonly property bool canSuspend: model.isTouchApp | 615 | Binding { |
36 | 616 | && !isExemptFromLifecycle(model.appId) | 616 | target: spreadTile.application |
37 | 617 | property: "exemptFromLifecycle" | ||
38 | 618 | value: !model.isTouchApp || isExemptFromLifecycle(model.appId) | ||
39 | 619 | } | ||
40 | 617 | 620 | ||
41 | 618 | Binding { | 621 | Binding { |
42 | 619 | target: spreadTile.application | 622 | target: spreadTile.application |
43 | 620 | property: "requestedState" | 623 | property: "requestedState" |
46 | 621 | value: !canSuspend | 624 | value: (isDash && root.keepDashRunning) |
45 | 622 | || (isDash && root.keepDashRunning) | ||
47 | 623 | || (!root.suspended && (model.appId == priv.mainStageAppId | 625 | || (!root.suspended && (model.appId == priv.mainStageAppId |
48 | 624 | || model.appId == priv.sideStageAppId)) | 626 | || model.appId == priv.sideStageAppId)) |
49 | 625 | ? ApplicationInfoInterface.RequestedRunning | 627 | ? ApplicationInfoInterface.RequestedRunning |
50 | 626 | 628 | ||
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 | 42 | , m_rotatesWindowContents(false) | 42 | , m_rotatesWindowContents(false) |
56 | 43 | , m_requestedState(RequestedRunning) | 43 | , m_requestedState(RequestedRunning) |
57 | 44 | , m_isTouchApp(true) | 44 | , m_isTouchApp(true) |
58 | 45 | , m_exemptFromLifecycle(false) | ||
59 | 45 | , m_manualSurfaceCreation(false) | 46 | , m_manualSurfaceCreation(false) |
60 | 46 | { | 47 | { |
61 | 47 | } | 48 | } |
62 | @@ -60,6 +61,7 @@ | |||
63 | 60 | , m_rotatesWindowContents(false) | 61 | , m_rotatesWindowContents(false) |
64 | 61 | , m_requestedState(RequestedRunning) | 62 | , m_requestedState(RequestedRunning) |
65 | 62 | , m_isTouchApp(true) | 63 | , m_isTouchApp(true) |
66 | 64 | , m_exemptFromLifecycle(false) | ||
67 | 63 | , m_manualSurfaceCreation(false) | 65 | , m_manualSurfaceCreation(false) |
68 | 64 | { | 66 | { |
69 | 65 | } | 67 | } |
70 | @@ -263,3 +265,17 @@ | |||
71 | 263 | } | 265 | } |
72 | 264 | } | 266 | } |
73 | 265 | } | 267 | } |
74 | 268 | |||
75 | 269 | bool ApplicationInfo::exemptFromLifecycle() const | ||
76 | 270 | { | ||
77 | 271 | return m_exemptFromLifecycle; | ||
78 | 272 | } | ||
79 | 273 | |||
80 | 274 | void ApplicationInfo::setExemptFromLifecycle(bool exemptFromLifecycle) | ||
81 | 275 | { | ||
82 | 276 | if (m_exemptFromLifecycle != exemptFromLifecycle) | ||
83 | 277 | { | ||
84 | 278 | m_exemptFromLifecycle = exemptFromLifecycle; | ||
85 | 279 | Q_EMIT exemptFromLifecycleChanged(m_exemptFromLifecycle); | ||
86 | 280 | } | ||
87 | 281 | } | ||
88 | 266 | 282 | ||
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 | 92 | bool isTouchApp() const override; | 92 | bool isTouchApp() const override; |
94 | 93 | void setIsTouchApp(bool isTouchApp); // only in mock | 93 | void setIsTouchApp(bool isTouchApp); // only in mock |
95 | 94 | 94 | ||
96 | 95 | bool exemptFromLifecycle() const override; | ||
97 | 96 | void setExemptFromLifecycle(bool) override; | ||
98 | 97 | |||
99 | 95 | public: | 98 | public: |
100 | 96 | void setSession(Session* session); | 99 | void setSession(Session* session); |
101 | 97 | Session* session() const { return m_session; } | 100 | Session* session() const { return m_session; } |
102 | @@ -125,6 +128,7 @@ | |||
103 | 125 | bool m_rotatesWindowContents; | 128 | bool m_rotatesWindowContents; |
104 | 126 | RequestedState m_requestedState; | 129 | RequestedState m_requestedState; |
105 | 127 | bool m_isTouchApp; | 130 | bool m_isTouchApp; |
106 | 131 | bool m_exemptFromLifecycle; | ||
107 | 128 | 132 | ||
108 | 129 | bool m_manualSurfaceCreation; | 133 | bool m_manualSurfaceCreation; |
109 | 130 | }; | 134 | }; |
110 | 131 | 135 | ||
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 | 107 | return app->focused(); | 107 | return app->focused(); |
116 | 108 | case RoleIsTouchApp: | 108 | case RoleIsTouchApp: |
117 | 109 | return app->isTouchApp(); | 109 | return app->isTouchApp(); |
118 | 110 | case RoleExemptFromLifecycle: | ||
119 | 111 | return app->exemptFromLifecycle(); | ||
120 | 110 | case RoleSession: | 112 | case RoleSession: |
121 | 111 | return QVariant::fromValue(app->session()); | 113 | return QVariant::fromValue(app->session()); |
122 | 112 | case RoleFullscreen: | 114 | case RoleFullscreen: |
123 | 113 | 115 | ||
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 | 43 | static ApplicationManager *singleton(); | 43 | static ApplicationManager *singleton(); |
129 | 44 | 44 | ||
130 | 45 | enum MoreRoles { | 45 | enum MoreRoles { |
132 | 46 | RoleSession = RoleIsTouchApp+1, | 46 | RoleSession = RoleExemptFromLifecycle+1, |
133 | 47 | RoleFullscreen, | 47 | RoleFullscreen, |
134 | 48 | }; | 48 | }; |
135 | 49 | enum Flag { | 49 | enum Flag { |
136 | 50 | 50 | ||
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 | 59 | Qt::ScreenOrientations supportedOrientations() const override { return Qt::PortraitOrientation; } | 59 | Qt::ScreenOrientations supportedOrientations() const override { return Qt::PortraitOrientation; } |
142 | 60 | bool rotatesWindowContents() const override { return false; } | 60 | bool rotatesWindowContents() const override { return false; } |
143 | 61 | bool isTouchApp() const override { return true; } | 61 | bool isTouchApp() const override { return true; } |
144 | 62 | bool exemptFromLifecycle() const override { return false; } | ||
145 | 63 | void setExemptFromLifecycle(bool) override {} | ||
146 | 62 | 64 | ||
147 | 63 | // Methods used for mocking (not in the interface) | 65 | // Methods used for mocking (not in the interface) |
148 | 64 | void setFocused(bool focused) { m_focused = focused; Q_EMIT focusedChanged(focused); } | 66 | void setFocused(bool focused) { m_focused = focused; Q_EMIT focusedChanged(focused); } |
149 | 65 | 67 | ||
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 | 1857 | loadShell(data.formFactor); | 1857 | loadShell(data.formFactor); |
155 | 1858 | shell.usageScenario = data.usageScenario; | 1858 | shell.usageScenario = data.usageScenario; |
156 | 1859 | 1859 | ||
157 | 1860 | // Add two main stage apps, the second in order to suspend the first. | ||
158 | 1861 | // LibreOffice has isTouchApp set to false by our mocks. | ||
159 | 1860 | var app1 = ApplicationManager.startApplication("libreoffice"); | 1862 | var app1 = ApplicationManager.startApplication("libreoffice"); |
160 | 1861 | waitUntilAppWindowIsFullyLoaded(app1); | 1863 | waitUntilAppWindowIsFullyLoaded(app1); |
162 | 1862 | var app2 = ApplicationManager.startApplication("dialer-app"); | 1864 | var app2 = ApplicationManager.startApplication("gallery-app"); |
163 | 1863 | waitUntilAppWindowIsFullyLoaded(app2); | 1865 | waitUntilAppWindowIsFullyLoaded(app2); |
164 | 1864 | 1866 | ||
169 | 1865 | // Make sure app1 is unfocused but still running | 1867 | // Sanity checking |
170 | 1866 | compare(app1.session.lastSurface.activeFocus, false); | 1868 | compare(app1.stage, ApplicationInfoInterface.MainStage); |
171 | 1867 | compare(app1.isTouchApp, false); // sanity check our mock, which sets this for us | 1869 | compare(app2.stage, ApplicationInfoInterface.MainStage); |
172 | 1868 | compare(app1.requestedState, ApplicationInfoInterface.RequestedRunning); | 1870 | verify(!app1.isTouchApp); |
173 | 1871 | verify(!app1.session.lastSurface.activeFocus); | ||
174 | 1872 | |||
175 | 1873 | // Make sure app1 is exempt with a requested suspend | ||
176 | 1874 | verify(app1.exemptFromLifecycle); | ||
177 | 1875 | compare(app1.requestedState, ApplicationInfoInterface.RequestedSuspended); | ||
178 | 1869 | } | 1876 | } |
179 | 1870 | 1877 | ||
180 | 1871 | function test_lifecyclePolicyExemption_data() { | 1878 | function test_lifecyclePolicyExemption_data() { |
181 | 1872 | return [ | 1879 | return [ |
184 | 1873 | {tag: "phone", formFactor: "phone", usageScenario: "phone", suspendsApps: true}, | 1880 | {tag: "phone", formFactor: "phone", usageScenario: "phone"}, |
185 | 1874 | {tag: "tablet", formFactor: "tablet", usageScenario: "tablet", suspendsApps: true} | 1881 | {tag: "tablet", formFactor: "tablet", usageScenario: "tablet"} |
186 | 1875 | ] | 1882 | ] |
187 | 1876 | } | 1883 | } |
188 | 1877 | 1884 | ||
189 | @@ -1881,14 +1888,20 @@ | |||
190 | 1881 | 1888 | ||
191 | 1882 | GSettingsController.setLifecycleExemptAppids(["webbrowser-app"]); | 1889 | GSettingsController.setLifecycleExemptAppids(["webbrowser-app"]); |
192 | 1883 | 1890 | ||
193 | 1891 | // Add two main stage apps, the second in order to suspend the first | ||
194 | 1884 | var app1 = ApplicationManager.startApplication("webbrowser-app"); | 1892 | var app1 = ApplicationManager.startApplication("webbrowser-app"); |
195 | 1885 | waitUntilAppWindowIsFullyLoaded(app1); | 1893 | waitUntilAppWindowIsFullyLoaded(app1); |
197 | 1886 | var app2 = ApplicationManager.startApplication("dialer-app"); | 1894 | var app2 = ApplicationManager.startApplication("gallery-app"); |
198 | 1887 | waitUntilAppWindowIsFullyLoaded(app2); | 1895 | waitUntilAppWindowIsFullyLoaded(app2); |
199 | 1888 | 1896 | ||
203 | 1889 | // Make sure app1 is unfocused but still running | 1897 | // Sanity checking |
204 | 1890 | compare(app1.session.lastSurface.activeFocus, false); | 1898 | compare(app1.stage, ApplicationInfoInterface.MainStage); |
205 | 1891 | compare(app1.requestedState, ApplicationInfoInterface.RequestedRunning); | 1899 | compare(app2.stage, ApplicationInfoInterface.MainStage); |
206 | 1900 | verify(!app1.session.lastSurface.activeFocus); | ||
207 | 1901 | |||
208 | 1902 | // Make sure app1 is exempt with a requested suspend | ||
209 | 1903 | verify(app1.exemptFromLifecycle); | ||
210 | 1904 | compare(app1.requestedState, ApplicationInfoInterface.RequestedSuspended); | ||
211 | 1892 | } | 1905 | } |
212 | 1893 | 1906 | ||
213 | 1894 | function test_switchToStagedForcesLegacyAppClosing_data() { | 1907 | function test_switchToStagedForcesLegacyAppClosing_data() { |
FAILED: Continuous integration, rev:2078 jenkins. qa.ubuntu. com/job/ unity8- ci/6880/ jenkins. qa.ubuntu. com/job/ generic- deb-autopilot- vivid-touch/ 5590/console jenkins. qa.ubuntu. com/job/ generic- deb-autopilot- xenial- touch/295/ console jenkins. qa.ubuntu. com/job/ unity-phablet- qmluitests- vivid/1591/ console jenkins. qa.ubuntu. com/job/ unity8- qmluitest- xenial- amd64/294/ console jenkins. qa.ubuntu. com/job/ unity8- vivid-amd64- ci/1486/ console jenkins. qa.ubuntu. com/job/ unity8- vivid-i386- ci/1486/ console jenkins. qa.ubuntu. com/job/ unity8- xenial- amd64-ci/ 293/console jenkins. qa.ubuntu. com/job/ unity8- xenial- i386-ci/ 292/console jenkins. qa.ubuntu. com/job/ generic- mediumtests- builder- vivid-armhf/ 5604/console jenkins. qa.ubuntu. com/job/ generic- mediumtests- builder- xenial- armhf/294/ console
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/unity8- ci/6880/ rebuild
http://