Merge lp:~dandrader/qtmir/multiSessionApp into lp:qtmir
Status: | Merged |
---|---|
Approved by: | Gerry Boland on 2017-04-05 |
Approved revision: | 639 |
Merged at revision: | 638 |
Proposed branch: | lp:~dandrader/qtmir/multiSessionApp |
Merge into: | lp:qtmir |
Diff against target: |
1193 lines (+292/-239) 9 files modified
src/modules/Unity/Application/application.cpp (+146/-90) src/modules/Unity/Application/application.h (+12/-13) src/modules/Unity/Application/application_manager.cpp (+29/-58) src/modules/Unity/Application/application_manager.h (+3/-7) src/modules/Unity/Application/dbusfocusinfo.cpp (+28/-25) src/modules/Unity/Application/session.cpp (+1/-1) src/modules/Unity/Application/session_interface.h (+6/-5) tests/modules/Application/application_test.cpp (+15/-15) tests/modules/ApplicationManager/application_manager_test.cpp (+52/-25) |
To merge this branch: | bzr merge lp:~dandrader/qtmir/multiSessionApp |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Michał Sawicz | Approve on 2017-04-05 | ||
Gerry Boland (community) | 2017-03-31 | Approve on 2017-04-05 | |
Unity8 CI Bot (community) | continuous-integration | Approve on 2017-03-31 | |
Review via email:
|
Commit message
Support multiple sessions per Application
Description of the change
* Are there any related MPs required for this MP to build/function as expected? Please list.
No
* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes
* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A
Gerry Boland (gerboland) wrote : | # |
+ if (session->state() > combinedState) {
+ combinedState = session->state();
+ }
I guess this is as good a "combined" state as any.
The code, you're using the convenience to the order of the states in its enum? Strikes me as something easily broken in future - but I'm glad to see you've added a comment to make that less likely.
+void Application::die()
terminate() would match the "kill" term
Going from memory, the setApplicationPid stuff was a workaround for supporting apps that are not launched via UAL - we had to save the pid we got from Mir. Are apps launched with desktop_file_hint still working?
How are you testing this? I made this simple client which might be handy: git clone -b multiple-
Is moving the InitialSurfaceSize calls into the Application required for this MP?
Daniel d'Andrada (dandrader) wrote : | # |
On 31/03/2017 13:26, Gerry Boland wrote:
> +void Application::die()
> terminate() would match the "kill" term
Ok. Renamed.
> Going from memory, the setApplicationPid stuff was a workaround for supporting apps that are not launched via UAL - we had to save the pid we got from Mir. Are apps launched with desktop_file_hint still working?
Ooops! You're right. Being able to remove this bookkeeping of pids from
ApplicationManager was too good to be true.
Fixed.
> How are you testing this? I made this simple client which might be handy: git clone -b multiple-
That's not handy. Launching some random app from the application drawer
is handy. I tried with nautilus and mahjongg.
As a general rule: try to launch some apps from the app drawer without
this patch. You should get endless splash screens for them. Then install
this patch and try the same ones again. They should now work.
You can also add debug-level logging to surface and application
categories and check in unity8.log that such applications are indeed
getting *3* separate mir sessions each (at least the gtk ones I checked).
> Is moving the InitialSurfaceSize calls into the Application required for this MP?
That was how things were being done before the latest landing (had the
patch ready) and that sure is simpler as well as the application has
many sessions and thus entries in that InitialSurfaceSize singleton.
Application knows his own pids and his own initialSurfaceSize. So it can
update InitialSurfaceSize on his own. No need to involve
ApplicationManager in that. Don't know what you gain from it.
Furthermore, ApplicationManager no longer keeps tracks of all pids going
around. It just momentarily holds them between the moment a session is
authorized and the moment it shows up to be assigned to an Application
object.
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:638
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:639
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Gerry Boland (gerboland) wrote : | # |
<bregma> Saviq, the first way to test the multiple-connection situation would probably be to create a custom .desktop file that removes GDK_BACKEND from the app's environment, which will force it to autoprobe and use Mir instead
Saviq tried this and got http://
Michał Sawicz (saviq) wrote : | # |
Sorry, apparently GTK_BACKEND="" is incorrect, ="*" or unsetting it altogether showed GTK apps launch fine.
PASSED: Continuous integration, rev:636 /unity8- jenkins. ubuntu. com/job/ lp-qtmir- ci/652/ /unity8- jenkins. ubuntu. com/job/ build/4810 /unity8- jenkins. ubuntu. com/job/ build-0- fetch/4838 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= xenial+ overlay/ 4650 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= xenial+ overlay/ 4650/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= zesty/4650 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= zesty/4650/ artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= xenial+ overlay/ 4650 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= xenial+ overlay/ 4650/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= zesty/4650 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= zesty/4650/ artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= xenial+ overlay/ 4650 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= xenial+ overlay/ 4650/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= zesty/4650 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= zesty/4650/ artifact/ output/ *zip*/output. zip
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild: /unity8- jenkins. ubuntu. com/job/ lp-qtmir- ci/652/ rebuild
https:/