RUM-2600: Add traversal flag to snapshot items

[jonathanmos]

What does this PR do?

Adds a flag to snapshot items that indicates that they have finished traversing the view tree. Also, increases the interval that snapshots are considered stale from 200ms to 1 second

Motivation

The RecordedDataQueueHandler in some situations could attempt to process an item that was still performing a traversal. Because items are inserted into the queue at the beginning of their traversals without nodes, the handler could regard the item as invalid and drop it from the queue. With this change, items that haven't finished traversals will only be dropped if stale.

Additional Notes

Anything else we should know when reviewing?

Review checklist (to be filled by reviewers)

• Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
• Make sure you discussed the feature or bugfix with the maintaining team in an Issue
• Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

[codecov-commenter]

Codecov Report

Merging #1837 (b24509a) into develop (8145782) will increase coverage by 0.01%.
Report is 2 commits behind head on develop.
The diff coverage is 100.00%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1837      +/-   ##
===========================================
+ Coverage    83.49%   83.50%   +0.01%     
===========================================
  Files          467      467              
  Lines        16491    16491              
  Branches      2483     2485       +2     
===========================================
+ Hits         13769    13770       +1     
+ Misses        2043     2038       -5     
- Partials       679      683       +4     

Files	Coverage Δ
...nreplay/internal/async/RecordedDataQueueHandler.kt	97.78% <ø> (ø)
...ay/internal/async/SnapshotRecordedDataQueueItem.kt	100.00% <100.00%> (ø)
...nternal/recorder/listener/WindowsOnDrawListener.kt	89.66% <100.00%> (+0.37%)	⬆️

... and 33 files with indirect coverage changes

[0xnm]

if this one can be modified from the different threads, it should be either @Volatile or AtomicBoolean

[jonathanmos]

It's modified only on the main thread - created as false and modified to true after the traversal. It may indeed be ww to make it volatile since the value could be read on another thread

[0xnm]

But it is read in the isValid, isReady functions, which may be called from the worker thread:

• isValid is called from triggerProcessingLoop, which is annotated with @WorkerThread and from shouldRemoveItem which is called from triggerProcessingLoop.
• isReady is called also from multiple places of triggerProcessingLoop.

Looking on the threading model and call-chain I can see that RecordedDataQueueRefs#tryToConsumeItem may call that triggerProcessingLoop and RecordedDataQueueRefs#tryToConsumeItem may be called during the traversal.

So to be on the safe side and to protect from the changes of the the threading model in the future, it is still better to add it.

[0xnm]

Same may be applied to the nodes property as well btw, maybe we should add @Volatile/AtomicReference there.

Anyway, passing mutable structures between the threads is already a dangerous idea.