17:26:56 INFO - TEST-START | jetpack-package/addon-sdk/source/test/test-selection.js.test Selection Listener on frame 17:26:57 INFO - ++DOCSHELL 1A361000 == 33 [pid = 2820] [id = 357] 17:26:57 INFO - ++DOMWINDOW == 140 (1A86C080) [pid = 2820] [serial = 946] [outer = 00000000] 17:26:57 INFO - ++DOMWINDOW == 141 (1A86CA80) [pid = 2820] [serial = 947] [outer = 1A86C080] 17:26:57 INFO - [2820] WARNING: NS_ENSURE_TRUE(mMutable) failed: file c:\builds\moz2_slave\try-w32-d-00000000000000000000\build\src\netwerk\base\nsSimpleURI.cpp, line 265 17:26:57 INFO - --DOMWINDOW == 140 (10653200) [pid = 2820] [serial = 893] [outer = 00000000] [url = data:text/html;charset=utf-8,%3Chtml%3E%20%20%3Cbody%3E%20%20%20%20%3Cdiv%3Efoo%3C%2Fdiv%3E%20%20%20%20%3Cdiv%3Eand%3C%2Fdiv%3E%20%20%20%20%3Ctextarea%3Enoodles%3C%2Ftextarea%3E%20%20%3C%2Fbody%3E%3C%2Fhtml%3E] 17:26:57 INFO - --DOMWINDOW == 139 (0F4B4280) [pid = 2820] [serial = 887] [outer = 00000000] [url = data:text/html;charset=utf-8,%3Chtml%3E%20%20%3Cbody%3E%20%20%20%20%3Cdiv%3Efoo%3C%2Fdiv%3E%20%20%20%20%3Cdiv%3Eand%3C%2Fdiv%3E%20%20%20%20%3Ctextarea%3Enoodles%3C%2Ftextarea%3E%20%20%3C%2Fbody%3E%3C%2Fhtml%3E] 17:26:57 INFO - --DOMWINDOW == 138 (10650A00) [pid = 2820] [serial = 889] [outer = 00000000] [url = data:text/html;charset=utf-8,%3Chtml%3E%20%20%3Cbody%3E%20%20%20%20%3Cdiv%3Efoo%3C%2Fdiv%3E%20%20%20%20%3Cdiv%3Eand%3C%2Fdiv%3E%20%20%20%20%3Ctextarea%3Enoodles%3C%2Ftextarea%3E%20%20%3C%2Fbody%3E%3C%2Fhtml%3E] 17:26:57 INFO - --DOMWINDOW == 137 (10652A80) [pid = 2820] [serial = 891] [outer = 00000000] [url = data:text/html;charset=utf-8,%3Chtml%3E%20%20%3Cbody%3E%20%20%20%20%3Cdiv%3Efoo%3C%2Fdiv%3E%20%20%20%20%3Cdiv%3Eand%3C%2Fdiv%3E%20%20%20%20%3Ctextarea%3Enoodles%3C%2Ftextarea%3E%20%20%3C%2Fbody%3E%3C%2Fhtml%3E] 17:26:57 INFO - ++DOCSHELL 1ADFB500 == 34 [pid = 2820] [id = 358] 17:26:57 INFO - ++DOMWINDOW == 138 (10650A00) [pid = 2820] [serial = 948] [outer = 00000000] 17:26:57 INFO - ++DOMWINDOW == 139 (1115FE00) [pid = 2820] [serial = 949] [outer = 10650A00] 17:26:57 INFO - Assertion failure: (((aRep->flags) & 0x1) != 0), at c:\builds\moz2_slave\try-w32-d-00000000000000000000\build\src\dom\base\ScriptSettings.cpp:484 17:26:57 INFO - #01: js::CallErrorReporter(JSContext *,char const *,JSErrorReport *) [js/src/jscntxt.cpp:794] 17:26:57 INFO - #02: ReportError [js/src/jscntxt.cpp:238] 17:26:57 INFO - #03: js::ReportErrorNumberVA(JSContext *,unsigned int,JSErrorFormatString const * (*)(void *,unsigned int),void *,unsigned int,js::ErrorArgumentsType,char *) [js/src/jscntxt.cpp:739] 17:26:57 INFO - #04: JS_ReportErrorNumberUC(JSContext *,JSErrorFormatString const * (*)(void *,unsigned int),void *,unsigned int,...) [js/src/jsapi.cpp:5071] 17:26:57 INFO - #05: js::AutoEnterPolicy::reportErrorIfExceptionIsNotPending(JSContext *,jsid) [js/src/proxy/Proxy.cpp:52] 17:26:57 INFO - #06: js::AutoEnterPolicy::AutoEnterPolicy(JSContext *,js::BaseProxyHandler const *,JS::Handle<JSObject *>,JS::Handle<jsid>,unsigned int,bool) [js/public/Proxy.h:618] 17:26:57 INFO - #07: js::Proxy::get(JSContext *,JS::Handle<JSObject *>,JS::Handle<JSObject *>,JS::Handle<jsid>,JS::MutableHandle<JS::Value>) [js/src/proxy/Proxy.cpp:276] 17:26:57 INFO - #08: js::proxy_GetProperty(JSContext *,JS::Handle<JSObject *>,JS::Handle<JSObject *>,JS::Handle<jsid>,JS::MutableHandle<JS::Value>) [js/src/proxy/Proxy.cpp:585] 17:26:57 INFO - #09: MaybeCallMethod [js/src/jsobj.cpp:3430] 17:26:57 INFO - #10: JS::OrdinaryToPrimitive(JSContext *,JS::Handle<JSObject *>,JSType,JS::MutableHandle<JS::Value>) [js/src/jsobj.cpp:3459] 17:26:57 INFO - #11: js::SecurityWrapper<js::CrossCompartmentWrapper>::defaultValue(JSContext *,JS::Handle<JSObject *>,JSType,JS::MutableHandle<JS::Value>) [js/src/proxy/SecurityWrapper.cpp:79] 17:26:57 INFO - #12: xpc::FilteringWrapper<js::SecurityWrapper<js::CrossCompartmentWrapper>,xpc::Opaque>::defaultValue(JSContext *,JS::Handle<JSObject *>,JSType,JS::MutableHandle<JS::Value>) [js/xpconnect/wrappers/FilteringWrapper.cpp:163] 17:26:57 INFO - #13: js::Proxy::defaultValue(JSContext *,JS::Handle<JSObject *>,JSType,JS::MutableHandle<JS::Value>) [js/src/proxy/Proxy.cpp:493] 17:26:57 INFO - #14: js::proxy_Convert(JSContext *,JS::Handle<JSObject *>,JSType,JS::MutableHandle<JS::Value>) [js/src/proxy/Proxy.cpp:666] 17:26:57 INFO - #15: js::ToPrimitive(JSContext *,JS::Handle<JSObject *>,JSType,JS::MutableHandle<JS::Value>) [js/src/jsobj.cpp:3310] 17:26:57 INFO - #16: js::ToPrimitive(JSContext *,JSType,JS::MutableHandle<JS::Value>) [js/src/jsobjinlines.h:544] 17:26:57 INFO - #17: js::ToStringSlow<1>(js::ExclusiveContext *,JS::Handle<JS::Value>) [js/src/jsstr.cpp:4202] 17:26:57 INFO - #18: js::ErrorReport::init(JSContext *,JS::Handle<JS::Value>) [js/src/jsexn.cpp:744] 17:26:57 INFO - #19: mozilla::dom::AutoJSAPI::~AutoJSAPI() [dom/base/ScriptSettings.cpp:331] 17:26:57 INFO - #20: nsXBLProtoImplAnonymousMethod::Execute(nsIContent *,JSAddonId *) [dom/xbl/nsXBLProtoImplMethod.cpp:336] 17:26:57 INFO - #21: nsXBLPrototypeBinding::BindingAttached(nsIContent *) [dom/xbl/nsXBLPrototypeBinding.cpp:263] 17:26:57 INFO - #22: nsXBLBinding::ExecuteAttachedHandler() [dom/xbl/nsXBLBinding.cpp:610] 17:26:57 INFO - #23: PresShell::FlushPendingNotifications(mozilla::ChangesToFlush) [layout/base/nsPresShell.cpp:4303] 17:26:57 INFO - #24: nsRefreshDriver::Tick(__int64,mozilla::TimeStamp) [layout/base/nsRefreshDriver.cpp:1603] 17:26:57 INFO - #25: nsRefreshDriver::DoTick() [layout/base/nsRefreshDriver.cpp:1383] 17:26:57 INFO - #26: nsRefreshDriver::WillRefresh(mozilla::TimeStamp) [layout/base/nsRefreshDriver.cpp:1904] 17:26:57 INFO - #27: nsRefreshDriver::DoTick() [layout/base/nsRefreshDriver.cpp:1383] 17:26:57 INFO - #28: nsRefreshDriver::FinishedWaitingForTransaction() [layout/base/nsRefreshDriver.cpp:1844] 17:26:57 INFO - #29: mozilla::layers::ClientLayerManager::DidComposite(unsigned __int64) [gfx/layers/client/ClientLayerManager.cpp:395] 17:26:57 INFO - #30: mozilla::layers::CompositorChild::RecvDidComposite(unsigned __int64 const &,unsigned __int64 const &) [gfx/layers/ipc/CompositorChild.cpp:296] 17:26:57 INFO - #31: mozilla::layers::PCompositorChild::OnMessageReceived(IPC::Message const &) [obj-firefox/ipc/ipdl/PCompositorChild.cpp:820] 17:26:57 INFO - #32: mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const &) [ipc/glue/MessageChannel.cpp:1218] 17:26:57 INFO - #33: mozilla::ipc::MessageChannel::DispatchMessageW(IPC::Message const &) [ipc/glue/MessageChannel.cpp:1144] 17:26:57 INFO - #34: mozilla::ipc::MessageChannel::OnMaybeDequeueOne() [ipc/glue/MessageChannel.cpp:1128] 17:26:57 INFO - #35: MessageLoop::RunTask(Task *) [ipc/chromium/src/base/message_loop.cc:362] 17:26:57 INFO - #36: MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const &) [ipc/chromium/src/base/message_loop.cc:372] 17:26:57 INFO - #37: MessageLoop::DoWork() [ipc/chromium/src/base/message_loop.cc:456] 17:26:57 INFO - #38: mozilla::ipc::DoWorkRunnable::Run() [ipc/glue/MessagePump.cpp:234] 17:26:57 INFO - #39: nsThread::ProcessNextEvent(bool,bool *) [xpcom/threads/nsThread.cpp:855] 17:26:57 INFO - #40: NS_ProcessNextEvent(nsIThread *,bool) [xpcom/glue/nsThreadUtils.cpp:265] 17:26:57 INFO - #41: mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate *) [ipc/glue/MessagePump.cpp:99] 17:26:57 INFO - #42: MessageLoop::RunInternal() [ipc/chromium/src/base/message_loop.cc:233] 17:26:57 INFO - #43: MessageLoop::RunHandler() [ipc/chromium/src/base/message_loop.cc:227] 17:26:57 INFO - #44: MessageLoop::Run() [ipc/chromium/src/base/message_loop.cc:201] 17:26:57 INFO - #45: nsBaseAppShell::Run() [widget/nsBaseAppShell.cpp:166] 17:26:57 INFO - #46: nsAppShell::Run() [widget/windows/nsAppShell.cpp:180] 17:26:57 INFO - #47: nsAppStartup::Run() [toolkit/components/startup/nsAppStartup.cpp:282] 17:26:57 INFO - #48: XREMain::XRE_mainRun() [toolkit/xre/nsAppRunner.cpp:4202] 17:26:57 INFO - #49: XREMain::XRE_main(int,char * * const,nsXREAppData const *) [toolkit/xre/nsAppRunner.cpp:4278] 17:26:57 INFO - #50: XRE_main [toolkit/xre/nsAppRunner.cpp:4498] 17:26:57 INFO - #51: do_main [browser/app/nsBrowserApp.cpp:294] 17:26:57 INFO - #52: NS_internal_main(int,char * *) [browser/app/nsBrowserApp.cpp:667] 17:26:57 INFO - #53: wmain [toolkit/xre/nsWindowsWMain.cpp:124] 17:26:57 INFO - #54: __tmainCRTStartup [f:/dd/vctools/crt/crtw32/startup/crt0.c:255] 17:26:57 INFO - #55: kernel32 + 0x17067

I can't reproduce this locally but looking at the stack this seems like the JS engine is doing something wrong? Bobby, you have blame for the assertion, can you shed any light here?

So. AutoJSAPI shims in WarningOnlyErrorReporter when it takes ownership of error reporting, because the JS engine is never supposed to call the error reporter for real errors when AutoJSPI is running the show. But as the stack shows, we do indeed end up triggering the error reporter. This is because we end up recursively generating an error, and turning _that_ into an exception fails for some reason, and ReportError falls back to invoking the error reporter. So I think we need ReportError (the one defined in jscntxt.cpp) to not fall back to the error reporter if the ErrorToException call fails. Ideally we'd NS_WARNING when that happens, but I don't think we can do that from within the JS engine. :-( Should be a 1-line change (plus comment). I'm happy to r+ it if it's green on try.

I may be misunderstanding you. Here is what I tried but it came out very not green on try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=66af64839c9d&exclusion_profile=false

Comment on attachment 8580151 [details] [diff] [review] patch Review of attachment 8580151 [details] [diff] [review]: ----------------------------------------------------------------- Not quite - I was suggesting making return following ErrorToException(...) unconditional, rather than only doing it in the case where ErrorToException succeeds.

So I tried returning unconditionally (https://hg.mozilla.org/try/rev/42906d06fe10) but that still caused a bunch of try failures: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f4f5221f449c. Then I tried only returning if it seems that AutoJSAPI owns error reporting (https://hg.mozilla.org/try/rev/e94179fc5756). That passes try, but didn't fix the failure :(

(In reply to Dave Townsend [:mossop] from comment #5) > So I tried returning unconditionally > (https://hg.mozilla.org/try/rev/42906d06fe10) but that still caused a bunch > of try failures: > https://treeherder.mozilla.org/#/jobs?repo=try&revision=f4f5221f449c. Then I > tried only returning if it seems that AutoJSAPI owns error reporting > (https://hg.mozilla.org/try/rev/e94179fc5756). That passes try, but didn't > fix the failure :( Hm, that last patch really should do it, I'd think. Can you link me to the crash stack a la comment 0? I found https://treeherder.mozilla.org/#/jobs?repo=try&revision=e94179fc5756&exclusion_profile=false , but there doesn't seem to be anything useful in the JP logs.

(In reply to Bobby Holley (:bholley) from comment #6) > (In reply to Dave Townsend [:mossop] from comment #5) > > So I tried returning unconditionally > > (https://hg.mozilla.org/try/rev/42906d06fe10) but that still caused a bunch > > of try failures: > > https://treeherder.mozilla.org/#/jobs?repo=try&revision=f4f5221f449c. Then I > > tried only returning if it seems that AutoJSAPI owns error reporting > > (https://hg.mozilla.org/try/rev/e94179fc5756). That passes try, but didn't > > fix the failure :( > > Hm, that last patch really should do it, I'd think. Can you link me to the > crash stack a la comment 0? > > I found > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=e94179fc5756&exclusion_profile=false , but there > doesn't seem to be anything useful in the JP logs. There are some other fixes that have to land before it will show up, this try run shows it though: https://treeherder.mozilla.org/logviewer.html#?job_id=5815869&repo=try

Hm, the last few frames of that stack appear garbled, which is very unfortunate - I have no idea how we end up invoking CallErrorReporter on that patch given your patch.

Oh it isn't the patch there causing it, it has probably been happening for a while but mochitest-jetpack hasn't been running for very long and for as long as it has been running the debug builds have been failing for other reasons too.

(In reply to Dave Townsend [:mossop] from comment #9) > Oh it isn't the patch there causing it, it has probably been happening for a > while but mochitest-jetpack hasn't been running for very long and for as > long as it has been running the debug builds have been failing for other > reasons too. Right, but given the second patch you linked to in comment 5 (unconditionally returning if autoJSAPIOwnsErrorReporting()), it seems like we should never be hitting that assertion.

Looks like this has been going on for a while, this is probably the same as bug 847840. There are some more stacks in there

Getting somewhere slowly. This fixes the assertion, but a new assertion pops up in its place: JavaScript error: , line 0: uncaught exception: unknown (can't convert to string) --DOCSHELL 0x13087c000 == 12 [pid = 3008] [id = 13] Assertion failure: !JS_IsExceptionPending(cx), at ./NodeBinding.cpp:146 #01: mozilla::dom::NodeBinding::get_parentNode(JSContext*, JS::Handle<JSObject*>, nsINode*, JSJitGetterCallArgs) (.NodeBinding.cpp:146, in XUL) #02: mozilla::dom::GenericBindingGetter(JSContext*, unsigned int, JS::Value*) (BindingUtils.cpp:2422, in XUL) #03: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (jscntxtinlines.h:235, in XUL) #04: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (Interpreter.cpp:502, in XUL) #05: js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>) (Interpreter.cpp:558, in XUL) #06: js::InvokeGetterOrSetter(JSContext*, JSObject*, JS::Value, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) (Interpreter.cpp:628, in XUL) #07: CallGetter(JSContext*, JS::Handle<JSObject*>, JS::Handle<js::Shape*>, JS::MutableHandle<JS::Value>) (NativeObject.cpp:1614, in XUL) #08: bool GetExistingProperty<(js::AllowGC)1>(JSContext*, js::MaybeRooted<JSObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<js::NativeObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<js::Shape*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::Value, (js::AllowGC)1>::MutableHandleType) (NativeObject.cpp:1661, in XUL) #09: bool NativeGetPropertyInline<(js::AllowGC)1>(JSContext*, js::MaybeRooted<js::NativeObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JSObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<jsid, (js::AllowGC)1>::HandleType, IsNameLookup, js::MaybeRooted<JS::Value, (js::AllowGC)1>::MutableHandleType) (NativeObject.cpp:1878, in XUL) #10: js::NativeGetProperty(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>) (NativeObject.cpp:1912, in XUL) #11: js::GetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>) (NativeObject.h:1434, in XUL) #12: GetPropertyOperation(JSContext*, js::InterpreterFrame*, JS::Handle<JSScript*>, unsigned char*, JS::MutableHandle<JS::Value>, JS::MutableHandle<JS::Value>) (Interpreter.cpp:260, in XUL) #13: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:2431, in XUL) #14: js::RunScript(JSContext*, js::RunState&) (Interpreter.cpp:452, in XUL) #15: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (Interpreter.cpp:521, in XUL) #16: js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>) (Interpreter.cpp:558, in XUL) #17: JS_CallFunctionValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) (jsapi.cpp:4335, in XUL) #18: JS::Call(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) (jsapi.h:3837, in XUL) #19: nsXBLProtoImplAnonymousMethod::Execute(nsIContent*, JSAddonId*) (nsXBLProtoImplMethod.cpp:333, in XUL) #20: nsXBLPrototypeBinding::BindingAttached(nsIContent*) (nsXBLPrototypeBinding.cpp:263, in XUL) #21: nsXBLBinding::ExecuteAttachedHandler() (nsXBLBinding.cpp:610, in XUL) #22: nsBindingManager::ProcessAttachedQueue(unsigned int) (nsBindingManager.cpp:435, in XUL) #23: PresShell::Initialize(int, int) (nsPresShell.cpp:1927, in XUL) #24: nsDocumentViewer::InitPresentationStuff(bool) (nsDocumentViewer.cpp:686, in XUL) #25: nsDocumentViewer::Show() (nsDocumentViewer.cpp:2074, in XUL) #26: nsDocShell::SetVisibility(bool) (nsDocShell.cpp:6358, in XUL) #27: non-virtual thunk to nsDocShell::SetVisibility(bool) (nsDocShell.cpp:6363, in XUL) #28: nsFrameLoader::Show(int, int, int, int, nsSubDocumentFrame*) (nsFrameLoader.cpp:754, in XUL) #29: nsSubDocumentFrame::ShowViewer() (nsSubDocumentFrame.cpp:173, in XUL) #30: AsyncFrameInit::Run() (nsSubDocumentFrame.cpp:84, in XUL) #31: nsContentUtils::RemoveScriptBlocker() (nsContentUtils.cpp:5052, in XUL) #32: nsAutoScriptBlocker::~nsAutoScriptBlocker() (nsContentUtils.h:2420, in XUL) #33: nsAutoScriptBlocker::~nsAutoScriptBlocker() (nsContentUtils.h:2420, in XUL) #34: PresShell::FlushPendingNotifications(mozilla::ChangesToFlush) (nsPresShell.cpp:4281, in XUL) #35: nsRefreshDriver::Tick(long long, mozilla::TimeStamp) (nsRefreshDriver.cpp:1607, in XUL) #36: mozilla::RefreshDriverTimer::TickDriver(nsRefreshDriver*, long long, mozilla::TimeStamp) (nsRefreshDriver.cpp:199, in XUL) #37: mozilla::RefreshDriverTimer::Tick(long long, mozilla::TimeStamp) (nsRefreshDriver.cpp:183, in XUL) #38: mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers(mozilla::TimeStamp) (nsRefreshDriver.cpp:441, in XUL) #39: mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) (nsRefreshDriver.cpp:376, in XUL) #40: void nsRunnableMethodArguments<mozilla::TimeStamp>::apply<mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver, void (mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::*)(mozilla::TimeStamp)>(mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver*, void (mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::*)(mozilla::TimeStamp)) (nsThreadUtils.h:588, in XUL) #41: nsRunnableMethodImpl<void (mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::*)(mozilla::TimeStamp), true, mozilla::TimeStamp>::Run() (nsThreadUtils.h:666, in XUL) #42: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:856, in XUL) #43: NS_ProcessPendingEvents(nsIThread*, unsigned int) (nsThreadUtils.cpp:207, in XUL) #44: nsBaseAppShell::NativeEventCallback() (nsBaseAppShell.cpp:99, in XUL) #45: nsAppShell::ProcessGeckoEvents(void*) (nsAppShell.mm:377, in XUL) #46: __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ (in CoreFoundation) + 17 #47: __CFRunLoopDoSources0 (in CoreFoundation) + 269 #48: __CFRunLoopRun (in CoreFoundation) + 927 #49: CFRunLoopRunSpecific (in CoreFoundation) + 296 #50: RunCurrentEventLoopInMode (in HIToolbox) + 235 #51: ReceiveNextEventCommon (in HIToolbox) + 431 #52: _BlockUntilNextEventMatchingListInModeWithFilter (in HIToolbox) + 71 #53: _DPSNextEvent (in AppKit) + 964 #54: -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (in AppKit) + 194 #55: -[GeckoNSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (nsAppShell.mm:118, in XUL) #56: -[NSApplication run] (in AppKit) + 594 #57: nsAppShell::Run() (nsAppShell.mm:651, in XUL) #58: nsAppStartup::Run() (nsAppStartup.cpp:281, in XUL) #59: XREMain::XRE_mainRun() (nsAppRunner.cpp:4202, in XUL) #60: XREMain::XRE_main(int, char**, nsXREAppData const*) (nsAppRunner.cpp:4278, in XUL) #61: XRE_main (nsAppRunner.cpp:4498, in XUL) #62: do_main(int, char**, nsIFile*) (nsBrowserApp.cpp:294, in firefox) #63: main (nsBrowserApp.cpp:667, in firefox) The bottom of the stack seems to match the last one but the assertion stems from an earlier point in nsXBLProtoImplAnonymousMethod::Execute which is weird but I guess it's just a later invocation or something. Still, that line looks a little suspect based on the comment above: http://mxr.mozilla.org/mozilla-central/source/dom/xbl/nsXBLProtoImplMethod.cpp#331 I did confirm that when the assertion happens AutoJSAPIOwnsErrorReporting is still true but I'm mostly stumbling in the dark here, any suggestions?

Comment on attachment 8582509 [details] [diff] [review] patch Review of attachment 8582509 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/ScriptSettings.cpp @@ +343,5 @@ > } > + > + // Now that any pending exception has been reported reset > + // AutoJSAPIOwnsErrorReporting. This must be the last thing before resetting > + // the error reporter. Explain why (i.e. mention the code in jscntxt.cpp).

So that assertion means that somebody is leaving an exception dangling on cx. My guess is that this is happening in ErrorReport::init (you'll see that it asserts against a pending exception at the top of the function). There should be no pending exception at the end of the AutoJSAPI destructor in the mOwnErrorReporting (i.e. !JS_IsExceptionPending), and there should be no pending exception when we return from ErrorReport::init. These bugs are annoying but pretty straightforward to debug. Just add assertions against pending exceptions in places where there shouldn't be one, and work backwards until you find the culprit. Culprits are usually someone that ignores failure return values from JSAPI methods. But the code in comment 12 looks ok, because the AutoEntryScript should be handling the exception as the comment suggests.

Tracked it down. IsDuckTypedErrorObject can generate exceptions so I've made it clear them after each JSAPI call which seems to match what ErrorReport::init does too. Only the first actually matters but I figured it was a good idea to clear them throughout.

Oh and this has had a nice green try run aside from known failures

Comment on attachment 8583880 [details] [diff] [review] patch Review of attachment 8583880 [details] [diff] [review]: ----------------------------------------------------------------- Is there any reason to add all the exception clearing in IsDuckTypedErrorObject rather than just doing it in the caller? ::: dom/base/ScriptSettings.cpp @@ +343,5 @@ > } > + > + // Now that any pending exception has been reported reset > + // AutoJSAPIOwnsErrorReporting. This must be the last thing before resetting > + // the error reporter or ReportError may try to send errors to our error This is still a tiny bit too vague. I'd say: We need to do this _after_ processing the existing exception, because the JS engine can throw while doing that, and uses this bit to determine what to do in that case: squelch the exception if the bit is set, otherwise call the error reporter. Calling WarningOnlyErrorReporter with a non-warning will assert, so we need to make sure we do the former.

(In reply to Bobby Holley (:bholley) from comment #17) > Comment on attachment 8583880 [details] [diff] [review] > patch > > Review of attachment 8583880 [details] [diff] [review]: > ----------------------------------------------------------------- > > Is there any reason to add all the exception clearing in > IsDuckTypedErrorObject rather than just doing it in the caller? I don't think it makes much difference but since it's called in an if statement It would be simplest to just clear the exception at the end of this large if block: http://mxr.mozilla.org/mozilla-central/source/js/src/jsexn.cpp#760 at which point I might as well remove all the exception clearing that goes on in there. Either that or define a new boolean with the result that the if statement uses. Not sure which you'd prefer.

Oh I see. How about just creating a tiny RAII class called AutoClearPendingException and declaring one in IsDuckTypedErrorObject? That's the cleanest thing I can think of anyway.

Comment on attachment 8584103 [details] [diff] [review] patch Review of attachment 8584103 [details] [diff] [review]: ----------------------------------------------------------------- Looks great! thanks. ::: js/src/jsexn.h @@ +119,5 @@ > +{ > + JSContext *cx; > + > + public: > + AutoClearPendingException(JSContext *cx) nit - call this cxArg to disambiguate the member.

Backed out for static analysis failures: https://hg.mozilla.org/integration/fx-team/rev/08cef2ba28a0 https://treeherder.mozilla.org/logviewer.html#?job_id=2425859&repo=fx-team

Ctor needs an 'explicit' keyword.

Pushed with that fix: https://hg.mozilla.org/integration/fx-team/rev/29d740684613