Skip to content

avoid calling messageUIDelegate.loaded repeatedly#650

Merged
andresilveirah merged 1 commit into
developfrom
guard_agaisnt_repeated_onMessageReady
May 18, 2026
Merged

avoid calling messageUIDelegate.loaded repeatedly#650
andresilveirah merged 1 commit into
developfrom
guard_agaisnt_repeated_onMessageReady

Conversation

@andresilveirah
Copy link
Copy Markdown
Member

If the rendering app dispatches the event sp.showMessage multiple times and the consentUI is being presented, clients' app can crash if their code is not guarding against re-presenting the consent ViewController.

Almost entirely inspired on #649

If the rendering app dispatches the event `sp.showMessage` multiple times and the consentUI is being presented, clients' app can crash if their code is not guarding against re-presenting the consent ViewController
@andresilveirah andresilveirah merged commit 2599394 into develop May 18, 2026
13 of 14 checks passed
@sampaioroberto
Copy link
Copy Markdown

@andresilveirah Thanks again for merging and crediting #649! 😊

One small concern with the guard: isBeingPresented and view.window only change after the host app calls present(), which happens inside the DispatchQueue.main.async in onSPUIReady. A second event arriving before that block runs would still pass the check and call loaded() twice.

There's also a minor issue in the test: loadMessage() instantiates GenericWebMessageViewController, not MockedGenericWebMessageViewController, so the mockIsBeingPresented override is never active in the duplicated-event scenario. The test passes because toEventually catches the count between the two deliveries rather than the guard actually blocking the second call.

Happy to be wrong if I'm missing something though!

@andresilveirah
Copy link
Copy Markdown
Member Author

Hey @sampaioroberto you are absolutely right about both, the test and the racing condition when calling loaded(). In the end, I think it'll be easier to deal with it with the flag as you first suggested.

@sampaioroberto
Copy link
Copy Markdown

Hey @andresilveirah Glad it helped! Are you planning to open a new PR with the flag approach, or would you like me to reopen #649?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants