Closed Bug 632290 Opened 14 years ago Closed 6 years ago

Intermittent browser_about.js | Test timed out | Found unexpected add-ons manager window still open

Categories

(Toolkit :: Add-ons Manager, defect, P3)

Product:
Toolkit
Component:
Add-ons Manager
Platform:
x86
Windows 7
Type:
defect

Tracking

()

Status:
RESOLVED INACTIVE
Tracking Flags:
Tracking Status
firefox37 --- disabled
firefox38 --- disabled
firefox39 --- disabled

People

(Reporter: philor, Assigned: RyanVM)

References

Details

(Keywords: intermittent-failure, Whiteboard: [test disabled on Linux and Windows])

Attachments

(3 files, 1 obsolete file)

tentative patch
(deleted), patch
mossop
: review-
Details | Diff | Splinter Review
WIP 1: add some diagnostics to waitForFocus, try waiting for focus on specific window
(deleted), patch
ehsan.akhgari
: feedback+
Details | Diff | Splinter Review
Rewrite test to Task, unwind stacks from event handlers
(deleted), patch
mossop
: review+
Details | Diff | Splinter Review 
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1297136072.1297137836.19702.gz
Rev3 WINNT 6.1 mozilla-central opt test mochitest-other on 2011/02/07 19:34:32
s: talos-r3-w7-028

