🐛 Fixed first contentful paint race condition - #25259
Conversation
|
@keithwrightbos The variable was introduced by #13015. When we tested the behavior we found that there's a race condition between the analytics ping and the first_contentful_paint. if the first_contentful_paint happens after the analytics ping, the value will be resolved to null. This brings us back to the discussion of async vs sync macro : ) We think that the current behavior is not consistent and want to fix the race condition. Please let us know what do you think about the fix. Thank you. |
|
cc @jonkeller as well to understand the impact to the metrics. (The requests may be delayed to wait for the value) Also I wonder how we handle |
zhouyx
left a comment
zhouyx
left a comment
There was a problem hiding this comment.
Could you please sync with the ads team offline to make sure they're aware of the change. SYNC macro becomes ASYNC, and understand if we want to provide a SYNC version of these variables. Thanks.
Ads team signed off on the changes. |
|
LGTM. to @jridgewell for owner's approval. Thank you |
* edge case handling and testing * type fixing * cleaning up * changing to only serve chrome * install platform service for fcp * Clean up integration test * viewport * remove platform test
4 participants
Fixed a race condition where an analytics request for
FIRST_CONTENTFUL_PAINTwould sometimes resolve to null because it hadn't be set yet.Applied this fix to
FIRST_VIEWPORT_READYandMAKE_BODY_VISIBLEas well.cc @zhouyx @jridgewell @keithwrightbos @ampproject/wg-ads