Closed Bug 646512 Opened 9 years ago Closed 9 years ago

HttpChannelParent::OnStartRequest deep-copies nsHttpHeaderArray

Categories

(Core :: Networking: HTTP, defect)

x86
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla12

People

(Reporter: mayhemer, Assigned: jaas)

Details

Attachments

(1 file)

17.18 KB, patch
jduell.mcbugs
: review+
Details | Diff | Splinter Review
See [1].  I don't think this is made purposely.  Just a typo - forgotten & at the declaration.  We might get a small perf/memory gain here.

As part of this bug, I'm curious why we use RequestHeaderTuples (that we have to copy the headers into) instead of nsHttpHeaderArray directly for which both we have ParamTraits defined.  But maybe I just forgot why we don't.

Fixing that is related to how/when bug 597706 and bug 646507 gets fixed.

[1] http://hg.mozilla.org/mozilla-central/diff/3a794e272196/netwerk/protocol/http/HttpChannelParent.cpp
You're right--we could use a reference to avoid a copy here.

And actually we could pass nsHttpHeaderArray directly to IPDL here, as you suggest.  We use the Tuple class when going from the child->parent, in order to set 'merge' correctly (the idea being that it's generally cheaper to send a log of the SetHeader calls the necko client has made, rather than copy the entire header array including default headers that the parent creates for us anyway).  But we don't need it in the parent->child direction.
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
Good first bug...
Assignee: honzab.moz → nobody
Status: ASSIGNED → NEW
Assignee: nobody → joshmoz
Attached patch fix v1.0Splinter Review
Attachment #589661 - Flags: review?(jduell.mcbugs)
Fix v1.0 passes on try server.
Comment on attachment 589661 [details] [diff] [review]
fix v1.0

Review of attachment 589661 [details] [diff] [review]:
-----------------------------------------------------------------

Very nice!  Thanks.  I'm adding a comment to nsIHttpChannelChild.idl describing the clientSetHeaders field, and then I'll land it.
Attachment #589661 - Flags: review?(jduell.mcbugs) → review+
https://hg.mozilla.org/mozilla-central/rev/a6e3185f8aa9
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
You need to log in before you can comment on or make changes to this bug.