1252268 - If the slow script dialog aborts a script, that was invoked by an interval timer, kill the timer.

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

This fixes bug 1251475 on my Linux system.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8724974 [details] [diff] [review]
Patch

>+++ b/dom/bindings/CallbackObject.cpp
>+    if (mErrorResult.ErrorCodeIs(NS_ERROR_JS_CALL_FAILED)) {
>+      // Translate the exception to something more useful.
>+      if (needToDealWithException) {
>+        mErrorResult.Throw(NS_ERROR_DOM_JS_EXCEPTION);

This is going to fail an assert in ErrorResult::AssignErrorCode.  You'd probably want to throw NS_ERROR_UNEXPECTED to match the old behavior in this case.

But also, the other option is to put this branching inside the codegenned code and just directly ThrowUncatchableException() in there if !JS_HasPendingException(cx).  This has the benefit of not needing NS_ERROR_JS_CALL_FAILED and slightly simpler code in ~CallSetup (no changes needed to it at all) but the drawback of slightly bloatier codegenned code.  I think in practice I'd prefer this option because there's less magic involved.

r=me with that.  And good catch on all those things that claim to throw and never do!

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8725443 [details] [diff] [review]
Patch

>+ErrorResult::NoteJSContextException(JSContext* aCx) {

Curly on next line, please.

r=me

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Fixup some fallout

Now with a test, and one important fix.  If NoteJSContextException sets the ErrorResult to NS_ERROR_DOM_EXCEPTION_ON_JSCONTEXT, and ~CallSetup doesn't rethrow the exception, we have to change the result code to something else.  For now I just used NS_ERROR_UNEXPECTED to preserve the existing behavior.

testPromiseWithThrowingChromePromiseInit caught that.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8726462 [details] [diff] [review]
Fixup some fallout

We probably want the fixup to NS_ERROR_UNEXPECTED for promise then callbacks too.

>+    'TestFunctions.cpp',

Should that be debug-only, perhaps, and the header file export?  See the TestInterface* bits that are a bit below this part.

r=me

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f0afcff63e38

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f0afcff63e38