550029 - Random mochitest failure due to test_tree_view.xul

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

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

Running mochitests under record-and-replay I got

162519 INFO Running /tests/toolkit/content/tests/widgets/test_videocontrols.html...
162520 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/content/tests/widgets/test_videocontrols.html | [SimpleTest/SimpleTest.js, window.onerror] An error occurred - netscape is not defined at http://localhost:8888/tests/toolkit/content/tests/widgets/test_tree_view.xul:50
162521 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/content/tests/widgets/test_videocontrols.html | [SimpleTest/SimpleTest.js, window.onerror] An error occurred - treeData is not defined at http://localhost:8888/tests/toolkit/content/tests/widgets/test_tree_view.xul:27
162522 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/content/tests/widgets/test_videocontrols.html | [SimpleTest/SimpleTest.js, window.onerror] An error occurred - treeData is not defined at http://localhost:8888/tests/toolkit/content/tests/widgets/test_tree_view.xul:27
162523 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/content/tests/widgets/test_videocontrols.html | [SimpleTest/SimpleTest.js, window.onerror] An error occurred - treeData is not defined at http://localhost:8888/tests/toolkit/content/tests/widgets/test_tree_view.xul:27
162524 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/content/tests/widgets/test_videocontrols.html | [SimpleTest/SimpleTest.js, window.onerror] An error occurred - treeData is not defined at http://localhost:8888/tests/toolkit/content/tests/widgets/test_tree_view.xul:27
162525 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/content/tests/widgets/test_videocontrols.html | [SimpleTest/SimpleTest.js, window.onerror] An error occurred - treeData is not defined at http://localhost:8888/tests/toolkit/content/tests/widgets/test_tree_view.xul:27
162526 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/content/tests/widgets/test_videocontrols.html | [SimpleTest/SimpleTest.js, window.onerror] An error occurred - treeData is not defined at http://localhost:8888/tests/toolkit/content/tests/widgets/test_tree_view.xul:27
162527 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/content/tests/widgets/test_videocontrols.html | [SimpleTest/SimpleTest.js, window.onerror] An error occurred - treeData is not defined at http://localhost:8888/tests/toolkit/content/tests/widgets/test_tree_view.xul:27
162528 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/content/tests/widgets/test_videocontrols.html | [SimpleTest/SimpleTest.js, window.onerror] An error occurred - treeData is not defined at http://localhost:8888/tests/toolkit/content/tests/widgets/test_tree_view.xul:27

It looks like we're calling back into the tree-view JS after we've already moved on to the next test, which doesn't work and also somehow triggers an error report to the window for test_videocontrols.html. That's quite disturbing actually since test_tree_view's JS should not be running in the context of the other page...

Comment 1

[Boris Zbarsky [:bzbarsky]]

Huh.  That onerror handler triggering there is indeed very weird.  We should probably have some checks about globals there or something to prevent that event from going to the new inner window due to the error on the old inner.

Comment 2

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

I'll try to get more details, but unfortunately the VMWare Visual Studio plugin crashed and somehow uninstalled itself... This is probably a good time to update to VMWare 7.0.1.

Comment 3

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

Unsurprisingly perhaps, this error occurs while we're painting the window.

I guess this is a situation where the old document is a zombie and the content viewer for the new document hasn't been shown yet because not enough of the new document has loaded.

Comment 4

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

Attachment: fix

I think this will fix it. I hope it makes sense. It does mean that when the document enters the zombie state, trees will paint differently from before, so in theory we should invalidate any trees... but who cares, really.

Comment 5

[Johnny Stenback (:jst)]

Roc, bz is out for at least a week, could someone else (dbaron?) review here?

Comment 6

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

Comment on attachment 432493 [details] [diff] [review]
fix

Could we bail out earlier when building display list? Probably when stepping into a new presshell?

Comment 7

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

We still want to paint these zombie documents. The new document isn't ready to paint yet.

Comment 8

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

So is the problem that error event from previous page is fired in the next page?
What about preventing error event in this case.
I wonder if the problem happens because of the async error event.

ScriptErrorEvent::Run could check that the current inner window is the same as
what it was when creating the object.

Comment 9

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

That is a problem, yes.

However, I don't think we should be calling into script that can't possibly run correctly.

Comment 10

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

I don't think the problem is because of the async error event. By the time we run the old script, the current inner window is already the new window.

Comment 12

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

I don't have a copy of the stack, and I think I deleted the VM recording, but it was very simple. It's a regular asynchronous paint, which goes into nsTreeBodyFrame::PaintColumn, which calls mView->GetColumnProperties, which runs page script, which raises an error because all the properties of its global object are gone. I'm not sure how that error ends up firing on the window of the new doc, though.

Comment 13

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

Comment on attachment 432493 [details] [diff] [review]
fix

Ok, let's try this.

Comment 14

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

http://hg.mozilla.org/mozilla-central/rev/6a398023b956

Comment 15

[Timothy Nikkel (:tnikkel)]

I think I saw another instance of this on try server
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1277260471.1277261468.18111.gz

17564 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/content/tests/widgets/test_videocontrols.html | [SimpleTest/SimpleTest.js, window.onerror] An error occurred - treeData is not defined at http://mochi.test:8888/tests/toolkit/content/tests/widgets/test_tree_view.xul:27
JavaScript error: http://mochi.test:8888/tests/toolkit/content/tests/widgets/test_tree_view.xul, line 27: treeData is not defined

and a bunch of

###!!! ASSERTION: XPConnect is being called on a scope without a 'Components' property!: 'Error', file /builds/slave/tryserver-macosx-debug/build/js/src/xpconnect/src/xpcwrappednativescope.cpp, line 784