refactor: try just using regular [Sync] for MessageSync

[nornagon]

Description of Change

This aligns us closer with how Mojo expects to work, and as a bonus fixes some tricky race conditions that can happen if the browser process closes its end of the mojo pipe while we're waiting for a synchronous response.

Checklist

• PR description included and stakeholders cc'd
• npm test passes
• tests are changed or added
• relevant documentation is changed or added
• PR title follows semantic commit guidelines
• PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

Release Notes

Notes: none

[nornagon]

This seems to be passing CI... could it be that we've removed our need for synchronous IPC to be well-ordered w.r.t asynchronous IPC?

[zcbenz]

I think tests pass this time because lots of tests have been moved to main process.

This might still cause problems with remote module:

win = remote.getCurrentWindow()
win = null && gc()  // async DereferenceRemoteJSObject sent to main process
win = remote.getCurrentWindow() // sync GetGlobal sent to main process
// DereferenceRemoteJSObject handled, remote reference cleared
// win now references to non-exist remote object.

[nornagon]

Hm, I don't think the scenario you laid out would be a problem in the usual case. The only time that sync calls will be reordered with respect to async calls is if the main process is waiting on a sync call, which almost never happens. We previously observed some issues relating to window close, because there's a compositor shutdown call that the browser invokes synchronously during window close. If I recall correctly, it was because we were triggering synchronous calls during window close to release objects, which I don't think is the case any more? But you're right that we're exercising the remote module a lot less.

[zcbenz]

Thanks for the clarification, I'm good with the change then.

[nornagon]

Let's merge this and see how it goes. We can always revert if we discover issues.

[release-clerk]

No Release Notes

[trop]

@MarshallOfSound has manually backported this PR to "7-1-x", please check out #21776

[miniak]

@nornagon what about 8-x-y? should it be back-ported there as well?

[nornagon]

it should.

@MarshallOfSound want to do the honors?

[trop]

@erickzhao has manually backported this PR to "8-x-y", please check out #21948