Closed Bug 995108 Opened 11 years ago Closed 10 years ago

Intermittent browser_cancelCompatCheck.js | leaked 1 window(s) until shutdown [url = about:blank], leaked 1 window(s) until shutdown [url = chrome://mozapps/content/extensions/update.xul]

Categories

(Toolkit :: Add-ons Manager, defect)

Product:
Toolkit
Component:
Add-ons Manager
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla35
Tracking Flags:
Tracking Status
firefox33 --- wontfix
firefox34 --- fixed
firefox35 --- fixed
firefox-esr31 --- wontfix

People

(Reporter: philor, Assigned: Irving)

References

Details

(Keywords: intermittent-failure)

Attachments

(2 files)

Don't register a couple of (unused) observers
2.27 KB, patch
mossop
: review+
Details | Diff | Splinter Review
Make sure timer-based addon background updates are disabled during tests
33.67 KB, patch
mossop
: review+
lmandel
: approval-mozilla-aurora+
Details | Diff | Splinter Review
Kind of hard to figure why this would have started when we switched the linux debug browser-chrome-in-three-chunks to doing chunk-by-dir, since all of mozapps/extensions/ should have been in bc3 before and should still be there, but there you go, now it leaks. https://tbpl.mozilla.org/php/getParsedLog.php?id=37616583&tree=Fx-Team Ubuntu VM 12.04 fx-team debug test mochitest-browser-chrome-3 on 2014-04-10 23:21:55 PDT for push 783c5013dbec slave: tst-linux32-spot-451 23:51:49 WARNING - TEST-UNEXPECTED-FAIL | toolkit/mozapps/extensions/test/browser/browser_cancelCompatCheck.js | leaked 1 window(s) until shutdown [url = about:blank] 23:51:49 WARNING - TEST-UNEXPECTED-FAIL | toolkit/mozapps/extensions/test/browser/browser_cancelCompatCheck.js | leaked 1 window(s) until shutdown [url = chrome://mozapps/content/extensions/update.xul]
Dave, please could you take a look at this intermittent failure? :-)
Flags: needinfo?(dtownsend+bugmail)
Reading through the log messages, I see a bunch of noise from the GMP (media plugin) and Browser Experiments during this test, so it's possible the recent up-tick in errors was caused by one of those modules not cleaning up after themselves. That said, I did spot a couple of places in this test where I registered observers that I didn't use, so this patch removes those.
Assignee: nobody → irving
Status: NEW → ASSIGNED
Attachment #8480157 - Flags: review?(dtownsend+bugmail)
Flags: needinfo?(dtownsend+bugmail)
Attachment #8480157 - Flags: review?(dtownsend+bugmail) → review+
Welp, that didn't fix it. The patch should stay landed.
Keywords: leave-open
Well that escalated quickly. One of the tests was messing with the prefs that control whether background updates are enabled or not, and leaving them enabled, which affected later tests. In an attempt to clean this up, I ended up: 1) Fixing the test that was leaving updates enabled, so that it cleans up after itself. 2) Adding code to explicitly insert and remove the updated timers, so that tests that *need* to enable/disable the preferences (e.g. to test the UI) can do so without fear of unexpected timer events. 3) Separating the code path for the timers from that used by most of the tests, so that tests can invoke background updates directly without enabling the prefs (and thus exposing themselves to the possibility of conflicting timer-driven updates) - this also led to switching some tests from Observer to Promise synchronization with the background update (there are more that could be changed). It ran clean through a few rounds of Try on linux32 debug, which is the platform that was reporting the failures: https://tbpl.mozilla.org/?tree=Try&rev=a72dde21c66f
Attachment #8482784 - Flags: review?(dtownsend+bugmail)
Attachment #8482784 - Flags: review?(dtownsend+bugmail) → review+
https://hg.mozilla.org/mozilla-central/rev/df679bea2e78
Flags: in-testsuite+
Whiteboard: [fixed-in-fx-team]
Declaring victory.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: leave-open
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Can we can an Aurora approval request on the last patch? :)
status-firefox33: --- → wontfix
status-firefox34: --- → affected
status-firefox35: --- → fixed
status-firefox-esr31: --- → wontfix
Flags: needinfo?(irving)
Comment on attachment 8482784 [details] [diff] [review] Make sure timer-based addon background updates are disabled during tests Approval Request Comment [Feature/regressing bug #]: [User impact if declined]: Sheriff annoyance at having to star frequent oranges [Describe test coverage new/current, TBPL]: https://tbpl.mozilla.org/?tree=Try&rev=a72dde21c66f, and six days of no oranges on trunk. [Risks and why]: Low risk; makes a slight changes to an AddonManager internal API but otherwise test-only. [String/UUID change made/needed]: None.
Attachment #8482784 - Flags: approval-mozilla-aurora?
Flags: needinfo?(irving)
Comment on attachment 8482784 [details] [diff] [review] Make sure timer-based addon background updates are disabled during tests I try to keep the sheriffs as happy as possible. :) Aurora+
Attachment #8482784 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: