Comment on attachment 8511670 [details] [diff] [review] 1089368.diff Review of attachment 8511670 [details] [diff] [review]: ----------------------------------------------------------------- Thanks! r=me with the two callback args removed. ::: browser/base/content/newtab/updater.js @@ +139,5 @@ > node.parentNode.removeChild(node); > resolve(); > }); > + }); > + }).then(aCallback).catch(console.exception); There's no reason to still support the callback, right? Catching errors should probably be moved to the caller as well. @@ +152,4 @@ > let {cells, sites} = gGrid; > > // Find empty cells and fill them. > + return Promise.all(sites.map((aSite, aIndex) => { We can remove the callback argument here for _fillEmptyCells() as well.

(In reply to Tim Taubert [:ttaubert] from comment #2) > Comment on attachment 8511670 [details] [diff] [review] > 1089368.diff > > Review of attachment 8511670 [details] [diff] [review]: > ----------------------------------------------------------------- > > Thanks! r=me with the two callback args removed. > > ::: browser/base/content/newtab/updater.js > @@ +139,5 @@ > > node.parentNode.removeChild(node); > > resolve(); > > }); > > + }); > > + }).then(aCallback).catch(console.exception); > > There's no reason to still support the callback, right? Catching errors > should probably be moved to the caller as well. > > @@ +152,4 @@ > > let {cells, sites} = gGrid; > > > > // Find empty cells and fill them. > > + return Promise.all(sites.map((aSite, aIndex) => { > > We can remove the callback argument here for _fillEmptyCells() as well. sounds good!

Code was removed in bug 1433133 https://hg.mozilla.org/mozilla-central/rev/fc80dd3e4fae