GoogleChromeLabs / tti-polyfill

Time-to-interactive polyfill
Other
347 stars 65 forks source link

Possible timing issue #9

Open jamietre opened 6 years ago

jamietre commented 6 years ago

When using PerformanceObserver, I found that it was possible for events to be lost when using the same pattern tti-polyfill uses to cache events that occur between registering the first PerformanceObserver in head, and calling disconnect & registering another one. The callback appears to be called asynchronously, so there could be an event loop between when a performance event is captured and the callback invoked. That is, this sequence of events could occur:

  1. Register first PerformanceObserver with callback in head
  2. Event is captured by PerformanceObserver, but callback not invoked yet.
  3. First PerformanceObserver is disconnected, causing event from #2 to be lost.

The correct way to handle this appears to be using takeRecords() to flush events that have been received, but not yet captured by the callback, before calling disconnect. In src/firstConsistentlyInteractiveDetector.js this isn't done before the first observer is disconnected at line 63. Theoretically events could be lost.

Documentation's a bit sparse on this so I could be doing this wrong, but this seemed to be the correct solution when events went missing for me. (I haven't actually experienced this problem with tti-polyfill just to be clear, but did using the exact same pattern with other types of events like mark).

philipwalton commented 6 years ago

Yeah, takeRecords sounds like the right approach to me, both with PerformanceObserver as well with all the other observers (IntersectionObserver, ResizeObserver, etc.).