Closed Bug 587960 Opened 9 years ago Closed 9 years ago

mochitest failure due to test_tree_view.xul with 130078


(Core :: XUL, defect)

Not set





(Reporter: tnikkel, Assigned: tnikkel)




(2 files)

Basically the same thing as bug 550029 is happening with 130078 on try server.

I can't seem to reproduce without forcing pages to paint while the next page has painting suppressed.

Is there something simple we can do like bug 550029 to stop this from being a problem?
Probably. What's the stack for the entry point to treeview script?
I have to make changes to the source in order to reproduce, so I can't trust the stacks I would get locally. But looking at the "xpconnect is being called with a scope" assert stacks from tinderbox, they all either go through nsTreeBodyFrame::PaintTreeBody or nsTreeBodyFrame::GetCursor, with GetCursor looking like it is the only one that causes the javascript errors that cause the test to fail. So I think a similar check in GetCursor should do it.
OK, I'm ready to review a patch :-)
Review should be straight forward, I need to try server it to make sure it fixes the problem first.
Looks like just bailing in GetCursor was not enough. I'll try bailing in PaintTreeBody as well.
Hmm, we still get the "XPConnect is being called on a scope without a 'Components' property!" asserts with GetCursor in the stack with the "GetContent()->GetCurrentDoc()->GetScriptGlobalObject()" check in GetCursor. Maybe we need something else to fix this?
I don't know, because I don't know XPConnect/JS well enough.
The assertions themselves don't make the test fail. I was just using them to get an idea of what is being called at that time.

So the test fails when getCellText is called, we can tell that from the line number of the error. I tried guarding every thing I could think of in nsTreeBodyFrame.cpp with mView and GetContent()->GetCurrentDoc()->GetScriptGlobalObject() check. That didn't work. I tried to use nsIDebug to raise an assertion of when treeData is null from js to see whose calling the function that is the problem, but that fails because I need UniversalXPConnect for that and when the problems happens "netscape" is undefined.

I'm out of ideas, so I think I'll just try guarding access to treeData in the test with null checks. I guess the real bug here will have to wait.
I don't like this patch, but without being able to reproduce this failure in any form where I can get my hands on it (even with clumsy try server debugging) leaves me few options.
Attachment #467693 - Flags: review?(roc)
Actually, after playing around with it with some patches applied to make the problem happen more easily locally I think it is the GetCursor call, it's just that GetContent()->GetCurrentDoc()->GetScriptGlobalObject() check passes, but the window is still in a bad state. There must be some property we can check on script global object to see if calling code which can execute js is going to fail.
Let's wait for someone who knows about that to respond.
Summary: Random mochitest failure due to test_tree_view.xul with 130078 → mochitest failure due to test_tree_view.xul with 130078
GetScriptGlobalObject can return non-null after the document is removed from its docshell; it's a pretty messed-up API.

What does GetScriptHandlingObject() return in your situation?

I'll look into fixing this properly and comment 12 yet.
Checking GetScriptHandlingObject() for null seems to fix the problem.
Attached patch patchSplinter Review
Assignee: nobody → tnikkel
Attachment #475017 - Flags: review?(bzbarsky)
Comment on attachment 475017 [details] [diff] [review]

Add a comment that explains why we're doing that call (to prevent running code in cases when our document is a zombie), and r=me.
Attachment #475017 - Flags: review?(bzbarsky) → review+
Attachment #475017 - Flags: approval2.0?
Backed out the test workaround
Landed the fix
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.