-
Notifications
You must be signed in to change notification settings - Fork 250
fix: keep WebView cached to reduce network traffic #284
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
fix: keep WebView cached to reduce network traffic #284
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 10fa03c44a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| guard let self, let entry else { return } | ||
| entry.isBusy = false | ||
| entry.lastUsedAt = Date() | ||
| entry.host.close() | ||
| self.entries.removeValue(forKey: key) | ||
| // Hide instead of close - keep WebView cached for reuse. | ||
| // This avoids re-downloading the ChatGPT SPA bundle on every refresh, | ||
| // saving significant network bandwidth. See GitHub issues #269, #251. | ||
| entry.host.hide() | ||
| self.prune(now: Date()) |
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.
Ensure cache reuse with stable data-store identity
Keeping the WebView alive on release only helps if the same WKWebsiteDataStore instance is reused on the next fetch. In the current call path, OpenAIDashboardWebsiteDataStore.store(forAccountEmail:) creates a new WKWebsiteDataStore each time, so the cache key (object identity) changes per refresh. With this new hide-and-keep behavior, each refresh inserts a new cached entry that is never reused, so the network savings won’t materialize and you’ll keep extra hidden WebViews alive until pruning. Consider keying by a stable store identifier or reusing the same WKWebsiteDataStore instance per account.
Useful? React with 👍 / 👎.
The OpenAI dashboard WebView was being destroyed after every fetch, causing the full ChatGPT SPA bundle to be re-downloaded on each refresh. Users reported 15+ GB of network traffic from chatgpt.com over a two-week period. Root cause: Commit 1c55dc7 changed the release behavior from hide() to close() to reduce idle CPU usage. However, this caused every refresh to create a new WebView and download all assets fresh. The fix has two parts: 1. OpenAIDashboardWebViewCache: On release, call hide() instead of close() to keep WebView in cache. The existing prune mechanism (10 minute idle timeout) handles cleanup. The inactiveSchedulingPolicy=.suspend setting still reduces idle CPU. 2. OpenAIDashboardWebsiteDataStore: Cache WKWebsiteDataStore instances so the same object is returned for the same account email. This ensures stable object identity for WebView cache lookups (the cache uses ObjectIdentifier as the key). This gives us the best of both worlds: low idle CPU (WebView suspended when hidden) and low network usage (WebView reused across refreshes). Added test instrumentation and 9 unit tests to verify: - WKWebsiteDataStore returns same instance for same email - WebView is cached after release, not destroyed - Different data stores have separate cached WebViews - Idle timeout pruning works correctly - Sequential fetches reuse the same WebView instance - Integration test with real data store factory Fixes steipete#269, steipete#251
10fa03c to
515ae98
Compare
|
Addressed the Codex review feedback in the updated commit. The issue was that The fix adds instance caching to |
|
Landed via temp rebase onto main. Thanks @vignesh07! |
Summary
The OpenAI dashboard WebView was being destroyed after every fetch, causing the full ChatGPT SPA bundle to be re-downloaded on each refresh. Users reported 15+ GB of network traffic from
chatgpt.comover a two-week period.Root Cause
Commit 1c55dc7 ("perf: reduce OpenAI web idle CPU") changed the release behavior:
This was intended to reduce idle CPU usage by destroying cached WebViews. However, it caused every refresh to create a new WebView and download all assets fresh.
With the default 5-minute refresh interval (multiplied by 5 for OpenAI web = 25 minutes), this resulted in approximately 57 page loads per day. If each load downloads 20-50 MB of assets, that adds up to 1-3 GB per day.
The Fix
On release, call
hide()instead ofclose()to keep the WebView in cache:This gives us the best of both worlds:
inactiveSchedulingPolicy=.suspend+alphaValue=0)Testing
Added 7 unit tests to verify cache behavior:
webViewCachedAfterReleaseseparateCachesPerDataStorewebViewPrunedAfterIdleTimeoutrecentlyUsedWebViewNotPrunedevictRemovesSpecificWebViewbusyWebViewCreatesTemporarysequentialFetchesReuseWebViewAll tests pass.
Impact
Fixes #269, #251