I noticed we have some leaks reported when running reftests. It turns out that there were a couple of issues, which I've got patches for here and on the Servo side.

This requires corresponding servo code in https://github.com/servo/servo/pull/12725

Comment on attachment 8777545 [details] [diff] [review] Part 2 - Use dont_AddRef for already-addrefed ServoComputedValues. v1 Review of attachment 8777545 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/style/ServoStyleSet.cpp @@ +350,5 @@ > MOZ_ASSERT(aType < CSSPseudoElementType::Count); > nsIAtom* pseudoTag = nsCSSPseudoElements::GetPseudoAtom(aType); > > RefPtr<ServoComputedValues> computedValues = > + dont_AddRef(Servo_GetComputedValuesForPseudoElement( Hmm... Seeing this makes me sad, we should really tackle bug 1275913. Though in practice I don't think it'd be a big deal once we have leak checks?

Comment on attachment 8777546 [details] [diff] [review] Part 3 - Use leak logging in a few more places. v1 Review of attachment 8777546 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/style/nsStyleContext.cpp @@ +91,5 @@ > #ifdef DEBUG > , mFrameRefCnt(0) > , mComputingStruct(nsStyleStructID_None) > #endif > +{ MOZ_COUNT_CTOR(nsStyleContext); } Maybe putting this in its own line? { MOZ_COUNT_CTOR(nsStyleContext); }

(In reply to Emilio Cobos Álvarez [:emilio] from comment #5) > Hmm... Seeing this makes me sad, we should really tackle bug 1275913. Though > in practice I don't think it'd be a big deal once we have leak checks? I think it's definitely still important and that we should fix it ASAP. It's might be kind of up Manish's alley given the other FFI safety stuff he's been working on. Manish, any chance you might have the cycles to fix our already_AddRefed story at some point?

(In reply to Emilio Cobos Álvarez [:emilio] from comment #6) > Comment on attachment 8777546 [details] [diff] [review] > Part 3 - Use leak logging in a few more places. v1 > > Review of attachment 8777546 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: layout/style/nsStyleContext.cpp > @@ +91,5 @@ > > #ifdef DEBUG > > , mFrameRefCnt(0) > > , mComputingStruct(nsStyleStructID_None) > > #endif > > +{ MOZ_COUNT_CTOR(nsStyleContext); } > > Maybe putting this in its own line? > { > MOZ_COUNT_CTOR(nsStyleContext); > } Done. https://treeherder.mozilla.org/#/jobs?repo=try&revision=3a8f0dbcb786

Pushed by bholley@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/867f638fcc50 Rejigger init hook and add shutdown hook. r=emilio https://hg.mozilla.org/integration/mozilla-inbound/rev/14c9cd79e79e Use dont_AddRef for already-addrefed ServoComputedValues. r=emilio https://hg.mozilla.org/integration/mozilla-inbound/rev/2dbf104fa4b4 Use leak logging in a few more places. r=emilio

https://hg.mozilla.org/mozilla-central/rev/867f638fcc50 https://hg.mozilla.org/mozilla-central/rev/14c9cd79e79e https://hg.mozilla.org/mozilla-central/rev/2dbf104fa4b4

> It's might be kind of up Manish's alley given the other FFI safety stuff > he's been working on. Manish, any chance you might have the cycles to fix > our already_AddRefed story at some point? At some point -- probably, but not too soon. I can prioritize it if it needs to be though. I'm also not yet clear on how we're using it in Servo at the moment, so I'd have to investigate that. Leaving this needinfo open till I have a closer look.