Closed
Bug 1078163
Opened 10 years ago
Closed 10 years ago
WorkerNavigator strings should honor general.*.override prefs in e10s mode
Categories
(Core :: DOM: Workers, defect)
Tracking
()
VERIFIED
FIXED
mozilla35
People
(Reporter: gk, Assigned: baku)
References
Details
Attachments
(1 file, 1 obsolete file)
5.90 KB,
patch
|
lmandel
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta-
|
Details | Diff | Splinter Review |
general.*.override prefs are not honored in e10s mode. Steps to reproduce: 1) Take the latest nightly on a Linux system. 2) Start it with a new profile. 3) If getting asked whether to enable e10s mode, choose "Yes". 4) After restarting open about:config and create a new pref "general.platform.override" with a value of "Win32" 5) Visit https://securehomes.esat.kuleuven.be/~gacar/dev/worker_test.html. 6) Result: The test fails as the script detects the Linux platform.
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8500419 -
Flags: review?(khuey)
I'm going to need some more explanation of what the problem was and how this fixes it.
Flags: needinfo?(amarchesini)
Assignee | ||
Comment 3•10 years ago
|
||
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #2) > I'm going to need some more explanation of what the problem was and how this > fixes it. It turned out that in RuntimeService::GetOrCreateService we set gRuntimeService when: nsRefPtr<RuntimeService> service = new RuntimeService(); if (NS_FAILED(service->Init())) { Init() finishes its task. But, in the Init we call |Preferences::RegisterCallbackAndCall()| for several preferences and many of these work with static methods such as PlatformOverrideChanged, LoadRuntimeOptions, etc etc. These methods are immediately called and many of them do: RuntimeService* rts = RuntimeService::GetService(); that returns null, because we are still into the Init(). The second issue is that Navigator::GetAcceptLanguages() assumes that the input array is empty.
Flags: needinfo?(amarchesini)
Attachment #8500419 -
Flags: review?(khuey) → review+
If you're going to change this please clean out the |gRuntimeServiceDuringInit| stuff then?
Updated•10 years ago
|
tracking-e10s:
--- → +
Assignee | ||
Comment 5•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=f892660d6fc2 pushed to try before landing
Attachment #8500419 -
Attachment is obsolete: true
Assignee | ||
Comment 6•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/8c00d59b1c00
https://hg.mozilla.org/mozilla-central/rev/8c00d59b1c00
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Comment 8•10 years ago
|
||
Reproduced the issue using the following build: - http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-10-08-03-02-02-mozilla-central/ Went through verification using the following builds: - http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-10-08-03-02-02-mozilla-central/ Test Cases: - ensured that "general.platform.override" is correctly being displayed when e10s is enabled - ensured that "general.platform.override" is correctly being displayed when e10s is disabled (normal/private/e10s) - ensured that "general.platform.override" is still being correctly displayed under about:config - ensured that "general.platform.override" is still correctly being displayed after closing/re-opening fx - ensured that updating from an older fx to the latest version fixes the original problem Went through the following OS's: - Win 8.1 x64 - PASSED - Ubuntu 14.0.4.1 x64 (VM) - PASSED - OSX 10.9.5 x64 (VM) - PASSED Should this also be uplifted into fx34 and fx33 to fix Bug #1062920? This is still happening under fx34 and fx33 (used today's builds). Once you restart the browser, you'll get: - FAIL - navigator.platform: Win32 != IntelMac Is the problem in fx34 and fx33 related to this? Or is this strictly for e10s?
Flags: needinfo?(amarchesini)
Assignee | ||
Comment 9•10 years ago
|
||
> http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-10-08-03-02-02-
> mozilla-central/
I think this patch was not in that build yet.
What I see is that the patch landed in nightly at this at this time: Wed Oct 08 23:37:42 2014 +0000
Can I ask you to try again with the current nightly? Thanks!
Flags: needinfo?(amarchesini)
Assignee | ||
Comment 10•10 years ago
|
||
Comment on attachment 8501615 [details] [diff] [review] e10s.patch Approval Request Comment [Feature/regressing bug #]: workers. [User impact if declined]: the overridden platform, appName, etc values from the preferences are not read at startup of firefox. [Describe test coverage new/current, TBPL]: TBPL is fully green. [Risks and why]: none. the patch is very simple. [String/UUID change made/needed]: none This bug is deeply connected to bug 1062920. I would like to have it uplift to aurora and beta.
Attachment #8501615 -
Flags: approval-mozilla-beta?
Attachment #8501615 -
Flags: approval-mozilla-aurora?
Comment 11•10 years ago
|
||
> I think this patch was not in that build yet.
> What I see is that the patch landed in nightly at this at this time: Wed Oct
> 08 23:37:42 2014 +0000
>
> Can I ask you to try again with the current nightly? Thanks!
I accidentally pasted the same link twice, this passed and worked fine under m-c. I'll finish up testing once the uplifts are completed for fx34 and fx33
Comment 12•10 years ago
|
||
Comment on attachment 8501615 [details] [diff] [review] e10s.patch Too late for beta as the release is next tuesday.
Attachment #8501615 -
Flags: approval-mozilla-beta? → approval-mozilla-beta-
Updated•10 years ago
|
Comment 13•10 years ago
|
||
Verified as per comment #8
Comment 14•10 years ago
|
||
(In reply to Andrea Marchesini (:baku) from comment #10) > Comment on attachment 8501615 [details] [diff] [review] > e10s.patch > > Approval Request Comment > [Feature/regressing bug #]: workers. > [User impact if declined]: the overridden platform, appName, etc values from > the preferences are not read at startup of firefox. As I understand it, this bug is an issue with e10s. As we're not going to enable e10s in Firefox 34, is there any reason to uplift this fix?
Flags: needinfo?(amarchesini)
Comment 15•10 years ago
|
||
(In reply to Lawrence Mandel [:lmandel] from comment #14) > As I understand it, this bug is an issue with e10s. As we're not going to > enable e10s in Firefox 34, is there any reason to uplift this fix? I'm going to answer my own question. This fix is required to fix bug 1062920.
Flags: needinfo?(amarchesini)
Comment 16•10 years ago
|
||
Comment on attachment 8501615 [details] [diff] [review] e10s.patch Aurora+ Andrea - We have no sheriff coverage this weekend. Can you handle the uplift before the merge on Monday?
Attachment #8501615 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: needinfo?(amarchesini)
Comment 17•10 years ago
|
||
Workahol, I haz it. https://hg.mozilla.org/releases/mozilla-aurora/rev/e96a7a4f3bbe
Flags: needinfo?(amarchesini)
Comment 18•10 years ago
|
||
Went through the verification process using the following build: - http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-10-13-00-40-02-mozilla-aurora/ I went through the same test cases from comment #8 other than testing with e10s as it's currently not available under aurora. Marking this as verified as fx33 is marked as "wontfix".
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•