Open Bug 528706 Opened 13 years ago Updated 6 years ago

nsIWindowMediator::getMostRecentWindow should not return closed windows

Categories

(Core :: Widget, defect, P3)

defect

Tracking

()

People

(Reporter: dao, Unassigned)

References

Details

(Whiteboard: tpi:+)

I don't know if there's a legitimate use for getMostRecentWindow returning a window that is already closed, but in browser code it's probably never what we need. See bug 528440 where we worked around this.
or eventually could provide getMostRecentOpenWindow if the behavior is expected.
or an optional aOnlyOpenWindows param.
(In reply to comment #0)
> I don't know if there's a legitimate use for getMostRecentWindow returning a
> window that is already closed

I doubt it. It looks to me like it can only happen if you call window.close() from JS and then call getMostRecentWindow() before returning to the event loop (i.e. before the nsCloseEvent fires).
(In reply to comment #3)
> It looks to me like it can only happen if you call window.close()
> from JS and then call getMostRecentWindow() before returning to the event loop
> (i.e. before the nsCloseEvent fires).

Doesn't that mean that http://hg.mozilla.org/mozilla-central/rev/9eeafcb0fc7d wouldn't have helped for the "Only one window should exist after cleanup - Got 2, expected 1" case in bug 527074, since executeSoon is used between tests?
I don't know. There may well be other cases where it can occur, since the code isn't super easy to follow.

Are you confident that that change helped?
Yes, that failure is the first one that I managed to get rid of by simply not having the windows counting function count closed windows.
I should note that I got that failure locally quite constantly, which is why I was so fast pushing the fix. See bug 527074 comment 29.
Blocks: 530298
This bug alone just made me lose HOURS of debugging to figure why on a yield my objects were dying when with the old synchronous API they were not.

Considered we are moving more and more to async, this may now have a much larger impact.

I think we should just change it to not return a window if .closed is true.
Anything against that?
Flags: qe-verify-
Flags: in-testsuite?
Flags: firefox-backlog+
Flags: in-testsuite?
Priority: -- → P3
Whiteboard: tpi:+
You need to log in before you can comment on or make changes to this bug.