Bug 1224007 part 6. Change MaybeSetPendingException to set the ErrorResult state to "not failed", just like SuppressException and StealNSResult already do, and assert in the destructor that the ErrorResult is not Failed().

This is not quite as strong as being able to assert that all codepaths that might
lead to failure call one of the above methods, but being able to assert that
would involve a lot of extra noise at callsites.  Or at least changing the
signature of StealNSResult to use an outparam and return a boolean indicating
whether the ErrorResult was failure or not, or something, so StealNSResult
can be usefully called on successful ErrorResults too.
This commit is contained in:
Boris Zbarsky 2015-11-20 16:29:41 -05:00
parent ef284c00d0
commit 3e01108efe
2 changed files with 23 additions and 5 deletions

View File

@ -209,6 +209,7 @@ ErrorResult::SetPendingExceptionWithMessage(JSContext* aCx)
argCount > 0 ? args : nullptr);
ClearMessage();
mResult = NS_OK;
}
void
@ -263,9 +264,7 @@ ErrorResult::SetPendingJSException(JSContext* cx)
// what, go ahead and unroot mJSException.
js::RemoveRawValueRoot(cx, &mJSException);
// We no longer have a useful exception but we do want to signal that an error
// occured.
mResult = NS_ERROR_FAILURE;
mResult = NS_OK;
#ifdef DEBUG
mUnionState = HasNothing;
#endif // DEBUG
@ -333,6 +332,7 @@ ErrorResult::SetPendingDOMException(JSContext* cx)
dom::Throw(cx, mDOMExceptionInfo->mRv, mDOMExceptionInfo->mMessage);
ClearDOMExceptionInfo();
mResult = NS_OK;
}
void
@ -372,6 +372,7 @@ ErrorResult::SetPendingGenericErrorException(JSContext* cx)
MOZ_ASSERT(!IsJSException());
MOZ_ASSERT(!IsDOMException());
dom::Throw(cx, ErrorCode());
mResult = NS_OK;
}
ErrorResult&
@ -468,12 +469,14 @@ ErrorResult::SetPendingException(JSContext* cx)
JS_ClearPendingException(cx);
// Don't do any reporting. Just return, to create an
// uncatchable exception.
mResult = NS_OK;
return;
}
if (IsJSContextException()) {
// Whatever we need to throw is on the JSContext already. We
// can't assert that there is a pending exception on it, though,
// because in the uncatchable exception case there won't be one.
mResult = NS_OK;
return;
}
if (IsErrorWithMessage()) {

View File

@ -6,6 +6,17 @@
/**
* A struct for tracking exceptions that need to be thrown to JS.
*
* Conceptually, an ErrorResult represents either success or an exception in the
* process of being thrown. This means that a failing ErrorResult _must_ be
* handled in one of the following ways before coming off the stack:
*
* 1) Suppressed via SuppressException().
* 2) Converted to a pure nsresult return value via StealNSResult().
* 3) Converted to an actual pending exception on a JSContext via
* MaybeSetPendingException.
* 4) Converted to an exception JS::Value (probably to then reject a Promise
* with) via dom::ToJSValue.
*/
#ifndef mozilla_ErrorResult_h
@ -87,8 +98,9 @@ public:
#ifdef DEBUG
~ErrorResult() {
MOZ_ASSERT_IF(IsErrorWithMessage(), !mMessage);
MOZ_ASSERT_IF(IsDOMException(), !mDOMExceptionInfo);
// Consumers should have called one of MaybeSetPendingException
// (possibly via ToJSValue), StealNSResult, and SuppressException
MOZ_ASSERT(!Failed());
MOZ_ASSERT(!mMightHaveUnreportedJSException);
MOZ_ASSERT(mUnionState == HasNothing);
}
@ -158,6 +170,9 @@ public:
// exception on aCx, due to uncatchable exceptions. It should still be
// considered equivalent to a JSAPI failure in terms of what callers should do
// after true is returned.
//
// After this call, the ErrorResult will no longer return true from Failed(),
// since the exception will have moved to the JSContext.
bool MaybeSetPendingException(JSContext* cx)
{
WouldReportJSException();