fix: prevent duplicate onSPUIReady delivery from repeated rendering app events#649
Closed
sampaioroberto wants to merge 1 commit into
Closed
Conversation
…pp events GenericWebMessageViewController had no idempotency guard on onMessageReady() and onPmReady(), so a rendering app firing sp.showMessage more than once for the same instance caused loaded() — and ultimately delegate.onSPUIReady() — to be called multiple times. Host apps following the documented pattern of present(controller) in onSPUIReady would crash with "tried to present a view controller already being presented". Adds a hasCalledLoaded flag that ensures loaded() is delivered at most once per controller instance regardless of how many JS events arrive. Includes a regression test (DuplicatingRenderingAppMock) that fires sp.showMessage twice and asserts loadedCallCount == 1. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Member
|
Hi @sampaioroberto thank you for the PR. I've made a small modification, instead of relying on a boolean flag, I check if the ViewController is currently being presented or its view is != nil. I've used your tests and created #650 . I'm closing this PR in favour of that one. |
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.
Problem
Crash reported via Firebase Crashlytics:
The same
GenericWebMessageViewControllerinstance was triggeringdelegate.onSPUIReady()more than once. Host apps following the documented pattern (present(controller, animated: true)insideonSPUIReady) would crash on the second call.Root cause
SPJSReceiver.jsconverts asp.showMessage(withoutfromPM) postMessage into anonMessageReadynative event. If the rendering app fires this event more than once for the same view, the Swift side had no idempotency guard:onPmReady()had a partial guard viaisFirstLayerMessage, but that flag guards a state-machine transition (first layer → PM), not duplicate events on the same path.Fix
Add a
hasCalledLoadedboolean flag toGenericWebMessageViewControllerthat ensuresloaded()— and thereforeonSPUIReady()— is delivered at most once per controller instance, regardless of how many JS events arrive.The two guards are orthogonal:
isFirstLayerMessageremains for its original purpose (suppress PM-ready when coming from a first-layer flow);hasCalledLoadedadds the missing idempotency invariant across both paths.Test
Added
DuplicatingRenderingAppMock— aWKWebViewsubclass that firessp.showMessagetwice in the same tick — and a regression test that assertsloadedCallCount == 1.loadedCallCountwould be 2 → test failsloadedCallCountis 1 → test passes ✅All 5 tests in
GenericWebMessageViewControllerSpecpass.🤖 Generated with Claude Code