Improve and debug ghost swipe situation

Change-Id: Ic6f2956a139516050e2b7e5ec248969326b7ec43

vector/src/main/java/im/vector/app/AppStateHandler.kt

@@ -21,6 +21,7 @@ import androidx.lifecycle.LifecycleOwner
 import androidx.lifecycle.asFlow
 import arrow.core.Option
 import arrow.core.getOrElse
+import de.spiritcroc.matrixsdk.util.DbgUtil
 import im.vector.app.core.di.ActiveSessionHolder
 import im.vector.app.core.utils.BehaviorDataSource
 import im.vector.app.features.analytics.AnalyticsTracker
@@ -45,6 +46,7 @@ import org.matrix.android.sdk.api.session.getRoomSummary
 import org.matrix.android.sdk.api.session.group.model.GroupSummary
 import org.matrix.android.sdk.api.session.initsync.SyncStatusService
 import org.matrix.android.sdk.api.session.room.model.RoomSummary
+import timber.log.Timber
 import javax.inject.Inject
 import javax.inject.Singleton
 
@@ -53,6 +55,17 @@ sealed class RoomGroupingMethod {
     data class BySpace(val spaceSummary: RoomSummary?) : RoomGroupingMethod()
 }
 
+enum class SelectSpaceFrom {
+    // Initialized / uiStateRepository
+    INIT,
+    // Swiped in home pager
+    SWIPE,
+    // Persisted after swipe in home pager
+    PERSIST_SWIPE,
+    // Selected from non-pager UI
+    SELECT,
+}
+
 fun RoomGroupingMethod.space() = (this as? RoomGroupingMethod.BySpace)?.spaceSummary
 fun RoomGroupingMethod.group() = (this as? RoomGroupingMethod.ByLegacyGroup)?.groupSummary
 
