embedders should be able to override the default socket type used for HTTP connections

VERIFIED FIXED in mozilla1.2alpha

Status

()

P2
normal
VERIFIED FIXED
16 years ago
16 years ago

People

(Reporter: darin.moz, Assigned: darin.moz)

Tracking

({arch, topembed})

Trunk
mozilla1.2alpha
arch, topembed
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [need-branch-approval])

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

16 years ago
embedders should be able to override the default socket type used for HTTP
connections.  we should provide a preference for this.
(Assignee)

Updated

16 years ago
Status: NEW → ASSIGNED
Keywords: arch, mozilla1.2, topembed
Priority: -- → P2
Target Milestone: --- → mozilla1.2alpha
(Assignee)

Comment 1

16 years ago
Created attachment 97611 [details] [diff] [review]
v1 patch

Comment 2

16 years ago
Comment on attachment 97611 [details] [diff] [review]
v1 patch


Do you have any fallback plans?  For example, suppose that a pref pointing to a
socket transport provider is left but the provider is removed from disk.

Also, what about overriding the default http SSL socket?
Attachment #97611 - Flags: review+
(Assignee)

Comment 3

16 years ago
good point... this could definitely could be expanded upon, and a sanity check
that a nsISocketProvider exists corresponding to the preference value is
probably needed.  ok, i'll rev the patch.  as for SSL, i think we should prefer
our builtin code for that.  i don't think we want to support an external SSL
socket implementation just yet.  anyways, it could always use XPCOM to make
itself the nsISocketProvider implementation of ssl.  this pref is about using a
socket type when none would normally be used.
(Assignee)

Comment 4

16 years ago
Created attachment 97708 [details] [diff] [review]
v2 patch

same patch, but now we try to instantiate the specified socket provider to
verify that the value provided in the preferences actually works.  if not, then
we fallback on the default setting (which is NULL).
Attachment #97611 - Attachment is obsolete: true

Comment 5

16 years ago
Comment on attachment 97708 [details] [diff] [review]
v2 patch

what about the ssl override?


     if (mConnectionInfo->UsingSSL())
	 ...GetDefaultSSLType...

also, what if the pref is set but it is empty.	You should do something like:
+	 if (NS_SUCCEEDED(rv) && val)
(Assignee)

Comment 6

16 years ago
can preferences really return NS_OK with a NULL result?  despite that, your
comment made me think of another problem... if someone wanted to reset the
preference to its default value, they'd want to call SetCharPref(..., "") with
an empty string value.  but, the pref observer code would fail to modify
mDefaultSocketType on account of the fact that "" is not a valid socket type. 
oops!  new patch coming up ;-)
(Assignee)

Comment 7

16 years ago
Created attachment 97785 [details] [diff] [review]
v3 patch
Attachment #97708 - Attachment is obsolete: true
(Assignee)

Comment 8

16 years ago
Comment on attachment 97785 [details] [diff] [review]
v3 patch

got an r=dougt over AIM.
Attachment #97785 - Flags: review+

Comment 9

16 years ago
Comment on attachment 97785 [details] [diff] [review]
v3 patch

sr=rpotts@netscape.com
Attachment #97785 - Flags: superreview+

Comment 10

16 years ago
Comment on attachment 97785 [details] [diff] [review]
v3 patch

a=chofmann for the trunk
Attachment #97785 - Flags: approval+
(Assignee)

Comment 11

16 years ago
fixed-on-trunk (for mozilla 1.2 alpha)
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Keywords: mozilla1.0.2
Resolution: --- → FIXED
(Assignee)

Updated

16 years ago
Whiteboard: [need-branch-approval]
a=rjesup@wgate.com for 1.0 branch.  Change mozilla1.0.2+ to fixed1.0.2 when
checked in 
Keywords: mozilla1.0.2 → mozilla1.0.2+
(Assignee)

Updated

16 years ago
Keywords: mozilla1.0.2+ → fixed1.0.2

Comment 13

16 years ago
verified - checked with lxr that patch was applied
Status: RESOLVED → VERIFIED

Comment 14

16 years ago
Please verify the bug. Once verified, change the keyword fixed1.0.2 to
verified1.0.2 

Comment 15

16 years ago
Tom, can you do the same verification within LXR for the mozilla 1.0 branch?

Comment 16

16 years ago
yes, verified patch applied to 1.0 branch
Keywords: fixed1.0.2 → verified1.0.2
You need to log in before you can comment on or make changes to this bug.