feat: add contents.ipc (EventEmitter), which allows handling messages for given webContents

[miniak]

Description of Change

• The motivation is to simplify application code, when messages only from a particular webContents instance are being handled.
• This is especially handy when using once, where doing the event.sender === someWebContents check can't be done in the listener, as it would be unregistered even if we don't handle the message.
• We cannot just emit the IPC events on the webContents directly as it could clash with existing events.
• It allows to use webContents.ipc.removeAllListeners(), which is safe(r) unlike ipcMain.removeAllListeners()

example

const win = new BrowserWindow()
...

// this does not work properly
ipcMain.once('ready', (event) => {
  if (event.sender === win.webContents) {
    console.log('ready')
  }
})

// this is annoying
ipcMain.on('ready', function handler (event) => {
  if (event.sender === win.webContents) {
    console.log('ready')
    ipcMain.removeListener('ready', handler)
  }
})

// this is much better
win.webContents.ipc.once('ready', () => {
  console.log('ready')
})

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: Added contents.ipc (EventEmitter), which allows handling messages for given webContents

[alexeykuzmin]

👍

Tests maybe?

[MarshallOfSound]

IMO this just adds another route to do the same stuff you can already do. In particular something I think was overlooked here was the ipc-message event.

webContents.on('ipc-message', handler)

I don't think adding another source of events is a good design choice here. Should discuss the advantage of this comparing to the above ipc-message event.

[miniak]

@MarshallOfSound ipc-message on webContents is not documented, it's an implementation detail basically. And it's not as convenient to use as you cannot directly listen for individual messages.

[miniak]

@MarshallOfSound also you would have to handle ipc-message-sync for sync messages separately. In this case we are also adding the returnValue property on the event in the internal code before doing ipcMain.emit

[miniak]

@MarshallOfSound it actually seems like the code around ipc-message should be refactored to make it more robust as you would currently break the forwarding to ipcMain by calling webContents.removeAllListeners()

[miniak]

@nornagon, @zcbenz, @codebytere what do you think?

[nornagon]

I agree with @MarshallOfSound that this seems like what the ipc-message event is for. I'd be interested to see a PR to clean up & refactor & document that event. I don't think it's a problem that ipc-message-sync is a separate event—synchronous messages are substantially different to asynchronous ones and you should know which kind you're dealing with.

[miniak]

@nornagon, @MarshallOfSound fair enough. I will create a new PR to make the ipc-message and ipc-message-sync events public + necessary refactoring

[miniak]

Replaced by #16468