@@ -74,10 +87,11 @@ class AppStateHandler @Inject constructor(
 
     // SchildiChat: the boolean means pendingSwipe and defaults to false. Set to true for swiping spaces, so you want to ignore updates which have pendingSwipe = true.
     // Call it different then the upstream one so we don't forget adding `first` to upstream's logic.
-    private val selectedSpaceDataSourceSc = BehaviorDataSource<Option<Pair<RoomGroupingMethod, Boolean>>>(Option.empty())
+    private val selectedSpaceDataSourceSc = BehaviorDataSource<Option<Pair<RoomGroupingMethod, SelectSpaceFrom>>>(Option.empty())
 
     val selectedRoomGroupingFlow = selectedSpaceDataSourceSc.stream().map { it.map { it.first } }
-    val selectedRoomGroupingFlowIgnoreSwipe = selectedSpaceDataSourceSc.stream().filter { it.getOrElse{ null }?.second != true }.map { it.map { it.first } }
+    val selectedRoomGroupingFlowIgnoreSwipe = selectedSpaceDataSourceSc.stream()
+            .filter { it.getOrElse{ null }?.second != SelectSpaceFrom.SWIPE }
 
     fun getCurrentRoomGroupingMethod(): RoomGroupingMethod? {
         // XXX we should somehow make it live :/ just a work around
@@ -94,7 +108,12 @@ class AppStateHandler @Inject constructor(
         }
     }
 
-    fun setCurrentSpace(spaceId: String?, session: Session? = null, persistNow: Boolean = false, pendingSwipe: Boolean = false) {
+    fun setCurrentSpace(spaceId: String?, session: Session? = null, persistNow: Boolean = false, from: SelectSpaceFrom = SelectSpaceFrom.SELECT) {
+        if (DbgUtil.isDbgEnabled(DbgUtil.DBG_VIEW_PAGER) && from == SelectSpaceFrom.SELECT) {
+            // We want a stack trace
+            Timber.w(Exception("Home pager: setCurrentSpace/SELECT"))
+        }
+
         val uSession = session ?: activeSessionHolder.getSafeActiveSession() ?: return
         if (selectedSpaceDataSourceSc.currentValue?.orNull()?.first is RoomGroupingMethod.BySpace &&
                 spaceId == selectedSpaceDataSourceSc.currentValue?.orNull()?.first?.space()?.roomId) return
@@ -105,7 +124,7 @@ class AppStateHandler @Inject constructor(
             uiStateRepository.storeSelectedSpace(spaceSum?.roomId, uSession.sessionId)
         }
 
-        selectedSpaceDataSourceSc.post(Option.just(Pair(RoomGroupingMethod.BySpace(spaceSum), pendingSwipe)))
+        selectedSpaceDataSourceSc.post(Option.just(Pair(RoomGroupingMethod.BySpace(spaceSum), from)))
         if (spaceId != null) {
             uSession.coroutineScope.launch(Dispatchers.IO) {
                 tryOrNull {
@@ -120,7 +139,7 @@ class AppStateHandler @Inject constructor(
         if (selectedSpaceDataSourceSc.currentValue?.orNull()?.first is RoomGroupingMethod.ByLegacyGroup &&
                 groupId == selectedSpaceDataSourceSc.currentValue?.orNull()?.first?.group()?.groupId) return
         val activeGroup = groupId?.let { uSession.groupService().getGroupSummary(groupId) }
-        selectedSpaceDataSourceSc.post(Option.just(Pair(RoomGroupingMethod.ByLegacyGroup(activeGroup), false)))
+        selectedSpaceDataSourceSc.post(Option.just(Pair(RoomGroupingMethod.ByLegacyGroup(activeGroup), SelectSpaceFrom.SELECT)))
         if (groupId != null) {
             uSession.coroutineScope.launch {
                 tryOrNull {
@@ -137,7 +156,7 @@ class AppStateHandler @Inject constructor(
                     // sessionDataSource could already return a session while activeSession holder still returns null
                     it.orNull()?.let { session ->
                         if (uiStateRepository.isGroupingMethodSpace(session.sessionId)) {
-                            setCurrentSpace(uiStateRepository.getSelectedSpace(session.sessionId), session)
+                            setCurrentSpace(uiStateRepository.getSelectedSpace(session.sessionId), session, from = SelectSpaceFrom.INIT)
                         } else {
                             setCurrentGroup(uiStateRepository.getSelectedGroup(session.sessionId), session)
                         }
@@ -193,8 +212,8 @@ class AppStateHandler @Inject constructor(
         val uSession = activeSessionHolder.getSafeActiveSession() ?: return
 
         // We want to persist it, so we also want to remove the pendingSwipe status
-        if (currentValue.second) {
-            selectedSpaceDataSourceSc.post(Option.just(Pair(currentMethod, false)))
+        if (currentValue.second == SelectSpaceFrom.SWIPE) {
+            selectedSpaceDataSourceSc.post(Option.just(Pair(currentMethod, SelectSpaceFrom.PERSIST_SWIPE)))
         }
 
         // Persist it across app restarts

vector/src/main/java/im/vector/app/features/home/HomeDetailFragment.kt

@@ -34,10 +34,12 @@ import com.airbnb.mvrx.fragmentViewModel
 import com.airbnb.mvrx.withState
 import com.google.android.material.badge.BadgeDrawable
 import de.spiritcroc.matrixsdk.util.DbgUtil
+import de.spiritcroc.matrixsdk.util.Dimber
 import de.spiritcroc.viewpager.reduceDragSensitivity
 import im.vector.app.AppStateHandler
 import im.vector.app.R
 import im.vector.app.RoomGroupingMethod
+import im.vector.app.SelectSpaceFrom
 import im.vector.app.core.extensions.restart
 import im.vector.app.core.extensions.toMvRxBundle
 import im.vector.app.core.platform.StateView
@@ -83,6 +85,7 @@ class HomeDetailFragment @Inject constructor(
         CurrentCallsView.Callback {
 
     private val DEBUG_VIEW_PAGER = DbgUtil.isDbgEnabled(DbgUtil.DBG_VIEW_PAGER)
+    private val viewPagerDimber = Dimber("Home pager", DbgUtil.DBG_VIEW_PAGER)
 
     private val viewModel: HomeDetailViewModel by fragmentViewModel()
     private val unknownDeviceDetectorSharedViewModel: UnknownDeviceDetectorSharedViewModel by activityViewModel()
@@ -153,7 +156,7 @@ class HomeDetailFragment @Inject constructor(
         // space pager: update appStateHandler's current page to update rest of the UI accordingly
         views.roomListContainerPager.registerOnPageChangeCallback(object : ViewPager2.OnPageChangeCallback() {
             override fun onPageSelected(position: Int) {
-                if (DEBUG_VIEW_PAGER) Timber.i("Home pager: selected page $position $initialPageSelected")
+                viewPagerDimber.i{"Home pager: selected page $position $initialPageSelected"}
                 super.onPageSelected(position)
                 if (!initialPageSelected) {
                     // Do not apply before we have restored the previous value
@@ -164,13 +167,7 @@ class HomeDetailFragment @Inject constructor(
                         initialPageSelected = true
                     }
                 }
-                val selectedId = getSpaceIdForPageIndex(position)
-                appStateHandler.setCurrentSpace(selectedId, pendingSwipe = true)
-                if (pagerPagingEnabled) {
-                    onSpaceChange(
-                            selectedId?.let { viewModel.getRoom(it)?.roomSummary() }
-                    )
-                }
+                selectSpaceFromSwipe(position)
             }
         })
 
@@ -255,6 +252,16 @@ class HomeDetailFragment @Inject constructor(
                 }
     }
 
+    private fun selectSpaceFromSwipe(position: Int) {
+        val selectedId = getSpaceIdForPageIndex(position)
+        appStateHandler.setCurrentSpace(selectedId, from = SelectSpaceFrom.SWIPE)
+        if (pagerPagingEnabled) {
+            onSpaceChange(
+                    selectedId?.let { viewModel.getRoom(it)?.roomSummary() }
+            )
+        }
+    }
+
     private fun handleCallStarted() {
         dismissLoadingDialog()
         val fragmentTag = HomeTab.DialPad.toFragmentTag()
@@ -542,17 +549,18 @@ class HomeDetailFragment @Inject constructor(
     }
      */
 
-    private fun setupViewPager(roomGroupingMethod: RoomGroupingMethod, spaces: List<RoomSummary>?, tab: HomeTab) {
+    private fun setupViewPager(roomGroupingMethodPair: Pair<RoomGroupingMethod, SelectSpaceFrom>, spaces: List<RoomSummary>?, tab: HomeTab) {
+        val roomGroupingMethod = roomGroupingMethodPair.first
         val oldAdapter = views.roomListContainerPager.adapter as? FragmentStateAdapter
         val pagingAllowed = vectorPreferences.enableSpacePager() && tab is HomeTab.RoomList
         if (pagingAllowed && spaces == null) {
-            Timber.i("Home pager: Skip initial setup, root spaces not known yet")
+            viewPagerDimber.i{"Home pager: Skip initial setup, root spaces not known yet"}
             views.roomListContainerStateView.state = StateView.State.Loading
             return
         } else {
             views.roomListContainerStateView.state = StateView.State.Content
         }
-        if (DEBUG_VIEW_PAGER) Timber.i("Home pager: setup, old adapter: $oldAdapter")
+        viewPagerDimber.i{"Home pager: setup, old adapter: $oldAdapter"}
         val unsafeSpaces = spaces?.map { it.roomId } ?: listOf()
         val selectedSpaceId = (roomGroupingMethod as? RoomGroupingMethod.BySpace)?.spaceSummary?.roomId
         val selectedIndex = getPageIndexForSpaceId(selectedSpaceId, unsafeSpaces)
@@ -561,10 +569,20 @@ class HomeDetailFragment @Inject constructor(
         // Check if we need to recreate the adapter for a new tab
         if (oldAdapter != null) {
             val changed = pagerTab != tab || pagerSpaces != safeSpaces || pagerPagingEnabled != pagingEnabled
-            if (DEBUG_VIEW_PAGER) Timber.i("Home pager: has changed: $changed (${pagerTab != tab} ${pagerSpaces != safeSpaces} ${pagerPagingEnabled != pagingEnabled} $selectedIndex ${views.roomListContainerPager.currentItem}) | $safeSpaces")
+            viewPagerDimber.i{ "has changed: $changed (${pagerTab != tab} ${pagerSpaces != safeSpaces} ${pagerPagingEnabled != pagingEnabled} $selectedIndex ${roomGroupingMethodPair.second} ${views.roomListContainerPager.currentItem}) | $safeSpaces" }
             if (!changed) {
+                // No need to re-setup pager, just check for selected page
                 if (pagingEnabled) {
-                    // No need to re-setup pager, just check for selected page
+                    // Prioritize the actually displayed space for all space changes except for SELECT ones, to avoid
+                    // unexpected page changes onResume for old updates
+                    if (roomGroupingMethodPair.second != SelectSpaceFrom.SELECT) {
+                        // Tell the rest of the UI that we want to keep displaying the current space
+                        viewPagerDimber.i { "Discard space change from ${roomGroupingMethodPair.second} (${selectedSpaceId}/$selectedIndex), persist own (${views.roomListContainerPager.currentItem})" }
+                        if (views.roomListContainerPager.currentItem != selectedIndex) {
+                            selectSpaceFromSwipe(views.roomListContainerPager.currentItem)
+                        }
+                        return
+                    }
                     if (selectedIndex != null) {
                         if (selectedIndex != views.roomListContainerPager.currentItem) {
                             // post() mitigates a case where we could end up in an endless loop circling around the same few spaces
@@ -613,16 +631,16 @@ class HomeDetailFragment @Inject constructor(
             }
 
             override fun createFragment(position: Int): Fragment {
-                if (DEBUG_VIEW_PAGER) Timber.i("Home pager: create fragment for position $position")
+                viewPagerDimber.i{"Home pager: create fragment for position $position"}
                 return when (tab) {
                     is HomeTab.DialPad -> createDialPadFragment()
                     is HomeTab.RoomList -> {
                         val params = if (pagingEnabled) {
                             val spaceId = getSpaceIdForPageIndex(position)
-                            if (DEBUG_VIEW_PAGER) Timber.i("Home pager: position $position -> space $spaceId")
+                            viewPagerDimber.i{"Home pager: position $position -> space $spaceId"}
                             RoomListParams(tab.displayMode, spaceId).toMvRxBundle()
                         } else {
-                            if (DEBUG_VIEW_PAGER) Timber.i("Home pager: paging disabled; position $position -> follow")
+                            viewPagerDimber.i{"Home pager: paging disabled; position $position -> follow"}
                             RoomListParams(tab.displayMode, SPACE_ID_FOLLOW_APP).toMvRxBundle()
                         }
                         childFragmentManager.fragmentFactory.instantiate(activity!!.classLoader, RoomListFragment::class.java.name).apply {
@@ -645,7 +663,7 @@ class HomeDetailFragment @Inject constructor(
                             return
                         }
                         try {
-                            if (DEBUG_VIEW_PAGER) Timber.i("Home pager: set initial page $selectedIndex")
+                            viewPagerDimber.i{"Home pager: set initial page $selectedIndex"}
                             views.roomListContainerPager.setCurrentItem(selectedIndex ?: 0, false)
                             initialPageSelected = true
                         } catch (e: Exception) {

vector/src/main/java/im/vector/app/features/home/HomeDetailViewModel.kt

@@ -225,7 +225,7 @@ class HomeDetailViewModel @AssistedInject constructor(
         appStateHandler.selectedRoomGroupingFlowIgnoreSwipe
                 .setOnEach {
                     copy(
-                            roomGroupingMethodIgnoreSwipe = it.orNull() ?: RoomGroupingMethod.BySpace(null)
+                            roomGroupingMethodIgnoreSwipe = it.orNull()
                     )
                 }
     }

vector/src/main/java/im/vector/app/features/home/HomeDetailViewState.kt

@@ -22,6 +22,7 @@ import com.airbnb.mvrx.MavericksState
 import com.airbnb.mvrx.Uninitialized
 import im.vector.app.R
 import im.vector.app.RoomGroupingMethod
+import im.vector.app.SelectSpaceFrom
 import org.matrix.android.sdk.api.session.initsync.SyncStatusService
 import org.matrix.android.sdk.api.session.room.model.RoomSummary
 import org.matrix.android.sdk.api.session.sync.SyncState
@@ -29,7 +30,7 @@ import org.matrix.android.sdk.api.util.MatrixItem
 
 data class HomeDetailViewState(
         val roomGroupingMethod: RoomGroupingMethod = RoomGroupingMethod.BySpace(null),
-        val roomGroupingMethodIgnoreSwipe: RoomGroupingMethod? = null,
+        val roomGroupingMethodIgnoreSwipe: Pair<RoomGroupingMethod, SelectSpaceFrom>? = null,
         val myMatrixItem: MatrixItem? = null,
         val asyncRooms: Async<List<RoomSummary>> = Uninitialized,
         val currentTab: HomeTab = HomeTab.RoomList(RoomListDisplayMode.ALL),