This repository has been archived by the owner on May 1, 2024. It is now read-only.
Force off-main-thread ItemsSource updates to update on main thread #11235
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
2 tasks
samhouts
added
Core
p/Android
a/collectionview
Android10
i/high
rmarinho
suggested changes
Jul 1, 2020
StephaneDelcroix
approved these changes
Jul 2, 2020
| // collection which are made off of the main thread remain invisible to consumers on the main thread | ||
| // until they have been processed by the main thread. | ||
|
|
||
| public class MarshalingObservableCollection : List<object>, INotifyCollectionChanged |
There was a problem hiding this comment.
Used by Platform.Android, so it needs to be public.
jsuarezruiz
reviewed
Jul 9, 2020
| case IList _ when itemsSource is INotifyCollectionChanged: | ||
| return new ObservableItemsSource(itemsSource as IList, notifier); | ||
| case IList list when itemsSource is INotifyCollectionChanged: | ||
| return new ObservableItemsSource(new MarshalingObservableCollection(list), notifier); |
There was a problem hiding this comment.
What about iOS? Would it be necessary to use MarshalingObservableCollection in iOS too?
jsuarezruiz
approved these changes
Jul 9, 2020
rmarinho
approved these changes
Jul 20, 2020
samhouts
added
the
approved
samhouts
added
the
API-change
|
@hartez, Still having this issue. My case.
|
Are you saying you have something like |
No, no. Let review the design. My collegue told me that the test regard this issue covers just the Issue also happens when code is refactored to |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Description of Change
If a developer
for the most part, these changes will be automatically marshaled by ObservableItemsSource to the main thread, and all will be well.
However, if the changes are timed correctly (or occur quickly enough) and there is any scrolling involved (e.g., using KeepLastItemInView, or opening/closing the keyboard, or an ill-timed user scrolling action), it's possible that the RecyclerView will attempt to access the underlying collection in a way which will make the viewholder collection inconsistent. At this point, the user may see errors like:
java.lang.IndexOutOfBoundsException: Inconsistency detected. Invalid item positionor
java.Lang.IndexOutOfBoundsException: Inconsistency detected. Invalid view holder adapter positionThe issue is that the RecyclerView (via the adapter) is accessing the underlying source between the time the changes have been made (from a non-UI thread) and the time the adapter has been notified of the changes (which must happen on the UI thread).
To work around this problem, these changes provide a wrapper class which queues up the changes requested on the non-ui-thread and later processes them on the UI thread; in the intervening time, any requests for data are answered with a snapshot of the data before it was modified. The wrapper class is injected by the ItemsSourceFactory on Android.
Additionally, these changes address an issue where the ItemsViewRenderer was using the adapter's item count for scrolling rather than the item count in the layout manager. This would occasionally cause issues with the KeepLastItemInView option.
Issues Resolved
API Changes
Added to Core:
public class MarshalingObservableCollection
Platforms Affected
Behavioral/Visual Changes
None
Before/After Screenshots
Not applicable
Testing Procedure
Issue 10735 manual test
UI tests
Automated platform tests
PR Checklist