Fix WebView process termination retry loop#451
Merged
Conversation
…retry counter The previous counter-based approach had a bug: processTerminationRetryCount reset to 0 on every successful didFinish, defeating the max-retry cap entirely. This caused infinite kill-reload loops on memory-constrained devices with preloaded paywalls. Replace with presentation-state logic: - Active (presenting) paywalls: always reload immediately on process termination - Background/preloaded paywalls: mark didFailToLoad for deferred reload on next viewWillAppear Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Caps active-paywall reloads at 3 to prevent indefinite loops under sustained memory pressure where didFinish never fires. The counter resets on successful load since that breaks the rapid-termination cycle. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Only didFinish should reset the counter, since it signals a genuinely successful load. Resetting in the else branch re-armed the cycle for active paywalls that hit the cap. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Two bugs fixed: - didFinish reset defeated the retry cap (terminate -> reload -> success -> counter back to 0 -> terminate again, infinitely) - else branch set didFailToLoad for active paywalls, but viewWillAppear never fires while presented, leaving users with a blank screen Now: no counter reset (lifetime cap per WebView instance), and active paywalls that exhaust retries are dismissed via webViewDidFail(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Reset activeProcessTerminationRetryCount in loadURL(from:) so cached SWWebView instances get a fresh retry budget per intentional load. This is the correct reset point: didFinish fires after terminate- triggered reloads too, but loadURL(from:) only fires for SDK- initiated loads. - Set didFailToLoad = true before calling webViewDidFail() so that cached VCs re-presenting trigger a fresh load via viewWillAppear. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The counter should cap consecutive unrecovered terminations (rapid kill loops where didFinish never fires), not total lifetime terminations. A didFinish after a terminate-triggered reload() means the termination was recovered — the counter should reset so devices with chronic but manageable memory pressure aren't penalised. This is safe because the counter now only gates the active path. The old bug was a shared counter where didFinish from an unrelated initial load reset the cap for background paywalls — that path now bypasses the counter entirely. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
processTerminationRetryCount) reset to 0 on every successfuldidFinish, defeating the max-retry cap entirely. Data showed 44% of affected users (2,856) exceeded the supposed 1-retry limit on SDK 4.12.8.delegate?.isActive: actively-presenting paywalls always reload immediately, while background/preloaded paywalls markdidFailToLoadfor deferred reload on nextviewWillAppear.isActivetoSWWebViewDelegateprotocol, already implemented byPaywallViewController.Changes
SWWebView.swift: RemovedprocessTerminationRetryCount/maxProcessTerminationRetriesproperties and the counter reset indidFinish. RewrotewebViewWebContentProcessDidTerminateto branch ondelegate?.isActive.SWWebViewDelegateprotocol: Addedvar isActive: Bool { get }.CHANGELOG.md: Added 4.14.2 entry.Test plan
CHANGELOG.mdfor any breaking changes, enhancements, or bug fixes.swiftlintin the main directory and fixed any issues.Testing note: The
webViewWebContentProcessDidTerminatehandler is aWKNavigationDelegatecallback triggered by iOS when the OS kills the WebView process. The branching logic itself is simple (checkisActive→ reload vs setdidFailToLoad).SWWebViewinstantiatesWKWebViewand depends onSuperwall.sharedfor tracking, making isolated unit tests impractical without significant mocking infrastructure. The existingSWWebViewLoadingHandlertests cover the downstreamdidFailToLoadpath.🤖 Generated with Claude Code
Greptile Summary
This PR replaces the broken counter-based WebView process termination retry logic with a presentation-state–aware approach. The previous
processTerminationRetryCountwas reset to zero on every successfuldidFinish, making the max-retry cap of 1 completely ineffective and causing infinite kill-reload loops on memory-constrained devices.The new logic branches on
delegate?.isActive:maxActiveProcessTerminationRetries), after whichwebViewDidFail()is called to dismiss immediately.didFailToLoad = trueand defer reloading to the nextviewWillAppear.The
activeProcessTerminationRetryCountis properly reset in bothSWWebView.loadURL()(on fresh paywall loads) andWKNavigationDelegate.didFinish(on successful recovery), andPaywallViewController.loadWebView()routes throughwebView.loadURL(), so re-presented paywalls also get a fresh counter.Key changes:
SWWebView.swift: RemovedprocessTerminationRetryCount/maxProcessTerminationRetries; addedactiveProcessTerminationRetryCount/maxActiveProcessTerminationRetries = 3; rewrotewebViewWebContentProcessDidTerminateto branch ondelegate?.isActive; counter resets inloadURLanddidFinish.SWWebViewDelegateprotocol: Addedvar isActive: Bool { get }, already implemented byPaywallViewController.CHANGELOG.md: Added 4.14.2 fix entry.Confidence Score: 4/5
loadURLanddidFinishcorrectly handles all re-presentation paths.PaywallViewController.loadWebView()routes throughwebView.loadURL(), soactiveProcessTerminationRetryCountis properly reset on every new load attempt. TheisActive-based branching cleanly separates active vs background paywall behaviour. Score is 4 rather than 5 only because theelse ifbranch setsdidFailToLoad = truealongsidewebViewDidFail()(which dismisses), creating slightly mixed signals — though this is functionally harmless sinceloadURLresets the flag on re-presentation.Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[webViewWebContentProcessDidTerminate] --> B{delegate?.isActive == true?} B -- Yes --> C{activeProcessTerminationRetryCount < 3?} B -- No --> G[Set didFailToLoad = true\nDefer reload to viewWillAppear] C -- Yes --> D[Increment counter\nwebView.reload] D --> E{didFinish fires?} E -- Yes --> F[Reset counter to 0\nContinue presenting] E -- No / terminated again --> A C -- No --> H[Set didFailToLoad = true\nCall webViewDidFail] H --> I[handleWebViewFailure:\nguard isActive else return\ndismiss paywall] G --> J[viewWillAppear\nCheck didFailToLoad] J --> K[loadWebView → loadURL\nReset counter to 0\nReset didFailToLoad on success] I --> L[VC dismissed] L -- Re-presented --> JLast reviewed commit: 02bc8fa