Closed Bug 751086 Opened 13 years ago Closed 13 years ago

Disable toolkit/mozapps/plugins/tests/browser_bug435788.js for leaks

Categories

(Toolkit :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: bholley, Assigned: bholley)

References

Details

Attachments

(1 file, 1 obsolete file)

Mano fixed some added leaks with CPG over in bug 730340. On central we're leaking 22-23 objects, with the leak threshold set to 23. I believe the fuzz is due to bug 751085, which I just filed. However, it's still occasionally orange on try with his patch. The reason appears to be that we're leaking 2 extra windows in a test that already leaks. Without CPG: [toolkit/mozapps/plugins/tests/browser_bug435788.js] 1 window(s) [url = chrome://mozapps/content/plugins/pluginInstallerWizard.xul] 1 window(s) [url = about:blank] With CPG: [toolkit/mozapps/plugins/tests/browser_bug435788.js] 2 window(s) [url = chrome://mozapps/content/plugins/pluginInstallerWizard.xul] 2 window(s) [url = about:blank] I think it's safe to bump the leak count here.
(In reply to Bobby Holley (:bholley) from comment #0) > However, it's still occasionally orange on try with his patch. How often? Increasing the limit for intermittent leaks is risky because it means that new persistent leaks can go unnoticed until you hit the intermittent leak again.
Well, I guess we're not talking about an intermittent leak here, i.e. the browser_bug435788.js change is persistent?
(In reply to Dão Gottwald [:dao] from comment #2) > Well, I guess we're not talking about an intermittent leak here, i.e. the > browser_bug435788.js change is persistent? It does not always happen, but it happens reliably enough that I don't think transgressing the threshold with something else intermittent is likely to go undetected. See https://tbpl.mozilla.org/?tree=Try&rev=2b7d9a8bfa12 and https://tbpl.mozilla.org/?tree=Try&rev=7d2e75665ce4
I'd also be fine with disabling the test though, given that it already leaks. Do you think we should do that instead?
I prefer disabling the teat and tracking down the leak in a new bug. By the way, this leak might just be the gPFS global var in the test, which is never cleared.
Sounds good to me.
Summary: Bump Max Leak Count slightly for compartment-per-global → Disable toolkit/mozapps/plugins/tests/browser_bug435788.js for leaks
Component: XPConnect → General
Product: Core → Toolkit
QA Contact: xpconnect → general
Flagging Mano for review.
Attachment #620242 - Flags: review?(mano)
Filed bug 751100 for tracking down the leak.
Comment on attachment 620242 [details] [diff] [review] Disable toolkit/mozapps/plugins/tests/browser_bug435788.js for leaks. v1 >+#browser_bug435788.js has been disabled for leaks. > ifneq (mobile,$(MOZ_BUILD_APP)) > _BROWSER_FILES = \ >- browser_bug435788.js \ Comment out instead, and put the comment right above it (and please reference bug 751100 in the comment)
Attachment #620242 - Flags: review?(mano) → review+
Blocks: bc-leaks
Blocks: 751100
(In reply to Mano from comment #5) > By the way, this leak might just be the gPFS global var in the test, which > is never cleared. browser-test.js automatically clears each test's scope.
Blocks: 435788
Blocks: 593064
(In reply to Mano from comment #9) > >+#browser_bug435788.js has been disabled for leaks. > > ifneq (mobile,$(MOZ_BUILD_APP)) > > _BROWSER_FILES = \ > >- browser_bug435788.js \ > > Comment out instead, and put the comment right above it (and please > reference bug 751100 in the comment) I can't, because it's a Makefile list. But I'll do the warning trick if you want.
Thanks.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
Blocks: 734554
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: