-
Notifications
You must be signed in to change notification settings - Fork 658
Hooks up the SessionLifecycleService to the FirelogPublisher, and has… #5427
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Generated by 🚫 Danger |
Size Report 1Affected Products
Test Logs |
| * Determines if the foregrounding that occurred at the given time should trigger a new session | ||
| * because the app has been idle for too long. | ||
| */ | ||
| private fun isSessionRestart(foregroundTimeMs: Long) = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The old SessionInitiator used Durations not Longs for this check. The WallClock has a better clock for measuring durations that won't be affected by funny things like the user changing the time or anything with daylight savings etc. See my comment #5420 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is getting the time out of the messages themselves where we don't actually know which clock was used to generate them other than that the framework did. Switching to use our own clock would mean that we'd be keeping track of timestamps based on when the service processes each message as opposed to when the event occurred. It's true that manipulating the system clock might cause a session to be dropped or created when it shouldn't have, but IMO that's a much less important or likely scenario than delays in message delivery.
… the FirelogPublisher handle the full SessionEvent creation since that's only relevant to Firelog.
9714062 to
a0b5558
Compare
… the FirelogPublisher handle the full SessionEvent creation since that's only relevant to Firelog.