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

Fix crashes #8410

Merged
merged 4 commits into from
May 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/8410.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix crash when opening "Protect access" screen, and various other issue with `repeatOnLifecycle`
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,9 @@ import android.text.Editable
import android.view.View
import android.view.inputmethod.EditorInfo
import androidx.autofill.HintConstants
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.LifecycleOwner
import androidx.lifecycle.lifecycleScope
import androidx.lifecycle.repeatOnLifecycle
import androidx.lifecycle.withResumed
import com.google.android.material.textfield.TextInputLayout
import im.vector.app.core.platform.SimpleTextWatcher
import kotlinx.coroutines.flow.launchIn
Expand Down Expand Up @@ -88,7 +87,7 @@ fun TextInputLayout.setOnImeDoneListener(action: () -> Unit) {
fun TextInputLayout.setOnFocusLostListener(lifecycleOwner: LifecycleOwner, action: () -> Unit) {
editText().setOnFocusChangeListener { _, hasFocus ->
when (hasFocus) {
false -> lifecycleOwner.lifecycleScope.launch { lifecycleOwner.repeatOnLifecycle(Lifecycle.State.RESUMED) { action() } }
false -> lifecycleOwner.lifecycleScope.launch { lifecycleOwner.withResumed { action() } }
else -> {
// do nothing
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,8 @@ import androidx.core.view.isVisible
import androidx.drawerlayout.widget.DrawerLayout
import androidx.fragment.app.Fragment
import androidx.fragment.app.FragmentManager
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.lifecycleScope
import androidx.lifecycle.repeatOnLifecycle
import androidx.lifecycle.withResumed
import com.airbnb.mvrx.Mavericks
import com.airbnb.mvrx.viewModel
import com.google.android.material.dialog.MaterialAlertDialogBuilder
Expand Down Expand Up @@ -402,7 +401,9 @@ class HomeActivity :
private fun handleStartRecoverySetup() {
// To avoid IllegalStateException in case the transaction was executed after onSaveInstanceState
lifecycleScope.launch {
repeatOnLifecycle(Lifecycle.State.RESUMED) { navigator.open4SSetup(this@HomeActivity, SetupMode.NORMAL) }
withResumed {
navigator.open4SSetup(this@HomeActivity, SetupMode.NORMAL)
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,8 @@ import androidx.core.view.isInvisible
import androidx.core.view.isVisible
import androidx.core.view.updatePadding
import androidx.fragment.app.setFragmentResultListener
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.lifecycleScope
import androidx.lifecycle.repeatOnLifecycle
import androidx.lifecycle.withResumed
import androidx.recyclerview.widget.ItemTouchHelper
import androidx.recyclerview.widget.LinearLayoutManager
import androidx.recyclerview.widget.RecyclerView
Expand Down Expand Up @@ -1112,29 +1111,31 @@ class TimelineFragment :
private fun updateJumpToReadMarkerViewVisibility() {
if (isThreadTimeLine()) return
viewLifecycleOwner.lifecycleScope.launch {
viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.RESUMED) {
val state = timelineViewModel.awaitState()
val showJumpToUnreadBanner = when (state.unreadState) {
UnreadState.Unknown,
UnreadState.HasNoUnread -> false
is UnreadState.ReadMarkerNotLoaded -> true
is UnreadState.HasUnread -> {
if (state.canShowJumpToReadMarker) {
val lastVisibleItem = layoutManager.findLastCompletelyVisibleItemPosition()
val positionOfReadMarker = withContext(Dispatchers.Default) {
timelineEventController.getPositionOfReadMarker()
}
if (positionOfReadMarker == null) {
false
withResumed {
viewLifecycleOwner.lifecycleScope.launch {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it expected to have this line twice?

Copy link
Contributor

Choose a reason for hiding this comment

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

it seems since this is done at several places

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, we are not in a coroutine scope in the withResumed block.

val state = timelineViewModel.awaitState()
val showJumpToUnreadBanner = when (state.unreadState) {
UnreadState.Unknown,
UnreadState.HasNoUnread -> false
is UnreadState.ReadMarkerNotLoaded -> true
is UnreadState.HasUnread -> {
if (state.canShowJumpToReadMarker) {
val lastVisibleItem = layoutManager.findLastCompletelyVisibleItemPosition()
val positionOfReadMarker = withContext(Dispatchers.Default) {
timelineEventController.getPositionOfReadMarker()
}
if (positionOfReadMarker == null) {
false
} else {
positionOfReadMarker > lastVisibleItem
}
} else {
positionOfReadMarker > lastVisibleItem
false
}
} else {
false
}
}
views.jumpToReadMarkerView.isVisible = showJumpToUnreadBanner
}
views.jumpToReadMarkerView.isVisible = showJumpToUnreadBanner
}
}
}
Expand Down Expand Up @@ -1625,14 +1626,16 @@ class TimelineFragment :

override fun onRoomCreateLinkClicked(url: String) {
viewLifecycleOwner.lifecycleScope.launch {
viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.RESUMED) {
permalinkHandler
.launch(requireActivity(), url, object : NavigationInterceptor {
override fun navToRoom(roomId: String?, eventId: String?, deepLink: Uri?, rootThreadEventId: String?): Boolean {
requireActivity().finish()
return false
}
})
withResumed {
viewLifecycleOwner.lifecycleScope.launch {
permalinkHandler
.launch(requireActivity(), url, object : NavigationInterceptor {
override fun navToRoom(roomId: String?, eventId: String?, deepLink: Uri?, rootThreadEventId: String?): Boolean {
requireActivity().finish()
return false
}
})
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ import androidx.core.content.ContextCompat
import androidx.core.view.isGone
import androidx.core.view.isVisible
import androidx.fragment.app.setFragmentResultListener
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.lifecycleScope
import androidx.lifecycle.repeatOnLifecycle
import com.airbnb.mvrx.fragmentViewModel
import com.airbnb.mvrx.withState
import com.google.android.material.dialog.MaterialAlertDialogBuilder
Expand Down Expand Up @@ -101,12 +99,10 @@ class LocationSharingFragment :
views.mapView.onCreate(savedInstanceState)

viewLifecycleOwner.lifecycleScope.launch {
viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.CREATED) {
views.mapView.initialize(
url = urlMapProvider.getMapUrl(),
locationTargetChangeListener = this@LocationSharingFragment
)
}
views.mapView.initialize(
url = urlMapProvider.getMapUrl(),
locationTargetChangeListener = this@LocationSharingFragment
)
}

initLocateButton()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ import android.view.MenuItem
import android.view.View
import android.view.ViewGroup
import androidx.core.view.isVisible
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.lifecycleScope
import androidx.lifecycle.repeatOnLifecycle
import com.airbnb.mvrx.args
import com.airbnb.mvrx.fragmentViewModel
import com.airbnb.mvrx.withState
Expand Down Expand Up @@ -81,9 +79,7 @@ class LocationPreviewFragment :
views.mapView.onCreate(savedInstanceState)

viewLifecycleOwner.lifecycleScope.launch {
viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.CREATED) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if this code was here for a certain reason, but I agree that it does not seem necessary since we're in the onViewCreated method

views.mapView.initialize(urlMapProvider.getMapUrl())
}
views.mapView.initialize(urlMapProvider.getMapUrl())
}

observeViewEvents()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@

package im.vector.app.features.settings

import androidx.lifecycle.Lifecycle
import androidx.lifecycle.lifecycleScope
import androidx.lifecycle.repeatOnLifecycle
import androidx.preference.Preference
import androidx.preference.SwitchPreference
import dagger.hilt.android.AndroidEntryPoint
Expand Down Expand Up @@ -80,13 +78,13 @@ class VectorSettingsPinFragment :

override fun onResume() {
super.onResume()

updateBiometricPrefState(isPinCodeChecked = usePinCodePref.isChecked)
viewLifecycleOwner.lifecycleScope.launch {
refreshPinCodeStatus()
}
}

override fun bindPref() {
refreshPinCodeStatus()

usePinCodePref.setOnPreferenceChangeListener { _, value ->
val isChecked = (value as? Boolean).orFalse()
updateBiometricPrefState(isPinCodeChecked = isChecked)
Expand Down Expand Up @@ -130,38 +128,34 @@ class VectorSettingsPinFragment :
.onFailure { Timber.e(it) }
}

private fun refreshPinCodeStatus() {
viewLifecycleOwner.lifecycleScope.launch {
viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.RESUMED) {
val hasPinCode = pinCodeStore.hasEncodedPin()
usePinCodePref.isChecked = hasPinCode
usePinCodePref.onPreferenceClickListener = Preference.OnPreferenceClickListener {
if (hasPinCode) {
lifecycleScope.launch {
pinCodeStore.deletePinCode()
refreshPinCodeStatus()
}
} else {
navigator.openPinCode(
requireContext(),
pinActivityResultLauncher,
PinMode.CREATE
)
}
true
private suspend fun refreshPinCodeStatus() {
val hasPinCode = pinCodeStore.hasEncodedPin()
usePinCodePref.isChecked = hasPinCode
usePinCodePref.onPreferenceClickListener = Preference.OnPreferenceClickListener {
if (hasPinCode) {
lifecycleScope.launch {
pinCodeStore.deletePinCode()
refreshPinCodeStatus()
}
} else {
navigator.openPinCode(
requireContext(),
pinActivityResultLauncher,
PinMode.CREATE
)
}
true
}

changePinCodePref.onPreferenceClickListener = Preference.OnPreferenceClickListener {
if (hasPinCode) {
navigator.openPinCode(
requireContext(),
pinActivityResultLauncher,
PinMode.MODIFY
)
}
true
}
changePinCodePref.onPreferenceClickListener = Preference.OnPreferenceClickListener {
if (hasPinCode) {
navigator.openPinCode(
requireContext(),
pinActivityResultLauncher,
PinMode.MODIFY
)
}
true
}
}

Expand All @@ -170,6 +164,6 @@ class VectorSettingsPinFragment :
}

private val pinActivityResultLauncher = registerStartForActivityResult {
refreshPinCodeStatus()
// Nothing to do, refreshPinCodeStatus() will be called by `onResume`
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,8 @@ import android.view.View
import android.view.ViewGroup
import androidx.core.content.ContextCompat
import androidx.core.view.isVisible
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.lifecycleScope
import androidx.lifecycle.repeatOnLifecycle
import androidx.lifecycle.withResumed
import androidx.preference.Preference
import androidx.preference.PreferenceCategory
import androidx.preference.SwitchPreference
Expand Down Expand Up @@ -190,6 +189,10 @@ class VectorSettingsSecurityPrivacyFragment :
rawService
.getElementWellknown(session.sessionParams)
?.isE2EByDefault() == false

refreshXSigningStatus()
// My device name may have been updated
refreshMyDevice()
}
}

Expand Down Expand Up @@ -288,19 +291,6 @@ class VectorSettingsSecurityPrivacyFragment :
true
}

lifecycleScope.launch {
repeatOnLifecycle(Lifecycle.State.RESUMED) {
refreshXSigningStatus()
}
}

lifecycleScope.launch {
repeatOnLifecycle(Lifecycle.State.RESUMED) {
// My device name may have been updated
refreshMyDevice()
}
}

secureBackupPreference.icon = activity?.let {
ThemeUtils.tintDrawable(
it,
Expand Down Expand Up @@ -429,16 +419,18 @@ class VectorSettingsSecurityPrivacyFragment :

private fun openPinCodePreferenceScreen() {
viewLifecycleOwner.lifecycleScope.launch {
viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.RESUMED) {
val hasPinCode = pinCodeStore.hasEncodedPin()
if (hasPinCode) {
navigator.openPinCode(
requireContext(),
pinActivityResultLauncher,
PinMode.AUTH
)
} else {
doOpenPinCodePreferenceScreen()
withResumed {
viewLifecycleOwner.lifecycleScope.launch {
val hasPinCode = pinCodeStore.hasEncodedPin()
if (hasPinCode) {
navigator.openPinCode(
requireContext(),
pinActivityResultLauncher,
PinMode.AUTH
)
} else {
doOpenPinCodePreferenceScreen()
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,8 @@ import android.view.View
import android.view.ViewGroup
import android.widget.ScrollView
import androidx.core.view.isVisible
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.lifecycleScope
import androidx.lifecycle.repeatOnLifecycle
import androidx.lifecycle.withResumed
import com.airbnb.mvrx.activityViewModel
import com.airbnb.mvrx.args
import com.airbnb.mvrx.withState
Expand Down Expand Up @@ -178,7 +177,7 @@ class UserListFragment :
// Scroll to the bottom when adding chips. When removing chips, do not scroll
if (newNumberOfChips >= currentNumberOfChips) {
viewLifecycleOwner.lifecycleScope.launch {
viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.RESUMED) {
withResumed {
views.chipGroupScrollView.fullScroll(ScrollView.FOCUS_DOWN)
}
}
Expand Down