HttpChannelParent::OnStartRequest deep-copies nsHttpHeaderArray

RESOLVED FIXED in mozilla12

Status

()

Core
Networking: HTTP
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: mayhemer, Assigned: Josh Aas)

Tracking

Trunk
mozilla12
x86
Windows 7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
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.
(Reporter)

Updated

6 years ago
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
(Reporter)

Comment 2

6 years ago
Good first bug...
Assignee: honzab.moz → nobody
Status: ASSIGNED → NEW
(Assignee)

Updated

6 years ago
Assignee: nobody → joshmoz
(Assignee)

Comment 3

6 years ago
Created attachment 589661 [details] [diff] [review]
fix v1.0
(Assignee)

Updated

6 years ago
Attachment #589661 - Flags: review?(jduell.mcbugs)
(Assignee)

Comment 4

6 years ago
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/integration/mozilla-inbound/rev/a6e3185f8aa9

Comment 7

6 years ago
https://hg.mozilla.org/mozilla-central/rev/a6e3185f8aa9
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
You need to log in before you can comment on or make changes to this bug.