Here's a patch from Tor Browser for possible uplift: https://torpat.ch/16337 The original ticket is at https://trac.torproject.org/16337

Oh right. Should we reuse Performance::RoundTime() [1] somehow? [1] https://hg.mozilla.org/mozilla-central/file/eb1d92b2b6a4/dom/performance/Performance.cpp#l264

I think we should extend the approach in Bug 1217238 to cover the Animation API. The implementation is https://hg.mozilla.org/mozilla-central/rev/1a7356fac9ba and regression tests are https://hg.mozilla.org/mozilla-central/rev/e967714a341a

Comment on attachment 8899307 [details] Bug 1382545 - Part 1: Rounding the time of Animation API to 100ms when 'privacy.resistFingerprinting'is true. I think birtles should review this.

Comment on attachment 8899307 [details] Bug 1382545 - Part 1: Rounding the time of Animation API to 100ms when 'privacy.resistFingerprinting'is true. https://reviewboard.mozilla.org/r/170530/#review176174 There's a general invariant that when an animation is in play: current time = (timeline time - start time) × playback rate It seems to me that with this patch that relationship might not hold and content that relies on that holding could suffer minor glitches. Likewise, anything using those values for synchronization will not be able to do a very good job. Then there's the fact that setting any of these values to a non-rounded value and then getting it back will give a different result which is a little counter-intuitive, could break content (unlikely) and makes it very obvious you've flipped this pref (not a problem I expect). I'm not aware of the broader picture of fingerprinting here, what sort of attacks are foreseen, and what sort content breakage is acceptable, but I trust you are and assuming this is ok with you the code changes seem fine to me (except for the one minor comment below). ::: dom/animation/AnimationUtils.h:36 (Diff revision 1) > { > dom::Nullable<double> result; > > if (!aTime.IsNull()) { > - result.SetValue(aTime.Value().ToMilliseconds()); > + result.SetValue( > + nsRFPService::ReduceTimePrecisionAsMSecs(aTime.Value().ToMilliseconds()) Any chance you could re-factor ReduceTimePrecisionAsMSecs so that the !IsResistFingerprintingEnabled() check is inline (so there's minimal overhead in the 99.9% case where that setting is not in-effect). And the method should probably be renamed MaybeReduceTimePrecisionAsMSecs too.

Comment on attachment 8899308 [details] Bug 1382545 - Part 2: Add a test case for making sure that Animation API doesn't expose a high resolution time when 'privacy.resistFingerprinting' is true. https://reviewboard.mozilla.org/r/170532/#review176176 ::: browser/components/resistfingerprinting/test/mochitest/test_animation_api.html:17 (Diff revision 1) > + > + /** Test for Bug 1382545 **/ > + SimpleTest.waitForExplicitFinish(); > + > + window.onload = () => { > + SimpleTest.waitForFocus(() => { Curious, any need to wait for focus? Or for 'load' for that matter?) ::: browser/components/resistfingerprinting/test/mochitest/test_animation_api.html:21 (Diff revision 1) > + window.onload = () => { > + SimpleTest.waitForFocus(() => { > + SpecialPowers.pushPrefEnv({"set": > + [ > + ["privacy.resistFingerprinting", true], > + ["dom.animations-api.core.enabled", true] I'm not totally sure this works. I think for prefs that affect the Element interface, we need to tweak them before creating any Element instances (see bug 1159743 comment 4). Previously we've had to work around this using a separate opener file although as of bug 1328830 you can set the pref in the mochitest.ini but only for all tests in manifest (which probably doesn't make sense here). ::: browser/components/resistfingerprinting/test/mochitest/test_animation_api.html:28 (Diff revision 1) > + let isRounded = x => (Math.floor(x/100)*100) === x; > + let testDiv = document.getElementById("testDiv"); > + let animation = testDiv.animate({ opacity: [0,1] }, 100000); (nit: s/let/const/) ::: browser/components/resistfingerprinting/test/mochitest/test_animation_api.html:33 (Diff revision 1) > + SimpleTest.waitForCondition( > + () => animation.currentTime > 1000, > + function () { 1s seems like a long time to wait. It means this test adds 1s to every test run ever. Can we make this a little shorter? ::: browser/components/resistfingerprinting/test/mochitest/test_animation_api.html:54 (Diff revision 1) > + } > + > + </script> > +</head> > +<body> > +<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=">Mozilla Bug </a> Not sure you need this line, but if you do you probably want to add some bug numbers to it.

Comment on attachment 8899307 [details] Bug 1382545 - Part 1: Rounding the time of Animation API to 100ms when 'privacy.resistFingerprinting'is true. https://reviewboard.mozilla.org/r/170530/#review177136

Comment on attachment 8899308 [details] Bug 1382545 - Part 2: Add a test case for making sure that Animation API doesn't expose a high resolution time when 'privacy.resistFingerprinting' is true. https://reviewboard.mozilla.org/r/170532/#review177140 r+, pending the issues identified by Brian.

(In reply to Brian Birtles (:birtles) from comment #6) > > Any chance you could re-factor ReduceTimePrecisionAsMSecs so that the > !IsResistFingerprintingEnabled() check is inline (so there's minimal > overhead in the 99.9% case where that setting is not in-effect). > > And the method should probably be renamed MaybeReduceTimePrecisionAsMSecs > too. Sure, I will open a follow-up bug for this, thanks.

Comment on attachment 8899308 [details] Bug 1382545 - Part 2: Add a test case for making sure that Animation API doesn't expose a high resolution time when 'privacy.resistFingerprinting' is true. https://reviewboard.mozilla.org/r/170532/#review176176 > Curious, any need to wait for focus? Or for 'load' for that matter?) It is not really necessary here, so I remove this.

Try looks fine. https://treeherder.mozilla.org/#/jobs?repo=try&revision=bb557db98cee

Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/e2213726a3d3 Part 1: Rounding the time of Animation API to 100ms when 'privacy.resistFingerprinting'is true. r=arthuredelstein,birtles https://hg.mozilla.org/integration/autoland/rev/ad3bc2177e89 Part 2: Add a test case for making sure that Animation API doesn't expose a high resolution time when 'privacy.resistFingerprinting' is true. r=arthuredelstein,birtles

https://hg.mozilla.org/mozilla-central/rev/e2213726a3d3 https://hg.mozilla.org/mozilla-central/rev/ad3bc2177e89