Merge lp:~unity-team/qtmir/fix-some-lifecycle-bugs into lp:qtmir
Status: | Merged |
---|---|
Approved by: | Albert Astals Cid on 2014-09-29 |
Approved revision: | 255 |
Merged at revision: | 264 |
Proposed branch: | lp:~unity-team/qtmir/fix-some-lifecycle-bugs |
Merge into: | lp:qtmir |
Diff against target: |
456 lines (+236/-24) 9 files modified
debian/control (+1/-1) src/modules/Unity/Application/application.cpp (+2/-2) src/modules/Unity/Application/application.h (+1/-2) src/modules/Unity/Application/application_manager.cpp (+43/-2) src/modules/Unity/Application/application_manager.h (+3/-0) src/modules/Unity/Application/taskcontroller.cpp (+13/-7) src/modules/Unity/Application/taskcontroller.h (+2/-2) tests/modules/ApplicationManager/application_manager_test.cpp (+148/-0) tests/modules/TaskController/taskcontroller_test.cpp (+23/-8) |
To merge this branch: | bzr merge lp:~unity-team/qtmir/fix-some-lifecycle-bugs |
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Albert Astals Cid (community) | Approve on 2014-09-29 | ||
PS Jenkins bot (community) | continuous-integration | Needs Fixing on 2014-09-29 | |
Gerry Boland (community) | 2014-09-10 | Needs Fixing on 2014-09-22 | |
Review via email:
|
Commit message
Fix some bugs in app lifecycle management.
* Correctly determine PID of apps not started by UAL
* Delayed suspending of apps if the first suspending failed because of them being in Starting phase still
Description of the change
* 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?
yes
* 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?
no
* If you changed the UI, has there been a design review?
no
Gerry Boland (gerboland) wrote : | # |
+bool TaskController:
const Application* please. Same for resume.
Could we also get a test checking if you start app1 & app2, one after the other, and if app2 comes up first, that app1 is suspended when it comes up. I believe your code fixes that too?
Gerry Boland (gerboland) wrote : | # |
Testing problem:
- installed package, rebooted phone, verified dash suspended via SSH. Unlocked screen. Dash was there, but only wallpaper was to be seen, no scope contents was visible. When I entered spread and selected Dash, it finally drew the scope contents.
Gerry Boland (gerboland) wrote : | # |
Nitpick not related to this MR - I noticed Dash remains suspended when greeter is shown, but resumed when lock-screen is revealed by swiping greeter away.
I think this MR fixes these bugs too:
https:/
https:/
please associate them with this MR.
Michael Zanetti (mzanetti) wrote : | # |
> Testing problem:
> - installed package, rebooted phone, verified dash suspended via SSH. Unlocked
> screen. Dash was there, but only wallpaper was to be seen, no scope contents
> was visible. When I entered spread and selected Dash, it finally drew the
> scope contents.
Did you install the related unity8 branch? I believe this happened because unity8 trunk doesn't focus any already running app when starting up. The linked branch fixes that in unity8.
Michael Zanetti (mzanetti) wrote : | # |
> +bool TaskController:
> const Application* please. Same for resume.
done
>
> Could we also get a test checking if you start app1 & app2, one after the
> other, and if app2 comes up first, that app1 is suspended when it comes up. I
> believe your code fixes that too?
done
Michael Zanetti (mzanetti) wrote : | # |
> Nitpick not related to this MR - I noticed Dash remains suspended when greeter
> is shown, but resumed when lock-screen is revealed by swiping greeter away.
>
Right... ApplicationMana
> I think this MR fixes these bugs too:
> https:/
> https:/
> please associate them with this MR.
done.
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:248
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:252
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
Gerry Boland (gerboland) wrote : | # |
https:/
this was superceded by a branch which merged, it can be removed from the prereq list
Gerry Boland (gerboland) wrote : | # |
+ qWarning() << "Dash doesn't seem to be running... Ignoring.";
By your own edict, needs fixing :)
+ && m_sideStageAppl
possible?? I hope not
+ if (m_dashActive && application-
+ return false;
+ }
resumeApplication should have this too, no?
Test for the new dashActive property would be good. And test to make sure dash isn't changed by suspendApplication & resumeApplication
- 253. By Michael Zanetti on 2014-09-29
-
merge/update
- 254. By Michael Zanetti on 2014-09-29
-
dashActive -> forceDashActive
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:253
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:254
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
- 255. By Michael Zanetti on 2014-09-29
-
add a test for forceDashActive
Michael Zanetti (mzanetti) wrote : | # |
> + qWarning() << "Dash doesn't seem to be running... Ignoring.";
> By your own edict, needs fixing :)
>
fixed
> + && m_sideStageAppl
> possible?? I hope not
not atm, but yeah, should be according to design.
>
> + if (m_dashActive && application-
> + return false;
> + }
> resumeApplication should have this too, no?
I don't think so...
>
>
> Test for the new dashActive property would be good. And test to make sure dash
> isn't changed by suspendApplication & resumeApplication
done
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:255
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
Albert Astals Cid (aacid) wrote : | # |
Looks good codewise, need to test.
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? If not, please explain why.
No, requires other packages to build not present in the archive
* Did you make sure that the branch does not contain spurious tags?
Yes
PASSED: Continuous integration, rev:247 jenkins. qa.ubuntu. com/job/ qtmir-ci/ 66/ jenkins. qa.ubuntu. com/job/ qtmir-utopic- amd64-ci/ 66 jenkins. qa.ubuntu. com/job/ qtmir-utopic- armhf-ci/ 66
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/qtmir- ci/66/rebuild
http://