Skip to content
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

Fixes #3257 Fixes #3256 Make sure session listeners are set when first LoadRequest is called #3265

Merged
merged 1 commit into from
Apr 28, 2020

Conversation

keianhzo
Copy link
Collaborator

@keianhzo keianhzo commented Apr 28, 2020

Fixes #3257 Fixes #3256 We are setting the session listeners after activating the sessions. That means that in some cases (like restoring when activating the session) the listeners are not ready. This cause issues like permissions (or anything that we do in the session listeners) not being correctly checked for restored sessions.

@keianhzo keianhzo self-assigned this Apr 28, 2020
@keianhzo keianhzo changed the title Fixes #3257 Make sure session listeners are set when LoadRequest is called Fixes #3257 Make sure session listeners are set when first LoadRequest is called Apr 28, 2020
@keianhzo keianhzo changed the title Fixes #3257 Make sure session listeners are set when first LoadRequest is called Fixes #3257 Fixes #3256 Make sure session listeners are set when first LoadRequest is called Apr 28, 2020
Copy link
Contributor

@MortimerGoro MortimerGoro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since setupListeners is always followed by a setSession I'd like a safer API to handle this instead of separate calls. What about adding an additional (probably optional) enum or bitflag to the SetSession call which indicates if we want to activate, deactivate or don't change active state of the session?

We can do it as a follow-up

@bluemarvin bluemarvin merged commit fe05ee7 into master Apr 28, 2020
@bluemarvin bluemarvin deleted the v10/load_request_restore branch April 28, 2020 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants