[38] refactor Outgoing & Incoming flows to represent call-engine state - #67
Conversation
| videoApp.streamVideo, | ||
| StreamRouterImpl(this), | ||
| requireNotNull(intent.getSerializableExtra(KEY_CALL_DATA) as? IncomingCallData) | ||
| // TODO passing [this] may lead to memory leak, cause VM may live longer than Activity |
There was a problem hiding this comment.
Could you elaborate on this a bit? Any suggestions on how to avoid it?
There was a problem hiding this comment.
So IncomingCallViewModel keeps ref to StreamRouterImpl, which keeps ref to IncomingCallActivity, and in case of Activity recreation (like screen rotation etc), IncomingCallViewModel will keep the ref to the dead obj, through StreamRouterImpl , because VM instance will still gonna be the same.
We could resolve this by using ActivityLifecycleCallbacks inside StreamRouterImpl to let it use any running activity
There was a problem hiding this comment.
This sounds good yeah - given that these are built for hte purposes of our apps, maybe it's not as important to expose all of this on the get go
Although, based on what we expose to users (e.g. Outgoing/Incoming screens), it might be good to just ship it too and make it clean!
| logger.d { "[createCall] input: $input" } | ||
| // this._callState.value = videoClient.getCall(callId) TODO - load details | ||
|
|
||
| // TODO CallClient is supposed to live longer than VM |
There was a problem hiding this comment.
Good point - maybe we can create the client before the ViewModel and hten pass it in as a parameter
filbabic
left a comment
There was a problem hiding this comment.
Approved and can be merged, just left a few small comments
Relates to: #38
I've created this PR to avoid a chonker one in the end.
A final merge of 3 screens will be completed in the next PR.
Main changes:
CallEngineto handle the following cases:meetingif someone leaves it (but regular calls are dropped normally)regular callif not all participants reject incoming call