Code review comment for lp:~ted/ubuntu-app-launch/fdo-application-open

Revision history for this message
Ted Gould (ted) wrote :

On Wed, 2013-09-25 at 09:41 +0000, Loïc Minier wrote:

> * the variable name "connection" is confusing in get_pid_cb() and
> contact_app(), could we call it connection_name?

Fixed r76.

> * please add a TODO in main and in parse_uris() to use something else
> than space for splitting URIs; in fact it might be better to just
> remove support for multiple uris in this new code

Comment added r77. At this point I think it's better to be consistent.
I hope that, assuming fires get put out, to fix this later this week.

> So IIUC, we're first scanning by matching PID (PID that we think we've
> launched with upstart) then comparing the DBus bus name of the target
> app to match the app name; I find this a bit ugly because:
> a) pids are reused, even if it's unlikely that it happens quickly and
> at the right time, it's bad when you can't guarantee it didn't happen
> b) app names are not unique, only namespace + app name is
>
> I'd rather we use namespace + app name as the bus name we're searching
> for, and connect to that; seems simple and solid. But I guess this
> was discussed elsewhere since we have to use the same scheme in e.g.
> qtubuntu?

I think you might have misread the code slightly. We're not using any
well known names on DBus, those aren't allowed by confinement. We're
only using object paths on the bus. So it is only important that the
object path be unique for the connection, which it is likely that the
application name would be. Also, this is what is specified by the FD.o
application interface on where to put the object. They assume all
application names will become something like
"org.freedesktop.foo.desktop" eventually, and we handle that case, but
also the case of "foo.desktop".

The lack of well-known names (which really have their own set of race
conditions) is what makes us interested in PIDs. Unfortunately DBus
doesn't have a quick way to turn a PID into a connection. So we have to
get the list of connections and get their PIDs. Then we can know the
unique name of the PID of the Upstart job. And once we know the DBus
unique name for it, we can send it a message.

« Back to merge proposal