Seems mozmill tests are hitting this Assertion failure: IsOuterWindow(), at /builds/slave/tb-c-cen-lx-d-0000000000000000/build/mozilla/dom/base/nsGlobalWindow.cpp:7678 TEST-UNEXPECTED-FAIL | Disconnect Error: Application unexpectedly closed 908352 Permanent orange on Windows mozmill across many tests: TEST-UNEXPECTED-FAIL | Disconnect Error: Application unexpectedly closed OSError: [Errno 2] No such file or directory 566330 Linux64 Talos failure: OSError: [Errno 2] No such file or directory TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run Assertion failure: IsOuterWindow(), at /builds/slave/tb-c-cen-lx-d-0000000000000000/build/mozilla/dom/base/nsGlobalWindow.cpp:7678 TEST-UNEXPECTED-FAIL | Disconnect Error: Application unexpectedly closed 908352 Permanent orange on Windows mozmill across many tests: TEST-UNEXPECTED-FAIL | Disconnect Error: Application unexpectedly closed OSError: [Errno 2] No such file or directory 566330 Linux64 Talos failure: OSError: [Errno 2] No such file or directory TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run http://archive.mozilla.org/pub/thunderbird/tinderbox-builds/comm-central-linux-debug/1447499202/comm-central_ubuntu32_vm-debug_test-mozmill-bm01-tests1-linux32-build6.txt.gz

Assertion happens in nsGlobalWindow::OpenDialog() and was added with Bug 1216401. Assertion seem to happen during the following tests: TEST-START | C:\slave\test\build\tests\mozmill\composition\test-attachment-reminder.js | test_attachment_reminder_dismissal TEST-START | C:\slave\test\build\tests\mozmill\folder-display\test-message-commands-on-msgstore.js | test_mark_messages_replied

Running test-attachment-reminder.js locally, I see a couple of JavaScript error: chrome://messenger/content/messengercompose/MsgComposeCommands.js, line 4851: TypeError: gMsgCompose is null which is in InitEditor(), along with a bunch of timeouts. (May or may not be related to the assertion.)

