Conversation
There was a problem hiding this comment.
Hey - 我发现了两个问题,并给出了一些整体性的反馈:
- 临时解压目录
temp_dir.join("mxu_webview2_extract")是固定路径;建议改为每次运行唯一的目录(例如包含 PID 或随机后缀),以避免在多个实例并发运行时或之前的运行遗留目录时出现冲突或竞争条件。
给 AI Agent 的提示词
请根据本次代码审查中的评论进行修改:
## 总体说明
- 临时解压目录 `temp_dir.join("mxu_webview2_extract")` 是固定路径;建议改为每次运行唯一的目录(例如包含 PID 或随机后缀),以避免在多个实例并发运行时或之前的运行遗留目录时出现冲突或竞争条件。
## 单条评论
### Comment 1
<location> `src-tauri/src/webview2/install.rs:97-99` </location>
<code_context>
+}
+
+/// 解压 cab 文件到 WebView2 运行时目录
+fn extract_cab_to_runtime(cab_path: &std::path::Path, runtime_dir: &std::path::Path) -> Result<(), String> {
+ let temp_dir = std::env::temp_dir();
+ let extract_temp = temp_dir.join("mxu_webview2_extract");
+
+ let _ = std::fs::remove_dir_all(&extract_temp);
</code_context>
<issue_to_address>
**issue (bug_risk):** 使用固定的临时目录进行解压,在多实例情况下可能产生冲突。
解压路径始终是 `%TEMP%/mxu_webview2_extract`,并且在每次运行前都会被删除。如果多个实例并发运行,或者其他进程也在复用这个 helper,它们可能会互相删除或覆盖对方的临时目录,导致偶发失败或清理不完全。应为每次运行使用唯一的临时目录(例如基于 PID/时间戳/随机后缀,或使用类似 `tempfile` 的 API)以避免这些竞争问题。
</issue_to_address>
### Comment 2
<location> `src-tauri/src/webview2/install.rs:184-193` </location>
<code_context>
+ }
+
+ // 优先使用架构匹配的 cab
+ if let Some(cab_path) = matched {
+ let progress_dialog = CustomDialog::new_progress(
+ "正在解压 WebView2",
+ "检测到本地 WebView2 运行时 cab 文件,正在解压...",
+ );
+
+ let result = extract_cab_to_runtime(&cab_path, runtime_dir);
+
+ if let Some(pw) = progress_dialog {
+ pw.close();
+ }
+
+ if result.is_ok() {
+ let _ = std::fs::remove_file(&cab_path);
+ }
+ return Some(result);
+ }
+
</code_context>
<issue_to_address>
**issue:** 本地 cab 解压失败时,并不会像注释所说那样回退到在线下载。
`try_extract_local_cab` 的文档说明在本地 cab 不可用时应该“返回 None 继续下载”,但如果存在匹配的 cab 且 `extract_cab_to_runtime` 失败(例如 cab 损坏/不完整),当前分支会返回 `Some(Err(_))`。此时 `download_and_extract` 会直接退出,而不会尝试在线下载,因此一次错误的本地 cab 可能会长期阻塞后续恢复。建议在解压失败时:(1) 删除该 cab 文件;(2) 返回 `None` 以触发在线下载;(3) 仅在在线下载也失败时再向上抛出错误。
</issue_to_address>帮我变得更有用!请对每条评论点 👍 或 👎,我会根据你的反馈改进后续的代码审查。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- The temporary extraction directory
temp_dir.join("mxu_webview2_extract")is a fixed path; consider using a per-run unique directory (e.g., including PID or a random suffix) to avoid clashes or race conditions when multiple instances run concurrently or when a previous run left the directory behind.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The temporary extraction directory `temp_dir.join("mxu_webview2_extract")` is a fixed path; consider using a per-run unique directory (e.g., including PID or a random suffix) to avoid clashes or race conditions when multiple instances run concurrently or when a previous run left the directory behind.
## Individual Comments
### Comment 1
<location> `src-tauri/src/webview2/install.rs:97-99` </location>
<code_context>
+}
+
+/// 解压 cab 文件到 WebView2 运行时目录
+fn extract_cab_to_runtime(cab_path: &std::path::Path, runtime_dir: &std::path::Path) -> Result<(), String> {
+ let temp_dir = std::env::temp_dir();
+ let extract_temp = temp_dir.join("mxu_webview2_extract");
+
+ let _ = std::fs::remove_dir_all(&extract_temp);
</code_context>
<issue_to_address>
**issue (bug_risk):** Using a fixed temp directory for extraction can cause conflicts between multiple instances.
The extraction path is always `%TEMP%/mxu_webview2_extract`, which is deleted before each run. If multiple instances run concurrently or another process shares this helper, they could delete or overwrite each other’s temp directory, leading to intermittent failures or incomplete cleanup. Use a per-run unique temp directory (e.g., PID/timestamp/random suffix or a `tempfile`-style API) to avoid these races.
</issue_to_address>
### Comment 2
<location> `src-tauri/src/webview2/install.rs:184-193` </location>
<code_context>
+ }
+
+ // 优先使用架构匹配的 cab
+ if let Some(cab_path) = matched {
+ let progress_dialog = CustomDialog::new_progress(
+ "正在解压 WebView2",
+ "检测到本地 WebView2 运行时 cab 文件,正在解压...",
+ );
+
+ let result = extract_cab_to_runtime(&cab_path, runtime_dir);
+
+ if let Some(pw) = progress_dialog {
+ pw.close();
+ }
+
+ if result.is_ok() {
+ let _ = std::fs::remove_file(&cab_path);
+ }
+ return Some(result);
+ }
+
</code_context>
<issue_to_address>
**issue:** Local cab extraction failures do not fall back to online download as the comment suggests.
The doc for `try_extract_local_cab` says it should “返回 None 继续下载” when the local cab is unusable, but if a matching cab exists and `extract_cab_to_runtime` fails (e.g. corrupted/partial cab), this branch returns `Some(Err(_))`. `download_and_extract` then exits without trying the online download, so a single bad local cab can permanently block recovery. Consider, on extraction failure: (1) delete the cab, (2) return `None` to trigger the online download, and (3) only bubble up an error if the online download also fails.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This pull request migrates WebView2 deployment from system-wide installation (Evergreen Bootstrapper) to a local fixed-version runtime approach. The new implementation downloads WebView2 Fixed Version Runtime as a CAB file from Microsoft's CDN, extracts it to a local webview2_runtime/ directory alongside the executable, and uses the WEBVIEW2_BROWSER_EXECUTABLE_FOLDER environment variable to direct Tauri to use this isolated runtime instead of system-wide installation.
Changes:
- Replaces system installation workflow with local CAB download and extraction using Windows' native
expand.exe - Implements local CAB file detection for offline/pre-downloaded runtimes with architecture validation
- Adds UI improvements including proper window sizing with
AdjustWindowRect, throttled progress updates (≥200ms) to prevent blocking, and correct process exit behavior when users close the progress dialog
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 11 comments.
| File | Description |
|---|---|
| src-tauri/src/webview2/install.rs | Core migration: replaced Evergreen Bootstrapper download/install with Fixed Version CAB download/extraction logic, local CAB detection, streaming download with throttled UI updates, and architecture-aware GUID selection |
| src-tauri/src/webview2/dialog.rs | Fixed window sizing calculation using AdjustWindowRect, added dialog_type tracking for correct WM_CLOSE handling (exit process for progress dialogs) |
| src-tauri/src/webview2/detection.rs | Formatting only: added blank line before function |
| src-tauri/src/main.rs | Added early detection and environment variable setup for existing webview2_runtime directory, skip ensure_webview2() call when local runtime already configured |
|
我草 这Copilot怎么找了这么多自己的毛病 |
- 改进架构不支持的错误信息,注明仅支持 x64/ARM64 - 递归复制时检查并跳过符号链接 - 添加 TOCTOU 竞态条件注释说明 - 删除/创建 runtime_dir 时改进文件锁错误提示 - 下载完成后 flush 文件缓冲 - HTTP 客户端显式设置 TLS 证书验证 - 添加 HTTP 整体超时 (600s) 防止无限挂起 - SendMessageW 安全性注释说明必须同步调用 - 验证 webview2_runtime 目录包含 msedgewebview2.exe Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - 我发现了 1 个问题,并且给出了一些总体反馈:
extract_cab_to_runtime辅助函数假设expand.exe已经在 PATH 中,并且除了一个通用错误之外,不会检查或细化失败原因;建议显式地从System32解析该可执行文件,并在其缺失时给出更清晰的错误信息,以避免在环境配置有问题时出现令人困惑的失败。- 在
download_and_extract中,临时 cab 路径只是temp_dir.join(cab_name),因此应用的并发实例可能会争用同一个文件;在临时文件名中加入进程 ID 或随机后缀会让这一点更加健壮。
给 AI 代理的提示
Please address the comments from this code review:
## Overall Comments
- `extract_cab_to_runtime` 辅助函数假设 `expand.exe` 已经在 PATH 中,并且除了一个通用错误之外,不会检查或细化失败原因;建议显式地从 `System32` 解析该可执行文件,并在其缺失时给出更清晰的错误信息,以避免在环境配置有问题时出现令人困惑的失败。
- 在 `download_and_extract` 中,临时 cab 路径只是 `temp_dir.join(cab_name)`,因此应用的并发实例可能会争用同一个文件;在临时文件名中加入进程 ID 或随机后缀会让这一点更加健壮。
## Individual Comments
### Comment 1
<location> `src-tauri/src/webview2/install.rs:106-102` </location>
<code_context>
+fn extract_cab_to_runtime(cab_path: &std::path::Path, runtime_dir: &std::path::Path) -> Result<(), String> {
</code_context>
<issue_to_address>
**suggestion:** 考虑增强对 `expand.exe` 调用的健壮性,并处理 `expand.exe` 缺失或不在 PATH 中的情况。
当前实现依赖 `expand.exe` 在 PATH 中;在锁定或精简的 Windows 环境中它可能不存在,从而只会产生一个通用的“运行 expand.exe 失败”错误信息。
建议:
- 通过 `%SystemRoot%\System32\expand.exe`(例如使用 `GetSystemDirectoryW`)解析可执行文件,而不是依赖 PATH。
- 区分“找不到可执行文件”和“非零退出码”两种情况,这样错误信息可以清楚地表明何时是 `expand.exe` 本身不可用。
这将使 cab 解压失败对用户来说更容易诊断。
</issue_to_address>帮我变得更有用!请在每条评论上点击 👍 或 👎,我会根据反馈改进后续的评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The
extract_cab_to_runtimehelper assumesexpand.exeis available on PATH and doesn’t exist or resolve failures beyond a generic error; consider resolving it explicitly fromSystem32and surfacing a clearer message when it’s missing to avoid confusing failures on misconfigured environments. - In
download_and_extract, the temporary cab path is justtemp_dir.join(cab_name), so concurrent instances of the app could contend for the same file; including the process ID or a random suffix in the temp filename would make this more robust.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `extract_cab_to_runtime` helper assumes `expand.exe` is available on PATH and doesn’t exist or resolve failures beyond a generic error; consider resolving it explicitly from `System32` and surfacing a clearer message when it’s missing to avoid confusing failures on misconfigured environments.
- In `download_and_extract`, the temporary cab path is just `temp_dir.join(cab_name)`, so concurrent instances of the app could contend for the same file; including the process ID or a random suffix in the temp filename would make this more robust.
## Individual Comments
### Comment 1
<location> `src-tauri/src/webview2/install.rs:106-102` </location>
<code_context>
+fn extract_cab_to_runtime(cab_path: &std::path::Path, runtime_dir: &std::path::Path) -> Result<(), String> {
</code_context>
<issue_to_address>
**suggestion:** Consider hardening the `expand.exe` invocation and handling the case where `expand.exe` is missing or not on PATH.
This relies on `expand.exe` being on PATH; in locked-down or minimal Windows setups it may be missing, resulting in a generic “运行 expand.exe 失败” message.
I’d suggest:
- Resolving `%SystemRoot%\System32\expand.exe` (e.g., via `GetSystemDirectoryW`) instead of depending on PATH.
- Distinguishing between “executable not found” and a non‑zero exit code so the error clearly indicates when `expand.exe` itself is unavailable.
That will make cab extraction failures more diagnosable for users.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| CustomDialog::show_error( | ||
| "系统 WebView2 已被禁用", | ||
| &format!( | ||
| "检测到系统 WebView2 已被禁用:\r\n{}\r\n\r\n\ | ||
| 【什么是 WebView2?】\r\n\ | ||
| WebView2 是微软提供的网页渲染组件,本程序依赖它来\r\n\ | ||
| 显示界面。如果 WebView2 被禁用,程序将无法正常运行。\r\n\r\n\ | ||
| 【如何解决?】\r\n\ | ||
| 方法一:如果使用了 Edge Blocker 等工具\r\n\ | ||
| - 打开 Edge Blocker,点击\"Unblock\"解除禁用\r\n\ | ||
| - 或删除注册表中的 IFEO 拦截项\r\n\r\n\ | ||
| 方法二:修改组策略(需要管理员权限)\r\n\ | ||
| 1. 按 Win + R,输入 gpedit.msc\r\n\ | ||
| 2. 导航到:计算机配置 > 管理模板 > Microsoft Edge WebView2\r\n\ | ||
| 3. 将相关策略设置为\"未配置\"或\"已启用\"\r\n\r\n\ | ||
| 方法三:加入我们的 QQ 群,获取帮助和支持\r\n\ | ||
| - 群号可在我们的官网或文档底部找到\r\n\r\n\ | ||
| 点击确定后将尝试下载独立 WebView2 运行时以继续运行。\r\n\ | ||
| 若想恢复使用系统 WebView2,请手动删除 exe 目录下的 webview2_runtime 文件夹", | ||
| reason | ||
| ), | ||
| ); |
There was a problem hiding this comment.
此处新增/扩展了大量面向用户的提示文案(标题/正文均为硬编码字符串)。仓库规范要求“所有面向用户的文本必须定义在 src/i18n/locales/ 中”(见 AGENTS.md 3.2)。建议将这些提示文案迁移到统一的本地化资源(至少提供 zh-CN/en-US),并在 Rust 侧通过一套可复用的字符串表/桥接方式读取,避免后续无法维护多语言一致性。
c4408b0 to
4c60ca1
Compare
- 改进架构不支持的错误信息,注明仅支持 x64/ARM64 - 递归复制时检查并跳过符号链接 - 添加 TOCTOU 竞态条件注释说明 - 删除/创建 runtime_dir 时改进文件锁错误提示 - 下载完成后 flush 文件缓冲 - HTTP 客户端显式设置 TLS 证书验证 - 添加 HTTP 整体超时 (600s) 防止无限挂起 - SendMessageW 安全性注释说明必须同步调用 - 验证 webview2_runtime 目录包含 msedgewebview2.exe - expand.exe 从 System32 解析并为下载临时文件添加 PID 前缀 - 从 %SystemRoot%\System32 解析 expand.exe 完整路径,不依赖 PATH - expand.exe 不存在时给出明确路径提示 - 下载的临时 cab 文件名添加 PID 前缀,避免并发实例冲突 - show_download_failed_dialog 不再默认回退 x64,架构不支持时展示专门提示 - 删除 runtime_dir 前通过 symlink_metadata 检查符号链接/重解析点,拒绝操作以防任意目录删除 - 新增 validate_runtime_dir 在设置环境变量前校验 msedgewebview2.exe 存在 - 下载临时 cab 路径已在上次提交中添加 PID 前缀 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
移除 清空缓存 按钮 在调试中添加 Webview2 目录
865043b to
052e0f0
Compare
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - 我发现了 1 个问题,并留下了一些高层次的反馈:
- WebView2 运行时目录路径目前既在
get_webview2_runtime_dir()中构造了一次,又在main.rs中再次构造(exe_dir.join("cache").join("webview2_runtime"));建议通过该辅助函数统一这段逻辑,以避免未来路径定义出现漂移。 try_extract_local_cab在多个 IO 失败场景下(例如current_exe、read_dir)会静默返回None,导致代码在没有任何痕迹的情况下回退到网络下载;建议至少记录这些错误日志,这样在排查为什么本地 CAB 被忽略时会更容易。- 硬编码的
WEBVIEW2_VERSION和 GUID(GUID_X64/GUID_ARM64)在更新时很容易被忘记;可以考虑把它们集中到一个结构体或辅助函数中(或在 GUID 附近添加一个简短注释,明确说明其对应的版本),以降低更新时不一致的风险。
给 AI 代理的提示
请根据本次代码评审中的评论进行修改:
## 整体评论
- WebView2 运行时目录路径目前既在 `get_webview2_runtime_dir()` 中构造了一次,又在 `main.rs` 中再次构造(`exe_dir.join("cache").join("webview2_runtime")`);建议通过该辅助函数统一这段逻辑,以避免未来路径定义出现漂移。
- `try_extract_local_cab` 在多个 IO 失败场景下(例如 `current_exe`、`read_dir`)会静默返回 `None`,导致代码在没有任何痕迹的情况下回退到网络下载;建议至少记录这些错误日志,这样在排查为什么本地 CAB 被忽略时会更容易。
- 硬编码的 `WEBVIEW2_VERSION` 和 GUID(`GUID_X64`/`GUID_ARM64`)在更新时很容易被忘记;可以考虑把它们集中到一个结构体或辅助函数中(或在 GUID 附近添加一个简短注释,明确说明其对应的版本),以降低更新时不一致的风险。
## 具体评论
### 评论 1
<location path="src-tauri/src/commands/system.rs" line_range="729-724" />
<code_context>
+
+/// 获取当前使用的 WebView2 目录
+#[tauri::command]
+pub fn get_webview2_dir() -> Option<WebView2DirInfo> {
+ if let Ok(folder) = std::env::var("WEBVIEW2_BROWSER_EXECUTABLE_FOLDER") {
+ Some(WebView2DirInfo {
+ path: folder,
+ system: false,
+ })
+ } else {
+ // 没有设置自定义目录,使用系统 WebView2
+ Some(WebView2DirInfo {
+ path: String::new(),
+ system: true,
+ })
+ }
+}
</code_context>
<issue_to_address>
**suggestion:** `get_webview2_dir` 始终返回 `Some`,因此在其公共 API 中不需要使用 `Option`。
两个分支都会返回 `Some(WebView2DirInfo { .. })`,因此 `Option` 从不会用来表示“无值”;并且 TS 端已经将结果视为非空。直接返回 `WebView2DirInfo` 可以简化 API,同时去掉前端中不必要的空值处理。
建议实现如下:
```rust
pub fn get_webview2_dir() -> WebView2DirInfo {
```
```rust
WebView2DirInfo {
```
```rust
// 没有设置自定义目录,使用系统 WebView2
WebView2DirInfo {
path: String::new(),
system: true,
}
```
1. 确保 `system.rs` 中 `get_webview2_dir` 的第二个分支(`else`)与搜索文本完全一致;如果注释/空白有差异,请同步调整 SEARCH 块。
2. 检查所有 Rust 调用点以及生成的 TypeScript 绑定(如果你使用带代码生成的 `tauri::command`),将它们的预期返回类型从 `Option<WebView2DirInfo>` 更新为 `WebView2DirInfo`,并移除前端中所有不必要的 null/undefined 处理。
</issue_to_address>帮我变得更有用!请在每条评论上点 👍 或 👎,我会基于你的反馈改进评审质量。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The WebView2 runtime directory path is now constructed both in
get_webview2_runtime_dir()and again inmain.rs(exe_dir.join("cache").join("webview2_runtime")); consider centralizing this logic via the helper to avoid future path drift. try_extract_local_cabsilently returnsNoneon several IO failures (e.g.current_exe,read_dir), which causes the code to fall back to network download without any trace; it may be helpful to at least log these errors so diagnosing why a local CAB was ignored is easier.- The hardcoded
WEBVIEW2_VERSIONand GUIDs (GUID_X64/GUID_ARM64) are easy to forget when updating; consider grouping them into a single struct or helper (or adding a small comment near the GUIDs that explicitly names the matching version) to reduce the risk of mismatched updates.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The WebView2 runtime directory path is now constructed both in `get_webview2_runtime_dir()` and again in `main.rs` (`exe_dir.join("cache").join("webview2_runtime")`); consider centralizing this logic via the helper to avoid future path drift.
- `try_extract_local_cab` silently returns `None` on several IO failures (e.g. `current_exe`, `read_dir`), which causes the code to fall back to network download without any trace; it may be helpful to at least log these errors so diagnosing why a local CAB was ignored is easier.
- The hardcoded `WEBVIEW2_VERSION` and GUIDs (`GUID_X64`/`GUID_ARM64`) are easy to forget when updating; consider grouping them into a single struct or helper (or adding a small comment near the GUIDs that explicitly names the matching version) to reduce the risk of mismatched updates.
## Individual Comments
### Comment 1
<location path="src-tauri/src/commands/system.rs" line_range="729-724" />
<code_context>
+
+/// 获取当前使用的 WebView2 目录
+#[tauri::command]
+pub fn get_webview2_dir() -> Option<WebView2DirInfo> {
+ if let Ok(folder) = std::env::var("WEBVIEW2_BROWSER_EXECUTABLE_FOLDER") {
+ Some(WebView2DirInfo {
+ path: folder,
+ system: false,
+ })
+ } else {
+ // 没有设置自定义目录,使用系统 WebView2
+ Some(WebView2DirInfo {
+ path: String::new(),
+ system: true,
+ })
+ }
+}
</code_context>
<issue_to_address>
**suggestion:** `get_webview2_dir` always returns `Some` and doesn’t need an `Option` in its public API.
Both branches return `Some(WebView2DirInfo { .. })`, so `Option` never signals “no value”, and the TS side already treats the result as non-null. Returning `WebView2DirInfo` directly would simplify the API and remove unnecessary nullability handling in the frontend.
Suggested implementation:
```rust
pub fn get_webview2_dir() -> WebView2DirInfo {
```
```rust
WebView2DirInfo {
```
```rust
// 没有设置自定义目录,使用系统 WebView2
WebView2DirInfo {
path: String::new(),
system: true,
}
```
1. Ensure the second branch (`else`) of `get_webview2_dir` in `system.rs` matches the searched text exactly; if comments/whitespace differ, adjust the SEARCH blocks accordingly.
2. Check all Rust call sites and the generated TypeScript bindings (if you use `tauri::command` with codegen) to update their expected return type from `Option<WebView2DirInfo>` to `WebView2DirInfo`, removing any unnecessary null/undefined handling on the frontend.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - 我发现了两个问题,并给出了一些整体性的反馈:
- 在
extract_cab_to_runtime中,临时解压目录只会在正常返回路径以及一个早期失败分支中被清理;建议把整个函数包裹在一个作用域中,在发生错误时都尝试调用remove_dir_all(&extract_temp),以避免临时目录长期泄漏。 - 目前选择的 WebView2 变体(系统版本 vs 本地固定运行时,包括本地 cab 的使用/回退逻辑)只在 UI 中有所体现;在
ensure_webview2/download_and_extract/try_extract_local_cab中增加一些log::info!调用,会让我们在真实环境的失败场景中更容易诊断实际走的是哪条逻辑路径。
面向 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- In `extract_cab_to_runtime`, the temporary extract directory is only cleaned up on the happy path and on one early failure; consider wrapping the whole function in a scope that always attempts `remove_dir_all(&extract_temp)` on error to avoid leaking temp directories over time.
- Right now the selected WebView2 variant (system vs local fixed runtime, including local cab usage/fallbacks) is only reflected in UI; adding a few `log::info!` calls in `ensure_webview2` / `download_and_extract` / `try_extract_local_cab` would make it much easier to diagnose which path was taken in real-world failures.
## Individual Comments
### Comment 1
<location path="src-tauri/src/webview2/dialog.rs" line_range="239-240" />
<code_context>
+ right: width,
+ bottom: height,
+ };
+ let _ = AdjustWindowRect(&mut rc, wnd_style, false);
+ let wnd_w = rc.right - rc.left;
+ let wnd_h = rc.bottom - rc.top;
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Handle potential AdjustWindowRect failure instead of ignoring the result.
`AdjustWindowRect` returns a BOOL, and on failure `rc` is undefined. Relying on `rc.right - rc.left` / `rc.bottom - rc.top` in that case can produce invalid sizes. Please check the return value and fall back to the original `width`/`height` if the call fails:
```rust
let success = AdjustWindowRect(&mut rc, wnd_style, false).as_bool();
let (wnd_w, wnd_h) = if success {
(rc.right - rc.left, rc.bottom - rc.top)
} else {
(width, height)
};
```
Suggested implementation:
```rust
let title_wide = to_wide(&title_owned);
let wnd_style = WS_OVERLAPPED | WS_CAPTION | WS_SYSMENU;
// width/height represent desired client area; compute actual window size
let mut rc = windows::Win32::Foundation::RECT {
left: 0,
top: 0,
right: width,
bottom: height,
};
let success = AdjustWindowRect(&mut rc, wnd_style, false).as_bool();
let (wnd_w, wnd_h) = if success {
(rc.right - rc.left, rc.bottom - rc.top)
} else {
(width, height)
};
```
1. Ensure that the code which previously used `wnd_w` / `wnd_h` (or `width` / `height` directly after the `AdjustWindowRect` call) now uses the `(wnd_w, wnd_h)` variables from this block.
2. If `AdjustWindowRect` is not already imported into this module, add `use windows::Win32::UI::WindowsAndMessaging::AdjustWindowRect;` near the other `use` statements.
3. If this call is currently inside an `unsafe` block (as is typical with the windows crate APIs), keep it there; if not, wrap the `AdjustWindowRect` call in `unsafe { ... }` as required by your existing conventions.
</issue_to_address>
### Comment 2
<location path="src-tauri/src/webview2/install.rs" line_range="323-253" />
<code_context>
+ let _ = std::fs::remove_file(&cab_path);
+ return Some(Ok(()));
+ }
+ Err(_) => {
+ // 本地 cab 解压失败(可能文件损坏或被移除),删除并回退到在线下载
+ let _ = std::fs::remove_file(&cab_path);
+ return None;
+ }
+ }
</code_context>
<issue_to_address>
**suggestion:** Consider surfacing an explicit warning/log when local cab extraction fails before falling back to online download.
In this failure branch we delete the cab and silently fall back to online download. For users in network‑restricted environments who intentionally provide the local cab, this can be confusing (especially if the download also fails). Since `log::warn` is already available, please emit a warning here (and/or surface a small user‑visible message on later download failure) explaining that the local WebView2 cab was invalid/corrupted and we’re falling back to online download.
</issue_to_address>帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据反馈持续改进评审质量。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- In
extract_cab_to_runtime, the temporary extract directory is only cleaned up on the happy path and on one early failure; consider wrapping the whole function in a scope that always attemptsremove_dir_all(&extract_temp)on error to avoid leaking temp directories over time. - Right now the selected WebView2 variant (system vs local fixed runtime, including local cab usage/fallbacks) is only reflected in UI; adding a few
log::info!calls inensure_webview2/download_and_extract/try_extract_local_cabwould make it much easier to diagnose which path was taken in real-world failures.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `extract_cab_to_runtime`, the temporary extract directory is only cleaned up on the happy path and on one early failure; consider wrapping the whole function in a scope that always attempts `remove_dir_all(&extract_temp)` on error to avoid leaking temp directories over time.
- Right now the selected WebView2 variant (system vs local fixed runtime, including local cab usage/fallbacks) is only reflected in UI; adding a few `log::info!` calls in `ensure_webview2` / `download_and_extract` / `try_extract_local_cab` would make it much easier to diagnose which path was taken in real-world failures.
## Individual Comments
### Comment 1
<location path="src-tauri/src/webview2/dialog.rs" line_range="239-240" />
<code_context>
+ right: width,
+ bottom: height,
+ };
+ let _ = AdjustWindowRect(&mut rc, wnd_style, false);
+ let wnd_w = rc.right - rc.left;
+ let wnd_h = rc.bottom - rc.top;
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Handle potential AdjustWindowRect failure instead of ignoring the result.
`AdjustWindowRect` returns a BOOL, and on failure `rc` is undefined. Relying on `rc.right - rc.left` / `rc.bottom - rc.top` in that case can produce invalid sizes. Please check the return value and fall back to the original `width`/`height` if the call fails:
```rust
let success = AdjustWindowRect(&mut rc, wnd_style, false).as_bool();
let (wnd_w, wnd_h) = if success {
(rc.right - rc.left, rc.bottom - rc.top)
} else {
(width, height)
};
```
Suggested implementation:
```rust
let title_wide = to_wide(&title_owned);
let wnd_style = WS_OVERLAPPED | WS_CAPTION | WS_SYSMENU;
// width/height represent desired client area; compute actual window size
let mut rc = windows::Win32::Foundation::RECT {
left: 0,
top: 0,
right: width,
bottom: height,
};
let success = AdjustWindowRect(&mut rc, wnd_style, false).as_bool();
let (wnd_w, wnd_h) = if success {
(rc.right - rc.left, rc.bottom - rc.top)
} else {
(width, height)
};
```
1. Ensure that the code which previously used `wnd_w` / `wnd_h` (or `width` / `height` directly after the `AdjustWindowRect` call) now uses the `(wnd_w, wnd_h)` variables from this block.
2. If `AdjustWindowRect` is not already imported into this module, add `use windows::Win32::UI::WindowsAndMessaging::AdjustWindowRect;` near the other `use` statements.
3. If this call is currently inside an `unsafe` block (as is typical with the windows crate APIs), keep it there; if not, wrap the `AdjustWindowRect` call in `unsafe { ... }` as required by your existing conventions.
</issue_to_address>
### Comment 2
<location path="src-tauri/src/webview2/install.rs" line_range="323-253" />
<code_context>
+ let _ = std::fs::remove_file(&cab_path);
+ return Some(Ok(()));
+ }
+ Err(_) => {
+ // 本地 cab 解压失败(可能文件损坏或被移除),删除并回退到在线下载
+ let _ = std::fs::remove_file(&cab_path);
+ return None;
+ }
+ }
</code_context>
<issue_to_address>
**suggestion:** Consider surfacing an explicit warning/log when local cab extraction fails before falling back to online download.
In this failure branch we delete the cab and silently fall back to online download. For users in network‑restricted environments who intentionally provide the local cab, this can be confusing (especially if the download also fails). Since `log::warn` is already available, please emit a warning here (and/or surface a small user‑visible message on later download failure) explaining that the local WebView2 cab was invalid/corrupted and we’re falling back to online download.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
怎么七点了写不动 description 了找 LLM 写了
移除 清空缓存 按钮
在调试中添加 Webview2 目录
WebView2 Fixed Version Runtime 支持
将 WebView2 从系统安装(Evergreen Bootstrapper)改为本地固定版本运行时(Fixed Version Runtime),下载后解压到 exe 同目录使用,不影响系统环境,且在 MXU 自更新后仍然生效。
主要变更
核心功能:本地 WebView2 运行时
触发逻辑
本地 cab 文件支持
下载体验优化
UI 修复
Summary by Sourcery
将 WebView2 集成方式从系统范围的 Evergreen 安装器切换为在可执行文件旁解压的本地固定版本运行时,并改进下载处理和对话框体验。
New Features:
webview2_runtime目录中运行。.cab文件,以便在离线或低网络环境中完成安装。Bug Fixes:
Enhancements:
AdjustWindowRect调整窗口大小以保证控件可见。Original summary in English
Summary by Sourcery
Switch WebView2 integration from system-wide Evergreen installer to a local fixed-version runtime unpacked alongside the executable, with improved download handling and dialogs.
New Features:
Bug Fixes:
Enhancements:
Original summary in English
Summary by Sourcery
将 WebView2 集成方式从系统范围的 Evergreen 安装器切换为在可执行文件旁解压的本地固定版本运行时,并改进下载处理和对话框体验。
New Features:
webview2_runtime目录中运行。.cab文件,以便在离线或低网络环境中完成安装。Bug Fixes:
Enhancements:
AdjustWindowRect调整窗口大小以保证控件可见。Original summary in English
Summary by Sourcery
Switch WebView2 integration from system-wide Evergreen installer to a local fixed-version runtime unpacked alongside the executable, with improved download handling and dialogs.
New Features:
Bug Fixes:
Enhancements:
由 Sourcery 撰写的摘要
将 WebView2 集成方式从系统范围安装切换为本地解压的固定版本运行时,并改进相关诊断与对话框。
新功能:
cache/webview2_runtime目录下载、解压并使用捆绑的 WebView2 固定版本运行时。错误修复:
AdjustWindowRect调整对话框窗口尺寸,确保进度和状态控件始终完全可见。改进:
杂项:
Original summary in English
Summary by Sourcery
切换 WebView2 集成方式,改为使用本地解压的固定版本运行时,并改进安装/下载流程和诊断信息。
新功能:
cache/webview2_runtime目录下下载、解压并使用 WebView2 固定版本运行时,可通过WEBVIEW2_BROWSER_EXECUTABLE_FOLDER进行配置。错误修复:
AdjustWindowRect调整自定义对话框窗口尺寸,确保内容和控件保持完全可见。增强改进:
expand.exe、运行时目录校验,以及更清晰的错误/修复指引。日常维护:
Original summary in English
Summary by Sourcery
Switch WebView2 integration to use a locally extracted fixed-version runtime with improved install/download handling and diagnostics.
New Features:
Bug Fixes:
Enhancements:
Chores: