feat: capture Electron IPC messages for opensumi devtools #1583
Conversation
Codecov ReportBase: 57.69% // Head: 57.81% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1583 +/- ##
==========================================
+ Coverage 57.69% 57.81% +0.12%
==========================================
Files 1254 1271 +17
Lines 78152 79119 +967
Branches 16333 16575 +242
==========================================
+ Hits 45088 45745 +657
- Misses 30093 30365 +272
- Partials 2971 3009 +38
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
tyn1998
force-pushed
the
feat/capture-ipc-messages
branch
from
bb48aaf
to
9a2e7f8
Compare
tyn1998
changed the title
|
收到！学到了～
Dan ***@***.***>于2022年9月5日 周一11:02写道：
… ***@***.**** requested changes on this pull request.
------------------------------
In packages/connection/src/common/utils.ts
<#1583 (comment)>:
> @@ -37,8 +37,8 @@ export function parse(input: string, reviver?: (this: any, key: string, value: a
}
export function getCapturer() {
- if (typeof window !== 'undefined' && window.__OPENSUMI_DEVTOOLS_GLOBAL_HOOK__?.capture) {
- return window.__OPENSUMI_DEVTOOLS_GLOBAL_HOOK__.capture;
+ if (typeof window !== 'undefined' && window.__OPENSUMI_DEVTOOLS_GLOBAL_HOOK__?.captureRpc) {
那些地方的写法也是不规范的，有机会可以一起改掉，这种专有名词出现的时候还是要大写比较规范，例如 macOS，后面的 OS 全程是 Operating
System，所以简称一般用大写表示，这里的 IPC，RPC 同样也是缩写，因此规范的写法也要是大写，比较好识别
------------------------------
In packages/core-electron-main/src/bootstrap/window.ts
<#1583 (comment)>:
> @@ -94,6 +95,10 @@ export class CodeWindow extends Disposable implements ICodeWindow {
...this.appConfig.overrideBrowserOptions,
...options,
});
+
+ // initialize for OpenSumi DevTools
+ initForDevtools(this.browser);
一般对于这种开发工具代码，有点类似“后门”的程序，最好还是把是否集成的逻辑交给集成方去决定，默认开启的话会造成一些不必要的问题，用户可能在未知的情况下升级版本过程中集成了你这个特性，这里比较建议通过
AppCofig 中追加配置项来控制，例如 devtool: boolean, 默认值在开发态时为 True，这样不会影响已集成的用户
—
Reply to this email directly, view it on GitHub
<#1583 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHXOSWAOCMI2RMGKKPQH2Y3V4VPDTANCNFSM57WMZDEQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
@erha19 老师，“专有名词大小写”的问题已经解决，在devtools项目中也有对应的修改。 但是，“增加配置项”这个要求我虽然push了一个commit，但还没有完成，需要帮助。我的思路和遇到的困难是这样的： 截止到此PR最新的commit，在opensumi/core中，与devtools相关的代码有多处：
目前，通过新加入的 此外需要注意的是，对于Electron端来说，控制devtools的开关需要考虑两个地方：
所以在此PR中，我分别在
（可能描述得不是很清晰，再配一个图） |
|
IClientAppOpts 里面配置 devtools 就好了，至于 electron preload 的内容，可以加到 metadata 中，通过 metadata 去获取是否开启 devtools |
谢谢 @yantze 老师！metadata这部分能不能稍微再展开一下~ |
这里的 electron 的 appConfig 和 browser 的 appConfig 是两个实例，确实都要声明的 |
|
感谢 @yantze 老师的保姆级指南，使得browser-preload在现在也能被配置了！ 此外，利用依赖注入能力，我在chrome-devtools.contribution.ts中根据
目前唯一遗留下来的是connection模块的配置，ide-core-browser模块似乎不能在这里被import，于是没法使用
其实分开设置也能得到合理的解释：如果把core-electron-main的 |
@erha19 老师的requested changes里，最重要的是上面这个，因为影响功能的正确性，所以我仔细捋了一下，在此更新：
图中提到的套娃问题是刚刚发现并验证的，之前没有考虑到；第二个问题“ipcMain.on从各个renderer收到的消息都会被每一个窗口捕获”不知道是不是Dan老师说的“串了”，这的确是不合适的。
这个测试是这样的：打开两个窗口，拖移其中一个窗口，两个窗口的IPC消息列表中都出现了“ipcRenderer.on”事件。于是我马上回复了“的确有这个Bug！”。但今天在捋思路的时候，我发现了这个： core/packages/core-electron-main/src/bootstrap/api.ts Lines 99 to 104 in ebe8d66 opensumi/core中已经存在对Electron API的代理，上面代码是其中的一部分。会遍历app的所有窗口并发送消息，昨天我遇到的是这种情况。所以其实不是我以为的“串了”，而是设计就是这样。Q：为什么要遍历发送消息呢？ 以上是关于“消息串了”的一些思考，下一个comment是我的新思路。 |
|
新思路： 我认为不要捕获main进程这侧的消息了（ipcMain.on、ipcMain.handle...），在devtools中只呈现renderer侧的消息（ipcRenderer.on、ipcRenderer.send、ipcRenderer.invoke、ipcRenderer.sendSync）。 理由如下：
若采用这个新思路，上面的几个requested changes都迎刃而解了。 |
But capture the return values of ipcRenderer.sendSync and ipcRenderer.invoke
tyn1998
force-pushed
the
feat/capture-ipc-messages
branch
from
2129d90
to
b3840bc
Compare
|
以下是新思路下的IPC view，“↑”和“↓”都是以renderer（即窗口）的视角看待的：
由于core中使用ipcRenderer.sendSync()和ipcRenderer.invoke()基本没有，所以自己本地加了两个测试channel用于测试效果。OpenSumi DevTools那里也做了相应的修改。 |
|
@tyn1998 新思路可以的，也能减少对于 IPC 和 RPC 的兼容写法，只关注 Browser/Renderer 进程的消息就可以了 |
|
ipcRenderer.invoke 之后会有，目前只监听 renderer 侧也可以的 |
#1098 also requires implementing IPC messages capturing and presenting within opensumi devtools. This PR provides messages capturing support for opensumi devtools.
Types
Background or solution
By proxies of a few Electron IPC API to capture IPC messages for opensumi/devtools#5.
Also modify RPC capturing code (only name, not logic).
Changelog
Electron IPC messages capturing support for opensumi devtools.