If the code fits into async function and also it's test code, replaced legacy generator with async function. for other cases, just replaced legacy generator with ES6 generator. some notes: * browser/base/content/test/chrome/test_aboutCrashed.xul Task.jsm and Promise.jsm are no more used. * browser/components/downloads/test/browser/browser_libraryDrop.js |drop| function (previously |task_drop|) now returns promise, so |yield*| in caller is changed to |yield| * browser/components/privatebrowsing/test/browser/browser_privatebrowsing_DownloadLastDirWithCPS.js all functions called by |runTest| can return promise, so changed |runTest| to async function and just await for them. also, checkDownloadLastDir and checkDownloadLastDirNull didn't return value. fixed them. * browser/components/privatebrowsing/test/browser/browser_privatebrowsing_downloadLastDir_toggle.js what |testHelper| does is Task.async, that can be just replaced with async. * browser/components/privatebrowsing/test/browser/browser_privatebrowsing_windowtitle.js |testTabTitle| is also used like Task.async, so replaced with async * browser/components/sessionstore/SessionMigration.jsm this can also use async, but I used ES6 generator since it's not test. also, replaced |throw new Task.Result(state);| with |return state;|, since ES6 generator can return value. * browser/components/sessionstore/test/browser_capabilities.js |createTab| returns a promise that is resolved to tab. so that can be rewritten by async that returns tab. * browser/components/sessionstore/test/browser_formdata_format.js this test waits for the all tabs to be closed, so created an array of promises, and pushed all promises that is resolved when the tab is closed, and the test finishes when those all promises are resolved |testTabRestoreData| function does partially Task.async, so replaced whole function to async.

Thanks Arai! Not sure I can get to this review before the company meeting. Feel free to reassign if you want, or I'll be able to do this shortly after the meeting week for sure.

Comment on attachment 8815629 [details] [diff] [review] Remove legacy generator from browser/. Review of attachment 8815629 [details] [diff] [review]: ----------------------------------------------------------------- (In reply to :Paolo Amadini from comment #1) > Thanks Arai! Not sure I can get to this review before the company meeting. > Feel free to reassign if you want, or I'll be able to do this shortly after > the meeting week for sure. Sorry for the delay, I've been quite busy recently. Thanks a lot for the patch! I think my "for sure" wasn't quite right, although I can still make up an excuse based on the ambiguous meaning of "shortly after"... I didn't try to apply the patch locally, because this is likely to need rebasing by now. I generally prefer patches pushed to the MozReview repository, because it makes it easier for me to apply them locally and see if there is any real conflict when rebasing. ::: browser/base/content/test/chrome/test_aboutCrashed.xul @@ +47,3 @@ > } > > + async function doTest() { There's a issue introduced by using async functions as entry points when the callers expect a normal function, like in this case with "doTest". Unfortunately JavaScript isn't a typed language, so while nothing stops you from doing this, the caller will happily ignore the returned Promise, resulting in an uncaught rejection if something in the test fails. Some of our testing frameworks notice those rejections already, and others are on the way in bug 1242505, but even in that case the best practice is to ensure all Promise chains are terminated, which gives better stacks and earlier reporting. In this case this probably amounts to: function doTest() { (async function() { /*...*/ })().catch(ex => ok(false, ex)); } I haven't run the code above to verify it's the right syntax for this testing framework. Different frameworks require different reporting, for example do_report_unexpected_exception in xpcshell. You can look up existing uses of this pattern in the tree. Note that in this case the missing "catch" was an existing issue with "Task.async" too. The same kind of issues should be fixed in the rest of the patch too.

Comment on attachment 8815629 [details] [diff] [review] Remove legacy generator from browser/. thank you for reviewing :) updated the rejection handling.

Comment on attachment 8828746 [details] Bug 1321215 - Remove legacy generator from browser/. https://reviewboard.mozilla.org/r/106044/#review107076 Looks great. Thanks a lot!

https://hg.mozilla.org/integration/mozilla-inbound/rev/969012588cf0ba98e99b95d6079851c982a4d78b Bug 1321215 - Remove legacy generator from browser/. r=Paolo