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

Status: RESOLVED WONTFIX

(DevTools :: General, defect, P3)

Description

[Tooru Fujisawa [:arai]]

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.

Comment 1

[Tooru Fujisawa [:arai]]

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.

Comment 2

[Tooru Fujisawa [:arai]]

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

Comment 3

[Tooru Fujisawa [:arai]]

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

Comment 4

[Tooru Fujisawa [:arai]]

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.

Comment 5

[Tooru Fujisawa [:arai]]

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.

Comment 6

[Tooru Fujisawa [:arai]]

forgot one more fix.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7ecce716a4ea50bbb6b7219fe8cf12c202795a78
and checking other envs as well.

Comment 7

[Tooru Fujisawa [:arai]]

with the last one fixed by bug 1430383, I think we can avoid touching `once` implementation for now.