Remove dead code to set the call state #4359
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
danxuliu
added
bug
|
/backport to stable-20.0 |
"IN_CONVERSATION" was set when the activity was created and "state" in the intent extras had the value "resume". However, there is no "state" extra set by default in Android intents, it should be explicitly set, but as it is not set anywhere in Talk Android code that would make it dead code and safe to remove. Moreover, the connection to the call should be initialized again in any case rather than resumed when "onCreate" is called, as it is likely that any previous connection would have been ended if the previous activity instance was destroyed. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
mahibi
force-pushed
the
remove-dead-code-to-set-the-call-state
branch
from
68d8698
to
fdb8692
Compare
|
APK file: https://www.kaminsky.me/nc-dev/android-artifacts/4359-talk.apk |
Great, thanks for the confirmation :-) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change was part of another fix, but I have extracted it to its own pull request for better reviewing, as it is related to the Android activity lifecycle and I am a bit worried to be missing something.
IN_CONVERSATIONwas set when the activity was created andstatein the intent extras has the valueresume. As far as I know there is nostateextra set by default in Android intents, it should be explicitly set, but it is not set anywhere in Talk Android code, so that would make it dead code and safe to remove.Moreover, it was added when switching between call view and chat during a call was introduced. At that time the check was done on a BlueLine Labs Controller rather than on an Android Activity, so maybe back then the controller received the
resumestate when switching between controllers in the same activity, but that would be no longer the case after dropping the controllers in favour of standard activities.And even if resuming an activity provided a
resumestate, if I am not mistakenonCreateis called when the activity is initially created (so it would not make sense to receive aresumestate) or after it was destroyed while in the background and recreated again when moving it to the foregroud again, which I guess would have caused the call to be interrupted anyway, and thus it should start the connection again rather than being marked asIN_CONVERSATION.But as mentioned I am a bit worried to be missing something in all of the above, so please verify it :-) Thanks!