730773 - Track shutdown leaks when DOMWindows *or* DocShells leaked (not and)

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Tim Taubert [:ttaubert] (inactive)]

Attachment: trivial patch

Trivial patch attached.

Comment 1

[Serge Gautherie (:sgautherie)]

Comment on attachment 600858 [details] [diff] [review]
trivial patch

Right, per my bug 683953 comment 36 ;-)
(Of course, need to Try this wrt MAX_LEAK_COUNT.)

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

Attachment: trivial patch v2

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=602fc521eb1a

Looks like we need to increase the threshold to 130 for now.

Comment 3

[Serge Gautherie (:sgautherie)]

Comment on attachment 600891 [details] [diff] [review]
trivial patch v2

Ted is away for one more week.

Comment 4

[Serge Gautherie (:sgautherie)]

Comment on attachment 600891 [details] [diff] [review]
trivial patch v2

Or you could just push as "bustage-fix" then wait for post-repush review...

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

It's not exactly busted - it just doesn't report some leaks but still the situation is better than before. I think we can and should wait a bit more.

Comment 6

[cmtalbert]

Comment on attachment 600891 [details] [diff] [review]
trivial patch v2

I can take this review. The code changes look fine, but let me make sure I understand the premise behind this. We're just increasing our threshold so that we don't hit a random orange when a system suddenly decides to leak like a sieve?

And bug 658738 (and friends) is filed to capture and fix the underlying issues here?

If so, r+

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Clint Talbert ( :ctalbert ) from comment #6)
> I can take this review. The code changes look fine, but let me make sure I
> understand the premise behind this. We're just increasing our threshold so
> that we don't hit a random orange when a system suddenly decides to leak
> like a sieve?

We increase the threshold because this patch makes us detect leaks that we accidentally didn't track before. It's also a bit higher because the number of leaks might slightly vary between test runs.

> And bug 658738 (and friends) is filed to capture and fix the underlying
> issues here?

Yes. Ideally we don't want any threshold at all but we focused on not regressing the current efforts to reduce the number of leaks. The threshold will be lowered when leaks have been fixed.

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/integration/fx-team/rev/5724c5ba0f84

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/5724c5ba0f84