fix: cancel downloads when the userscript panel or tab closes - #108
Conversation
Closing the panel/tab while Chromium was starting could miss cancel because page fetch is blocked by Private Network Access and mid-launch close was a no-op.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe change adds cancellable browser lifecycle management to provider downloaders. It also adds active-job cancellation during userscript panel close and page lifecycle events, with tests for launch cancellation, downloader shutdown ordering, and cancellation request sequencing. ChangesCancellable browser lifecycle
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@userscript/sc-gate-dl.test.ts`:
- Around line 190-203: Add an assertion in the active-job cancellation test for
the production `beforeunload` listener registration, alongside the existing
`pagehide` assertion, so removal of either host-unload handler fails the test.
In `@userscript/sc-gate-dl.user.js`:
- Around line 1240-1249: Update the closePanel cleanup flow around
gmXmlHttpRequest so synchronous bridge exceptions are caught and do not reject
the operation. Treat the request as best-effort, allowing the subsequent iframe
reset and panel hiding logic to execute even when the GM bridge throws.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 837c6a43-38a3-4173-8d28-45313a730e75
📒 Files selected for processing (13)
src/browserLaunch.test.tssrc/browserLaunch.tssrc/downloadgater.tssrc/droploud.tssrc/gateCancellation.test.tssrc/gaterush.tssrc/hypeddit.tssrc/mypresskit.tssrc/pumpyoursound.tssrc/stillhype.tsuserscript/sc-gate-dl.test.tsuserscript/sc-gate-dl.user.jswebui/src/components/App.tsx
Keep panel teardown moving if the cancel bridge throws, and assert both unload listeners in the regression test.
|
@coderabbitai full review |
Closing the userscript panel or SoundCloud tab while Chromium was starting often left the download running. Cancel from the host page used a normal
fetchto localhost, which Chrome blocks via Private Network Access, and blanking the iframe raced the embedded UI’s unload handler. Separately,close()during an in-flight browser launch was a no-op, so the process that finished starting could stay orphaned.This change routes host cancel through
GM_xmlhttpRequest, cancels onpagehide/beforeunloadbefore tearing down the iframe, hardens the WebUI unload path with keepalivefetch, and addsCancellableBrowserLaunchso cancel waits out launch and closes that browser.Test plan
bun testfor cancellation / browser launch / userscript suitesImplemented with Composer in Cursor.
Summary by CodeRabbit