1443943 - Only Jitter Timestamps in non-System Principal Code

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Tom Ritter [:tjr] (OOTO until April)]

We can avoid a slew of debug output and improve performance by only clamping + jittering timestamps that are exposed to web content. (Jitter is the expensive one of the two.)

Comment 1

[Tom Ritter [:tjr] (OOTO until April)]

Attachment: Bug 1443943 Move LRUCache Initialization to startup rather than lazily

Before, we would initialize LRUCache on the first instance of
calling the Timer Precision Reduction functions. We would both
allocate and initialize it, and call ClearOnShutdown.

ClearOnShutdown can only be called on the Main Thread, but it
just so happened that we always did that, so there was no
problem. Now that we are not calling precision reduction for
system callers, we were initializing on a non-main-thread and
we need to avoid that.

In the future, we could reduce memory use IF we are not using
the timer precision reduction functions by figuring out how
to initialize this lazily but still on the main thread. For
now, because we are using the timer precision reduction
functions, doing so would not save us any memory.

Review commit: https://reviewboard.mozilla.org/r/226560/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/226560/

Comment 2

[Tom Ritter [:tjr] (OOTO until April)]

Attachment: Bug 1443943 Port the performance APIs only to only clamping/jittering on non-System Principal

Review commit: https://reviewboard.mozilla.org/r/226562/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/226562/

Comment 3

[Tom Ritter [:tjr] (OOTO until April)]

Attachment: Bug 1443943 Do not clamp/jitter in the JS Engine if it's system context

Review commit: https://reviewboard.mozilla.org/r/226564/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/226564/

Comment 4

[Tom Ritter [:tjr] (OOTO until April)]

Attachment: Bug 1443943 Do not clamp/jitter event timestamps if it's a system principal

This patch has an annoying gap in it. I follow the pointer chain
up like so: mPresContext->Document()->NodePrincipal()

However, sometimes either mPresContext or mPresContext->Document()
is null. I don't know why.

Review commit: https://reviewboard.mozilla.org/r/226566/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/226566/

Comment 5

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957653 [details]
Bug 1443943 Do not clamp/jitter event timestamps if it's a system principal

Nika, would you know why the pointer chain I'm following in Event.cpp may null out on me, and if the approach I'm taking is okay? I wonder if it's actually the case that if the pointer chain is not in-tact, then we _are_ a System Caller, but this approach is more conservative...

Comment 6

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957650 [details]
Bug 1443943 Move LRUCache Initialization to startup rather than lazily

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226560/diff/1-2/

Comment 7

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957651 [details]
Bug 1443943 Port the performance APIs only to only clamping/jittering on non-System Principal

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226562/diff/1-2/

Comment 8

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957652 [details]
Bug 1443943 Do not clamp/jitter in the JS Engine if it's system context

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226564/diff/1-2/

Comment 9

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957653 [details]
Bug 1443943 Do not clamp/jitter event timestamps if it's a system principal

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226566/diff/1-2/

Comment 10

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957653 [details]
Bug 1443943 Do not clamp/jitter event timestamps if it's a system principal

This patch moves some functions around in a way that doesn't seem required. It's not technically required here, but will be needed for Bug 1443943

Comment 11

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957650 [details]
Bug 1443943 Move LRUCache Initialization to startup rather than lazily

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226560/diff/2-3/

Comment 12

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957651 [details]
Bug 1443943 Port the performance APIs only to only clamping/jittering on non-System Principal

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226562/diff/2-3/

Comment 13

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957652 [details]
Bug 1443943 Do not clamp/jitter in the JS Engine if it's system context

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226564/diff/2-3/

Comment 14

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957653 [details]
Bug 1443943 Do not clamp/jitter event timestamps if it's a system principal

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226566/diff/2-3/

Comment 15

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957650 [details]
Bug 1443943 Move LRUCache Initialization to startup rather than lazily

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226560/diff/3-4/

Comment 16

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957651 [details]
Bug 1443943 Port the performance APIs only to only clamping/jittering on non-System Principal

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226562/diff/3-4/

Comment 17

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957652 [details]
Bug 1443943 Do not clamp/jitter in the JS Engine if it's system context

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226564/diff/3-4/

Comment 18

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957653 [details]
Bug 1443943 Do not clamp/jitter event timestamps if it's a system principal

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226566/diff/3-4/

