Closed
Bug 134165
Opened 22 years ago
Closed 20 years ago
nsHttpHandler uses nsXPIDLCString for no apparent reason
Categories
(Core :: Networking: HTTP, defect)
Core
Networking: HTTP
Tracking
()
VERIFIED
FIXED
Future
People
(Reporter: Biesinger, Assigned: mikael)
Details
(Keywords: perf)
Attachments
(1 file, 1 obsolete file)
1.19 KB,
patch
|
Biesinger
:
review+
darin.moz
:
superreview+
|
Details | Diff | Splinter Review |
Almost all string members of nsHttpHandler are nsXPIDLCStrings, even though at least some of them are never used in IDL calls; they are only assigned to. Also, these strings are assigned to using .Adopt(nsCRT::strdup(...)); This could be made much faster if nsCAutoString and .Assign would be used. This is true for at least these two members: mPlatform mOscpu
Reporter | ||
Comment 1•22 years ago
|
||
Other members which can be changed: mAppname mAppVersion mPlatform mLanguage And the others should probably use .Assign too, instead of .Adopt
Comment 2•22 years ago
|
||
the reason stems from the fact that many of the UA components are set via preferences, and we use getter_Copies to get string values from prefs. of course, not all string values are set via prefs as you've noticed. using nsCAutoString here is wasteful since the values are member variables, and allocation of the UA components is only done really once during startup. this is hardly a significant issue, but it probably should be cleaned up.
Severity: normal → trivial
Status: NEW → ASSIGNED
Target Milestone: --- → Future
Assignee | ||
Updated•20 years ago
|
Assignee: darin → mikael
Status: ASSIGNED → NEW
Assignee | ||
Updated•20 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Updated•20 years ago
|
OS: Linux → All
Hardware: PC → All
Assignee | ||
Comment 3•20 years ago
|
||
Attachment #172011 -
Flags: superreview?(darin)
Attachment #172011 -
Flags: review?(cbiesinger)
Reporter | ||
Comment 4•20 years ago
|
||
Comment on attachment 172011 [details] [diff] [review] patch hm... can you make the variable names align with the other ones? compare mUserAgent. r=biesi with that change; please attach a new patch with it
Attachment #172011 -
Flags: review?(cbiesinger) → review+
Assignee | ||
Comment 5•20 years ago
|
||
Reporter | ||
Comment 6•20 years ago
|
||
Comment on attachment 172039 [details] [diff] [review] updated with indent thanks
Attachment #172039 -
Flags: review+
Assignee | ||
Updated•20 years ago
|
Attachment #172011 -
Attachment is obsolete: true
Attachment #172011 -
Flags: superreview?(darin)
Assignee | ||
Updated•20 years ago
|
Attachment #172039 -
Flags: superreview?(darin)
Comment 7•20 years ago
|
||
Comment on attachment 172039 [details] [diff] [review] updated with indent sr=darin
Attachment #172039 -
Flags: superreview?(darin) → superreview+
Reporter | ||
Comment 8•20 years ago
|
||
I checked this patch in: Checking in netwerk/protocol/http/src/nsHttpHandler.h; /cvsroot/mozilla/netwerk/protocol/http/src/nsHttpHandler.h,v <-- nsHttpHandler.h new revision: 1.44; previous revision: 1.43 done
Assignee | ||
Updated•20 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•