Bug 171776 – Introduce ExceptionScope::assertNoException() and releaseAssertNoException().

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Lam]

Instead of ASSERT(!scope.exception()), we can now do scope.assertNoException().  Ditto for RELEASE_ASSERT and scope.releaseAssertNoException().  

The advantage of using ExceptionScope::assertNoException() and releaseAssertNoException() is that if the assertion fails, these utility functions will print the stack trace for where the unexpected exception is detected as well as where the unexpected exception was thrown from.  This makes it much easier to debug the source of unhandled exceptions.

[Mark Lam]

Created attachment 309300 [details]
proposed patch.

Let's try this on the EWS.

[Build Bot]

Comment on attachment 309300 [details]
proposed patch.

Attachment 309300 [details] did not pass jsc-ews (mac):
Output: http://webkit-queues.webkit.org/results/3689922

New failing tests:
ChakraCore.yaml/ChakraCore/test/Strings/HTMLHelpers.js.default

[Mark Lam]

(In reply to Build Bot from comment #2)
> Comment on attachment 309300 [details]
> proposed patch.
> 
> Attachment 309300 [details] did not pass jsc-ews (mac):
> Output: http://webkit-queues.webkit.org/results/3689922
> 
> New failing tests:
> ChakraCore.yaml/ChakraCore/test/Strings/HTMLHelpers.js.default

I can't reproduce this locally.  Will rebase and try again.

[Mark Lam]

(In reply to Mark Lam from comment #3)
> (In reply to Build Bot from comment #2)
> > Comment on attachment 309300 [details]
> > proposed patch.
> > 
> > Attachment 309300 [details] did not pass jsc-ews (mac):
> > Output: http://webkit-queues.webkit.org/results/3689922
> > 
> > New failing tests:
> > ChakraCore.yaml/ChakraCore/test/Strings/HTMLHelpers.js.default
> 
> I can't reproduce this locally.  Will rebase and try again.

Nope, not bogus.  I can reproduce it now after I rebased to ToT.  Probably not due to my patch, but I'll investigate.

[Mark Lam]

(In reply to Mark Lam from comment #4)
> (In reply to Mark Lam from comment #3)
> > (In reply to Build Bot from comment #2)
> > > Comment on attachment 309300 [details]
> > > proposed patch.
> > > 
> > > Attachment 309300 [details] did not pass jsc-ews (mac):
> > > Output: http://webkit-queues.webkit.org/results/3689922
> > > 
> > > New failing tests:
> > > ChakraCore.yaml/ChakraCore/test/Strings/HTMLHelpers.js.default
> > 
> > I can't reproduce this locally.  Will rebase and try again.
> 
> Nope, not bogus.  I can reproduce it now after I rebased to ToT.  Probably
> not due to my patch, but I'll investigate.

OK, I found out that:
1. the issue reproduces without my patch.
2. the issue is intermittent and doesn't always reproduce.

Given that, I don't think it should block this patch.  Let's get a review.

[Mark Lam]

(In reply to Mark Lam from comment #5)
> > Nope, not bogus.  I can reproduce it now after I rebased to ToT.  Probably
> > not due to my patch, but I'll investigate.
> 
> OK, I found out that:
> 1. the issue reproduces without my patch.
> 2. the issue is intermittent and doesn't always reproduce.

FYI, I filed https://bugs.webkit.org/show_bug.cgi?id=171786 to track the ChakraCore test failure bug.

[Keith Miller]

Comment on attachment 309300 [details]
proposed patch.

r=me.

[Mark Lam]

Thanks for the review.  Landed in r216428: <http://trac.webkit.org/r216428>.

[Ryan Haddad]

(In reply to Mark Lam from comment #8)
> Thanks for the review.  Landed in r216428: <http://trac.webkit.org/r216428>.

It appears that this change broke the Windows Debug build: https://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/1120

[Mark Lam]

(In reply to Ryan Haddad from comment #9)
> (In reply to Mark Lam from comment #8)
> > Thanks for the review.  Landed in r216428: <http://trac.webkit.org/r216428>.
> 
> It appears that this change broke the Windows Debug build:
> https://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/
> 1120

Speculative build fix landed in r216444: <http://trac.webkit.org/r216444>.