Comment 19

[Tom Ritter [:tjr] (OOTO until April)]

mozreview clearing flags on me...

Comment 20

[Andrea Marchesini [:baku]]

Comment on attachment 8957651 [details]
Bug 1443943 Port the performance APIs only to only clamping/jittering on non-System Principal

https://reviewboard.mozilla.org/r/226562/#review232540

::: dom/performance/Performance.cpp:99
(Diff revision 4)
>  Performance::Now() const
>  {
>    TimeDuration duration = TimeStamp::Now() - CreationTimeStamp();
> -  return RoundTime(duration.ToMilliseconds());
> +  DOMHighResTimeStamp minimallyClamped = RoundTime(duration.ToMilliseconds());
> +
> +  if (mSystemPrincipal)

{}

Comment 21

[Andrea Marchesini [:baku]]

Comment on attachment 8957652 [details]
Bug 1443943 Do not clamp/jitter in the JS Engine if it's system context

https://reviewboard.mozilla.org/r/226564/#review232542

You must ask a JS peer to review this patch.

Comment 22

[Andrea Marchesini [:baku]]

Comment on attachment 8957653 [details]
Bug 1443943 Do not clamp/jitter event timestamps if it's a system principal

https://reviewboard.mozilla.org/r/226566/#review232544

::: dom/events/Event.cpp:1101
(Diff revision 4)
>  
>  double
> -Event::TimeStampImpl() const
> +Event::TimeStamp()
>  {
> +  bool isSystemCaller = false;
> +  // Sometimes this chain is not complete: a fair number of unit tests will assert if it is not checked.

80chars max.

Comment 23

[Andrea Marchesini [:baku]]

Comment on attachment 8957650 [details]
Bug 1443943 Move LRUCache Initialization to startup rather than lazily

https://reviewboard.mozilla.org/r/226560/#review232548

nsRFPService::RandomMidpoint should be thread-safe, but with this change it's not.
What about if sCache is nullified on the main-thread when nsRFPService::RandomMidpoint runs on a different one?

Here how I would fix this:

1. in nsRFPService::StartShutdown: lock sLock and nullify sCache.

2. Doing this you don't need ClearOnShutdown() any more.

3. Make LRUCache refcounted.

4. Replace StaticAutoPtr with StaticRefPtr for sCache.

5. In nsRFPService::RandomMidpoint do:

RefPtr<LRUCache> cache;
{
  StaticMutexAutoLock lock(sLock)
  cache = sCache;
}

if (!cache) {
  .. we are shutting down.
  return NS_ERROR_FAILURE; // or any other error.
}

and use cache instead of sCache in the rest of RandomMidpoint.

::: toolkit/components/resistfingerprinting/nsRFPService.cpp:691
(Diff revision 4)
>  
>    // Call Update here to cache the values of the prefs and set the timezone.
>    UpdateRFPPref();
>  
> +  // Create the LRU Cache when we initialize, to avoid accidently trying to
> +  // cerate it (and call ClearOnShutdown) on a non-main-thread

create

Comment 24

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8957653 [details]
Bug 1443943 Do not clamp/jitter event timestamps if it's a system principal

https://reviewboard.mozilla.org/r/226566/#review232622

::: dom/events/Event.cpp:1103
(Diff revision 4)
> -Event::TimeStampImpl() const
> +Event::TimeStamp()
>  {
> +  bool isSystemCaller = false;
> +  // Sometimes this chain is not complete: a fair number of unit tests will assert if it is not checked.
> +  // If we can't check, we assume we are not in chrome code.
> +  if (mPresContext && mPresContext->Document() && mPresContext->Document()->NodePrincipal())

This works only in main thread, not in workers.
And even in main thread mOwner should be used.
Not every event has prescontext.

I'm a bit worried about performance here in general.

Comment 25

[Tom Ritter [:tjr] (OOTO until April)]

Attachment: Bug 1443943 Allow internal callers of performance.now() to opt-out of clamping/jittering

PresShell only uses performane.now to track refresh times, and notify internal
observers. We can provide more accurate times by not clamping and jittering
these numbers.

Review commit: https://reviewboard.mozilla.org/r/226962/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/226962/

Comment 26

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957650 [details]
Bug 1443943 Move LRUCache Initialization to startup rather than lazily

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226560/diff/4-5/

Comment 27

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957651 [details]
Bug 1443943 Port the performance APIs only to only clamping/jittering on non-System Principal

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226562/diff/4-5/

Comment 28

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957652 [details]
Bug 1443943 Do not clamp/jitter in the JS Engine if it's system context

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226564/diff/4-5/

Comment 29

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957653 [details]
Bug 1443943 Do not clamp/jitter event timestamps if it's a system principal

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226566/diff/4-5/

Comment 30

[Tom Ritter [:tjr] (OOTO until April)]

I think I've corrected all the issues noted.


(In reply to Andrea Marchesini [:baku] from comment #23)
> nsRFPService::RandomMidpoint should be thread-safe, but with this change
> it's not.
> What about if sCache is nullified on the main-thread when
> nsRFPService::RandomMidpoint runs on a different one?
> 
> Here how I would fix this:

Thank you for the explicit detail: I struggled for a while in another bug trying to figure out how to avoid shutdown leaks with this; your suggestion worked first try.

(In reply to Olli Pettay [:smaug] from comment #24)
> This works only in main thread, not in workers.
> And even in main thread mOwner should be used.
> Not every event has prescontext.
> 
> I'm a bit worried about performance here in general.

Fixed. You're worried about the performance of checking the principal; or performance of the entire approach of time jittering? If it's the former, I can check once and store a boolean, assuming the principal doesn't change. If it's the latter, this bug, as well as Bug 1441157, are focused on reducing the cost of jitter.

Comment 31

[Olli Pettay [:smaug][[e-mail]]]

checking principal.

Comment 32

[Tom Ritter [:tjr] (OOTO until April)]

I added significantly to the second patch to affect the rest of the performance APIs.

Comment 33

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957651 [details]
Bug 1443943 Port the performance APIs only to only clamping/jittering on non-System Principal

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226562/diff/5-6/

Comment 34

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957652 [details]
Bug 1443943 Do not clamp/jitter in the JS Engine if it's system context

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226564/diff/5-6/

Comment 35

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957653 [details]
Bug 1443943 Do not clamp/jitter event timestamps if it's a system principal

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226566/diff/5-6/

Comment 36

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8958023 [details]
Bug 1443943 Allow internal callers of performance.now() to opt-out of clamping/jittering

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226962/diff/1-2/

Comment 37

[Luke Wagner [:luke]]

Comment on attachment 8957652 [details]
Bug 1443943 Do not clamp/jitter in the JS Engine if it's system context

https://reviewboard.mozilla.org/r/226564/#review232806

Comment 38

[Tom Ritter [:tjr] (OOTO until April)]

(In reply to Olli Pettay [:smaug] from comment #31)
> checking principal.

I queued up a Talos run to see if caching the principal shows a performance improvement. (Results still coming in as-of writing.)

https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=9f375efa6b33&newProject=try&newRevision=843a750871f0&framework=1&showOnlyComparable=1

Comment 39

[Andrea Marchesini [:baku]]

Comment on attachment 8957650 [details]
Bug 1443943 Move LRUCache Initialization to startup rather than lazily

https://reviewboard.mozilla.org/r/226560/#review233142

::: toolkit/components/resistfingerprinting/nsRFPService.cpp:780
(Diff revision 5)
>    MOZ_ASSERT(NS_IsMainThread());
>  
>    nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
>  
> +  if (sCache) {
> +    StaticMutexAutoLock lock(sLock);

Do the opposite:

StaticMutexAutoLock lock(sLock);
{
  sCache = nullptr;
}

you don't need a if() stmt.

Comment 40

[Tom Ritter [:tjr] (OOTO until April)]

(In reply to Tom Ritter [:tjr] from comment #38)
> (In reply to Olli Pettay [:smaug] from comment #31)
> > checking principal.
> 
> I queued up a Talos run to see if caching the principal shows a performance
> improvement. (Results still coming in as-of writing.)
> 
> https://treeherder.mozilla.org/perf.html#/
> compare?originalProject=try&originalRevision=9f375efa6b33&newProject=try&newR
> evision=843a750871f0&framework=1&showOnlyComparable=1

It doesn't seem like caching the result of the lookup has a real affect on performance.

Resolved all other issues, just the last patch needs review (I added it after the first batch of reviews.)

Comment 41

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957650 [details]
Bug 1443943 Move LRUCache Initialization to startup rather than lazily

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226560/diff/5-6/

Comment 42

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957651 [details]
Bug 1443943 Port the performance APIs only to only clamping/jittering on non-System Principal

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226562/diff/6-7/

Comment 43

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957652 [details]
Bug 1443943 Do not clamp/jitter in the JS Engine if it's system context

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226564/diff/6-7/

Comment 44

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957653 [details]
Bug 1443943 Do not clamp/jitter event timestamps if it's a system principal

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226566/diff/6-7/

Comment 45

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8958023 [details]
Bug 1443943 Allow internal callers of performance.now() to opt-out of clamping/jittering

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226962/diff/2-3/

Comment 46

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8958023 [details]
Bug 1443943 Allow internal callers of performance.now() to opt-out of clamping/jittering

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226962/diff/3-4/

Comment 47

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957651 [details]
Bug 1443943 Port the performance APIs only to only clamping/jittering on non-System Principal

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226562/diff/7-8/

Comment 48

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957652 [details]
Bug 1443943 Do not clamp/jitter in the JS Engine if it's system context

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226564/diff/7-8/

Comment 49

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957653 [details]
Bug 1443943 Do not clamp/jitter event timestamps if it's a system principal

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226566/diff/7-8/

Comment 50

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8958023 [details]
Bug 1443943 Allow internal callers of performance.now() to opt-out of clamping/jittering

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226962/diff/4-5/

Comment 51

[Bobby Holley (:bholley)]

Comment on attachment 8958023 [details]
Bug 1443943 Allow internal callers of performance.now() to opt-out of clamping/jittering

https://reviewboard.mozilla.org/r/226962/#review233264

r=me with that.

::: dom/performance/Performance.h:67
(Diff revision 5)
>  
>    virtual PerformanceStorage* AsPerformanceStorage() = 0;
>  
>    void ClearResourceTimings();
>  
> -  DOMHighResTimeStamp Now() const;
> +  DOMHighResTimeStamp Now();

Keep this const?

Comment 52

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957652 [details]
Bug 1443943 Do not clamp/jitter in the JS Engine if it's system context

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226564/diff/8-9/

Comment 53

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957653 [details]
Bug 1443943 Do not clamp/jitter event timestamps if it's a system principal

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226566/diff/8-9/

Comment 54

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8958023 [details]
Bug 1443943 Allow internal callers of performance.now() to opt-out of clamping/jittering

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226962/diff/5-6/

Comment 55

[Jason Orendorff [:jorendorff]]

Comment on attachment 8957652 [details]
Bug 1443943 Do not clamp/jitter in the JS Engine if it's system context

https://reviewboard.mozilla.org/r/226564/#review233522

r=me with a few nits addressed. Please also disable the `clampAndJitterTime` option for the JavaScript shell (in `SetStandardCompartmentOptions` in js/src/shell/js.cpp).

::: js/src/jsdate.cpp:1352
(Diff revision 9)
>  
>  bool
>  js::date_now(JSContext* cx, unsigned argc, Value* vp)
>  {
>      CallArgs args = CallArgsFromVp(argc, vp);
> -    args.rval().set(TimeValue(NowAsMillis()));
> +    bool clampAndJitter = JS::CompartmentCreationOptionsRef(js::GetContextCompartment(cx)).clampAndJitterTime();

Style nit: instead of repeating this line of code, please make NowAsMillis take cx as an argument.

::: js/xpconnect/src/Sandbox.cpp:1129
(Diff revision 9)
>      // [SecureContext] API (bug 1273687).  In that case we'd call
>      // creationOptions.setSecureContext(true).
>  
> +    if (principal == nsXPConnect::SystemPrincipal()) {
> +        creationOptions.setClampAndJitterTime(false);
> +    }

Style nit: Please remove the curly braces on this if-statement. (This is the style in this component -- sorry, there are several dumb differences for the code in js...)

Comment 56

[Jason Orendorff [:jorendorff]]

Comment on attachment 8957652 [details]
Bug 1443943 Do not clamp/jitter in the JS Engine if it's system context

https://reviewboard.mozilla.org/r/226564/#review233536

::: js/src/jsapi.h:2079
(Diff revision 9)
>  
> +    bool clampAndJitterTime() const { return clampAndJitterTime_; }
> +    CompartmentCreationOptions& setClampAndJitterTime(bool flag) {
> +        clampAndJitterTime_ = flag;
> +        return *this;
> +    }

Please also disable the `clampAndJitterTime` option for the JavaScript shell (in `SetStandardCompartmentOptions` in js/src/shell/js.cpp).

Comment 57

[Tom Ritter [:tjr] (OOTO until April)]

(In reply to Jason Orendorff [:jorendorff] from comment #55)
> r=me with a few nits addressed. Please also disable the `clampAndJitterTime`
> option for the JavaScript shell (in `SetStandardCompartmentOptions` in
> js/src/shell/js.cpp).

I didn't do this because clamping and jittering is only performed in the js shell if one manually calls setTimeResolution (from Bug 1440539) - we want to support clamping and jittering in the js shell; but it is off by default.

Got the rest.

Comment 58

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957651 [details]
Bug 1443943 Port the performance APIs only to only clamping/jittering on non-System Principal

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226562/diff/8-9/

Comment 59

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957652 [details]
Bug 1443943 Do not clamp/jitter in the JS Engine if it's system context

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226564/diff/9-10/

Comment 60

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957653 [details]
Bug 1443943 Do not clamp/jitter event timestamps if it's a system principal

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226566/diff/9-10/

Comment 61

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8958023 [details]
Bug 1443943 Allow internal callers of performance.now() to opt-out of clamping/jittering

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226962/diff/6-7/

Comment 62

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a5ceb2a28cfd
Port the performance APIs only to only clamping/jittering on non-System Principal r=baku
https://hg.mozilla.org/integration/autoland/rev/b76d20f277db
Do not clamp/jitter in the JS Engine if it's system context r=jorendorff,luke
https://hg.mozilla.org/integration/autoland/rev/8d4312a248ec
Do not clamp/jitter event timestamps if it's a system principal r=baku
https://hg.mozilla.org/integration/autoland/rev/ba24b6cd4de9
Allow internal callers of performance.now() to opt-out of clamping/jittering r=bholley

Comment 63

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/a5ceb2a28cfd
https://hg.mozilla.org/mozilla-central/rev/b76d20f277db
https://hg.mozilla.org/mozilla-central/rev/8d4312a248ec
https://hg.mozilla.org/mozilla-central/rev/ba24b6cd4de9

Comment 64

[Tom Ritter [:tjr] (OOTO until April)]

Comment on attachment 8957651 [details]
Bug 1443943 Port the performance APIs only to only clamping/jittering on non-System Principal

Approval Request Comment
[Feature/Bug causing the regression]: The initial version of Jitter that landed (Bug 
1425462) would jitter even system principal calls. This was silly and hurt performance. This bug fixes that and improves performance.

[User impact if declined]: We might as well turn jitter off since this blocks something that's needed to make it secure.

[Is this code covered by automated tests?]: Indirectly, yes.

[Has the fix been verified in Nightly?]: Yes, it's baked for a few days

[Needs manual test from QE? If yes, steps to reproduce]: No.

[List of other uplifts needed for the feature/fix]: Bug 1444588

[Is the change risky?]: No.

[Why is the change risky/not risky?]: There is always a risk I got the patch wrong, but it's been reviewed and people seem to think it's good.

[String changes made/needed]: None

Comment 65

[Julien Cristau [:jcristau]]

Comment on attachment 8957651 [details]
Bug 1443943 Port the performance APIs only to only clamping/jittering on non-System Principal

timestamp work for beta60

Comment 66

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/releases/mozilla-beta/rev/80d6f981383a
https://hg.mozilla.org/releases/mozilla-beta/rev/ec9f524f70c5
https://hg.mozilla.org/releases/mozilla-beta/rev/bd233781f767
https://hg.mozilla.org/releases/mozilla-beta/rev/e3104f4386e8

Comment 67

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/53d478be2796
Ensure redirect SJS' serve the correct content types. r=jya

Comment 68

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Pulsebot from comment #67)
> Pushed by [e-mail]:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/53d478be2796
> Ensure redirect SJS' serve the correct content types. r=jya

Off by one error in my commit message for this bug, sorry. This commit isn't for this bug, it's for bug 1443942

Comment 69

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/53d478be2796