Daniel Wippermann wrote:
> Hi,
>
> *** BEGIN OFF TOPIC ***
>
>> no problem... but for future reference, you can cvs add the files to
>> your local tree, and then use cvs diff -uN to generate a diff that
>> includes the new files. it'd be good to add such a diff to the tree
>> to make it easier for others to review your changes.
>
>
>
> This does not work for me. CVS states something like
> ? nsISocketListener.idl
> and exits without a diff output. It seems that I have to create my own
> local repositority, import the Mozilla tree, "cvs add" my files and
> perform the diff afterwards. But I don't know yet how to keep a local
> and the global repositories in sync...
wierd... this works for me. is this basically what you did:
% cvs co mozilla/client.mk
% cd mozilla
% gmake -f client.mk
% cp /path/to/nsISocketListener.idl netwerk/base/public
% cd netwerk/base/public
% cvs add nsISocketListener.idl
% cvs diff -uN > diff
>
> *** END OFF TOPIC ***
>
>> it almost seems like you'd want to have an API to configure the socket
>> transport service to listen for incoming connections, the result of
>> which would be a nsISocketTransport object. so instead of using a
>> nsISocketTranport for the listener socket, we'd use some other
>> interface/object. it just doesn't feel right to clutter
>> nsISocketTransport, which represents a data pipe, with listen/accept
>> operations.
>>
>> i almost think we need an api resembling:
>>
>> nsISocketTransportService {
>> nsIServerSocket createServerSocket(in long port);
>> };
>>
>> nsIServerSocket {
>> void asyncAccept(in nsIServerSocketListener listener, in
>> nsISupports ctxt);
>> };
>>
>> nsIServerSocketListener {
>> void onNewConnection(in nsIServerSocket server, in nsISupports
>> ctxt, in nsISocketTransport connection, nsresult status);
>> };
>>
>> the nsIServerSocket would still need to participate in the same
>> PR_Poll loop, which would mean changes to nsSocketTransportService to
>> work with something other than just nsSocketTransport objects.
>
>
>
> The main reason for the aproach I made was to minimize alternations in
> the exisiting interfaces (I'm a newbie in sofware development of this
> magnitude). I also considered using nsISocketTransportService, but I
> thought the need of extending the work queue / PR_Poll loop would lead
> in too radical modifications.
understandable for sure.
>
> But I agree with your opinion to use the transport service for this.
> This eliminates the need to create a socket only to switch it into
> listening mode. Could the nsSocketTransport object implement the
> nsIServerSocket interface? This could reduce the alternations in the
> work queue.
probably, however a common base class would probably be more
appropriate, since nsSocketTransport allocates a bunch of stuff that
wouldn't be needed by a listening socket.
>
> I don't believe that an extra method "asyncAccept" is still needed. If
> you call "createServerSocket" the socket could be created, bound to the
> port and made accepting incoming connections. Or dou you have a
> situation in mind, where it is vital to separate this two steps?
yeah, i think you're right... we should try to combine these two things.
>
> Furthermore there should be a method to close the socket. In the case of
> the PORT command in the FTP protocol the server socket should only be
> used once for the corresponding data connection.
>
> The interfaces could look like this
>
> nsISocketTransportService {
> ... // existing functionality
> nsIServerSocket createServerSocket(in long port, in
> nsIServerSocketListener listener, in nsISupports ctxt);
> };
>
> nsIServerSocket {
> void close ();
> };
>
> nsIServerSocketListener {
> void onNewConnection(in nsIServerSocket server, in nsISupports ctxt,
> in nsISocketTransport connection, nsresult status);
> };
>
>
> Daniel
>
we could actually eliminate the need for creating a nsIServerSocket
interface by just using nsIRequest.
nsIRequest::status - current status, NS_FAILED if there was a problem
nsIRequest::isPending - TRUE if polling for incoming connections
nsIRequest::cancel - to stop polling
nsIRequest::suspend - to temporarily stop polling
nsIRequest::resume - to resume polling after being suspended
nsIRequest::loadGroup - ignored
nsIRequest::loadFlags - ignored
then, we'd have something like this:
nsISocketTransportService {
// ...
nsIRequest createServerSocket(in long port,
in nsIServerSocketListener listener,
in nsISupports ctxt);
};
I'm not sure if createServerSocket is the best name for this function...
might be nice to have something resembling asyncAccept or something like
that.
Also, I don't think there is any need to write code for
nsServerSocketListenerProxy... we can just use GetProxyForObject from
the nsIProxyObjectManager. take a look at how nsIProgressEventSink
messages are proxied to the UI thread. nsStreamListenerProxy is special
cased because it actually needs to do more than just execute the calls
on a separate thread.
I would also just declare nsIServerSocketListener in
nsISocketTransportService, instead of adding a new file for it.
darin