TEST-PASS | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser-window/browser_about.js | Should be closing window with correct URI
TEST-INFO | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser-window/browser_about.js | Longer timeout required, waiting longer...  Remaining timeouts: 1
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser-window/browser_about.js | Test timed out
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser-window/browser_about.js | Found unexpected add-ons manager window still open
INFO TEST-END | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser-window/browser_about.js | finished in 60053ms
Attached patch tentative patch (deleted) — — Splinter Review 
Dave, is there a specific reason this test is using waitForFocus in the domwindowopen handler? locally I see this test perma failing, while with a simple load event it works fine.
I also see a globalwindow assertion, but it disappears with the fix for bug 651315, since this uses openDialog could be related.
Attachment #527898 - Flags: review?(dtownsend)
(In reply to comment #63)
> Created attachment 527898 [details] [diff] [review]
> tentative patch
> 
> Dave, is there a specific reason this test is using waitForFocus in the
> domwindowopen handler? locally I see this test perma failing, while with a
> simple load event it works fine.
> I also see a globalwindow assertion, but it disappears with the fix for bug
> 651315, since this uses openDialog could be related.

When we first landed it in bug 610661 we found it failed with just a load handler.
I've also very commonly seen issues when attempting to close windows that have only just opened but haven't fully got focus on linux so I'm not sure I'd want to take this. Why would it be failing for you with waiting for focus?
Comment on attachment 527898 [details] [diff] [review]
tentative patch

Review of attachment 527898 [details] [diff] [review]:

This is something that I'd only expect to make things worse unless waitForFocus is broken in some way, so I don't think we should take it.
Attachment #527898 - Flags: review?(dtownsend) → review-
(In reply to comment #65)
> I've also very commonly seen issues when attempting to close windows that have
> only just opened but haven't fully got focus on linux so I'm not sure I'd want
> to take this. Why would it be failing for you with waiting for focus?

I don't know, but I was able to reproduce failures constantly and this bug demonstrates that something is broken.  FWIW bug 521233 is still open.
Something interesting I found is that this test always asserts, see bug 511467 comment 4. A workaround to the assertion is to enqueue the aManagerWindow.close() call in close_manager, so that the docshell is destroyed after the modal dialog dismissal is completely handled. I have no idea if there is a relation with the missing "unload" event though :(
Summary: Intermittent browser_about.js | Test timed out → Intermittent browser_about.js | Test timed out | Found unexpected add-ons manager window still open
On 5/14, this morphed to "Intermittent browser_about.js | Test timed out | Found a tab after previous test timed out: about:addons".  I don't know if the subject should be changed or a new bug should be filed...
Whiteboard: [orange]
I don't suppose we have any new theories here? This is probably one of the oldest intermittent failures that we still hit.
Flags: needinfo?(mak77)
Flags: needinfo?(dtownsend+bugmail)
Flags: needinfo?(dtownsend+bugmail)
Nope sorry, I don't have any news compared to what I wrote in 2011, and I'm not sure that still applies after 2 years.
Flags: needinfo?(mak77)
Yet another recent surge in this. Can we get a fresh look at this or can we finally admit defeat and turn it off?
Flags: needinfo?(dtownsend+bugmail)
Flags: needinfo?(bmcbride)
(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #432)
> Yet another recent surge in this. Can we get a fresh look at this or can we
> finally admit defeat and turn it off?

I don't really have much time to look at this though it seems to have dropped off again. Hardware changes?

Anyway I'm ok with turning it off if that's helpful
Flags: needinfo?(dtownsend+bugmail)
Taking a quick look to see if I can find anything. One interesting point from the last log:

22:32:17     INFO -  TEST-INFO | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_about.js | About dialog opened, waiting for focus
22:32:17     INFO -  TEST-INFO | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_about.js | must wait for load
22:32:17     INFO -  TEST-INFO | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_about.js | must wait for focus
22:32:18     INFO -  TEST-INFO | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_about.js | Saw about dialog
22:32:18     INFO -  TEST-PASS | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_about.js | The correct add-on about window should have opened
22:32:18     INFO -  TEST-PASS | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_about.js | window.arguments[0] should refer to the add-on object
22:32:18     INFO -  TEST-INFO | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_about.js | About dialog closed, waiting for focus on browser window
22:32:18     INFO -  TEST-INFO | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_about.js | must wait for focus
22:32:18     INFO -  [Parent 1672] WARNING: Should not try to set the focus on a disabled window: file c:/builds/moz2_slave/m-in-w32-d-0000000000000000000/build/dom/base/nsGlobalWindow.cpp, line 6411

which probably confirms Mak's observation oh so long ago, that the timing is racy between closing the modal dialog and waiting for focus to come back to the main window.
Assignee: nobody → irving
Status: NEW → ASSIGNED
If we take the approach of fixing waitForFocus(), what I see happening is that when SimpleTest.waitForFocus calls SpecialPowers.getFocusedElementForWindow(), the call chains down to nsFocusManager::GetFocusedDescendant() at http://dxr.mozilla.org/mozilla-central/source/dom/base/nsFocusManager.cpp#266

That method follows the 'window->getFocusedNode()' chain to its tip, without checking any other properties of the windows it's walking through. In our failure case, it returns a pointer to a window that has its 'enabled' flag set to false. I don't know the structure of the C++ window tree that well, but I'm guessing this happens when the modal child window that was focused is closed, but not everything is cleaned up yet.

If that's not intentional, cleaning up the window tree so that we never leave a !enabled window as the leaf of the getFocusedNode() chain would make this problem go away.

We might be able to paper over this by modifying SimpleTest.waitForFocus (or SpecialPowers.getFocusedElementForWindow) to walk up the window's parent chain, if nsFocusManager::GetFocusedDescendant returns a !enabled window.

The alternative is to modify the test to not use waitForFocus(); in that case we'd need some other way to reliably tell when the main AddonManager window gets focus back from the pop-up. Could we get this just by hanging a focus observer directly on the AddonManager window before we start the pop-up? That's what I'll look at next.
Ehsan's fought similar battles with other tests in the past. Maybe he has some ideas too :)
(In reply to :Irving Reid from comment #448)
> If we take the approach of fixing waitForFocus(), what I see happening is
> that when SimpleTest.waitForFocus calls
> SpecialPowers.getFocusedElementForWindow(), the call chains down to
> nsFocusManager::GetFocusedDescendant() at
> http://dxr.mozilla.org/mozilla-central/source/dom/base/nsFocusManager.cpp#266
> 
> That method follows the 'window->getFocusedNode()' chain to its tip, without
> checking any other properties of the windows it's walking through. In our
> failure case, it returns a pointer to a window that has its 'enabled' flag
> set to false. I don't know the structure of the C++ window tree that well,
> but I'm guessing this happens when the modal child window that was focused
> is closed, but not everything is cleaned up yet.

Yeah, looking through the code, the enabled flag actually lives on the widget (see nsWindow::IsEnabled), and I believe that is set to false when we tear down the OS level widget which happens as part of closing the window.  Note that closing the window doesn't destroy the DOM object.

> If that's not intentional, cleaning up the window tree so that we never
> leave a !enabled window as the leaf of the getFocusedNode() chain would make
> this problem go away.
> 
> We might be able to paper over this by modifying SimpleTest.waitForFocus (or
> SpecialPowers.getFocusedElementForWindow) to walk up the window's parent
> chain, if nsFocusManager::GetFocusedDescendant returns a !enabled window.
> 
> The alternative is to modify the test to not use waitForFocus(); in that
> case we'd need some other way to reliably tell when the main AddonManager
> window gets focus back from the pop-up. Could we get this just by hanging a
> focus observer directly on the AddonManager window before we start the
> pop-up? That's what I'll look at next.

I'm not sure what the above proposals are trying to fix here.  Which window are we closing and which one we're focusing here?  If I'm reading the test code right, we focus the about window, call .close() on it, and wait for domwindowclosed to be dispatched and then try to focus the main window.  Is it here where we're seeing the disabled window?  That seems like a bug!  Or am I missing something?

(BTW, this is exactly the same pattern that I've been seeing in bug 772823 and I tested a patch there on the try server which fixes this issue by reordering these operations.  We can do the same thing here but it looks like we're on our way to finally understand the root cause of these failures!  Also, note that Neil Deakin is on vacation until the 29th.)
Flags: needinfo?(irving)
See Also: → https://bugzilla.mozilla.org/show_bug.cgi?id=772823
The sequence is:

Open Addon Manager window

Request 'about' window for one add-on
Wait for domwindowopen (hopefully it's the 'about' window)
waitForFocus()
Close 'about' window
Wait for domwindowclose
waitForFocus() (hopefully on the Addon Manager window)

... repeat steps for another add-on

The last TBPL test failure was in the last step, where we wait for Addon Manager to get the focus back. I have a WIP patch that just waits for onfocus on the Addon Manager, rather than using the 'waitForFocus()' test hook; it works fine on my desktop, I'm cleaning up a couple more things before I sent it to Try.
Flags: needinfo?(irving)
Flags: needinfo?(ehsan)
I wonder if bug 608820 has the same root cause.
Based on a quick look at the code & TBPL log for bug 608820, there is a different (as yet unknown) mechanism at fault. That will have to be next week's orange squash ;->
(In reply to :Irving Reid from comment #451)
> The sequence is:
> 
> Open Addon Manager window
> 
> Request 'about' window for one add-on
> Wait for domwindowopen (hopefully it's the 'about' window)
> waitForFocus()
> Close 'about' window
> Wait for domwindowclose
> waitForFocus() (hopefully on the Addon Manager window)
> 
> ... repeat steps for another add-on

OK, that confirms what I understood from the test.  Thanks!

> The last TBPL test failure was in the last step, where we wait for Addon
> Manager to get the focus back.

OK but I still don't understand this part, specifically the theory in comment 448.  So, waitForFocus() calls getFocusedElementForWindow in two places, once in <http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/tests/SimpleTest/SimpleTest.js#683> and then in <http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/tests/SimpleTest/SimpleTest.js#736>.  Which one of these calls were you talking about in comment 448?

I'm specifically trying to understand where we encounter the window with the 'enabled' flag set to false and where that window comes from.  That's basically where our bug is.

> I have a WIP patch that just waits for
> onfocus on the Addon Manager, rather than using the 'waitForFocus()' test
> hook; it works fine on my desktop, I'm cleaning up a couple more things
> before I sent it to Try.

I'm afraid that won't work, we can never assume that the focus will sanely go back to the window that we expect it to go back to in our tests which is why we had to add waitForFocus in the first place.  The reason is that the focus is a shared state across all of the apps running on the same desktop/OS so any other window that is opened on the test machine can mess with the onfocus idea.

(Note that waitForFocus is really badly named, it should probably be called focusAndWaitForLoad as it actually ensures that the window is focused and the content inside it has finished loading -- see the discussion in bug 564276.)
Flags: needinfo?(ehsan)
Flags: needinfo?(irving)
Ah, I see. (aside: browser_about.js might work without focus, because it does all its operations by calling handlers directly rather than by simulating keypresses and clicks; since the window we're waiting for in this case is modal, we actually have synchronous behaviour at the JS API level so we might get away without onfocus. I did a try run with no wait for focus on the AddonManager window and it passed: https://tbpl.mozilla.org/?tree=Try&rev=61e8449bc257)

But to answer your question, we're getting the log message "must wait for load" at SimpleTest.js:729 just before the "WARNING: Should not try to set the focus on a disabled window", which implies it's the call at SimpleTest.js:683 to retrieve 'childTargetWindow' - search for the messages at 22:32:18 in https://tbpl.mozilla.org/php/getParsedLog.php?id=37679337&full=1&branch=mozilla-inbound#error0

Is there useful information about the window available through the JS API? It would be easy add some diagnostic output to SimpleTest.js in cases where childTargetWindow.enabled is false.
Flags: needinfo?(irving) → needinfo?(ehsan)
(In reply to :Irving Reid from comment #455)
> Ah, I see. (aside: browser_about.js might work without focus, because it
> does all its operations by calling handlers directly rather than by
> simulating keypresses and clicks; since the window we're waiting for in this
> case is modal, we actually have synchronous behaviour at the JS API level so
> we might get away without onfocus. I did a try run with no wait for focus on
> the AddonManager window and it passed:
> https://tbpl.mozilla.org/?tree=Try&rev=61e8449bc257)

Haha, good point!  That would be a great fix for this specific bug.  :-)

(BTW I really appreciate your help with this.  This issue is affecting a whole bunch of tests so I think it's perfectly fine for us to not fix this like that and go the rest of the way towards fixing it for real!)

> But to answer your question, we're getting the log message "must wait for
> load" at SimpleTest.js:729 just before the "WARNING: Should not try to set
> the focus on a disabled window", which implies it's the call at
> SimpleTest.js:683 to retrieve 'childTargetWindow' - search for the messages
> at 22:32:18 in
> https://tbpl.mozilla.org/php/getParsedLog.
> php?id=37679337&full=1&branch=mozilla-inbound#error0

Ah great!  Then this is this call site: <http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/tests/SimpleTest/SimpleTest.js#683>

> Is there useful information about the window available through the JS API?
> It would be easy add some diagnostic output to SimpleTest.js in cases where
> childTargetWindow.enabled is false.

Yes, it's location.href would be very interesting to know (see the getHref function inside waitForFocus.)  So would the |closed| property of the window object.  It seems that based on the above, we're somehow getting the about dialog window as a result of getFocusedElementForWindow on the _main_ window, which is frightening!  Can you please post the diagnostic patch to try?
Flags: needinfo?(ehsan)
I ran this on Try and retriggered a few of the tests (the test i'm working on is in the bc3 set).

I tried two things in this patch: add some diagnostics to see which window we're trying to focus on in waitForFocus(), and an alternate version of browser_about.js that specifies we want to wait for the Addon Manager window rather than picking up the default global Window. I'm not sure if this will help - we're running this in a 'domwindowclosed' event handler for a modal dialog; if we're losing the race such that the global 'window' object points at the (closing, but still around) modal dialog rather than the Addon Manager, I'd expect any attempt to set focus on the Addon Manager window to be blocked by the modal dialog.

Unfortunately the intermittent failure hasn't shown up in the Try test runs yet, so I can't tell whether either of these approaches helps.
https://tbpl.mozilla.org/?tree=Try&rev=8db5624369b5&showall=1
Attachment #8410981 - Flags: feedback?(ehsan)
Flags: needinfo?(bmcbride)
This patch takes a different approach - first, in an attempt to improve clarity I rewrote what I could using the add_task() test driver.

Second, because the addon 'about' dialog is modal and all the test operations are executed by calling underlying handlers directly rather than by simulating UI interactions, the test might not need to wait for focus on the addon manager window at all.

I put this through one Try run and it was all green, but that's not much evidence given how intermittent the failure is.
Attachment #8410984 - Flags: feedback?(dtownsend+bugmail)
(In reply to :Irving Reid from comment #457)
> Unfortunately the intermittent failure hasn't shown up in the Try test runs
> yet, so I can't tell whether either of these approaches helps.
> https://tbpl.mozilla.org/?tree=Try&rev=8db5624369b5&showall=1

That's a pretty tame number of retriggers :P. I've queued a few more for you :)
Comment on attachment 8410981 [details] [diff] [review]
WIP 1: add some diagnostics to waitForFocus, try waiting for focus on specific window

Review of attachment 8410981 [details] [diff] [review]:
-----------------------------------------------------------------

(f+ for testing this on try but I don't think we should land this yet before we understand what's going on here.)

::: testing/mochitest/tests/SimpleTest/SimpleTest.js
@@ +718,5 @@
> +    var childTargetWindow = {};
> +    SpecialPowers.getFocusedElementForWindow(targetWindow, true, childTargetWindow);
> +    childTargetWindow = childTargetWindow.value;
> +    info("waitForFocus: child window href " + getHref(childTargetWindow)
> +            + " closed " + childTargetWindow.closed);

So this basically adds two logging info() calls, right?  (Not sure why you're moving the code but if I'm not mistaken the code move should not change anything.)

::: toolkit/mozapps/extensions/test/browser/browser_about_alt.js
@@ +57,5 @@
> +    if (aTopic == "domwindowclosed") {
> +      Services.ww.unregisterNotification(TEST_ww_observer);
> +
> +      info("About dialog closed, waiting for focus on browser window");
> +      waitForFocus(aCallback, gManagerWindow);

If I understand this correctly, this is the only line changed, right?  If the add-on manager is opened in a tab, passing either |window| or |gManagerWindow| here should not change the semantics of the test, but we'll see on try.

But to be honest, I don't understand the racing theory in comment 457.  |window| should always point to the browser window running the tests, not sure how it can point to the modal dialog here...
Attachment #8410981 - Flags: feedback?(ehsan) → feedback+
BTW, I'm going to laugh (read: cry) if this is a heisenbug where adding logging disrupts the test just enough to avoid it failing.
(In reply to :Irving Reid from comment #457)
> Created attachment 8410981 [details] [diff] [review]
> WIP 1: add some diagnostics to waitForFocus, try waiting for focus on
> specific window
> 
> I ran this on Try and retriggered a few of the tests (the test i'm working
> on is in the bc3 set).
> 
> I tried two things in this patch: add some diagnostics to see which window
> we're trying to focus on in waitForFocus(), and an alternate version of
> browser_about.js that specifies we want to wait for the Addon Manager window
> rather than picking up the default global Window. I'm not sure if this will
> help - we're running this in a 'domwindowclosed' event handler for a modal
> dialog; if we're losing the race such that the global 'window' object points
> at the (closing, but still around) modal dialog rather than the Addon
> Manager, I'd expect any attempt to set focus on the Addon Manager window to
> be blocked by the modal dialog.

The global "window" will never point to the modal dialog. Now it is possible the attempt to focus could be blocked but I think it shouldn't be because we only do it after the modal has been completely closed.
Comment on attachment 8410984 [details] [diff] [review]
WIP 2: Rewrite to task-style async, remove waitForFocus() after modal pop-up

Review of attachment 8410984 [details] [diff] [review]:
-----------------------------------------------------------------

I don't see how this can be a step in the right direction. You basically just remove the attempt to wait for the model dialog to close completely before proceeding so you'll race with that and the test ending.
Attachment #8410984 - Flags: feedback?(dtownsend+bugmail) → feedback-
(In reply to Dave Townsend [:mossop] from comment #463)
> Comment on attachment 8410984 [details] [diff] [review]
> WIP 2: Rewrite to task-style async, remove waitForFocus() after modal pop-up
> 
> Review of attachment 8410984 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I don't see how this can be a step in the right direction. You basically
> just remove the attempt to wait for the model dialog to close completely
> before proceeding so you'll race with that and the test ending.

I thought the call in the test that pops up the dialog:

  gManagerWindow.gViewController.doCommand("cmd_showItemAbout", selectedItem.mAddon);

is completely synchronous - that it won't return until the modal dialog is closed & gone. Is that not true?


(In reply to Dave Townsend [:mossop] from comment #462)
> (In reply to :Irving Reid from comment #457)
> > Created attachment 8410981 [details] [diff] [review]
> > WIP 1: add some diagnostics to waitForFocus, try waiting for focus on
> > specific window
> > 
> The global "window" will never point to the modal dialog. Now it is possible
> the attempt to focus could be blocked but I think it shouldn't be because we
> only do it after the modal has been completely closed.

OK, I'm still learning how the nesting of DOM UI elements works - I had seen some debug messages that made me think I was in a different window hierarchy when the modal was up, but now I've figured out that I was misreading the context of those messages.

That brings me back to my even-earlier theory (https://bugzilla.mozilla.org/show_bug.cgi?id=632290#c448), that the code we're running under the 'domwindowclosed' notification is racing with the teardown of the modal such that we sometimes find the object at the end of the thing->getFocusedNode() chain is something that can't actually be focused, or that it's OK when we get it but by the time waitForFocus() tries to reset focus on it, it's closed.
Flags: needinfo?(dtownsend+bugmail)
(In reply to :Irving Reid from comment #464)
> (In reply to Dave Townsend [:mossop] from comment #463)
> > Comment on attachment 8410984 [details] [diff] [review]
> > WIP 2: Rewrite to task-style async, remove waitForFocus() after modal pop-up
> > 
> > Review of attachment 8410984 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > I don't see how this can be a step in the right direction. You basically
> > just remove the attempt to wait for the model dialog to close completely
> > before proceeding so you'll race with that and the test ending.
> 
> I thought the call in the test that pops up the dialog:
> 
>   gManagerWindow.gViewController.doCommand("cmd_showItemAbout",
> selectedItem.mAddon);
> 
> is completely synchronous - that it won't return until the modal dialog is
> closed & gone. Is that not true?

I don't believe that it is completely true, it returns after the dialog has started to close but before it is completely gone away.

> (In reply to Dave Townsend [:mossop] from comment #462)
> > (In reply to :Irving Reid from comment #457)
> > > Created attachment 8410981 [details] [diff] [review]
> > > WIP 1: add some diagnostics to waitForFocus, try waiting for focus on
> > > specific window
> > > 
> > The global "window" will never point to the modal dialog. Now it is possible
> > the attempt to focus could be blocked but I think it shouldn't be because we
> > only do it after the modal has been completely closed.
> 
> OK, I'm still learning how the nesting of DOM UI elements works - I had seen
> some debug messages that made me think I was in a different window hierarchy
> when the modal was up, but now I've figured out that I was misreading the
> context of those messages.
> 
> That brings me back to my even-earlier theory
> (https://bugzilla.mozilla.org/show_bug.cgi?id=632290#c448), that the code
> we're running under the 'domwindowclosed' notification is racing with the
> teardown of the modal such that we sometimes find the object at the end of
> the thing->getFocusedNode() chain is something that can't actually be
> focused, or that it's OK when we get it but by the time waitForFocus() tries
> to reset focus on it, it's closed.

Plausible, there might be an even later window notification we need to use, maybe outer-window-destroyed. I really forget what order all these window notifications happen in
Flags: needinfo?(dtownsend+bugmail)
getFocusedNode doesn't (shouldn't) look at the DOM of another document (namely the modal window), right?  That's basically the main problem I have understanding the theory in comment 448.
Flags: needinfo?(enndeakin)
FYI, this isn't Linux only; it started out on Windows but has been seen on MacOS and Linux.
Irving, any new ideas here by chance? Would love to not take this failure into another ESR release.
Flags: needinfo?(irving)
In the hope that we can reduce this nasty intermittent, I've reworked the test a bit - aside from switching to Task-style async, this patch makes sure we unwind the stack before acting on the window in the 'domwindowopened' event handler, and waits for the modal dialog call to return (thus also unwinding the stack) before waiting for focus to come back to the AddonManager main window.

This doesn't address any of the underlying concerns about FocusManager, but hopefully letting the event loop spin renders that moot.

https://tbpl.mozilla.org/?tree=Try&rev=420afb446af7
Attachment #8410984 - Attachment is obsolete: true
Attachment #8449414 - Flags: review?(dtownsend+bugmail)
Flags: needinfo?(irving)
Attachment #8449414 - Flags: review?(dtownsend+bugmail) → review+
Assignee: irving → nobody
Status: ASSIGNED → NEW
IIUC, there's a patch here that got r+ in July and never landed?!?! Now that Irving's gone, can you please help find a new owner, Dave?
Flags: needinfo?(dtownsend)
Let's at least get the patch landed and see what happens: https://hg.mozilla.org/integration/fx-team/rev/83689aa1a00f
Flags: needinfo?(dtownsend)
(In reply to TBPL Robot from comment #583)

Hah, literally failed on the push from comment 582.
Keywords: leave-open
And backed out ... https://hg.mozilla.org/integration/fx-team/rev/16f2c2fe21d5
Flags: needinfo?(enndeakin)
Given this test has been giving us fits for literally years now, I think it's time to disable it if nobody has the time/inclination to sort out whatever's going on here.
Flags: needinfo?(dtownsend)
Actually, I've been looking at it the last couple of days, along with some other focus related test failures. I think the Linux failures may be a slightly different cause. I've added some logging but they are hard to reproduce with the logging added. If I can't get anything more in a few days, I'll disable it.
Over to Neil but yes if we can't figure this out just disable it. This is a little-used UI feature that frankly has little that could go wrong (and if it did it wouldn't be the end of the world) so I'd be fine with no testing.
Assignee: nobody → enndeakin
Flags: needinfo?(dtownsend)
(In reply to Dave Townsend [:mossop] from comment #709)
> Over to Neil but yes if we can't figure this out just disable it. This is a
> little-used UI feature that frankly has little that could go wrong (and if
> it did it wouldn't be the end of the world) so I'd be fine with no testing.

Disabled on Windows and Linux.
https://hg.mozilla.org/integration/mozilla-inbound/rev/c20505bb8fa0
status-firefox39: --- → disabled
Whiteboard: [test disabled on Linux and Windows]
https://hg.mozilla.org/mozilla-central/rev/c20505bb8fa0
Assignee: enndeakin → nobody
Bulk assigning P3 to all open intermittent bugs without a priority set in Firefox components per bug 1298978.
Priority: -- → P3
Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → INACTIVE
Assignee: nobody → ryanvm
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: