646512 - HttpChannelParent::OnStartRequest deep-copies nsHttpHeaderArray

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Honza Bambas (:mayhemer)]

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

Comment 1

[Jason Duell]

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.

Comment 2

[Honza Bambas (:mayhemer)]

Good first bug...

Comment 3

[Josh Aas]

Attachment: fix v1.0

Comment 4

[Josh Aas]

Fix v1.0 passes on try server.

Comment 5

[Jason Duell]

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.

Comment 6

[Jason Duell]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a6e3185f8aa9

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/a6e3185f8aa9