675179 - Errors in cached dynamic overlay script cause XBL to trip up with an assertion

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[[e-mail]]

I have a preference window. As with many preference windows, it uses dynamic overlays to populate itself. One of these overlays has itself an overlay provided by an extension. The extension's overlay unfortunately has a JavaScript error in one of its scripts.

When the preference window is first loaded, and the overlaid pane is selected, the JavaScript error is reported, but then execution continues normally with the next script.

However when the preference window is next loaded, the JavaScript error is not handled correctly, and XBL asserts and fails to bind its fields:

###!!! ASSERTION: Shouldn't get here when an exception is pending!: '!::JS_IsExceptionPending(cx)', file content/xbl/src/nsXBLProtoImplField.cpp, line 122

Obviously the assertion does not occur if the XUL cache is disabled.

Comment 1

[Boris Zbarsky [:bzbarsky]]

What sends the pending exception the second time the preference window is loaded?  Presumably something should report the exception after that, but it's hard to tell what JSAPI entry point is involved without a testcase.  Want to just get a stack to the JS_SetPendingException call?

Comment 2

[[e-mail]]

This is the stack to JS_SetPendingException when the overlay is loaded for the second time, i.e. cached. (JS_SetPendingException does not get hit when the overlay is loaded for the first time, i.e. uncached.)

mozjs!JS_SetPendingException
mozjs!js_ErrorToException+0x2b0
mozjs!ReportError+0x76
mozjs!js_ReportErrorNumberVA+0xb1
mozjs!JS_ReportErrorNumber+0x27
mozjs!js_ReportIsNotDefined+0x19
mozjs!js::Interpret+0x18ef3
mozjs!js::RunScript+0x10b
mozjs!js::Execute+0x189
mozjs!js::ExternalExecute+0xdd
mozjs!JS_ExecuteScript+0xd2
xul!nsJSContext::ExecuteScript+0x2af
xul!nsXULDocument::ExecuteScript+0xcd
xul!nsXULDocument::ExecuteScript+0x1bd
xul!nsXULDocument::ResumeWalk+0x5f5
xul!nsXULDocument::OnPrototypeLoadDone+0x85
xul!nsXULDocument::LoadOverlayInternal+0x229
xul!nsXULDocument::LoadOverlay+0x136
xul!NS_InvokeByIndex_P+0x27

Comment 3

[Boris Zbarsky [:bzbarsky]]

> JS_SetPendingException does not get hit when the overlay is loaded for the
> first time

Uh.... but an exception _is_ thrown that time, no?

Comment 4

[[e-mail]]

Well, one appears in the Error Console, yes...

Comment 5

[Boris Zbarsky [:bzbarsky]]

It's really not possible for that to happen without a JS_SetPendingException somewhere....

Comment 6

[[e-mail]]

Indeed. Maybe I had disabled the breakpoint by mistake. Anyway, stack:

mozjs!JS_SetPendingException
mozjs!js_ErrorToException+0x2b0
mozjs!ReportError+0x76
mozjs!js_ReportErrorNumberVA+0xb1
mozjs!JS_ReportErrorNumber+0x27
mozjs!js_ReportIsNotDefined+0x19
mozjs!js::Interpret+0x18ef3
mozjs!js::RunScript+0x10b
mozjs!js::Execute+0x189
mozjs!js::ExternalExecute+0xdd
mozjs!JS_ExecuteScript+0xd2
xul!nsJSContext::ExecuteScript+0x2af
xul!nsXULDocument::ExecuteScript+0xcd
xul!nsXULDocument::ExecuteScript+0x1bd
xul!nsXULDocument::ResumeWalk+0x5f5
xul!nsXULDocument::OnPrototypeLoadDone+0x85
xul!nsXULDocument::EndLoad+0x2eb
xul!XULContentSinkImpl::DidBuildModel+0x50
xul!nsParser::DidBuildModel+0xb8
xul!nsParser::ResumeParse+0x302
xul!nsParser::OnStopRequest+0x12f
xul!nsJARChannel::OnStopRequest+0x99
xul!nsInputStreamPump::OnStateStop+0xde
xul!nsInputStreamPump::OnInputStreamReady+0x90
xul!nsInputStreamReadyEvent::Run+0x4a
xul!nsThread::ProcessNextEvent+0x2a4
xul!NS_ProcessNextEvent_P+0x53
xul!mozilla::ipc::MessagePump::Run+0xfd
xul!MessageLoop::RunInternal+0x4e
xul!MessageLoop::RunHandler+0x82
xul!MessageLoop::Run+0x1d
xul!nsBaseAppShell::Run+0x50
xul!nsAppShell::Run+0x47
xul!nsAppStartup::Run+0x6a
xul!XRE_main+0x2f97

Comment 7

[Boris Zbarsky [:bzbarsky]]

Right, so the difference is that in the comment 2 situation script is on the stack.

This looks like just a bug in nsJSContext::ExecuteScript.  It should ReportPendingException() just like nsJSContext::EvaluateStringWithValue.  Does doing that help?

Comment 8

[[e-mail]]

Attachment: Proposed patch

This prevents the assertion, and the pref pane now opens correctly.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 549464 [details] [diff] [review]
Proposed patch

r=me

Comment 10

[[e-mail]]

Pushed changeset 3efdbc58a094 to mozilla-central.