600703 - "ASSERTION: mArguments wasn't cleaned up properly" with showModalDialog

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Jesse Ruderman]

Attachment: testcase

1. Load the testcase (with popup blocking disabled).
2. Press Cmd+Q.

Result:

###!!! ASSERTION: Uh, LeaveModalState() called w/o a reachable top window?: 'Error', file dom/base/nsGlobalWindow.cpp, line 6140
(bug 398103?)

###!!! ASSERTION: mArguments wasn't cleaned up properly!: '!mArguments', file dom/base/nsGlobalWindow.cpp, line 861

Comment 1

[Jesse Ruderman]

Attachment: stack traces

Comment 2

[Nochum Sossonko [:Natch]]

Assigning a url that does it for me without having to quit the app. Just close the modal dialog and shortly thereafter the assert is hit.

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Nochum Sossonko [:Natch] from comment #2)
> Assigning a url that does it for me without having to quit the app. Just
> close the modal dialog and shortly thereafter the assert is hit.

I can reproduce with the same testcase.

Comment 4

[Jesse Ruderman]

This assertion was added in https://github.com/mozilla/mozilla-central/commit/0d8b79a0bf88c75d1f4507f07cb5f95228397f73

Author:   jst
Date:     2005 Aug 12
Summary:  Followup changes to bz's review for bug 296639. Reviews pending, a=drivers

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Isn't this a dup? And IIRC dup of a somewhat invalid bug. We should just remove the assertion.
(maybe change it to NS_WARN_IF_FALSE)

Comment 6

[Jesse Ruderman]

The only other bug I see that mentions "mArguments wasn't cleaned up properly" is bug 640127, which is still open.

Comment 7

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

I worked around this in:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d08c12966908
in prep for bug 404077.


That said... if we don't call the cleanup functions (DetachFromDocShell for outers, FreeInnerObjects for inners), can't we end up leaking windows if things aren't easy to clean up?  And isn't this assertion a sign that we didn't?  (Or is there actually a path from FreeInnerObjects to freeing mArguments?)

Comment 8

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

and the same to 2 more files in
https://hg.mozilla.org/integration/mozilla-inbound/rev/c7cedc21bdf2

Comment 9

[Andrew McCreight [:mccr8]]

Maybe I'll get a chance to look at this soon.

Comment 10

[Bobby Holley (:bholley)]

I recently embarked on an epic quest to make showModalDialog more sane.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Fixed by bug 860941.