Skip to content

perf: 优化 pushValue 处理#1403

Open
cyfung1031 wants to merge 3 commits intomainfrom
perf/pushValue-session-onChanged
Open

perf: 优化 pushValue 处理#1403
cyfung1031 wants to merge 3 commits intomainfrom
perf/pushValue-session-onChanged

Conversation

@cyfung1031
Copy link
Copy Markdown
Collaborator

@cyfung1031 cyfung1031 commented May 6, 2026

Checklist / 检查清单

  • Fixes mentioned issues / 修复已提及的问题
  • Code reviewed by human / 代码通过人工检查
  • Changes tested / 已完成测试

Description / 描述

  1. 在 chrome 环境,可以改指定为 session 而非 local, 减少 onChanged 触发
  2. 把原本一分为三的 pushValue 合并为一个
  3. scripting.ts: 如该页面没有该脚本的执行,则不触发 broadcast event
  4. runtime.ts: 如没有后台脚本使用此 storageName, 则不广播至 offscreen/sandbox
  5. 新增 example/tests/gm_value_test.js. 里面包括了 GM_getValue, GM_setValue, GM_deleteValue, GM_addValueChangeListener, GM_removeValueChangeListener

Screenshots / 截图

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

该 PR 主要围绕 GM value(GM_setValue / GM_addValueChangeListener 等)的“值更新推送”链路做性能与结构调整:把原先分散在 ValueService / MessageQueue 的推送逻辑收敛到 RuntimeService,并在 scripting 侧按页面实际是否运行对应脚本来减少不必要的广播,同时新增一个用于手工验证 GM value 行为的示例脚本。

Changes:

  • 将 value 更新推送入口从 ValueService.pushValueToTab + mq.emit 合并为 RuntimeService.pushValueUpdate 统一处理(包含 early-start 脚本的资源更新逻辑)。
  • scripting 侧引入 activeStorageNames,仅在当前页面确实存在对应 storageName 的脚本执行环境时才转发 runtime/valueUpdate 到 content / inject。
  • 新增 example/tests/gm_value_test.js,用于在真实 iframe 场景下验证 GM value 读写与监听器行为。

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/app/service/service_worker/value.ts 移除原本的 pushValueToTab/mq.emit 逻辑,改为委托 RuntimeService.pushValueUpdate 统一推送
src/app/service/service_worker/value.test.ts 适配 ValueService API 变化(mock & 断言从 pushValueToTab 改为 pushValueUpdate)
src/app/service/service_worker/runtime.ts 新增 pushValueUpdate,集中处理 storage 投递、offscreen 转发、early-start 脚本资源更新
src/app/service/sandbox/runtime.ts 仅补充注释,标注 valueUpdate 消息路径
src/app/service/content/scripting.ts 增加 activeStorageNames 与按需广播策略,改用 deliveryStorage.onChanged 处理投递
src/app/service/content/script_executor.ts 仅补充注释,标注 valueUpdate 消息路径
example/tests/gm_value_test.js 新增 GM value 行为测试用用户脚本(主页面 + 多 iframe 控制台)
Comments suppressed due to low confidence (1)

src/app/service/service_worker/value.test.ts:90

  • 这个测试用例里仍然 mock 了 mockMessageQueue.emit = vi.fn(),但本次改动已经移除了 ValueService.setValues 内对 mq.emit 的调用;这里的 mock 目前是冗余的,建议删除以减少噪音并避免误导后续读者。
    // Mock pushValueUpdate 方法
    valueService.pushValueUpdate = vi.fn();

    // Mock mq.emit 方法
    mockMessageQueue.emit = vi.fn();
  });

Comment thread src/app/service/content/scripting.ts
Comment on lines +86 to +92
const bgScriptStorageNames = new Set<string>();

// For Firefox, StorageArea.setAccessLevel is not implemented.
// See https://bugzilla.mozilla.org/show_bug.cgi?id=1724754
// const deliveryStorage = isFirefox() ? chrome.storage.local : chrome.storage.session;
const deliveryStorage = chrome.storage.local; // 日后再处理

Copy link
Copy Markdown
Collaborator Author

@cyfung1031 cyfung1031 May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

故意的。因為 storageName 可以不只一個腳本。
就刪掉了所有 storageName, 也只不過是多廣播一下

Comment on lines +88 to +91
// For Firefox, StorageArea.setAccessLevel is not implemented.
// See https://bugzilla.mozilla.org/show_bug.cgi?id=1724754
// const deliveryStorage = isFirefox() ? chrome.storage.local : chrome.storage.session;
const deliveryStorage = chrome.storage.local; // 日后再处理
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

local <-> session 問題先不在這PR處理

@CodFrm
Copy link
Copy Markdown
Member

CodFrm commented May 6, 2026

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants