613823 - [SeaMonkey] test_idcheck.xul causes "mochitest-chrome: T-FAIL L-FAIL", after bug 612447 push, due to an ASSERTION

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect)

Description

[Serge Gautherie (:sgautherie)]

Regression timeframe:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=91980a82eeae&tochange=0012ca751ce1
-> (3 changesets for) bug 612447 !

{
You are not authorized to access bug #612447.
}
:-/

Comment 1

[Serge Gautherie (:sgautherie)]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1290292368.1290295468.26264.gz&fulltext=1
Linux comm-central-trunk debug test mochitest-other on 2010/11/20 14:32:48
The 3+1 js issue are already reported and the next test should be
{
11689 INFO TEST-PASS | chrome://mochitests/content/chrome/suite/common/tests/chrome/test_idcheck.xul | check id: inspector.xul#winInspectorMain
}


http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1290301313.1290303676.27357.gz&fulltext=1
Linux comm-central-trunk debug test mochitest-other on 2010/11/20 17:01:53
{
11688 INFO TEST-PASS | chrome://mochitests/content/chrome/suite/common/tests/chrome/test_idcheck.xul | check id: messenger.xul#contentAreaCommandsBundle
...
JavaScript strict warning: chrome://messenger-smime/content/msgHdrViewSMIMEOverlay.js, line 216: reference to undefined property msgWindow.msgHeaderSink
JavaScript strict warning: chrome://messenger-smime/content/msgHdrViewSMIMEOverlay.js, line 216: reference to undefined property msgWindow.msgHeaderSink
JavaScript strict warning: chrome://messenger-smime/content/msgHdrViewSMIMEOverlay.js, line 216: reference to undefined property msgWindow.msgHeaderSink
...
JavaScript error: , line 0: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIObserverService.removeObserver]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://navigator/content/tabbrowser.xml ::  :: line 2688"  data: no]
...
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/suite/common/tests/chrome/test_idcheck.xul | Exited with code 1 during test run
INFO | automation.py | Application ran for: 0:05:31.303596
}

Comment 2

[Serge Gautherie (:sgautherie)]

