Last Comment Bug 646512 - HttpChannelParent::OnStartRequest deep-copies nsHttpHeaderArray
: HttpChannelParent::OnStartRequest deep-copies nsHttpHeaderArray
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: Trunk
: x86 Windows 7
-- normal (vote)
: mozilla12
Assigned To: Josh Aas
: Patrick McManus [:mcmanus]
Depends on:
  Show dependency treegraph
Reported: 2011-03-30 09:45 PDT by Honza Bambas (:mayhemer)
Modified: 2012-01-21 07:11 PST (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

fix v1.0 (17.18 KB, patch)
2012-01-18 14:54 PST, Josh Aas
jduell.mcbugs: review+
Details | Diff | Splinter Review

Description User image Honza Bambas (:mayhemer) 2011-03-30 09:45:47 PDT
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.

Comment 1 User image Jason Duell [:jduell] (needinfo me) 2011-04-18 11:33:54 PDT
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 User image Honza Bambas (:mayhemer) 2012-01-18 08:15:50 PST
Good first bug...
Comment 3 User image Josh Aas 2012-01-18 14:54:18 PST
Created attachment 589661 [details] [diff] [review]
fix v1.0
Comment 4 User image Josh Aas 2012-01-18 19:51:42 PST
Fix v1.0 passes on try server.
Comment 5 User image Jason Duell [:jduell] (needinfo me) 2012-01-19 13:50:08 PST
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 User image Jason Duell [:jduell] (needinfo me) 2012-01-19 14:27:43 PST
Comment 7 User image Ed Morley [:emorley] 2012-01-21 07:11:27 PST

Note You need to log in before you can comment on or make changes to this bug.