Closed Bug 1361500 Opened 3 years ago Closed 3 years ago

UTCToLocalStandardOffsetSeconds blocks startup with main thread IO on Windows (calling _tzset)

Categories

(Core :: JavaScript Engine, enhancement)

x86_64
Windows 10
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: florian, Assigned: dthayer)

References

(Blocks 1 open bug)

Details

(Whiteboard: [qf:p1])

Attachments

(2 files)

Bug 1330890 is adding a resist fingerprinting service which wants to call _tzset().  It seems like that would be a nice place to centralize the logic of calling _tzset off the main thread on Windows.
Whiteboard: [qf] → [qf:p1]
Assignee: nobody → dothayer
Status: NEW → ASSIGNED
Should this even be happening in the chrome process if e10s is enabled? Is there a purpose to this other than masking a user's timezone for JS running in the content process?
Flags: needinfo?(ehsan)
That is a great question!  Arthur?
Flags: needinfo?(ehsan) → needinfo?(arthuredelstein)
(In reply to :Ehsan Akhgari (needinfo please, extremely long backlog) from comment #3)
> That is a great question!  Arthur?

I think Doug is right -- for anti-fingerprinting purposes, we only need to mask the true timezone in the content process.
Flags: needinfo?(arthuredelstein)
Comment on attachment 8880888 [details]
Bug 1361500 - Don't call _tzset on startup

https://reviewboard.mozilla.org/r/152216/#review157676

Very nice!
Attachment #8880888 - Flags: review?(ehsan) → review+
Comment on attachment 8880888 [details]
Bug 1361500 - Don't call _tzset on startup

https://reviewboard.mozilla.org/r/152216/#review158454

Looks good to me too. Thanks!
Attachment #8880888 - Flags: review?(arthuredelstein) → review+
Quick update on this: it's causing non-e10s tests to fail right now, because it checks if we're e10s before we've actually loaded the profile's prefs.js, which is where the e10s defaults are overridden. This causes us to cache the wrong e10s setting and seems to break Firefox in a bunch of random ways. In any case, Arthur, are there specific requirements for when nsRFPService::Init needs to be called? For my change to work, it needs to be called before the first content process is created, but are there other reasons why it's called specifically where it's called, or does it just need to happen some time during startup?
Flags: needinfo?(arthuredelstein)
Tim, do you have any advice for Doug? Thanks.
Flags: needinfo?(arthuredelstein) → needinfo?(tihuang)
The main purpose of nsRFPService::Init() is to add the observer for 'privacy.resistFingerprinting', cache its value and update TZ value accordingly. So, this initialization should happen before any fingerprinting resistance checks for making sure every check gets a correct value. 

Most of fingerprinting resistance checks are triggered by the content, so init it before the first content process will do for them. But, there are two exceptions, the timezone setting and the window size rounding. For timezone setting, it looks good to me for you to move the initialization. For the window size rounding, if the timing of the first content process is after the creation of chrome window, and I suppose it is, then I think it will also be good.
Flags: needinfo?(tihuang)
Comment on attachment 8884106 [details]
Bug 1361500 - (2) Move e10s check after profile load

https://reviewboard.mozilla.org/r/155032/#review160104
Attachment #8884106 - Flags: review?(ehsan) → review+
Pushed by dothayer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2dbe15749c1b
Don't call _tzset on startup r=arthuredelstein,Ehsan
https://hg.mozilla.org/integration/autoland/rev/dd8944d6956b
(2) Move e10s check after profile load r=Ehsan
https://hg.mozilla.org/mozilla-central/rev/2dbe15749c1b
https://hg.mozilla.org/mozilla-central/rev/dd8944d6956b
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.