Repository navigation
Criteo: Handle RemoveUserSyncIframe - #15522
Conversation
|
This PR introduces changes that may not work on all browsers. According to Babel, the following polyfills may be needed, and they are not automatically included:
The best way to address this is to provide good test coverage, as normal PR checks run unit tests on older browsers. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4cdaba616c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| export function politeInsertUserSyncIframe(url) { | ||
| runBackgroundTask(() => insertUserSyncIframe(url)); | ||
| export function politeInsertUserSyncIframe(url, done, timeout, onCleanup) { | ||
| runBackgroundTask(() => insertUserSyncIframe(url, done, timeout, onCleanup)); |
There was a problem hiding this comment.
Cancel deferred iframe insertion during cleanup
When userSync.usePoliteSync is enabled and scheduler.postTask or requestIdleCallback has not executed yet, pbjs.removeUserSyncs() finds no iframe and therefore never invokes onCleanup; this queued task subsequently inserts the iframe after cleanup. For adapters such as Criteo, that leaves the message handler active and recreates a sync iframe after an SPA navigation attempted to remove it, so pending polite insertions need to be tracked and cancelled or cleaned up by removeUserSyncs().
Useful? React with 👍 / 👎.
Barecheck - Code coverage reportTotal: 91.24%Your code coverage diff: 0.01% ▴ Uncovered files and lines
|
dgirardi
left a comment
There was a problem hiding this comment.
LGTM. Merging master should clear the test failures.
4cdaba6 to
690a265
Compare
Created as a response to the following issue: #15496
Basically this PR ensures that the new function of IFrame removal, doesn't let promises remain uncompleted indefinitely.