(In reply to aleth [:aleth] from comment #2) > which is in InitEditor(), along with a bunch of timeouts. (May or may not be > related to the assertion.) The timeouts are related to waiting for windows/dialogs to open or focus, which could be connected to the bug mentined in comment 1.

Sadly I'm getting the MOZ_ASSERT(IsOuterWindow()); from dom/base/nsGlobalWindow.cpp:7678 when trying to send a message in a debug build. Stack is: nsGlobalWindow::OpenDialog() Line 7678 C++ nsMsgProgress::OpenProgressDialog() Line 80 C++ nsMsgCompose::SendMsg() Line 1246 C++ In SendMsg at this point it tries display the progress dialog: OpenProgressDialog(... "chrome://messenger/content/messengercompose/sendProgress.xul", ... Since this is MOZ_ASSERT, it's only triggered in a debug build. That explains why the "official" Daily I'm using works fine. Altogether we're pretty much busted with this assertion in place.

See bug 1216401 comment #12.

As per bug 1216401 comment #13. Aleth, can you please run the test, review, approve and land. I checked that with this patch I can send my message again, since the progress dialog opens successfully.

Comment on attachment 8688172 [details] [diff] [review] Bustage fix. Review of attachment 8688172 [details] [diff] [review]: ----------------------------------------------------------------- LGTM, but I'm not a peer of this code, so forwarding.

Comment on attachment 8688172 [details] [diff] [review] Bustage fix. Review of attachment 8688172 [details] [diff] [review]: ----------------------------------------------------------------- ::: mailnews/base/src/nsMsgProgress.cpp @@ +55,1 @@ > NS_ENSURE_ARG_POINTER(parent); fwiw, if the original null check here was necessary, this really should be nsCOMPtr<nsPIDOMWindow> parent(do_QueryInterface(parentDOMWindow)); NS_ENSURE_ARG_POINTER(parent); parent = parent->GetOuterWindow(); NS_ENSURE_ARG_POINTER(parent); It's not immediately obvious that using parent without null checking it first is same (because someone could in theory pass in a null parentDOMWindow.

This patch doesn't appear to fix the test failure from comment 2. Maybe there are other instances of the same problem?

OK, added one more check as per Kyle's comment, thanks for watching! Aleth, yes, most likely there are other cases that need the same fix. Those tests (like test-attachment-reminder.js, see comment #1) most likely display a dialog.

Awesome this is getting quick, solid attention. I'm changing this to blocker, because it is blocking chiaki's development work that we're trying to get in for version 45.

Aleth, can you please run the test(s) with the debugger attached and a breakpoint on nsGlobalWindow::OpenDialog() Line 7678 C++ From the call stack it should be obvious where we need to fix. I've looked for calls to OpenDialog() in C-C and there aren't many in C++ code, but heaps in JS code: http://mxr.mozilla.org/comm-central/search?string=OpenDialog Wayne: This was holding up *my* development work, so I started looking at it on a side line. Please note bug 1216401 comment #13: (Quote): [The assertion was added] because nothing in mozilla-central made that call on an inner window.

Calls from JS go through a different codepath that is both not affected by the changes and is used in Firefox.

Thanks, shouldn't be to hard then, I only see these in C++ code: /mailnews/addrbook/src/nsAbContentHandler.cpp /mailnews/base/src/nsMsgProgress.cpp -- fixed already /mailnews/news/src/nsNNTPNewsgroupList.cpp Which makes me doubt that this is the cause of the test failure, since from the name of the test it's neither 'address book' nor 'news'. Hmm. Anyway, a run with a debugger attached should give us clarity pretty quickly.

(In reply to aleth [:aleth] from comment #2) > Running test-attachment-reminder.js locally, ... Please let me know how to run the test locally. I'm not familiar with Mozmill testing at all.

(In reply to Jorg K (GMT+1) from comment #15) > (In reply to aleth [:aleth] from comment #2) > > Running test-attachment-reminder.js locally, ... > > Please let me know how to run the test locally. I'm not familiar with > Mozmill testing at all. e.g., from the objdir, run make SOLO_TEST=composition/test-attachment-reminder.js mozmill-one Thanks for looking into this!

Comment on attachment 8688175 [details] [diff] [review] Bustage fix (v2). Review of attachment 8688175 [details] [diff] [review]: ----------------------------------------------------------------- rs=me Are you going to do the other cases (comment 14) in this bug too?

Not right now, I got side-tracked to bug 653342. I haven't tested the other cases from comment #14. Also, there is still the issue of the test failing, comment #9, which may have other reasons. I don't want to read up on Mozmill testing right now. The fix should be easy: Catch it in the debugger. I'd be happy to land this patch and leave the bug open for someone else to continue.

https://hg.mozilla.org/comm-central/rev/dcb45e37d6dced19f7499135dc89caa1276c48ec Bug 1224840 - Fix Assertion failure: IsOuterWindow() in nsMsgProgress.cpp. r=mkmelin

Let's leave test failures not due to this assertion for another bug.

Sure, but the ones from comment #1 are fixed or not? Or is Stefan pointing the finger at the wrong tests: TEST-START | C:\slave\test\build\tests\mozmill\composition\test-attachment-reminder.js | test_attachment_reminder_dismissal TEST-START | C:\slave\test\build\tests\mozmill\folder-display\test-message-commands-on-msgstore.js | test_mark_messages_replied

Comment on attachment 8688529 [details] [diff] [review] Fix remaining "Assertion failure: IsOuterWindow()" Review of attachment 8688529 [details] [diff] [review]: ----------------------------------------------------------------- Thx, r=mkmelin

https://hg.mozilla.org/comm-central/rev/7824b6bb1869a884b2a6849f55bf36716527f079 Bug 1224840 - Fix remaining "Assertion failure: IsOuterWindow()". r=mkmelin

(In reply to Jorg K (GMT+1) from comment #22) > Sure, but the ones from comment #1 are fixed or not? Or is Stefan pointing > the finger at the wrong tests: > > TEST-START | > C:\slave\test\build\tests\mozmill\composition\test-attachment-reminder.js | > test_attachment_reminder_dismissal > > TEST-START | > C:\slave\test\build\tests\mozmill\folder-display\test-message-commands-on- > msgstore.js | test_mark_messages_replied Looks like they are gone.

I can't seem to set uplift flags on these patches, but they are needed in c-a.

Comment on attachment 8688529 [details] [diff] [review] Fix remaining "Assertion failure: IsOuterWindow()" I seem to be able to set them. Not sure why Aleth had issues.

(In reply to Kent James (:rkent) from comment #27) > Attachment #8688529 [details] [diff] - Flags: approval-mozilla-aurora? Shouldn't it be approval-comm-aurora? Maybe I'm just confused.

Comment on attachment 8688529 [details] [diff] [review] Fix remaining "Assertion failure: IsOuterWindow()" Yes this is supposed to be approval-comm-aurora

https://hg.mozilla.org/releases/comm-aurora/rev/c87549fd9bd7 https://hg.mozilla.org/releases/comm-aurora/rev/9f1e5fcd89ba