Merge lp:~gerboland/qtmir/fix-lifecycle-exempt-keeps-wakelock into lp:qtmir
| Status: | Merged |
|---|---|
| Approved by: | Albert Astals Cid on 2015-03-06 |
| Approved revision: | 336 |
| Merged at revision: | 327 |
| Proposed branch: | lp:~gerboland/qtmir/fix-lifecycle-exempt-keeps-wakelock |
| Merge into: | lp:qtmir |
| Diff against target: |
1165 lines (+646/-246) 13 files modified
CMakeLists.txt (+2/-0) debian/control (+2/-0) src/common/abstractdbusservicemonitor.cpp (+2/-3) src/common/abstractdbusservicemonitor.h (+1/-7) src/modules/Unity/Application/application.cpp (+3/-1) src/modules/Unity/Application/application_manager.cpp (+5/-2) src/modules/Unity/Application/sharedwakelock.cpp (+120/-57) src/modules/Unity/Application/sharedwakelock.h (+14/-9) tests/modules/Application/application_test.cpp (+7/-7) tests/modules/ApplicationManager/application_manager_test.cpp (+78/-0) tests/modules/SharedWakelock/CMakeLists.txt (+6/-0) tests/modules/SharedWakelock/sharedwakelock_test.cpp (+365/-146) tests/modules/common/mock_shared_wakelock.h (+41/-14) |
| To merge this branch: | bzr merge lp:~gerboland/qtmir/fix-lifecycle-exempt-keeps-wakelock |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Albert Astals Cid (community) | Approve on 2015-03-06 | ||
| PS Jenkins bot | continuous-integration | Approve on 2015-03-06 | |
| Michał Sawicz | Abstain on 2015-03-05 | ||
| Gerry Boland | Abstain on 2015-03-05 | ||
| Michael Zanetti (community) | 2015-02-20 | Approve on 2015-02-25 | |
|
Review via email:
|
|||
Commit Message
Refactor wakelock handling. Lifecycle exempt apps now release wakelock when shell tries to suspend them
The previous Wakelock RAII design was faulty as it was wrapping an asynchronous service. It made it possible for wakelocks to be acquired and not be released.
This refactors SharedWakelock to hold a single instance of Wakelock, and Wakelock always holds a DBus connection.
Testing now includes testing the DBus calls are actually emitted.
Adds dependency on libqtdbusmock1-dev and libqtdbustest1-dev
| Michael Zanetti (mzanetti) wrote : | # |
Tested this and it works fine. If possible I think we could update the comment to make it a bit more clear what's going on with the implicit wake locks. (see inline)
| Michał Sawicz (saviq) wrote : | # |
With this MP in silo 19, I start music app and I can see a permanent wakelock in `sudo powerd-cli list`, that only goes away if I put the music app in background.
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:322
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
| Michał Sawicz (saviq) wrote : | # |
Should we have a test for setFocused(false)?
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:323
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
| Andrew Hayzen (ahayzen) wrote : | # |
Hey, just wanted to check the following usecase is not broken with this code...
1) Open Music-App
2) Start playing a set of songs
3) Switch to another application
4) Suspend/lock the phone
5) Wait until the track ends, the next track should start playing
The main reason left that we have the lifecycle exception is to be able to tell media-hub what the next track is to play, this should be fixed once background-
| Gerry Boland (gerboland) wrote : | # |
Hey Andrew
the music app/pulseaudio will be responsible for having its own wakelock, so that audio will continue to play when the screen is blanked. I tested this and it was working ok with this.
-G
| Gerry Boland (gerboland) wrote : | # |
2 wakelocks somehow acquired with this by the Music app, something wrong.
When I start Music app, here's logging info I get:
SharedWakelock:
Wakelock object created!
SharedWakelock:
Wakelock object destroyed!
qtmir.sessions: Wakelock released
SharedWakelock:
Wakelock object created!
qtmir.sessions: Wakelock acquired "8d92eec1-
SharedWakelock:
Wakelock object destroyed!
qtmir.sessions: Wakelock released
which results in 2 wakelocks held by QtMir. Suspect it the fact that Wakelock object is created and then destroyed rapidly means the cookie is not returned before the Wakelock object is destroyed. Need to fix that
| Gerry Boland (gerboland) wrote : | # |
This unfortunately turned into a refactoring. The previous design relied on RAII, where the existence of the Wakelock object was supposed to correspond to a wakelock being held.
This design was flawed because of the asynchronous communication between powerd and qtmir. The problem case:
1. create a Wakelock object. This calls an async method of powerd to acquire a wakelock, which will eventually return a cookie.
2. destroy the Wakelock object. As no cookie saved, Wakelock has no cleanup to do
3. the promised cookie from powerd arrives, there's no Wakelock object to receive it. Cookie is lost, and wakelock remains held by powerd.
So I had to abandon the RAII design and instead have a single permanent Wakelock object which always stays connected to DBus.
To test this more thoroughly, am using the QDBusMock & QDBusTest tools.
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:327
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:328
http://
Executed test runs:
FAILURE: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:328
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:332
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:333
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
| Albert Astals Cid (aacid) wrote : | # |
* Did you perform an exploratory manual test run of the code change and any related functionality?
Yes
* Did CI run pass?
Yes
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:334
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:334
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
| PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:335
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://

PASSED: Continuous integration, rev:320 jenkins. qa.ubuntu. com/job/ qtmir-ci/ 217/ jenkins. qa.ubuntu. com/job/ qtmir-vivid- amd64-ci/ 68 jenkins. qa.ubuntu. com/job/ qtmir-vivid- armhf-ci/ 68 jenkins. qa.ubuntu. com/job/ qtmir-vivid- armhf-ci/ 68/artifact/ work/output/ *zip*/output. zip
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/qtmir- ci/217/ rebuild
http://