Code review comment for lp:~dandrader/qtmir/multiSessionApp

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-connections https://git.launchpad.net/~gerboland/+git/basic_mir_client

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.