When a transaction is aborted due to an explicit call to transaction.abort(), we should let transaction.error remain null. Note though that we should not let transaction.error be null if a transaction is aborted due to a "success" or "error" event handler throwing.

I think this patch is making things more complicated than they need to be. Especially the fact that there are now 4 Abort* functions is unfortunate. And it's confusing that AbortWithCode both acts as an abort function, and as a factory function for creating DOMErrors. How about instead making AbortWithCode take an nsresult and an optional nsIDOMDOMError*. Then transaction.abort() can call AbortWithCode and pass NS_OK as nsresult. If the nsIDOMDOMError* argument is null, and the nsresult isn't NS_OK, we create the appropriate DOMError.

How about this?

Comment on attachment 637866 [details] [diff] [review] Patch Review of attachment 637866 [details] [diff] [review]: ----------------------------------------------------------------- Looks great. Please add a test to ensure that transaction.error is null after a transaction is aborted. ::: dom/indexedDB/IDBTransaction.cpp @@ +575,2 @@ > > + return AbortInternal(aRequest->GetErrorCode(), error.forget()); Maybe just inline this... @@ +580,5 @@ > +IDBTransaction::Abort(nsresult aErrorCode) > +{ > + NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); > + > + return AbortInternal(aErrorCode, DOMError::CreateForNSResult(aErrorCode)); ...and this function. That'll make it more clear what all variants do when looking in IDBTransaction.h

Inlining it leads to include hell with private dom headers in IPC stuff, unfortunately.

And there already is a test in test_transaction_abort.