(In reply to comment #1)

Hum, don't mind the various js issues nor when the build crash exactly...
(Next build is a little different.)

Oh, looking earlier in the log of this test:
{
###!!! ASSERTION: Observer already in the list: 's->mMutationObservers.IndexOf(aMutationObserver) == nsTArray<int>::NoIndex', file ../../dist/include/nsINode.h, line 742
nsINode::AddMutationObserver [:743]
nsHTMLEditor::Init [editor/libeditor/html/nsHTMLEditor.cpp:280]
nsEditingSession::SetupEditorOnWindow [editor/composer/src/nsEditingSession.cpp:502]
nsEditingSession::EndDocumentLoad [editor/composer/src/nsEditingSession.cpp:1049]
nsEditingSession::OnStateChange [editor/composer/src/nsEditingSession.cpp:799]
nsDocLoader::FireOnStateChange [uriloader/base/nsDocLoader.cpp:1334]
nsDocLoader::doStopDocumentLoad [uriloader/base/nsDocLoader.cpp:953]
nsDocLoader::DocLoaderIsEmpty [uriloader/base/nsDocLoader.cpp:820]
nsDocLoader::OnStopRequest [uriloader/base/nsDocLoader.cpp:705]
nsLoadGroup::RemoveRequest [netwerk/base/src/nsLoadGroup.cpp:680]
nsLoadGroup::Cancel [netwerk/base/src/nsLoadGroup.cpp:334]
nsDocLoader::Stop [uriloader/base/nsDocLoader.cpp:329]
nsDocShell::Stop [docshell/base/nsDocShell.h:230]
nsDocLoader::Stop [uriloader/base/nsDocLoader.cpp:320]
nsDocShell::Stop [docshell/base/nsDocShell.h:230]
nsDocShell::Stop [docshell/base/nsDocShell.cpp:4220]
nsDocShell::Destroy [docshell/base/nsDocShell.cpp:4495]
nsXULWindow::Destroy [xpfe/appshell/src/nsXULWindow.cpp:529]
nsWebShellWindow::Destroy [xpfe/appshell/src/nsWebShellWindow.cpp:832]
nsChromeTreeOwner::Destroy [xpfe/appshell/src/nsChromeTreeOwner.cpp:383]
nsGlobalWindow::ReallyCloseWindow [dom/base/nsGlobalWindow.cpp:6102]
nsCloseEvent::Run [dom/base/nsGlobalWindow.cpp:5894]
nsThread::ProcessNextEvent [xpcom/threads/nsThread.cpp:626]
NS_ProcessNextEvent_P [nsThreadUtils.cpp:250]
mozilla::ipc::MessagePump::Run [ipc/glue/MessagePump.cpp:110]
MessageLoop::RunInternal [ipc/chromium/src/base/message_loop.cc:220]
MessageLoop::RunHandler [ipc/chromium/src/base/message_loop.cc:203]
MessageLoop::Run [ipc/chromium/src/base/message_loop.cc:175]
nsBaseAppShell::Run [widget/src/xpwidgets/nsBaseAppShell.cpp:187]
nsAppStartup::Run [toolkit/components/startup/src/nsAppStartup.cpp:191]
XRE_main [toolkit/xre/nsAppRunner.cpp:3691]
main [suite/app/nsSuiteApp.cpp:103]
libc.so.6 + 0x15dec
}

Comment 3

[Serge Gautherie (:sgautherie)]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1290304145.1290306871.6817.gz&fulltext=1
WINNT 5.2 comm-central-trunk debug test mochitest-other on 2010/11/20 17:49:05
{
###!!! ASSERTION: unable to initialize resource: 'Error', file e:/builds/slave/comm-central-trunk-win32-debug/build/mozilla/rdf/base/src/nsRDFService.cpp, line 1030
xul!mozilla::layers::LayerManagerD3D10::operator=+0x0000000000379738
...

###!!! ASSERTION: Observer already in the list: 's->mMutationObservers.IndexOf(aMutationObserver) == nsTArray<int>::NoIndex', file e:\builds\slave\comm-central-trunk-win32-debug\build\objdir\mozilla\dist\include\nsINode.h, line 742
xul!mozilla::layers::LayerManagerOGL::operator=+0x000000000072D72E
...

###!!! ASSERTION: XPConnect is being called on a scope without a 'Components' property!: 'Error', file e:/builds/slave/comm-central-trunk-win32-debug/build/mozilla/js/src/xpconnect/src/xpcwrappednativescope.cpp, line 779
xul!DumpJSValue+0x0000000000033A95
...

JavaScript error: chrome://inspector/content/jsutil/rdf/RDFU.js, line 148: XPCU is not defined
}

Among the other assertions, and the useless stacks,
Windows reports the same assertion.

Comment 4

[Serge Gautherie (:sgautherie)]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1290297499.1290299400.9697.gz&fulltext=1
OS X 10.5 comm-central-trunk debug test mochitest-other on 2010/11/20 15:58:19
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1290295172.1290296997.32377.gz&fulltext=1
OS X 10.6 comm-central-trunk debug test mochitest-other on 2010/11/20 15:19:32
{
###!!! ASSERTION: Want to fire mutation events, but it's not safe: '(aNode->IsNodeOfType(nsINode::eCONTENT) && static_cast<nsIContent*>(aNode)-> IsInNativeAnonymousSubtree()) || sScriptBlockerCount == sRemovableScriptBlockerCount', file /builds/slave/comm-central-trunk-macosx-debug/build/mozilla/content/base/src/nsContentUtils.cpp, line 3662
nsContentUtils::HasMutationListeners [content/base/src/nsContentUtils.cpp:3665]
nsGenericElement::SetAttr [content/base/src/nsGenericElement.cpp:4589]
...

###!!! ASSERTION: Observer already in the list: 's->mMutationObservers.IndexOf(aMutationObserver) == nsTArray<int>::NoIndex', file ../../dist/include/nsINode.h, line 742
nsINode::AddMutationObserver [:743]
nsHTMLEditor::Init [editor/libeditor/html/nsHTMLEditor.cpp:280]
...

...
}

Same assertion, among others.

Comment 5

[(no longer active)]

Does this crash intermittently or on every run?  Can someone with a suite build please test if running the test standalone causes in the same crash or not?

Comment 6

[Serge Gautherie (:sgautherie)]

(In reply to comment #5)
> Does this crash intermittently or on every run?  Can someone with a suite build
> please test if running the test standalone causes in the same crash or not?

"near perma-orange" afaict.
(I don't have a Debug Win2K build atm.)

Comment 7

[Justin Wood (:Callek)]

ehsan I'll take a few shots at it for you later this week. It certainly is ominous. [might make sense for me also to split up test_idcheck a bit like we have been meaning to]

Comment 8

[Justin Wood (:Callek)]

ehsan, ok its 100% reproduceable with the test.

I used MSVC to grab the stack: http://callek.pastebin.mozilla.org/877049

Comment 9

[Justin Wood (:Callek)]

ok... ###!!! ASSERTION: Observer already in the list: 's->mMutationObservers.IndexOf(aMutationObserve
r) == nsTArray<int>::NoIndex', file c:\t\obj\mozilla\dist\include\nsINode.h, line 742

(http://callek.pastebin.mozilla.org/877055)

looks like the likely culprit given the crash stack.  I'm going to try and get a stack for that assert, locally. And see if there is an associated js stack to boot.

Comment 10

[Justin Wood (:Callek)]

Ok, I had bad luck getting a better stack. Ehsan if you need more let me know.

Comment 11

[(no longer active)]

OK, I'm building suite from source now to investigate this on my own, I guess...

Comment 12

[(no longer active)]

roc, I think this does need to block.

Comment 13

[Justin Wood (:Callek)]

assert stack: http://callek.pastebin.mozilla.org/879931

and a different crash stack: http://callek.pastebin.mozilla.org/879932

Comment 14

[Justin Wood (:Callek)]

Ok, it could probably be reduced more; but here is a reduced version of test_idcheck: http://callek.pastebin.mozilla.org/879982

Comment 15

[(no longer active)]

I have a fix, I'm trying to see if I can write a test for it...

Comment 16

[(no longer active)]

Attachment: Patch (v1) [Checked in: Comment 24]

I tried creating a test for this, and it seems we'd have to pull in too much editor stuff from composer, which is not really comfortable.  I think we can rely on SM tinderboxen to test this maybe!

Comment 17

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 496375 [details] [diff] [review]
Patch (v1)
[Checked in: Comment 24]

Can you explain how the editor is already an observer here?

Comment 18

[(no longer active)]

(In reply to comment #17)
> Comment on attachment 496375 [details] [diff] [review]
>   --> https://bugzilla.mozilla.org/attachment.cgi?id=496375
> Patch (v1)
> 
> Can you explain how the editor is already an observer here?

The first nsHTMLEditor::Init call is from the composer js calling makeWindowEditable directly (note that this does not happen in Firefox), and the second call comes from nsEditingSession::EndDocumentLoad (as noted in the assertion stack).

Comment 19

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Hmm, should we just exit Init earlier if it gets called twice?

Comment 20

[(no longer active)]

(In reply to comment #19)
> Hmm, should we just exit Init earlier if it gets called twice?

I guess we could, but neither nsEditor or nsPlaintextEditor work in that way, so I don't see why we should change nsHTMLEditor like that.  Do you think we should?

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

No.

Comment 22

[Serge Gautherie (:sgautherie)]

(In reply to comment #16)
> I tried creating a test for this, and it seems we'd have to pull in too much
> editor stuff from composer, which is not really comfortable.  I think we can
> rely on SM tinderboxen to test this maybe!

Yes, could you create a specific SeaMonkey test for this? (if one is wanted)

Comment 23

[Justin Wood (:Callek)]

(In reply to comment #22)
> (In reply to comment #16)
> > I tried creating a test for this, and it seems we'd have to pull in too much
> > editor stuff from composer, which is not really comfortable.  I think we can
> > rely on SM tinderboxen to test this maybe!
> 
> Yes, could you create a specific SeaMonkey test for this? (if one is wanted)

I'll be trying to reduce this even further for a real m-c test for ehsan, otherwise I'll get the closest-approximation of a test landed for us.

I don't think ehsan has any other actionable items here other than the landing of his patch.

Comment 24

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/f236810876e7

Comment 25

[Serge Gautherie (:sgautherie)]

V.Fixed, per
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1291886623.1291890348.18269.gz
Linux comm-central-trunk debug test mochitest-other on 2010/12/09 01:23:43