Closed
Bug 166396
Opened 22 years ago
Closed 22 years ago
embedders should be able to override the default socket type used for HTTP connections
Categories
(Core :: Networking: HTTP, defect, P2)
Core
Networking: HTTP
Tracking
()
VERIFIED
FIXED
mozilla1.2alpha
People
(Reporter: darin.moz, Assigned: darin.moz)
Details
(Keywords: arch, topembed, Whiteboard: [need-branch-approval])
Attachments
(1 file, 2 obsolete files)
4.68 KB,
patch
|
darin.moz
:
review+
rpotts
:
superreview+
chofmann
:
approval+
|
Details | Diff | Splinter Review |
embedders should be able to override the default socket type used for HTTP
connections. we should provide a preference for this.
Assignee | ||
Updated•22 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla1.2alpha
Assignee | ||
Comment 1•22 years ago
|
||
Comment 2•22 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•22 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•22 years ago
|
||
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•22 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•22 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•22 years ago
|
||
Attachment #97708 -
Attachment is obsolete: true
Assignee | ||
Comment 8•22 years ago
|
||
Comment on attachment 97785 [details] [diff] [review]
v3 patch
got an r=dougt over AIM.
Attachment #97785 -
Flags: review+
Comment 9•22 years ago
|
||
Attachment #97785 -
Flags: superreview+
Comment 10•22 years ago
|
||
Comment on attachment 97785 [details] [diff] [review]
v3 patch
a=chofmann for the trunk
Attachment #97785 -
Flags: approval+
Assignee | ||
Comment 11•22 years ago
|
||
fixed-on-trunk (for mozilla 1.2 alpha)
Assignee | ||
Updated•22 years ago
|
Whiteboard: [need-branch-approval]
Comment 12•22 years ago
|
||
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•22 years ago
|
Keywords: mozilla1.0.2+ → fixed1.0.2
Comment 13•22 years ago
|
||
verified - checked with lxr that patch was applied
Status: RESOLVED → VERIFIED
Comment 14•22 years ago
|
||
Please verify the bug. Once verified, change the keyword fixed1.0.2 to
verified1.0.2
Comment 15•22 years ago
|
||
Tom, can you do the same verification within LXR for the mozilla 1.0 branch?
Comment 16•22 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.
Description
•