Closed Bug 886903 Opened 7 years ago Closed 7 years ago

Send correct UserAgent for content process

Categories

(Core Graveyard :: Embedding: GRE Core, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla25

People

(Reporter: evilpie, Assigned: evilpie)

References

Details

(Whiteboard: [e10s])

Attachments

(1 file)

Attached patch v1Splinter Review
We have to send more AppInfo to the content process. (This bug is probably not in the right component)
Attachment #767329 - Flags: review?(benjamin)
Blocks: e10s-necko
Assignee: nobody → evilpies
Status: NEW → ASSIGNED
What effect does this have?
Component: Networking → Embedding: GRE Core
It sends the correct UserAgent header for http request. Without this pages like addons.mozilla.org are broken, because they don't detect Firefox.
Whiteboard: [e10s]
How much are we remoting HTTP? Couldn't we just be building the headers in the parent process instead?
Comment on attachment 767329 [details] [diff] [review]
v1

Does this bug affect B2G, or is this somehow specific to the new desktop-e10s work?

That said, I don't know how ContentChild::GetSingleton works any more or whether there are cases it can be null which would cause crashes in this code; so I'm going to defer this to somebody who hopefully does know.
Attachment #767329 - Flags: review?(benjamin) → review?(bent.mozilla)
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #4)
> Comment on attachment 767329 [details] [diff] [review]
> v1
> 
> Does this bug affect B2G, or is this somehow specific to the new
> desktop-e10s work?
> 
I am not sure, but I am pretty sure somebody would have noticed a bad UserAgent by now.
> That said, I don't know how ContentChild::GetSingleton works any more or
> whether there are cases it can be null which would cause crashes in this
> code; so I'm going to defer this to somebody who hopefully does know.
There are other cases of code that look exactly like mine and just return an other property, so I doubt that this is a problem.
To be clear, what's the UA before and after this patch?
I am going to try and find out what the difference is. On the first look the UA is the same, but addons.mozilla.org definitely reacts to this change.
Don't forget to consider HTTP headers and properties of navigator.
You are right the culprit was in Navigator. So what happens is (on amo):
This code https://addons.cdn.mozilla.net/media/js/preload-min.js?build=ecf37e2-51db415f:15 ends up calling NS_GetNavigatorUserAgent, which calls into GetUserAgent of the HTTP handler. There is a small difference between what we return with and without this patch. With it's: "Mozilla/5.0 (X11; Linux x86_64; rv:25.0) Gecko/20130710 Firefox/25.0", without however we are missing Firefox :( "Mozilla/5.0 (X11; Linux x86_64; rv:25.0) Gecko/20130710 /25.0a1".
Comment on attachment 767329 [details] [diff] [review]
v1

This patch seems fine to me.
Attachment #767329 - Flags: review?(bent.mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/86cba0042a68
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.