feat(ui): Do not update the viewport of the room list if it hasn't changed.

Imagine the room list has the viewport set to the range `0..=19`. The
user scrolls quickly to `50..=69` to see something below and immediately
scrolls back to its initial position, without stopping the scroll
at any moment in between. The app using this API might update the
viewport from `0..=19` to… `0..=19`. Updating the viewport, updates the
`visible_rooms` sliding sync list. Since a list is modified, the current
sync-loop iteration is skipped over and a new one restarts, i.e. the
current in-flight request is cancelled… for nothing.

This patch prevents this situation. The current viewport ranges is
stored in `RoomListService`. `RoomListService::apply_input` used to
return `Result<(), Error>`, now it returns `Result<InputResult, Error>`.
The `InputResult` enum is a new type. It represents whether an input has
been applied or ignored, which must be differentiate from errors.

So now, if the viewport changes and it's not different from the previous
value, `InputResult::Ignored` is returned.

crates/matrix-sdk-ui/src/room_list/mod.rs

@@ -87,15 +87,27 @@ use ruma::{
 };
 pub use state::*;
 use thiserror::Error;
-use tokio::sync::RwLock;
+use tokio::sync::{Mutex, RwLock};
 
 /// The [`RoomListService`] type. See the module's documentation to learn more.
 #[derive(Debug)]
 pub struct RoomListService {
+    /// The Sliding Sync instance.
     sliding_sync: Arc<SlidingSync>,
+
+    /// The current state of the `RoomListService`.
+    ///
+    /// `RoomListService` is a simple state-machine.
+    state: Observable<State>,
+
     /// Room cache, to avoid recreating `Room`s everytime users fetch them.
     rooms: Arc<RwLock<BTreeMap<OwnedRoomId, Room>>>,
-    state: Observable<State>,
+
+    /// The current viewport ranges.
+    ///
+    /// This is useful to avoid resetting the ranges to the same value,
+    /// which would cancel the current in-flight sync request.
+    viewport_ranges: Mutex<Ranges>,
 }
 
 impl RoomListService {
@@ -158,7 +170,12 @@ impl RoomListService {
             .map(Arc::new)
             .map_err(Error::SlidingSync)?;
 
-        Ok(Self { sliding_sync, state: Observable::new(State::Init), rooms: Default::default() })
+        Ok(Self {
+            sliding_sync,
+            state: Observable::new(State::Init),
+            rooms: Default::default(),
+            viewport_ranges: Mutex::new(vec![state::VISIBLE_ROOMS_DEFAULT_RANGE]),
+        })
     }
 
     /// Start to sync the room list.
@@ -258,19 +275,23 @@ impl RoomListService {
     }
 
     /// Pass an [`Input`] onto the state machine.
-    pub async fn apply_input(&self, input: Input) -> Result<(), Error> {
+    pub async fn apply_input(&self, input: Input) -> Result<InputResult, Error> {
         use Input::*;
 
         match input {
-            Viewport(ranges) => {
-                self.update_viewport(ranges).await?;
-            }
+            Viewport(ranges) => self.update_viewport(ranges).await,
         }
-
-        Ok(())
     }
 
-    async fn update_viewport(&self, ranges: Ranges) -> Result<(), Error> {
+    async fn update_viewport(&self, ranges: Ranges) -> Result<InputResult, Error> {
+        let mut viewport_ranges = self.viewport_ranges.lock().await;
+
+        // Is it worth updating the viewport?
+        // The viewport has the same ranges. Don't update it.
+        if *viewport_ranges == ranges {
+            return Ok(InputResult::Ignored);
+        }
+
         self.sliding_sync
             .on_list(VISIBLE_ROOMS_LIST_NAME, |list| {
                 list.set_sync_mode(SlidingSyncMode::new_selective().add_ranges(ranges.clone()));
@@ -278,9 +299,11 @@ impl RoomListService {
                 ready(())
             })
             .await
-            .ok_or_else(|| Error::InputHasNotBeenApplied(Input::Viewport(ranges)))?;
+            .ok_or_else(|| Error::InputCannotBeApplied(Input::Viewport(ranges.clone())))?;
 
-        Ok(())
+        *viewport_ranges = ranges;
+
+        Ok(InputResult::Applied)
     }
 
     /// Get a [`Room`] if it exists.
@@ -320,8 +343,8 @@ pub enum Error {
     UnknownList(String),
 
     /// An input was asked to be applied but it wasn't possible to apply it.
-    #[error("The input has been not applied")]
-    InputHasNotBeenApplied(Input),
+    #[error("The input cannot be applied")]
+    InputCannotBeApplied(Input),
 
     /// The requested room doesn't exist.
     #[error("Room `{0}` not found")]
@@ -342,6 +365,18 @@ pub enum Input {
     Viewport(Ranges),
 }
 
+/// An [`Input`] Ok result: whether it's been applied, or ignored.
+#[derive(Debug, Eq, PartialEq)]
+pub enum InputResult {
+    /// The input has been applied.
+    Applied,
+
+    /// The input has been ignored.
+    ///
+    /// Note that this is not an error. The input was valid, but simply ignored.
+    Ignored,
+}
+
 #[cfg(test)]
 mod tests {
     use matrix_sdk::{

crates/matrix-sdk-ui/tests/integration/room_list.rs

@@ -7,8 +7,9 @@ use imbl::vector;
 use matrix_sdk_test::async_test;
 use matrix_sdk_ui::{
     room_list::{
-        Error, Input, RoomListEntry, RoomListLoadingState, State, ALL_ROOMS_LIST_NAME as ALL_ROOMS,
-        INVITES_LIST_NAME as INVITES, VISIBLE_ROOMS_LIST_NAME as VISIBLE_ROOMS,
+        Error, Input, InputResult, RoomListEntry, RoomListLoadingState, State,
+        ALL_ROOMS_LIST_NAME as ALL_ROOMS, INVITES_LIST_NAME as INVITES,
+        VISIBLE_ROOMS_LIST_NAME as VISIBLE_ROOMS,
     },
     timeline::{TimelineItem, VirtualTimelineItem},
     RoomListService,
@@ -658,7 +659,7 @@ async fn test_sync_resumes_from_error() -> Result<(), Error> {
     pin_mut!(sync);
 
     // Update the viewport, just to be sure it's not reset later.
-    room_list.apply_input(Input::Viewport(vec![5..=10])).await?;
+    assert_eq!(room_list.apply_input(Input::Viewport(vec![5..=10])).await?, InputResult::Applied);
 
     // Do a regular sync from the `Error` state.
     sync_then_assert_request_and_fake_response! {
@@ -950,7 +951,7 @@ async fn test_sync_resumes_from_terminated() -> Result<(), Error> {
     pin_mut!(sync);
 
     // Update the viewport, just to be sure it's not reset later.
-    room_list.apply_input(Input::Viewport(vec![5..=10])).await?;
+    assert_eq!(room_list.apply_input(Input::Viewport(vec![5..=10])).await?, InputResult::Applied);
 
     // Do a regular sync from the `Terminated` state.
     sync_then_assert_request_and_fake_response! {
@@ -2141,7 +2142,7 @@ async fn test_input_viewport() -> Result<(), Error> {
     // present.
     assert_matches!(
         room_list.apply_input(Input::Viewport(vec![10..=15])).await,
-        Err(Error::InputHasNotBeenApplied(_))
+        Err(Error::InputCannotBeApplied(_))
     );
 
     sync_then_assert_request_and_fake_response! {
@@ -2185,7 +2186,17 @@ async fn test_input_viewport() -> Result<(), Error> {
         },
     };
 
-    assert!(room_list.apply_input(Input::Viewport(vec![10..=15, 20..=25])).await.is_ok());
+    // Now we can change the viewport..
+    assert_eq!(
+        room_list.apply_input(Input::Viewport(vec![10..=15, 20..=25])).await?,
+        InputResult::Applied
+    );
+
+    // Re-changing the viewport has no effect.
+    assert_eq!(
+        room_list.apply_input(Input::Viewport(vec![10..=15, 20..=25])).await?,
+        InputResult::Ignored
+    );
 
     // The `timeline_limit` is not repeated because it's sticky.
     sync_then_assert_request_and_fake_response! {