Code review comment for lp:~nick-dedekind/mir/trusted_sessions

> Looking at the client API:
>
> MirTrustSession* mir_connection_create_trust_session(MirConnection*
> connection);
>
> I would expect "create_trust_session" to take parameters (such as the event
> callback) and create the server-side trust session (which would implicitly
> make it an async call).
>
> AIUI this just creates a client-side object that still needs to be initialized
> before the server-side trust session is created using
> "mir_trust_session_start".
>
> Is there any reason to separate out these create/set_event_callback/start
> calls?

I guess not anymore. Previously it would be possible to start and stop trust sessions multiple times; but I think with the "post-start" adding of participants, it's no longer a possibility.
I've merged all of these into a mir_connection_start_trust_session which async returns a MirTrustSession.
Not sure if the stop/release should be separate now. Left as is for now.

>
> ~~~~
>
> mir_trust_session_add_trusted_session
>
> This is an async call - so presumably it is expected to be invoked after the
> trust session is started?
>
> Does that imply a round trip for each pid added to the trust session? If so
> would it be better to accept a list of pids?
>
> Can it be called on a created (but not started) trust session?
>

It does have a round trip, but as I don't think it's an issue. We call the start_trust_session with the pid of the "source" participant (whatever is "using" the trust helper. eg. gallery application). From there, thinking about the flow, we would be layering the trust as the user makes decisions (eg. content hub "share with google+", then online accounts "log in to google+"). So practically speaking we only do one layer of trust at a time.

> ~~~~
>
> Is there a missing mir_trust_session_remove_trusted_session() for revoking
> trust?

Closing a session will remove it from the server trust session, but I'm not sure about the helper explicitly removing it. For the moment at least, I think we can do without it.

>
> ~~~~
>
> mir_trust_session_set_event_callback
>
> The only state changes appear to be "start" and "stop" - which are under the
> control of the client. So does this do any more than duplicate the
> functionality of the WaitHandles returned by the start/stop functions?

The server can also stop an active trust session (if the application closes, we don't want it hanging around). Also, for first iteration, we want to stop the trust session when switching away from the app.
So we need the helper to get a notification of this.