418535 - Make socket attachment to nsSocketTransportService usable from XPCOM

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Florian Quèze [:florian]]

Attachment: patch v1

nsISocketTransportService exposes only a single method of nsSocketTransportService that creates a new transport (with a host and port given as parameters).

nsSocketTransportService can also watch already opened sockets but the methods related to this are not exposed in the interfaces. The current consumers use this feature from the global variable gSocketTransportService but it is not accessible to XPCOM components.

Instantbird needs an access to these methods to attach the sockets opened by libpurple.

Comment 1

[Boris Zbarsky [:bzbarsky]]

I'm not likely to get to this sr in the short term.  You may want to try someone else.

Comment 2

[Florian Quèze [:florian]]

Attachment: patch v2

Removed the canAttachSocket method from the interface as it doesn't seem needed. attachSocket will return an error if the limit is already reached and then the notifyWhenCanAttachSocket method can be used to get a notification.

Calling notifyWhenCanAttachSocket without calling attachSocket first is an acceptable use of the interface so I removed the warning for this case.

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 305567 [details] [diff] [review]
patch v2

+%{ C++

remove the space

+  class nsASocketHandler;
+  struct PRFileDesc;

normally these aren't indented...

+     * This will fail if the maximum number of sockets is already reached,
+     * in this case, the notifyWhenCanAttachSocket method should be used.

mention the error code here

+    if (!CanAttachSocket()) {
+        return NS_ERROR_FAILURE;

that's a very generic code. maybe NS_ERROR_NOT_AVAILABLE would be better? (or, maybe, _WOULD_BLOCK?)


and - nsASocketHandler is currently in nsSocketTransportService2.h. That's not an exported header. Therefore, please move that class to a new nsASocketHandler.h file in netwerk/base/public and add it to the EXPORTS list in the makefile there.

r+sr=biesi with those changes

Comment 4

[Florian Quèze [:florian]]

Attachment: patch v3

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 305635 [details] [diff] [review]
patch v3

a=beltzner for 1.9

Comment 6

[Florian Quèze [:florian]]

Checking in netwerk/base/public/Makefile.in;
/cvsroot/mozilla/netwerk/base/public/Makefile.in,v  <--  Makefile.in
new revision: 1.126; previous revision: 1.125
done
RCS file: /cvsroot/mozilla/netwerk/base/public/nsASocketHandler.h,v
done
Checking in netwerk/base/public/nsASocketHandler.h;
/cvsroot/mozilla/netwerk/base/public/nsASocketHandler.h,v  <--  nsASocketHandler.h
initial revision: 1.1
done
Checking in netwerk/base/public/nsISocketTransportService.idl;
/cvsroot/mozilla/netwerk/base/public/nsISocketTransportService.idl,v  <--  nsISocketTransportService.idl
new revision: 1.30; previous revision: 1.29
done
Checking in netwerk/base/src/nsSocketTransportService2.cpp;
/cvsroot/mozilla/netwerk/base/src/nsSocketTransportService2.cpp,v  <--  nsSocketTransportService2.cpp
new revision: 1.31; previous revision: 1.30
done
Checking in netwerk/base/src/nsSocketTransportService2.h;
/cvsroot/mozilla/netwerk/base/src/nsSocketTransportService2.h,v  <--  nsSocketTransportService2.h
new revision: 1.21; previous revision: 1.20
done

Comment 7

[Florian Quèze [:florian]]

Attachment: Follow up to improve documentation

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

Checking in nsISocketTransportService.idl;
/cvsroot/mozilla/netwerk/base/public/nsISocketTransportService.idl,v  <--  nsISocketTransportService.idl
new revision: 1.31; previous revision: 1.30
done