e10s HTTP: Create common AddHostHeader function

RESOLVED WONTFIX

Status

()

Core
Networking: HTTP
RESOLVED WONTFIX
8 years ago
8 years ago

People

(Reporter: jduell, Assigned: Jae-Seong Lee-Russo)

Tracking

Other Branch
x86
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

Right now we've got some logic in HttpChannelChild::Init to set the Host header (it's currently commented out) that's directly lifted from nsHttpChannel.  We should factor this out into a common function that both HttpChannelChild and nsHttpChannel can call.
(Reporter)

Updated

8 years ago
Blocks: 516730

Comment 1

8 years ago
I'll grab this one...  any preferred ways to share the code? Static method? Macro? Util-class? (I guess inheritance is not an option?)
Assignee: nobody → bjarne

Comment 2

8 years ago
Releasing this bug as requested in email.
Assignee: bjarne → nobody

Comment 3

8 years ago
Created attachment 425436 [details] [diff] [review]
Suggestion - not tested

This is a suggested way to do this. Feel free to grab it and modify.
(Assignee)

Comment 4

8 years ago
I will continue working based on the suggestion.
Assignee: nobody → lusian

Comment 5

8 years ago
(In reply to comment #4)
> I will continue working based on the suggestion.

You might want to double-check with Jason, Honza or others that this does not break other design-decisions in Electrolysis.
bug 445168 refactors this in a different way, perhaps that's good enough.

Comment 7

8 years ago
Yes Jae, you should really check with the guys driving the e10s-work whether there are any established best-practices or design patterns for such code-sharing (which I suspect will occur frequently). Using static methods may be safer since you don't risk unintentional shared state between the two subclasses. Check with Jason.
(Assignee)

Comment 8

8 years ago
Fixed by bug 445168.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Reporter)

Comment 9

8 years ago
Actually this isn't fixed:  we'd still need to call the function biesi created.

But I've decided we should simply share a common Init() class between nsHttpChannel and HttpChannelChild, so that makes this bug moot.

Jae or Bjarne: perhaps one of you could grab Bug 546581?
Resolution: FIXED → WONTFIX

Comment 10

8 years ago
Created attachment 427290 [details]
Proposed HttpChannelBase.cpp

Comment 11

8 years ago
Created attachment 427291 [details]
Proposed HttpChannelBase.h

Attached the proposed implementation of the common base-class. Can be used together with the patch as a start for bug #546581
You need to log in before you can comment on or make changes to this bug.