*** Original post on bio 673 at 2011-01-31 15:52:00 UTC *** JavaScript protocols need to easily be able to instantiate new socket connections easily. Note that it might be important for a protocol to generate multiple socket connections. I'll have a patch up soon.

*** Original post on bio 673 as attmnt 512 at 2011-01-31 16:30:00 UTC *** This adds a Socket object to jsProtoHelper which takes a hostname, port, whether to use SSL, a proxy (can be null), and a separator that is used to separate packets/messages/etc.

Comment on attachment 8352253 [details] [diff] [review] Initial cut adding a Socket object to jsProtoHelper *** Original change on bio 673 attmnt 512 at 2011-01-31 16:58:27 UTC *** Great first step! I'm going to give mostly high level comments for now. The API probably needs to handle 2 different cases: - reading until a delimiter is found - reading a fixed amount of data. In the first case, you need a way to add a listener. I don't think you need the _init method, it seems most of it can be merged with the constructor code. possible API: Socket.prototype = { read: function(aLength) { // read aLength bytes from the socket and return them. What should it do when there's not enough available data? }, get available() // returns the length of available data write: function(aData) ... addListener: function(aListener, aDelimiter) { // the aDelimiter parameter is optional. If not provided, the listener will be notified each time some more data is available. }, close: function() ... How do we handled binary data that can contain \0 characters? Maybe provide a way to use typed arrays (https://developer.mozilla.org/en/JavaScript_typed_arrays)?

*** Original post on bio 673 at 2011-01-31 17:04:26 UTC *** I forgot in my previous comment: If the 2 ways to get incoming data are incompatible, it's OK to make the read method throw if it's called after an addListener call with the aSeparator parameter provided. Also mentioned on IRC: the calling code needs to have a way to know if opening the connection failed, or if the remote host closed the connection.

*** Original post on bio 673 at 2011-02-02 19:22:52 UTC *** (In reply to comment #2) > (From update of attachment 8352253 [details] [diff] [review] (bio-attmnt 512) [details]) > The API probably needs to handle 2 different cases: > - reading until a delimiter is found > - reading a fixed amount of data. > > In the first case, you need a way to add a listener. Is this any particular special object or just a function & a delimiter? For just reading a set number of bytes, I'm not sure this is a good way to do it, I think we'd still want to create a listener and push whenever that number of bytes is available? I'll catch you on IRC at some point to talk about this. > I don't think you need the _init method, it seems most of it can be merged with > the constructor code. Very true. I've removed it.

*** Original post on bio 673 at 2011-02-27 15:46:16 UTC *** I just wanted to add a note that this needs to handle proxies before we could check it in. I think it can just use the default proxy from Instantbird (since JS accounts can't have their own proxy).

*** Original post on bio 673 as attmnt 558 at 2011-03-16 23:35:00 UTC *** This version is based on the msnmsgr code from http://www.mozdev.org/source/browse/msnmsgr/src/content/socket.js?rev=1.5 (which itself is based on the chatzilla code from http://hg.mozilla.org/chatzilla/raw-file/tip/js/lib/connection-xpcom.js). I've been using this in IRC and it's working well. I'm sure it'll need some work eventually (especially with binary protocols), but it can be done as follow ups.

*** Original post on bio 673 as attmnt 559 at 2011-03-18 01:11:00 UTC *** Review was denied and comments were given on IRC by flo. This (I think) meets all of flo's comments, but he wanted to look again while awake!

Comment on attachment 8352253 [details] [diff] [review] Initial cut adding a Socket object to jsProtoHelper *** Original change on bio 673 attmnt 512 at 2011-03-18 01:11:18 UTC was without comment, so any subsequent comment numbers will be shifted ***

Comment on attachment 8352299 [details] [diff] [review] v2 *** Original change on bio 673 attmnt 558 at 2011-03-18 01:11:18 UTC was without comment, so any subsequent comment numbers will be shifted ***

*** Original post on bio 673 as attmnt 561 at 2011-03-18 10:26:00 UTC *** I meant to include an interdiff last time, hopefully I'm not too late w/ this.

Comment on attachment 8352300 [details] [diff] [review] v2.1 *** Original change on bio 673 attmnt 559 at 2011-03-18 16:11:36 UTC was without comment, so any subsequent comment numbers will be shifted ***

*** Original post on bio 673 at 2011-03-18 16:11:36 UTC *** Comment on attachment 8352300 [details] [diff] [review] (bio-attmnt 559) v2.1 Your file has DOS line endings. >diff --git a/purple/purplexpcom/src/socket.jsm b/purple/purplexpcom/src/socket.jsm >+const BinaryInputStream = >+ Components.Constructor("@mozilla.org/binaryinputstream;1", >+ "nsIBinaryInputStream", "setInputStream"); >+const BinaryOutputStream = >+ Components.Constructor("@mozilla.org/binaryinputstream;1", >+ "nsIBinaryOutputStream", "setOututStream"); >+const ScriptableInputStream = >+ Components.Constructor("@mozilla.org/scriptableinputstream;1", >+ "nsIScriptableInputStream", "init"); Please remove the tabs on these lines. >+ // Use this to add a URI scheme to the hostname when resolving the proxy, this >+ // may be unnecessary for some protocols. >+ uriScheme: "", I'm not sure this is a good default value. >+ connect: function(aHost, aPort, aSecurity, aProxy) { >+ // Array of security options >+ if (aSecurity) >+ this.security = aSecurity; >+ else >+ this.security = []; this.security = aSecurity || []; ? >+ // Choose a proxy, use the given one, otherwise get one from the proxy >+ // service >+ if (aProxy) >+ this.proxy = aProxy; The consummer provided a proxy, cool, let's do nothing? :-P I think you meant this._openStreams(aProxy); >+ else { >+ try { >+ // Attempt to get a default proxy from the proxy service. >+ let proxyService = Cc["@mozilla.org/network/protocol-proxy-service;1"] >+ .getService(Ci.nsIProtocolProxyService); >+ >+ // Add a URI scheme since, by default, some protocols (i.e. IRC) don't >+ // have a URI scheme before the host. >+ let uri = Services.io.newURI(this.uriScheme + this.host, null, null); >+ this.proxy = proxyService.asyncResolve(uri, this.proxyFlags, this); The result here is an nsICancelable. I don't think this.proxy is a good variable to store it. And you should store it (delete it in onProxyAvailable) to be able to cancel the async resolution in disconnect. >+ } catch(e) { >+ // We had some error getting the proxy service, just don't use one >+ // Open the incoming and outgoing sockets Nit: Points at the end of the sentences? >+ this._openStreams(null); >+ } >+ } >+ >+ // XXX should this call an onConnected function? Is this different from the onConnection method you added in this version of the patch? >+ isAlive: function() { >+ return (this.transport && this.transport.isAlive()); If you want to return a boolean even when this.transport is null, you need to write something like: if (!this.transport) return false; return this.transport.isAlive(); >+ // Called after a client connection is accepted when we're listening for one. >+ onSocketAccepted: function(aSocket, aTransport) { >+ this.log("onSocketAccepted"); >+ this._openStreams(); This will set this.proxy to undefined. Is this an issue? >+ onDataAvailable: function(aRequest, aContext, aInputStream, aOffset, aCount) { >+ // This will be our ArrayBuffer >+ let buffer; >+ >+ if (this.inputSegmentSize) { >+ // If we're looking for a certain amount of data >+ while (this._incomingDataBuffer.length >= this.inputSegmentSize) { >+ // If we have enough data, report it >+ buffer = new ArrayBuffer(this.inputSegmentSize); >+ >+ // Create a new ArraybufferView >+ let uintArray = new Uint8Array(buffer); >+ // Set the data into the array while saving the extra data >+ uintArray.set(this._incomingDataBuffer >+ .splice(0, this.inputSegmentSize)); >+ >+ >+ // Notify we've received data >+ this.onBinaryDataReceived(buffer); >+ } >+ } else { >+ // Send all the data we've received >+ buffer = new ArrayBuffer(data.length); >+ let uintArray = new Uint8Array(buffer); >+ uintArray.set(data); >+ >+ // Notify we've received data >+ this.onBinaryDataReceived(buffer); >+ } This is way too complicated and the second part of the if/else doesn't work (data is undefined). >+ _resetBuffers: function() { >+ if (this.binaryMode) >+ this._incomingDataBuffer = []; >+ else >+ this._incomingDataBuffer = ""; this._incomingDataBuffer = this.binaryMode ? [] : ""; ? >+ onTransportStatus: function(aTransport, aStatus, aProgress, aProgressmax) { } >+} Missing semicolon.

*** Original post on bio 673 as attmnt 562 at 2011-03-18 16:13:00 UTC was without comment, so any subsequent comment numbers will be shifted ***

*** Original post on bio 673 as attmnt 563 at 2011-03-18 22:23:00 UTC was without comment, so any subsequent comment numbers will be shifted ***

*** Original post on bio 673 as attmnt 564 at 2011-03-18 22:24:00 UTC was without comment, so any subsequent comment numbers will be shifted ***

Comment on attachment 8352300 [details] [diff] [review] v2.1 *** Original change on bio 673 attmnt 559 at 2011-03-18 22:24:04 UTC was without comment, so any subsequent comment numbers will be shifted ***

Comment on attachment 8352303 [details] [diff] [review] onDataAvailable interdiff *** Original change on bio 673 attmnt 562 at 2011-03-18 22:24:04 UTC was without comment, so any subsequent comment numbers will be shifted ***

*** Original post on bio 673 as attmnt 565 at 2011-03-18 23:19:00 UTC *** Hopefully the final version, fixes a few more nits from flo.

Comment on attachment 8352305 [details] [diff] [review] v2.2 *** Original change on bio 673 attmnt 564 at 2011-03-18 23:19:27 UTC was without comment, so any subsequent comment numbers will be shifted ***

*** Original post on bio 673 as attmnt 566 at 2011-03-18 23:20:00 UTC was without comment, so any subsequent comment numbers will be shifted ***

Comment on attachment 8352306 [details] [diff] [review] v2.3 *** Original change on bio 673 attmnt 565 at 2011-03-18 23:36:29 UTC *** Looks good! :)

*** Original post on bio 673 at 2011-03-18 23:38:42 UTC *** https://hg.instantbird.org/instantbird/rev/804ac6412670