Now that we have async stacks, we can use those and remove the rewriting in Task.jsm.

Comment on attachment 8578962 [details] [diff] [review] Work in progress Review of attachment 8578962 [details] [diff] [review]: ----------------------------------------------------------------- Nice clean-up, I like it. ::: toolkit/modules/Task.jsm @@ +302,5 @@ > + if (this._isStarGenerator) { > + try { > + let continueFn = aSendResolved ? this._iterator.next > + : this._iterator.throw; > + continueFn = continueFn.bind(this._iterator, aSendValue); Nit: not a big fan of the naming convention and of reusing the same name twice. @@ +307,3 @@ > > + let result = this._stack ? Components.utils.callFunctionWithAsyncStack( > + continueFn, this._stack, "Task") Note: looking at xpccomponents.idl, it's not clear to me what the `asyncCause` arg means. @@ +430,5 @@ > * > * @param {string} topStack The stack provided by the error. > * @param {string=} prefix Optionally, a prefix for each line. > */ > generateReadableStack: function(topStack, prefix = "") { Does this function still do anything useful?

I'd like to ask for an automated test, please. I plan on converting Task.jsm to use native Promises (bug 1095267), and I really don't want to regress this when the time comes. :)

(In reply to Alex Vincent [:WeirdAl] from comment #3) > I'd like to ask for an automated test, please. I plan on converting > Task.jsm to use native Promises (bug 1095267), and I really don't want to > regress this when the time comes. :) Yeah, that's why the patch hadn't a review request yet. Unfortunately I won't be able to work on it this week, and since this isn't required to land bug 1038238, I don't think I'll be able to continue in the short term. If anyone feels like finishing the patch, they're more than welcome!

We've replaced most uses of Task.jsm with async functions, that now also report async stacks correctly. So, there is no need to modify the current Task.jsm async stack reporting.