Navigation API: fix an ordering issue

[domenic]

Because navigatesuccess/navigateerror events can run JavaScript code, they can change the "ongoing API method tracker" pointer. Thus, it's better to resolve/reject the finished promise for the API method tracker before firing the event.

This does not change the observable order in normal cases: it remains the case that JavaScript event handlers for navigatesuccess/navigateerror run before promise reactions, because promise resolution is always delayed by a microtask. However, it fixes some tricky reentrant cases.

---

This was discovered by @natechapin in https://chromium-review.googlesource.com/c/chromium/src/+/6622087, while he was implementing the fix from #11409. I'd like his review, as well as ideally @farre's.

I'm still working with Nate on getting clarity on an exact test case that exercises this bug, and what the before/after behavior is. It might be the test case already in https://chromium-review.googlesource.com/c/chromium/src/+/6622087; I'm not sure.

• At least two implementers are interested (and none opposed):
  • Chromium is interested
  • Probably we can treat this as an obvious bug fix, but let's wait until we have the specific WPT in hand.
• Tests are written and can be reviewed and commented upon at:
  • TODO
• Implementation bugs are filed:
  • Chromium: See https://chromium-review.googlesource.com/c/chromium/src/+/6622087
  • Gecko: TODO
  • WebKit: TODO
  • Deno (only for timers, structured clone, base64 utils, channel messaging, module resolution, web workers, and web storage): N/A
  • Node.js (only for timers, structured clone, base64 utils, channel messaging, and module resolution): N/A
• Corresponding HTML AAM & ARIA in HTML issues & PRs: N/A
• MDN issue is filed: N/A, too niche of an edge case
• The top of this comment includes a clear commit message to use.

(See WHATWG Working Mode: Changes for more details.)

---

/nav-history-apis.html ( diff )

[annevk]

This ordering seems like something we should stick with everywhere, though I suppose it really only manifests here because the resolving/rejecting is conditional. But still, consistency is good.

[farre]

This looks very reasonable. I wouldn't be surprised if we have the same bug as chromium here.

[natechapin]

The test in my CL (https://chromium-review.googlesource.com/c/chromium/src/+/6622087) fails the assertion at the start of the promote an upcoming API method tracker to ongoing steps. I haven't isolated a case where a promise resolves incorrectly or anything like that yet, but we definitely are violating spec assertions.

[natechapin]

I don't have official review permissions, but the spec diffs match what I expected based on my proposed code change.