[Bug 28367] Add StreamTube interface support

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=28367

--- Comment #19 from Olli Salli <ollisal at gmail.com> 2011-03-04 08:05:17 PST ---
channel-factory.h:

 208     // When merged, Tube channels should have export/import variants too
like FT has for send/receive

You've added those methods already, so please remove this comment!

channel-class-spec.h:

 186     static ChannelClassSpec outgoingStreamTube(const QVariantMap
&additionalProperties = QVariantMap());
 187     static ChannelClassSpec incomingStreamTube(const QVariantMap
&additionalProperties = QVariantMap());

Add const QString &service = QString() as the first argument (and don't insert
the key at all if the default value was passed, rather than inserting the empty
QString). This is because most often ChannelClassSpec will be used when
specifying Handler filters, and trying to be a Handler for all possible
services on stream tubes makes no sense (as the Handler does the actual
service-specific communication over the tube), so it should be convenient to
specify the service. However, the argument shouldn't be mandatory, because it's
likely that Observers and perhaps also Approvers want to observe / approve
tubes in a generic fashion.

I also second Daniele's suggestion about changing your use of serviceName to
mean the TCP-like service the StreamTube connects to to just "service" as
above; "service name" has a strong association with the well-known names of
D-Bus services, which might be misleading in this case, as we're also going to
add D-Bus tubes.

OutgoingStreamTubeChannel:

  50     PendingOperation *offerTcpSocket(QTcpServer *server, const QVariantMap
&parameters);
  54     PendingOperation *offerUnixSocket(QLocalServer *server, const
QVariantMap &parameters,

Couldn't the Q*Server args be pointers to const?

Otherwise, it's VeryGood (tm) that the API allows passing in bare addresses,
not just QSomethingServers, as otherwise you couldn't implement much anything
with it (for example, the tube-enabled krfb uses libvncserver (a C library) to
actually listen to incoming VNC connections, and consequently there's no
QTcpServer.

This is, however, in stark contrast with...

IncomingStreamTubeChannel:

Sorry for not noticing this earlier (by earlier I mean up to a year ago when I
first looked at this!), but the same "not everything uses the Qt network
classes" requirement holds here as well.

Currently, while you're exposing the bare address in
PendingStreamTubeConnection, which is Good, you're also auto-connecting a
QTcpSocket/QLocalSocket to that address, when the tube state goes Open, which
is Bad.

Consider an application which needs to use the bare address, because they use a
protocol library which doesn't communicate over QIODevices, but rather takes
the address as a parameter and uses internal implementation means to actually
establish a socket connection accordingly. This is true of all libraries I've
used in my entire programming "career", actually, I guess because libraries
usually aim to be as generic as possible, while QIODevices require e.g. running
the Qt event loop.

Now, it might appear that the application can pass the address to the library.
Well, it can, but at that point we've already auto-connected a QTcpSocket to
that address! If the CM and remote service are gracious enough to allow
multiple connections, this might not cause more than a log entry in the remote
service like "somebody just connected to us, but then disconnected without
sending anything", but it can also prevent the real connection using the bare
address passed to the library from succeeding.

Hence, we can't auto-connect a socket as part of the accept() PendingOp.
Rather, that op should just finish successfully once the tube state goes Open,
or with a failure if the tube channel is closed before that happens. If the
former happens, it should provide access to the address. At that point, any
protocol library or the application's own implementation can be easily used to
connect using that address information.

We can still offer convenience for getting a connected QIODevice to that
address, but that needs to be in an another PendingOp you get after the first
one (getting the address) finishes. That said, I don't think it's really that
useful: it's nothing Telepathy-specific, it'd just be an abstraction to
connecting a socket which is either an UNIX socket or a TCP one.

Also I'd like to see RCCSpec and CapabilitiesBase methods for stream tubes.
These are not essential though: I'd like to see this branch finally merged and
not bitrotting further as a first priority, we can add the capability stuff in
an another quick branch.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.