Closed Bug 608820 Opened 14 years ago Closed 9 years ago

Intermittent browser_bug567127.js | Test timed out and Found a tab after previous test timed out: about:addons

Categories

(Toolkit :: Add-ons Manager, defect)

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

Tracking

()

Status:
RESOLVED WORKSFORME
Tracking Flags:
Tracking Status
firefox30 --- wontfix
firefox31 --- fixed
firefox32 --- fixed
firefox33 --- affected
firefox-esr24 --- wontfix
b2g-v1.4 --- wontfix
b2g-v2.0 --- fixed

People

(Reporter: robert.strong.bugs, Unassigned)

References

Details

(Keywords: intermittent-failure)

Attachments

(2 files)

Set callback handlers before invoking actions, to avoid races
(deleted), patch
mossop
: review+
Details | Diff | Splinter Review
Part 2 - rewrite to task, which yields in another possibly critical place
(deleted), patch
mossop
: review+
Details | Diff | Splinter Review
ehsan%mozilla.com http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1288643678.1288644609.14210.gz Rev3 Fedora 12x64 mozilla-central opt test mochitest-other on 2010/11/01 13:34:38 s: talos-r3-fed64-022 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_bug567127.js | Test timed out TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_bug567127.js | Found a tab after previous test timed out: about:addons
Whiteboard: [orange]
Though that's not exactly [Linux], is it?
This may have originally been Linux only, but it's had a lot of Windows failures reported here since.
OS: Linux → All
Summary: [Linux] Intermittent browser_bug567127.js | Test timed out and Found a tab after previous test timed out: about:addons → Intermittent browser_bug567127.js | Test timed out and Found a tab after previous test timed out: about:addons
Whiteboard: [orange]
Resolving WFM any keyword:intermittent-failure bug where: * Changed: (is less than or equal to) -3m * Whiteboard: (contains none of the strings) random disabled marked fuzzy todo fails failing annotated time-bomb * Whiteboard: (does not contain the string) leave open There will inevitably be some false positives; for that (and the bugspam) I apologise, but at least this will clear out the open cruft (and thus reduce risk of mis-starring) on TBPL's annotated summary bug suggestions. Filter on orangewfm.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → WORKSFORME
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Blair, this failure has increased significantly over the last few weeks. Can you try to fix it or find someone to do so? Thanks!
Flags: needinfo?(bmcbride)
Justin, can you suggest someone who can look at this? The last needinfo? has been sitting for a month now.
Flags: needinfo?(dolske)
Disabled on Windows and OSX. https://hg.mozilla.org/integration/fx-team/rev/04abb7896b1d
Assignee: nobody → ryanvm
Whiteboard: [test disabled on OSX and Windows][leave open]
Assignee: ryanvm → nobody
Flags: needinfo?(dolske)
I tripped over this while cleaning up test failures in bug 760356... From examining logs, I suspect that the problem is that we're setting up our handlers after we open windows, and we may be losing the race to get the handler registered before the event in question fires. This patch: - Simplifies the Promise async code where it could be done without substantial restructuring - Adds info() calls at various places during async testing to make it easier to diagnose future issues - Registers handlers before opening windows or acting on UI (except for wait_for_view_load(), which is designed to be called after the action)
Assignee: nobody → irving
Status: REOPENED → ASSIGNED
Attachment #8414640 - Flags: review?(dtownsend+bugmail)
Flags: needinfo?(bmcbride)
Comment on attachment 8414640 [details] [diff] [review] Set callback handlers before invoking actions, to avoid races Review of attachment 8414640 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/mozapps/extensions/test/browser/head.js @@ -441,5 @@ > > - AddonManager.getInstallForURL(pathPrefix + path, (install) => { > - install.addListener({ > - onInstallEnded: () => { > - executeSoon(() => { Worried that this might cause issues but presumably the tests all pass?
Attachment #8414640 - Flags: review?(dtownsend+bugmail) → review+
This did go through a try run, though it was part of a large stack of patches and one of the others was causing one browser-mochitest to fail. This fix is necessary but I'm not certain that it will completely fix the issue, so we should keep the bug open and I'll watch for a while to see if we can re-enable the other platforms and close the bug.
Keywords: checkin-needed
Keywords: leave-open
Whiteboard: [test disabled on OSX and Windows][leave open] → [test disabled on OSX and Windows]
https://hg.mozilla.org/integration/fx-team/rev/bd3714386e29
Keywords: checkin-needed
Whiteboard: [test disabled on OSX and Windows] → [test disabled on OSX and Windows][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/bd3714386e29
Whiteboard: [test disabled on OSX and Windows][fixed-in-fx-team] → [test disabled on OSX and Windows]
https://hg.mozilla.org/mozilla-central/rev/3e7464b58203
Status: ASSIGNED → RESOLVED
Closed: 12 years ago → 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Rats, looks like there's a problem other than the one I patched...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
On top of the existing patch, this one: - adds more logging to try and pinpoint any future failures - rewrites to Task.jsm Studying the code, I found another place where we might be doing something sensitive on the stack during a callback: When we're done checking that the right addons are listed in test_confirmation() we call cancelDialog(). In the call to WindowOpenListener we set the test harness run_next_test function as our close handler, so when the file pop-up dialog signals unload, we end up calling the function chain run_next_test => end_test => close_manager => managerWindow.close() without yielding to the event loop. Since the confirmation dialog is modal, the attempt to close about:addons fails silently, as far as I can tell, if we do it too soon. The task-style rewrite just happens to fix this, since the Promise resolve inside the onunload handler (registered by WindowOpenListener) lets the stack unwind before continuing. I'm not wise in the ways of our windowing/widget code, but if I'm right about the sequence of events here, somebody who is wise in those ways should consider making "script can't close this parent window because there's a modal child open" log an error.
Attachment #8417161 - Flags: review?(dtownsend+bugmail)
Comment on attachment 8417161 [details] [diff] [review] Part 2 - rewrite to task, which yields in another possibly critical place Review of attachment 8417161 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/mozapps/extensions/test/browser/head.js @@ +348,5 @@ > is(aManagerWindow.location, MANAGER_URI, "Should be closing window with correct URI"); > > aManagerWindow.addEventListener("unload", function() { > + try { > + dump("Manager window unload handler"); Why not info? @@ +349,5 @@ > > aManagerWindow.addEventListener("unload", function() { > + try { > + dump("Manager window unload handler"); > + this.removeEventListener("unload", arguments.callee, false); Can this really ever throw?
Attachment #8417161 - Flags: review?(dtownsend+bugmail) → review+
(In reply to Dave Townsend [:mossop] from comment #282) > Comment on attachment 8417161 [details] [diff] [review] > Part 2 - rewrite to task, which yields in another possibly critical place > > + dump("Manager window unload handler"); > > Why not info? We have some test failures where callbacks are coming after the test harness is unloaded, leading to spurious "TypeError: info is not defined" as part of the failure trace. > @@ +349,5 @@ > > > > aManagerWindow.addEventListener("unload", function() { > > + try { > > + dump("Manager window unload handler"); > > + this.removeEventListener("unload", arguments.callee, false); > > Can this really ever throw? Probably not, but i'm getting getting paranoid to the point of wanting to instrument my coffee level as part of the log in case that's what is causing some of these oranges.
https://hg.mozilla.org/mozilla-central/rev/b26be6fe71be
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
I'm growing to love this test suite, can't you tell?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Who wouldn't? :P
status-firefox33: --- → affected
Target Milestone: mozilla32 → ---
Assignee: irving → nobody
Inactive; closing (see bug 1180138).
Status: REOPENED → RESOLVED
Closed: 11 years ago → 9 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: