Implement backend to electron IPC channels #10698
Conversation
msujew
added
electron
There was a problem hiding this comment.
Very preliminary comments only. @paul-marechal will certainly have more to say.
| } | ||
|
|
||
| protected closeChannels(): void { | ||
| for (const channel of Array.from(this.channels.values())) { |
There was a problem hiding this comment.
| for (const channel of Array.from(this.channels.values())) { | |
| for (const channel of this.channels.values()) { |
This should already be iterable.
| @@ -0,0 +1,59 @@ | |||
| /******************************************************************************** | |||
| * Copyright (C) 2012 TypeFox and others. | |||
There was a problem hiding this comment.
| * Copyright (C) 2012 TypeFox and others. | |
| * Copyright (C) 2022 TypeFox and others. |
Perhaps?
| protected handleMessage(data: string): void { | ||
| try { | ||
| // Start parsing the message to extract the channel id and route | ||
| const message: WebSocketChannel.Message = JSON.parse(data.toString()); |
There was a problem hiding this comment.
One point of the discussion re: performance in our communication layer is that conversions back and forth from string to JSON are relatively expensive. Will all messages be passing through here, or only some, or how will it fit into the refactor of the messaging layer currently underway? Is it expected that this will be one terminus of the messaging system that can delegate the parsed messages to handlers, or will further parsing be necessary?
|
@colin-grant-work Thanks for looking into this, but I think we discussed in one of our dev-meetings that I'll postpone this PR until Paul has finished his work on the messaging refactoring. I'll then have to rewrite most of this anyway ;) I'll mark it as a draft PR for now. |
|
Closing due to changes in IPC/Messaging. Will (possibly) continue working on this later. |
What it does
Closes #2162
Uses a similar architecture to the existing Electron Main <-> Frontend communication layer over
ipcRendererandipcMain. Contains additional changes to deal with the--no-clusterargument by using a special pipe object.For testing purposes I included a
TestConnectioninterface which returns the titles of all opened electron windows on startup to the backend server.In a second version of this we could mirror the proposed classes/APIs here to also enable electron -> backend requests (this PR only enables backend -> electron requests), since we create only a single backend server for each electron main.
Note: My use case for this change is that I plan to add proxy support for all requests in Theia. In particular, requests from the backend running in Electron should try to resolve to the Chromium proxy (which automatically picks up the OS proxy) in case no proxy is specified using the apps' preferences. For this, we need some kind of way to communicate with the electron process from the backend server process.
How to test
Titles:in the console. The console should contain thetitleof the newly opened Electron window.--no-clusterargument enabled.Review checklist
Reminder for reviewers