Make socket attachment to nsSocketTransportService usable from XPCOM

RESOLVED FIXED in mozilla1.9beta4

Status

()

defect
RESOLVED FIXED
12 years ago
9 years ago

People

(Reporter: florian, Assigned: florian)

Tracking

Trunk
mozilla1.9beta4
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

Posted patch patch v1 (obsolete) — Splinter Review
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.
Attachment #304362 - Flags: superreview?(bzbarsky)
Attachment #304362 - Flags: review?(cbiesinger)
I'm not likely to get to this sr in the short term.  You may want to try someone else.
Attachment #304362 - Flags: superreview?(bzbarsky) → superreview?(cbiesinger)
Posted patch patch v2 (obsolete) — Splinter Review
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.
Attachment #304362 - Attachment is obsolete: true
Attachment #304362 - Flags: superreview?(cbiesinger)
Attachment #304362 - Flags: review?(cbiesinger)
Attachment #305567 - Flags: superreview?(cbiesinger)
Attachment #305567 - Flags: review?(cbiesinger)
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
Attachment #305567 - Flags: superreview?(cbiesinger)
Attachment #305567 - Flags: superreview+
Attachment #305567 - Flags: review?(cbiesinger)
Attachment #305567 - Flags: review+
Summary: Make nsSocketTransportService usable from XPCOM → Make socket attachment to nsSocketTransportService usable from XPCOM
Posted patch patch v3Splinter Review
Attachment #305567 - Attachment is obsolete: true
Attachment #305635 - Flags: approval1.9?
Comment on attachment 305635 [details] [diff] [review]
patch v3

a=beltzner for 1.9
Attachment #305635 - Flags: approval1.9? → approval1.9+
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
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9beta4
Checking in nsISocketTransportService.idl;
/cvsroot/mozilla/netwerk/base/public/nsISocketTransportService.idl,v  <--  nsISocketTransportService.idl
new revision: 1.31; previous revision: 1.30
done
You need to log in before you can comment on or make changes to this bug.