[Telepathy] Review of MissionControl spec

[Simon McVittie]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Tue, 22 Jan 2008 at 10:16:23 +0200, Alberto Mardegan wrote:
> ext Simon McVittie wrote:
> > GetConnectionStatus() -> u / signal ConnectionStatusChanged(u, u)
> > Do we really need this? Connections already signal it. If we had a
> > robust way to go from an Account to a Connection, and from a Connection
> > to an Account, clients could just do a wildcard signal match on
> > StatusChanged (ignoring for the moment dbus-glib's inability to do such
> > things - this is the wonderful world of open source, we could just add
> > that code to dbus-glib and stop working around its absence.)
> 
> While I agree on the fact that it's not strictly needed, I don't know if 
> we want to remove it; monitoring the connection status is something that 
> MC has to do anyway, so I think it makes sense to expose it.

Once the Connection is up and running, I think it's an incredibly bad
idea for clients that are actively using it *not* to watch the Connection's
status directly - as soon as the connection goes Disconnected, all handles
become invalid, all channels become useless, etc., and failing to
respond to this *will* cause problems (unexpected handle re-use).

I'm going to code this assumption into the telepathy-glib API (i.e.
make TpChannel impossible to create unless you have a TpConnection), because
otherwise the meaning and validity of handles are anyone's guess.

On the other hand, perhaps we also want connection status monitoring that
doesn't involve talking to the Connection directly, for the
benefit of things like the "how online am I?" tray applet in Maemo.

In a way, we want these to be signals on the AccountManager or something,
rather than on the Account - if their goal is to aggregate info from
multiple connections to work around dbus-glib's inability to do wildcard
signal matching, there's no point in having that information
un-aggregated!

> 1) Incoming channel from the CM (suppress_handler = 0): MC will launch 
> the channel handlers for the incoming channel.
> 
> 2) Channel that was requested through MC RequestChannel() 
> (suppress_handler = 1): MC will launch the channel handlers for the 
> outgoing channel.
> 
> 3) Channel requested directly to the CM by a client (suppress_handler = 
> 1): MC will ignore the channel completely.
> 
> I cannot think of any case where a client would want to bypass the 
> filters, but I would leave the possibility open anyways. Well, on a 
> second thought, this feature might also be used by a malicious program 
> to make calls without the user noticing it, so maybe we might want to 
> have MC process all NewChannel signals...

These are useful use-cases to think about, thanks.

I'm not sure about your assertion that outgoing channels should always have
suppress_handler = TRUE, but it would indeed make our API simpler if it
was the case... if MC became a more central part of the Telepathy stack,
such that it could be assumed to be present, then we could do that.

At the moment it's possible for a client to do a fire-and-forget channel
request with suppress_handler = FALSE - perhaps when you right-clicked on
a contact and chose "Call" it'd do so, causing your default chat client
to start up. On the other hand, nothing useful would happen here unless
MC was already running, so a more correct behaviour would be for the
client to message MC (causing auto-activation if necessary) anyway.

So, yes, I think we could probably mandate that clients MUST NOT call
RequestChannel(..., suppress_handler=FALSE) in Telepathy 0.17. Which makes
that parameter rather useless, but hey.

(A mad idea that floated past while I was pondering accounts earlier
today: how about we make the contact-list part of e.g. Empathy or Kopete
into a channel-handler for channels of type ContactList? Then if some
random app auto-launches MC with a request that brings you online, your
preferred IM app will launch (in "minimized" mode) in response to the CM
coming online! Guaranteed winning...)

Unless you're planning to implement some sort of scary security model
(SELinux, OLPC Rainbow, something like that) I don't think there's
anything useful we can do about malicious programs making calls - if
they were that malicious, they could open a TCP socket themselves and
send /dev/dsp through it.

> > UpdateParameters() -> a{sv}
> > UnsetParameters(as)
> > Do we really need this, or is setting a parameter to its CM (or preset)
> > default equivalent to unsetting it? We just don't know. Something to think
> > about.
> 
> I tend to agree that "SetParameters" + "UpdateParameters" is probably 
> the most useful combination; "UnsetParameters" can be dropped, IMHO.

In something of a U-turn, on the way home yesterday I was pondering
replacing all of these with UpdateParameters(a{sv}: set, as: unset),
which can do everything the separate methods can.

I'm not sure whether things that are explicitly set to the default
should be stored as such, or deleted. I slightly lean towards "stored", because
that way, UIs can easily implement the other behaviour if they want it.

It's unclear whether we want an updated preset or .manager to take
precedence over what the user has seen and (at least passively) approved in a
dialog box already.

> > On the subject of parameters: how do we cope if we have a value
> > specified for a parameter the CM no longer supports? How do we cope if
> > we have no value for something the CM now needs? Probably, disable the
> > account and notify the user. The former case is tricky: most of the
> > time, the user will be irritated, because the correct resolution is
> > "obvious" and they'll resent having to say "OK". However, if the parameter
> > being removed (from Gabble) is "old-ssl", silently removing it would
> > make their connection no longer secure. Loss.
> 
> The account would disappear from the AccountManager: when MC starts, it 
> will check all the accounts, and if some required parameter is missing, 
> the account is considered invalid and ignored.

I don't like this at all! Silent loss of user data considered harmful.
Can't we just disable the account/flag it as invalid/etc., and let them sort
it out and enable it? Ideally, the CM-specific account UI (that we need
anyway) would include some sort of migration helper, which could even
exploit its CM-specific knowledge to make the corrections silently and
re-enable the account, in many cases.

    Simon
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFHljawWSc8zVUw7HYRAgXwAJ0dOJps5gQZR7aZoVh0LFk5LKkTcACeIa5i
rQPfZtxxtNIbJZ3Yb3JNV1A=
=ddpo
-----END PGP SIGNATURE-----