Closed
Bug 730773
Opened 13 years ago
Closed 13 years ago
Track shutdown leaks when DOMWindows *or* DocShells leaked (not and)
Categories
(Testing :: Mochitest, defect)
Testing
Mochitest
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla13
People
(Reporter: ttaubert, Assigned: ttaubert)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
cmtalbert
:
review+
sgautherie
:
feedback+
|
Details | Diff | Splinter Review |
Trivial patch attached.
Attachment #600858 -
Flags: review?(ted.mielczarek)
Comment 1•13 years ago
|
||
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.)
Attachment #600858 -
Flags: feedback+
Assignee | ||
Comment 2•13 years ago
|
||
Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=602fc521eb1a
Looks like we need to increase the threshold to 130 for now.
Attachment #600858 -
Attachment is obsolete: true
Attachment #600891 -
Flags: review?(ted.mielczarek)
Attachment #600858 -
Flags: review?(ted.mielczarek)
Updated•13 years ago
|
Attachment #600891 -
Flags: review?(mh+mozilla)
Comment 3•13 years ago
|
||
Comment on attachment 600891 [details] [diff] [review]
trivial patch v2
Ted is away for one more week.
Comment 4•13 years ago
|
||
Comment on attachment 600891 [details] [diff] [review]
trivial patch v2
Or you could just push as "bustage-fix" then wait for post-repush review...
Attachment #600891 -
Flags: feedback+
Assignee | ||
Comment 5•13 years ago
|
||
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.
Updated•13 years ago
|
Attachment #600891 -
Flags: review?(mh+mozilla) → review?(catlee)
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+
Attachment #600891 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Comment 7•13 years ago
|
||
(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.
Assignee | ||
Updated•13 years ago
|
Attachment #600891 -
Flags: review?(catlee)
Assignee | ||
Comment 8•13 years ago
|
||
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla13
Assignee | ||
Comment 9•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Updated•13 years ago
|
Flags: in-testsuite+
Updated•7 years ago
|
Component: BrowserTest → Mochitest
You need to log in
before you can comment on or make changes to this bug.
Description
•