QtMir

Merge lp://staging/~gerboland/qtmir/acquire-wakelock into lp://staging/qtmir

  1. acquire-wakelock
  2. Merge into trunk
Proposed by Gerry Boland
Status: Merged
Approved by: Michael Zanetti
Approved revision: 319
Merged at revision: 308
Proposed branch: lp://staging/~gerboland/qtmir/acquire-wakelock
Merge into: lp://staging/qtmir
Diff against target: 1098 lines (+814/-2)
16 files modified
src/common/abstractdbusservicemonitor.cpp (+101/-0)
src/common/abstractdbusservicemonitor.h (+68/-0)
src/modules/Unity/Application/CMakeLists.txt (+2/-0)
src/modules/Unity/Application/application.cpp (+20/-0)
src/modules/Unity/Application/application.h (+4/-0)
src/modules/Unity/Application/application_manager.cpp (+8/-0)
src/modules/Unity/Application/application_manager.h (+3/-0)
src/modules/Unity/Application/sharedwakelock.cpp (+168/-0)
src/modules/Unity/Application/sharedwakelock.h (+50/-0)
tests/modules/Application/CMakeLists.txt (+25/-0)
tests/modules/Application/application_test.cpp (+122/-0)
tests/modules/CMakeLists.txt (+3/-1)
tests/modules/SharedWakelock/CMakeLists.txt (+22/-0)
tests/modules/SharedWakelock/sharedwakelock_test.cpp (+170/-0)
tests/modules/common/mock_shared_wakelock.h (+44/-0)
tests/modules/common/qtmir_test.h (+4/-1)
To merge this branch: bzr merge lp://staging/~gerboland/qtmir/acquire-wakelock
Reviewer Review Type Date Requested Status
Michael Zanetti (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+245942@code.staging.launchpad.net

Commit message

Add Wakelock support - ensures device drops to deep-sleep mode only when all AppMan suspend tasks have completed

Implemented like so:
 - SharedWakelock manages a single wakelock instance, allowing multiple owners to keep the wakelock enabled, only releasing it when all owners have released it.
- Wakelock operates by communicating with PowerD
- the Application class decides whether a wakelock is necessary, based on the process' lifecycle state.
- added a mighty bunch of tests

Description of the change

Add Wakelock support - ensures device drops to deep-sleep mode only when all AppMan suspend tasks has completed

 * Are there any related MPs required for this MP to build/function as expected? Please list.
N
 * Did you perform an exploratory manual test run of your code change and any related functionality?
Y
 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

To post a comment you must log in.
Revision history for this message
Gerry Boland (gerboland) wrote :

To test, this command is handy:
sudo powerd-cli list
it lists the various wakelocks enabled:

Example output with system (grabbed by unity8 here) and display wakelock:
  Name: com.canonical.Unity.Screen, Owner: :1.16, State: 1
  Name: active, Owner: :1.48, State: 1

Example output with display wakelock alone:
  Name: com.canonical.Unity.Screen, Owner: :1.16, State: 1

Example output with display off, but system wakelock still held by unity8/qtmir - is held for 3 seconds after display is turned off, to give apps time to save state before suspend:
  Name: active, Owner: :1.48, State: 1

3 seconds after screen blank:
  None

Revision history for this message
Gerry Boland (gerboland) wrote :

Note also that I made it so that unity8-dash does not ever hold a wakelock.

lp://staging/~gerboland/qtmir/acquire-wakelock updated
316. By Gerry Boland

No need for the lambda, just use the existing method as a slot

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

PASSED: Continuous integration, rev:316
http://jenkins.qa.ubuntu.com/job/qtmir-ci/187/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/38
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/38
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/38/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)
lp://staging/~gerboland/qtmir/acquire-wakelock updated
317. By Gerry Boland

Fix typos

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

PASSED: Continuous integration, rev:317
http://jenkins.qa.ubuntu.com/job/qtmir-ci/188/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/39
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/39
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/39/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Michael Zanetti (mzanetti) wrote :

Ok. I think I fully understand the code. Looks ok except I'm not comfortable with the TODO in there (see inline comment)... Please explain if I'm wrong (or resolve it).

Still have to do testing.

review: Needs Information
Revision history for this message
Gerry Boland (gerboland) wrote :

> Ok. I think I fully understand the code. Looks ok except I'm not comfortable
> with the TODO in there (see inline comment)... Please explain if I'm wrong (or
> resolve it).

You're not wrong, wasn't sure how crashy unity8 is these days. Quick chat with ricmm, decided to save cookie to file and check for cookie file existence on startup

Revision history for this message
Michael Zanetti (mzanetti) wrote :

Ok, tried to test this:

* I cannot reproduce the app weirdness as described in the linked bug in vivid any more (I can in rtm) so I'm having problems verifying this fix.

