Last Comment Bug 675179 - Errors in cached dynamic overlay script cause XBL to trip up with an assertion
: Errors in cached dynamic overlay script cause XBL to trip up with an assertion
Status: RESOLVED FIXED
: assertion
Product: Core
Classification: Components
Component: XUL (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: neil@parkwaycc.co.uk
:
: Neil Deakin
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-07-29 04:30 PDT by neil@parkwaycc.co.uk
Modified: 2011-07-30 16:10 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Proposed patch (675 bytes, patch)
2011-07-29 14:10 PDT, neil@parkwaycc.co.uk
bzbarsky: review+
Details | Diff | Splinter Review

Description neil@parkwaycc.co.uk 2011-07-29 04:30:03 PDT
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 [:bz] (still a bit busy) 2011-07-29 07:44:55 PDT
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 neil@parkwaycc.co.uk 2011-07-29 07:52:01 PDT
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 [:bz] (still a bit busy) 2011-07-29 08:07:56 PDT
> 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 neil@parkwaycc.co.uk 2011-07-29 08:13:20 PDT
Well, one appears in the Error Console, yes...
Comment 5 Boris Zbarsky [:bz] (still a bit busy) 2011-07-29 09:41:06 PDT
It's really not possible for that to happen without a JS_SetPendingException somewhere....
Comment 6 neil@parkwaycc.co.uk 2011-07-29 09:53:07 PDT
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 [:bz] (still a bit busy) 2011-07-29 10:40:36 PDT
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 neil@parkwaycc.co.uk 2011-07-29 14:10:04 PDT
Created attachment 549464 [details] [diff] [review]
Proposed patch

This prevents the assertion, and the pref pane now opens correctly.
Comment 9 Boris Zbarsky [:bz] (still a bit busy) 2011-07-29 20:08:00 PDT
Comment on attachment 549464 [details] [diff] [review]
Proposed patch

r=me
Comment 10 neil@parkwaycc.co.uk 2011-07-30 16:10:25 PDT
Pushed changeset 3efdbc58a094 to mozilla-central.

Note You need to log in before you can comment on or make changes to this bug.