MXCallManager: Make call transfer requests sequential #1739
Conversation
manuroe
force-pushed
the
manu/call_transfer_requests_order
branch
from
93c9854 to
c6aa99e
Compare
manuroe
requested review from
a team and
alfogrillo
and removed request for
a team
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## develop #1739 +/- ##
============================================
+ Coverage 17.67% 37.51% +19.83%
============================================
Files 612 612
Lines 96547 96554 +7
Branches 40518 41731 +1213
============================================
+ Hits 17066 36220 +19154
+ Misses 78968 59285 -19683
- Partials 513 1049 +536
... and 196 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
| [callWithTarget transferToRoom:nil | ||
| user:transferee | ||
| createCall:nil | ||
| awaitCall:newCallId | ||
| success:^(NSString * _Nonnull eventId) { | ||
| dispatch_group_leave(dispatchGroupReplaces); | ||
| dispatch_group_leave(dispatchGroup); |
There was a problem hiding this comment.
I think you also need to leave the dispatch group in case of failure
There was a problem hiding this comment.
We definitely do not want to make the next request if the first one failed.
Does it create an issue (like a retain cycle) if we let the dispatch_group open?
The code before was worse. It could generate 2 calls of the failure callback without "releasing" the dispatch group.
There was a problem hiding this comment.
Does it create an issue (like a retain cycle) if we let the dispatch_group open
This is what I meant.
A dispatch group with a work count equal to "one" will stick somewhere forever I guess.
But I can't find easily a confirmation of this (DispatchGroup objects don't work as normal ARC objects).
Btw I don't see it as a huge problem, I'm going to approve it anyway.
| [callWithTarget transferToRoom:transferRoom.roomId | ||
| user:transferee | ||
| createCall:nil | ||
| awaitCall:newCallId | ||
| success:^(NSString * _Nonnull eventId) { | ||
| dispatch_group_leave(dispatchGroupReplaces); | ||
| dispatch_group_leave(dispatchGroup); |
Make sure we made requests one by one to avoid to create weird scenarii.