Closed Bug 1423008 Opened 7 years ago Closed 7 years ago

Wait for the next event tick before resolving promise in event handler in EventEmitter.promise.once and waitForNEvents

Categories

(DevTools :: General, defect, P3)

defect

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: arai, Assigned: arai)

References

Details

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a005fb8aae235b850857ce9954c81f2f6e4ffc41&selectedJob=149019744 https://treeherder.mozilla.org/#/jobs?repo=try&revision=a005fb8aae235b850857ce9954c81f2f6e4ffc41&selectedJob=149019822 EventEmitter.prototype.once in devtools/shared/old-event-emitter.js needs to wait for the next event tick before resolving the promise inside event handler, to avoid the remaining part of the test runs too early. also, another implementation of once uses waitForNEvents in devtools/client/framework/test/shared-head.js, that also has to wait for the next event tick, for same reason.
Priority: -- → P3
there are several `once` implementation in tree, and at least some of them are used in non-testing code. So the change will affect so many part of devtools.
delaying Promise resolution in `once` in old-event-emitter.js fixed the following tests failure: * devtools/client/webconsole/test/browser_console_keyboard_accessibility.js * devtools/client/webconsole/test/browser_webconsole_clear_method.js * devtools/client/inspector/boxmodel/test/browser_boxmodel_guides.js now checking the new failure caused by the change. https://treeherder.mozilla.org/#/jobs?repo=try&revision=83a58e011b64ba3e68b849c4cdab3e0c6ad93372 also, here's the rough list of `once` functions that returns promise, including outside of devtools. I wonder if it's better aligning the behavior across them... https://searchfox.org/mozilla-central/rev/7fb999d1d39418fd331284fab909df076b967ac6/devtools/shared/old-event-emitter.js#75 https://searchfox.org/mozilla-central/rev/7fb999d1d39418fd331284fab909df076b967ac6/devtools/shared/async-utils.js#53 https://searchfox.org/mozilla-central/rev/7fb999d1d39418fd331284fab909df076b967ac6/devtools/client/commandline/test/helpers.js#272 https://searchfox.org/mozilla-central/rev/7fb999d1d39418fd331284fab909df076b967ac6/devtools/server/tests/browser/browser_canvasframe_helper_04.js#59 https://searchfox.org/mozilla-central/rev/7fb999d1d39418fd331284fab909df076b967ac6/devtools/client/debugger/new/debugger.js#19793 https://searchfox.org/mozilla-central/rev/7fb999d1d39418fd331284fab909df076b967ac6/devtools/client/debugger/new/debugger.js#45269 https://searchfox.org/mozilla-central/rev/7fb999d1d39418fd331284fab909df076b967ac6/devtools/client/performance/test/helpers/event-utils.js#20 https://searchfox.org/mozilla-central/rev/7fb999d1d39418fd331284fab909df076b967ac6/dom/browser-element/mochitest/browserElement_Find.js#17 https://searchfox.org/mozilla-central/rev/7fb999d1d39418fd331284fab909df076b967ac6/dom/media/mediasource/test/mediasource.js#64 https://searchfox.org/mozilla-central/rev/7fb999d1d39418fd331284fab909df076b967ac6/dom/media/test/eme.js#442 https://searchfox.org/mozilla-central/rev/7fb999d1d39418fd331284fab909df076b967ac6/dom/media/test/manifest.js#1579 https://searchfox.org/mozilla-central/rev/7fb999d1d39418fd331284fab909df076b967ac6/toolkit/modules/EventEmitter.jsm#92
here are the new failure [dt1] TEST-UNEXPECTED-FAIL | devtools/client/inspector/test/browser_inspector_breadcrumbs_mutations.js | Test timed out - [dt2] TEST-UNEXPECTED-FAIL | devtools/client/webconsole/test/browser_webconsole_completion.js | 'document.getElem' completion - Got entById, expected entsByTagNameNS TEST-UNEXPECTED-FAIL | devtools/client/webconsole/test/browser_webconsole_completion.js | 'document.getElem' another tab completion - Got entsByTagNameNS, expected entsByTagName TEST-UNEXPECTED-FAIL | devtools/client/webconsole/test/browser_webconsole_completion.js | 'document.getElem' completion - Got entById, expected entsByTagNameNS [dt3] TEST-UNEXPECTED-FAIL | devtools/client/inspector/markup/test/browser_markup_mutation_02.js | Test timed out - [dt4] TEST-UNEXPECTED-FAIL | devtools/client/inspector/rules/test/browser_rules_cycle-angle.js | Angle displayed as a turn value again. - Got 360deg, expected 1turn TEST-UNEXPECTED-FAIL | devtools/client/inspector/rules/test/browser_rules_cycle-color.js | Color displayed as an authored value. - Got hsl(0, 100%, 50%), expected #f00 TEST-UNEXPECTED-FAIL | devtools/client/inspector/rules/test/browser_rules_cycle-color.js | Color displayed as an HSL value again. - Got rgb(255, 0, 0), expected hsl(0, 100%, 50%) [dt6] TEST-UNEXPECTED-FAIL | devtools/client/framework/test/browser_toolbox_window_title_changes.js | Test timed out - TEST-UNEXPECTED-FAIL | devtools/client/framework/test/browser_toolbox_window_title_frame_select.js | Test timed out - [dt7] TEST-UNEXPECTED-FAIL | devtools/client/webconsole/new-console-output/test/mochitest/browser_jsterm_completion.js | 'document.getElem' completion - Got entById, expected entsByTagNameNS TEST-UNEXPECTED-FAIL | devtools/client/webconsole/new-console-output/test/mochitest/browser_jsterm_completion.js | 'document.getElem' another tab completion - Got entsByTagNameNS, expected entsByTagName TEST-UNEXPECTED-FAIL | devtools/client/webconsole/new-console-output/test/mochitest/browser_jsterm_completion.js | 'document.getElem' completion - Got entById, expected entsByTagNameNS
now I'm wondering, if it's better keeping the implementations not to wait for the next event tick, and adding `await waitForTick()` to caller side. so that the impact is smaller. since waiting for the next event tick only in some impls maybe confusing, and modifying all impls may be too much troublesome (and may delay the bug 1193394 fix) I'll see if adding `await waitForTick()` to caller side can solve all failures.
now testing with small workarounds https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a8507efacd89af458c62a05a19725009c3cc768&group_state=expanded if it works, we can defer the `once` API design decision and land bug 1193394 fix sooner.
with the last one fixed by bug 1430383, I think we can avoid touching `once` implementation for now.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.