As part of rolling out ESLint across the tree, I've worked on enabling it for dom/presentation, especially as there's some production code here. The first patch is the automatic fixes, the second patch are the additional manual fixes.

Comment on attachment 8991404 [details] Bug 1475004 - Enable ESLint for dom/presentation - manual fixes. https://reviewboard.mozilla.org/r/256294/#review267824 ::: dom/presentation/tests/mochitest/PresentationSessionFrameScript.js:63 (Diff revision 1) > > const Cm = Components.manager; > > const mockedChannelDescription = { > - QueryInterface: ChromeUtils.generateQI(["nsIPresentationChannelDescription"]), > + /* eslint-disable-next-line mozilla/use-chromeutils-generateqi */ > + QueryInterface(iid) { This change should be reverted. It looks like maybe a rebase issue. ::: dom/presentation/tests/mochitest/PresentationSessionFrameScript.js:94 (Diff revision 1) > } > > const mockedSessionTransport = { > - QueryInterface: ChromeUtils.generateQI(["nsIPresentationSessionTransport", "nsIPresentationDataChannelSessionTransportBuilder", "nsIFactory"]), > + /* eslint-disable-next-line mozilla/use-chromeutils-generateqi */ > + QueryInterface(iid) { > + const interfaces = [Ci.nsIPresentationSessionTransport, Likewise. ::: dom/presentation/tests/mochitest/file_presentation_reconnect.html (Diff revision 1) > aResolve(); > }; > }, > function(aError) { > ok(false, "Error occurred when establishing a connection: " + aError); > - teardown(); Is this removal correct? This file is run from a script tag, along with another script tag that does define teardown(). Would that work? I could certainly believe that this doesn't work given that it is an error case.

Comment on attachment 8991403 [details] Bug 1475004 - Enable ESLint for dom/presentation - automatic fixes. https://reviewboard.mozilla.org/r/256292/#review267770 ::: dom/presentation/provider/PresentationControlService.js:362 (Diff revision 1) > } > }, > > classID: Components.ID("{f4079b8b-ede5-4b90-a112-5b415a931deb}"), > - QueryInterface : ChromeUtils.generateQI([Ci.nsIServerSocketListener, > + QueryInterface: ChromeUtils.generateQI([Ci.nsIServerSocketListener, > Ci.nsIPresentationControlService, indentation got broken here ::: dom/presentation/tests/xpcshell/test_presentation_device_manager.js (Diff revision 1) > add_test(terminateRequest); > add_test(reconnectRequest); > add_test(removeDevice); > add_test(removeProvider); > - > -function run_test() { This removal looks wrong to me. Isn't this how we actually run the tests? I don't know why this was removed for two of these files and left alone for 3 others. ::: dom/presentation/tests/xpcshell/test_presentation_state_machine.js (Diff revision 1) > add_test(disconnect); > add_test(receiverDisconnect); > add_test(abnormalDisconnect); > - > -function run_test() { // jshint ignore:line > - run_next_test(); I think this removal is also wrong. See above.

Comment on attachment 8991403 [details] Bug 1475004 - Enable ESLint for dom/presentation - automatic fixes. https://reviewboard.mozilla.org/r/256292/#review267770 > This removal looks wrong to me. Isn't this how we actually run the tests? I don't know why this was removed for two of these files and left alone for 3 others. The main xpcshell-test infrastructure automatically detects a missing run_test and calls run_next_test automatically, so in these cases the function is superfluous: https://searchfox.org/mozilla-central/rev/d0a0ae30d534dc55e712bd3eedc1b4553ba56323/testing/xpcshell/head.js#523-530 Generally people are moving towards add_test or add_task, and will run any setup work in an add_task(function setup() {...}), so these are just unnecessary (and tended to have been left behind, which is why ESLint is cleaning them up).

Comment on attachment 8991404 [details] Bug 1475004 - Enable ESLint for dom/presentation - manual fixes. https://reviewboard.mozilla.org/r/256294/#review267824 > This change should be reverted. It looks like maybe a rebase issue. I just realised what happened here. The automatic ESLint fixes rewrote this to use ChromeUtils, however running this test locally fails with ChromeUtils being undefined (I guess as this is loaded in a special scope). So in the manual patch, I reverted it. I'll drop the change from the automatic patch, and just add the eslint comment here.

(In reply to Andrew McCreight [:mccr8] (away Aug 6 - 10) from comment #3) > ::: dom/presentation/tests/mochitest/file_presentation_reconnect.html > (Diff revision 1) > > aResolve(); > > }; > > }, > > function(aError) { > > ok(false, "Error occurred when establishing a connection: " + aError); > > - teardown(); > > Is this removal correct? This file is run from a script tag, along with > another script tag that does define teardown(). Would that work? I could > certainly believe that this doesn't work given that it is an error case. Isn't this a iframe or am I reading something wrong? https://searchfox.org/mozilla-central/search?q=file_presentation_reconnect.html&case=false&regexp=false&path= I've just tried putting teardown() at various places in file_presentation_reconnect.html and it definitely appears to be undefined. So I think the removal is correct, though it may also be a bug in the test... I can file a follow-up if you wish?

(In reply to Mark Banner (:standard8) from comment #7) > So I think the removal is correct, though it may also be a bug in the > test... I can file a follow-up if you wish? Sounds good, thanks.

Filed bug 1480470.

Pushed by mbanner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0639e328afde Enable ESLint for dom/presentation - automatic fixes. r=mccr8 https://hg.mozilla.org/integration/autoland/rev/ee957e55c91e Enable ESLint for dom/presentation - manual fixes. r=mccr8

https://hg.mozilla.org/mozilla-central/rev/0639e328afde https://hg.mozilla.org/mozilla-central/rev/ee957e55c91e