-
-
Notifications
You must be signed in to change notification settings - Fork 78
chrome. implement offscreen document to keep service worker alive #209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
chrome. implement offscreen document to keep service worker alive #209
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements Chrome's Offscreen API to keep the service worker alive, replacing the previous keep-alive mechanism that used periodic alarm checks. The new approach creates an offscreen document that sends periodic heartbeat messages to the background service worker.
Key Changes:
- Replaced interval-based alarm polling with Chrome Offscreen API for service worker lifecycle management
- Added offscreen document that sends keep-alive messages every 20 seconds
- Added "offscreen" permission to the manifest
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/manifest.json | Added "offscreen" permission required for Chrome Offscreen API |
| src/offscreen.html | Created minimal HTML file to host the offscreen script |
| src/offscreen.ts | Implemented keep-alive logic that sends messages every 20 seconds to background |
| src/background/main.ts | Replaced old keep-alive mechanism with offscreen document creation and message handler |
| vite.config.ts | Added offscreen.html to build inputs for bundling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed everything up to 712cbab in 1 minute and 47 seconds. Click for details.
- Reviewed
122lines of code in5files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/background/main.ts:110
- Draft comment:
Duplicate invocation of setupOffscreen: It's added as listeners on onStartup and onInstalled, then called directly. If intentional, add a clarifying comment; otherwise, consider removing the direct call. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment is asking the author to either clarify or change the code. However, this pattern seems intentional: the direct call ensures immediate setup when the background script loads, while the event listeners handle browser restarts. The function has proper guards to prevent duplicate document creation. This comment is essentially asking the author to "confirm their intention" or "add clarifying comments," which violates the rules. The comment doesn't identify a bug or required code change - it's more of a "this might be confusing" observation. The pattern is actually quite common in browser extensions where you want something to run both immediately and on specific events. The pattern could genuinely be confusing to future maintainers, and a clarifying comment might improve code readability. The comment does suggest a concrete action (adding a comment or removing the call), which could be considered actionable rather than purely speculative. While the pattern might benefit from clarification, the rules explicitly state not to ask the author to confirm their intention or add clarifying comments. The comment uses the phrase "If intentional, add a clarifying comment; otherwise, consider removing" which is exactly the type of "verify/ensure/confirm" language the rules say to avoid. There's no actual bug here - the code works correctly with the guards in place. This comment should be deleted. It's asking the author to either confirm their intention or add a clarifying comment, which violates the rules. The code has proper guards and the pattern is intentional and functional. No actual code change is required.
2. src/offscreen.ts:3
- Draft comment:
The comment is in Chinese; consider using English (or a consistent language) for clarity across the codebase. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This is a style/consistency comment about using English instead of Chinese. While language consistency can be valuable, the rules state "Do NOT comment unless there is clearly a code change required" and "Do NOT make comments that are obvious or unimportant." This is more of a stylistic preference rather than a functional issue. The code works fine with Chinese comments. Without seeing the rest of the codebase, I can't verify if there's actually an established pattern of English-only comments. The comment is also somewhat presumptuous about what language should be used - maybe this is a Chinese-language project. I might be missing context about whether this codebase has an established English-only policy. If the entire codebase uses English comments and this is the only Chinese comment, it could be a legitimate consistency issue worth addressing. Even if there is an English-only policy, this falls under "obvious or unimportant" comments. The author can see the comment is in Chinese. This is a stylistic preference that doesn't affect functionality, and without strong evidence of a codebase-wide policy visible in this diff, I should err on the side of deletion per the rules. This comment is about style/language preference rather than a functional code issue. It falls under "obvious or unimportant" comments that don't clearly require a code change. Without strong evidence of a codebase policy in the diff itself, this should be deleted.
Workflow ID: wflow_BI7DONsYuOb4zSim
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
712cbab to
14418cf
Compare
|
Have you tested this locally? I keep getting error creating bucket for some reason. Properly set up cors and everything in the server. @BelKed Maybe you can give this a go. |
I have used it normally, and after modification, it works well at least at windows 11 edge(143.0.3650.96).
|
|
Got it working on the python server. Some cors issues on the rust server, I presume. Seems to be working perfectly.🚀 |
The issues on the Rust server appear to be CORS-related; likely because its security policy doesn't yet permit the chrome-extension:// origin/scheme. Since this has been a long-standing issue affecting many users, I’d appreciate it if we could expedite the merge and release of this fix. Thanks! |
|
The Rust server has hard-coded UUID which only extensions installed from the Chrome Web Store have I'll manually test it in the upcoming days, thank you for the fix 💙 |
BelKed
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Manually tested, seems to be working correctly :)


https://github.com/Tokisaki-Galaxy/aw-watcher-web/actions
Passed the action compilation and grammar check, and has been tested to be normal.
resolve #199
Important
Implements Offscreen API in Chrome to keep the service worker alive by adding offscreen document and periodic messaging.
setupOffscreen()inmain.tsto keep the service worker alive using the Offscreen API for Chrome.KEEP_ALIVEmessages fromoffscreen.tsto prevent service worker termination.KEEP_ALIVEmessages inmain.tsand responds with status 'ok'.offscreen.htmlto loadoffscreen.ts.offscreen.tsto send periodic messages to the background script.manifest.jsonto includeoffscreenpermission for Chrome.vite.config.tsto includesrc/offscreen.htmlinadditionalInputs.This description was created by
for 712cbab. You can customize this summary. It will automatically update as commits are pushed.