Recover from WebContent process termination on macOS by handling webViewWebContentProcessDidTerminate#5129
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a new macOS event Changes
Sequence Diagram(s)sequenceDiagram
participant WK as WKWebView (WebContent)
participant Delegate as WebviewWindowDelegate
participant App as Host App Event System
participant JS as JS Runtime / Client
WK->>Delegate: webViewWebContentProcessDidTerminate
Delegate->>App: processWindowEvent(EventWebViewWebContentProcessDidTerminate)
App->>JS: deliver "mac:WebViewWebContentProcessDidTerminate"
Delegate->>WK: reload()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
v3/pkg/application/webview_window_darwin.m (1)
792-801: Consider guarding against rapid crash→reload loops.Auto-reload is good, but repeated renderer crashes can cause an unbounded tight loop. A small per-window debounce/backoff would improve resilience.
♻️ Suggested hardening
- (void)webViewWebContentProcessDidTerminate:(WKWebView *)webView { + static NSTimeInterval const kMinReloadIntervalSeconds = 1.0; + static NSMutableDictionary<NSNumber *, NSDate *> *lastReloadByWindow = nil; + if (lastReloadByWindow == nil) { + lastReloadByWindow = [[NSMutableDictionary alloc] init]; + } + + NSNumber *windowKey = @(self.windowId); + NSDate *now = [NSDate date]; + NSDate *last = [lastReloadByWindow objectForKey:windowKey]; + if (last && [now timeIntervalSinceDate:last] < kMinReloadIntervalSeconds) { + return; + } + [lastReloadByWindow setObject:now forKey:windowKey]; + if( hasListeners(EventWebViewWebContentProcessDidTerminate) ) { processWindowEvent(self.windowId, EventWebViewWebContentProcessDidTerminate); } [webView reload]; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@v3/pkg/application/webview_window_darwin.m` around lines 792 - 801, The webViewWebContentProcessDidTerminate: handler currently calls [webView reload] immediately which can enter a tight crash→reload loop; add a per-window debounce/backoff: track per-window fields (e.g., _lastTerminateTimestamp and _terminateCount on the window/controller that owns self.windowId), on each invocation compute a delay = min(baseDelay * 2^_terminateCount, maxDelay) and if now - _lastTerminateTimestamp < cooldown skip the reload, otherwise schedule the reload after delay on the main thread and increment _terminateCount and set _lastTerminateTimestamp; call processWindowEvent(self.windowId, EventWebViewWebContentProcessDidTerminate) as before and reset _terminateCount/_lastTerminateTimestamp to zero on a successful webView:didFinishNavigation: (or similar) to restore normal behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@v3/pkg/application/webview_window_darwin.go`:
- Around line 1062-1074: The InvokeAsync closures in macosWebviewWindow.reload
and macosWebviewWindow.forceReload can run after the window is torn down; mirror
the execJS lifecycle guard by adding the same early-return check used there
inside each InvokeAsync closure (e.g., verify the window is not closed/destroyed
and nsWindow is valid) before calling C.windowReload or C.windowForceReload so
the C calls never run against a destroyed window; apply the same guard logic to
both reload and forceReload.
---
Nitpick comments:
In `@v3/pkg/application/webview_window_darwin.m`:
- Around line 792-801: The webViewWebContentProcessDidTerminate: handler
currently calls [webView reload] immediately which can enter a tight
crash→reload loop; add a per-window debounce/backoff: track per-window fields
(e.g., _lastTerminateTimestamp and _terminateCount on the window/controller that
owns self.windowId), on each invocation compute a delay = min(baseDelay *
2^_terminateCount, maxDelay) and if now - _lastTerminateTimestamp < cooldown
skip the reload, otherwise schedule the reload after delay on the main thread
and increment _terminateCount and set _lastTerminateTimestamp; call
processWindowEvent(self.windowId, EventWebViewWebContentProcessDidTerminate) as
before and reset _terminateCount/_lastTerminateTimestamp to zero on a successful
webView:didFinishNavigation: (or similar) to restore normal behavior.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d2390a8e-edd4-4b45-b7a5-833083fe8c16
📒 Files selected for processing (9)
v3/UNRELEASED_CHANGELOG.mdv3/internal/generator/collect/known_events.gov3/internal/runtime/desktop/@wailsio/runtime/src/event_types.tsv3/pkg/application/webview_window_darwin.gov3/pkg/application/webview_window_darwin.mv3/pkg/events/events.gov3/pkg/events/events.txtv3/pkg/events/events_darwin.hv3/pkg/events/known_events.go
| func (w *macosWebviewWindow) reload() { | ||
| //TODO: Implement | ||
| globalApplication.debug("reload called on WebviewWindow", "parentID", w.parent.id) | ||
| InvokeAsync(func() { | ||
| C.windowReload(w.nsWindow) | ||
| }) | ||
| } | ||
|
|
||
| func (w *macosWebviewWindow) forceReload() { | ||
| //TODO: Implement | ||
| globalApplication.debug("force reload called on WebviewWindow", "parentID", w.parent.id) | ||
| InvokeAsync(func() { | ||
| C.windowForceReload(w.nsWindow) | ||
| }) | ||
| } |
There was a problem hiding this comment.
Add lifecycle guards before async reload calls
InvokeAsync closures on Line 1064 and Line 1071 can run after teardown. Mirror the execJS guard pattern to avoid invoking reload against a destroyed window during shutdown/recovery.
🔧 Proposed fix
func (w *macosWebviewWindow) reload() {
globalApplication.debug("reload called on WebviewWindow", "parentID", w.parent.id)
InvokeAsync(func() {
+ globalApplication.shutdownLock.Lock()
+ performingShutdown := globalApplication.performingShutdown
+ globalApplication.shutdownLock.Unlock()
+ if performingShutdown || w.nsWindow == nil || w.parent.isDestroyed() {
+ return
+ }
C.windowReload(w.nsWindow)
})
}
func (w *macosWebviewWindow) forceReload() {
globalApplication.debug("force reload called on WebviewWindow", "parentID", w.parent.id)
InvokeAsync(func() {
+ globalApplication.shutdownLock.Lock()
+ performingShutdown := globalApplication.performingShutdown
+ globalApplication.shutdownLock.Unlock()
+ if performingShutdown || w.nsWindow == nil || w.parent.isDestroyed() {
+ return
+ }
C.windowForceReload(w.nsWindow)
})
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (w *macosWebviewWindow) reload() { | |
| //TODO: Implement | |
| globalApplication.debug("reload called on WebviewWindow", "parentID", w.parent.id) | |
| InvokeAsync(func() { | |
| C.windowReload(w.nsWindow) | |
| }) | |
| } | |
| func (w *macosWebviewWindow) forceReload() { | |
| //TODO: Implement | |
| globalApplication.debug("force reload called on WebviewWindow", "parentID", w.parent.id) | |
| InvokeAsync(func() { | |
| C.windowForceReload(w.nsWindow) | |
| }) | |
| } | |
| func (w *macosWebviewWindow) reload() { | |
| globalApplication.debug("reload called on WebviewWindow", "parentID", w.parent.id) | |
| InvokeAsync(func() { | |
| globalApplication.shutdownLock.Lock() | |
| performingShutdown := globalApplication.performingShutdown | |
| globalApplication.shutdownLock.Unlock() | |
| if performingShutdown || w.nsWindow == nil || w.parent.isDestroyed() { | |
| return | |
| } | |
| C.windowReload(w.nsWindow) | |
| }) | |
| } | |
| func (w *macosWebviewWindow) forceReload() { | |
| globalApplication.debug("force reload called on WebviewWindow", "parentID", w.parent.id) | |
| InvokeAsync(func() { | |
| globalApplication.shutdownLock.Lock() | |
| performingShutdown := globalApplication.performingShutdown | |
| globalApplication.shutdownLock.Unlock() | |
| if performingShutdown || w.nsWindow == nil || w.parent.isDestroyed() { | |
| return | |
| } | |
| C.windowForceReload(w.nsWindow) | |
| }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@v3/pkg/application/webview_window_darwin.go` around lines 1062 - 1074, The
InvokeAsync closures in macosWebviewWindow.reload and
macosWebviewWindow.forceReload can run after the window is torn down; mirror the
execJS lifecycle guard by adding the same early-return check used there inside
each InvokeAsync closure (e.g., verify the window is not closed/destroyed and
nsWindow is valid) before calling C.windowReload or C.windowForceReload so the C
calls never run against a destroyed window; apply the same guard logic to both
reload and forceReload.
|
@wayneforrest please can you edit the PR description with the requested information. Thanks |
Hey @leaanthony , I have updated this, and also provided a script I used to test this. |
|
CI is failing on this PR — Cross-Compile checks for darwin/arm64 and windows/arm64 are not passing. Additionally, the PR shows a merge conflict with the target branch. Please resolve the CI failures and rebase to fix the merge conflict, then we can proceed with testing. |
76fbaeb to
541d3f1
Compare
…iewWebContentProcessDidTerminate
541d3f1 to
dc8c1d8
Compare
|
rebased onto master. |
Description
Implemented automatic recovery when the web view process is killed.
Fixes #5130
Type of change
Please select the option that is relevant.
How Has This Been Tested?
Run the script, it kills the WebView, and app survives, the window re-centres on the screen.
Output:
If you checked Linux, please specify the distro and version.
Test Configuration
Please paste the output of
wails doctor. If you are unable to run this command, please describe your environment in as much detail as possible.Checklist:
website/src/pages/changelog.mdxwith details of this PRSummary by CodeRabbit
New Features
Bug Fixes