* After installing the packages from this branch, the very first suspend after booting the phone seems to fail. Couldn't repro that with vivid trunk, so seems to be caused by this.

review: Needs Fixing
lp://staging/~gerboland/qtmir/acquire-wakelock updated
318. By Gerry Boland

Merge trunk

319. By Gerry Boland

Cache cookie to disk so can restore it if qtmir crashes and fails to clean up

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

PASSED: Continuous integration, rev:319
http://jenkins.qa.ubuntu.com/job/qtmir-ci/192/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/43
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/43
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/43/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Gerry Boland (gerboland) wrote :

> * After installing the packages from this branch, the very first
> suspend after booting the phone seems to fail. Couldn't repro that
> with vivid trunk, so seems to be caused by this.

And naturally I'm unable to reproduce this. I followed your exact instructions on Mako:
<mzanetti> ubuntu-device-flash --device mako --channel devel-proposed
<mzanetti> phablet-shell
<mzanetti> wget jenkins:/output.zip
<mzanetti> unzip & dpkg -i qtmir-android qtdeclarative-qtmir
<mzanetti> *unplug* usb
<mzanetti> reboot phone
<mzanetti> open browser -> youtube.de
<mzanetti> play first video, press power button
<mzanetti> continues to play
<mzanetti> following retries make it stop after 5 secs.
<mzanetti> just the very first time the screen turns off after booting
<greyback> you let the screen turn itself off? Or you hit power key yourself?
<mzanetti> I press power

Can you have another look please?

Revision history for this message
kevin gunn (kgunn72) wrote :

I just tried with vivid/mako, installed the debs from jenkins.
For me, 1st try and every other....it always stops playing video after ~5 seconds.

> > * After installing the packages from this branch, the very first
> > suspend after booting the phone seems to fail. Couldn't repro that
> > with vivid trunk, so seems to be caused by this.
>
> And naturally I'm unable to reproduce this. I followed your exact instructions
> on Mako:
> <mzanetti> ubuntu-device-flash --device mako --channel devel-proposed
> <mzanetti> phablet-shell
> <mzanetti> wget jenkins:/output.zip
> <mzanetti> unzip & dpkg -i qtmir-android qtdeclarative-qtmir
> <mzanetti> *unplug* usb
> <mzanetti> reboot phone
> <mzanetti> open browser -> youtube.de
> <mzanetti> play first video, press power button
> <mzanetti> continues to play
> <mzanetti> following retries make it stop after 5 secs.
> <mzanetti> just the very first time the screen turns off after booting
> <greyback> you let the screen turn itself off? Or you hit power key yourself?
> <mzanetti> I press power
>
> Can you have another look please?

Revision history for this message
kevin gunn (kgunn72) wrote :

testing with the powerd-cli list
playing video with youtube, list is
System State Requests:
  Name: com.canonical.Unity.Screen, Owner: :1.17, State: 1
  Name: active, Owner: :1.43, State: 1
hit power button, screen off while youtube is playing list changes to
System State Requests:
  Name: active, Owner: :1.43, State: 1
after ~5 sec, youtube audio is halted (app suspended presumably)
System State Requests:
  None

on the very first attempt of monitoring list, i thot i saw the
    Name: active, Owner: :1.43, State: 1
hang around and be returned for powerd-cli list after striking the power button, and never go away for well over 10 seconds. Even without an app being launched. Retried many tries, even reflashed thinking that might be related. Never saw it again...

Revision history for this message
kevin gunn (kgunn72) wrote :

after even more testing, i am now convinced i must have not waited long enough.
This seems to be working solidly

> testing with the powerd-cli list
> playing video with youtube, list is
> System State Requests:
> Name: com.canonical.Unity.Screen, Owner: :1.17, State: 1
> Name: active, Owner: :1.43, State: 1
> hit power button, screen off while youtube is playing list changes to
> System State Requests:
> Name: active, Owner: :1.43, State: 1
> after ~5 sec, youtube audio is halted (app suspended presumably)
> System State Requests:
> None
>
> on the very first attempt of monitoring list, i thot i saw the
> Name: active, Owner: :1.43, State: 1
> hang around and be returned for powerd-cli list after striking the power
> button, and never go away for well over 10 seconds. Even without an app being
> launched. Retried many tries, even reflashed thinking that might be related.
> Never saw it again...

Revision history for this message
Michael Zanetti (mzanetti) wrote :

> * After installing the packages from this branch, the very first suspend after booting the phone
> seems to fail. Couldn't repro that with vivid trunk, so seems to be caused by this.

Installed the latest packages from jenkins and can't repro this any more either...

Code looks still good to me, seems to behave well. Thanks Kevin for the testing help.

 * Did you perform an exploratory manual test run of the code change and any related functionality?

yes

 * Did CI run pass? If not, please explain why.

yip yip

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches