1069401 - UserAgent cannot be changed for specific websites in workers

Status: RESOLVED FIXED

(Core :: DOM: Workers, defect)

Description

[Andrea Marchesini [:baku]]

Currently navigator.userAgent value can be changed using nsISiteSpecificUserAgent and properties as such as general.useragent.site_specific_overrides and general.useragent.override.*.

This feature should be supported in workers too.

Comment 1

[Andrea Marchesini [:baku]]

Attachment: useragent.patch

It turned out that we ignored general.useragent.override in the main-thread too.
Just because we can have overridden values per site, we cannot use the standard approach to retrieve prefs in workers.

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 8491691 [details] [diff] [review]
useragent.patch

Review of attachment 8491691 [details] [diff] [review]:
-----------------------------------------------------------------

I assume that you've tested that this behaves sanely in a worker created from a JSM/JS component?

::: dom/base/Navigator.cpp
@@ +2550,5 @@
> +  MOZ_ASSERT(NS_IsMainThread());
> +
> +  if (!nsContentUtils::IsCallerChrome()) {
> +    const nsAdoptingString& override =
> +      mozilla::Preferences::GetString("general.useragent.override");

Is a fully qualified mozilla:: really necessary here?

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 8491691 [details] [diff] [review]
useragent.patch

Review of attachment 8491691 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/workers/Navigator.cpp
@@ +366,5 @@
> +
> +} // anonymous namespace
> +
> +void
> +WorkerNavigator::GetUserAgent(nsString& aUserAgent) const

We should cache here, rather than go to the main thread every time.

Comment 4

[Andrea Marchesini [:baku]]

> We should cache here, rather than go to the main thread every time.

We don't need to do that because the attribute is marked as [Cached] in the WebIDL. The bindings do the magic.

I'll write a test for worker in JSM.

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Ah, nice.  Magic ftw.

Comment 6

[Andrea Marchesini [:baku]]

Attachment: id.patch

bz, can you take a look at this change?
I need language to be cached and to have the ClearCachedLanguageValue() method.

Comment 7

[Andrea Marchesini [:baku]]

Attachment: useragent.patch

https://tbpl.mozilla.org/?tree=Try&rev=4c22a5afaeef

Comment 8

[Andrea Marchesini [:baku]]

Attachment: id2.patch

khuey, a quick review on this interdiff? A test was failing with the previous patch.

Comment 9

[Andrea Marchesini [:baku]]

Attachment: useragent.patch

https://tbpl.mozilla.org/?tree=Try&rev=5b9797c9b9db

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8493621 [details] [diff] [review]
id.patch

r=me especially if you add a comment to the IDL about why it's cached.

Comment 11

[Andrea Marchesini [:baku]]

Last push to try before landing:

https://tbpl.mozilla.org/?tree=Try&rev=6f38ed97475d

Comment 12

[Andrea Marchesini [:baku]]

Attachment: useragent.patch

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/13028d2039e8

Comment 14

[Carsten Book [:Tomcat]]

sorry had to backout this change for test failures like https://tbpl.mozilla.org/php/getParsedLog.php?id=48754884&tree=Mozilla-Inbound

Comment 15

[Andrea Marchesini [:baku]]

Attachment: useragent.patch

it turned out that on b2g desktop the platform can be an empty string.

https://tbpl.mozilla.org/?tree=Try&rev=63fd93df5ddb

Comment 16

[Andrea Marchesini [:baku]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4f72287c59ca

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4f72287c59ca

Comment 18

[Andrea Marchesini [:baku]]

We should uplift this patch because it's similar to bug 1062920

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Happy to do so once it has approval ;)

Comment 20

[Andrea Marchesini [:baku]]

Comment on attachment 8494445 [details] [diff] [review]
useragent.patch

Approval Request Comment
[Feature/regressing bug #]: 925847
[User impact if declined]: The overridden prefs for userAgent will be ignored in workers.
[Describe test coverage new/current, TBPL]: https://tbpl.mozilla.org/?tree=Try&rev=63fd93df5ddb
[Risks and why]: The patch is not complex and it has good mochitests.
[String/UUID change made/needed]: none

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/bc8aad860662
https://hg.mozilla.org/releases/mozilla-beta/rev/e178848e43d1

Comment 22

[Boris Zbarsky [:bzbarsky]]

> r=me especially if you add a comment to the IDL about why it's cached.

This comment never got added.  Or rather, a comment got added, but it doesn't explain the _why_.