From e657a9210e9394d8c68f22e8a2eacf8e499567c3 Mon Sep 17 00:00:00 2001 From: Dmitry Gachkovsky Date: Tue, 10 Aug 2021 13:51:02 +0200 Subject: [PATCH] [CIS-1087] Fix data races in message list's message count (#1347) --- CHANGELOG.md | 1 + .../ChatMessageList/ChatMessageListVC.swift | 16 ++++++++++-- .../ChatMessageList/ChatMessageListView.swift | 3 +++ .../ChatMessageList/ChatThreadVC.swift | 25 ++++++++++++++----- 4 files changed, 37 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index adcb6c8bf7c..a5109c33e0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### 🔄 Changed +- Fixed race condition on `ChatMessageListVC` and `ChatThreadVC` that caused `UITableView` crashes [#1347](https://github.com/GetStream/stream-chat-swift/pull/1347) - Added `GalleryAttachmentViewInjector.galleryViewAspectRatio` to control the aspect ratio of a gallery inside a message cell [#1300](https://github.com/GetStream/stream-chat-swift/pull/1300) # [4.0.0-beta.9](https://github.com/GetStream/stream-chat-swift/releases/tag/4.0.0-beta.9) diff --git a/Sources/StreamChatUI/ChatMessageList/ChatMessageListVC.swift b/Sources/StreamChatUI/ChatMessageList/ChatMessageListVC.swift index a8464a91553..cc745b8ff01 100644 --- a/Sources/StreamChatUI/ChatMessageList/ChatMessageListVC.swift +++ b/Sources/StreamChatUI/ChatMessageList/ChatMessageListVC.swift @@ -24,6 +24,9 @@ open class ChatMessageListVC: ChatMessageListScrollOverlayDataSource { /// Controller for observing data changes within the channel open var channelController: ChatChannelController! + + /// We use private property for channels count so we can update it inside `performBatchUpdates` as [documented](https://developer.apple.com/documentation/uikit/uicollectionview/1618045-performbatchupdates#discussion) + private var numberOfMessages = 0 public var client: ChatClient { channelController.client @@ -106,6 +109,7 @@ open class ChatMessageListVC: override open func setUp() { super.setUp() + updateNumberOfMessages() let longPress = UILongPressGestureRecognizer(target: self, action: #selector(handleLongPress)) longPress.minimumPressDuration = 0.33 @@ -317,7 +321,11 @@ open class ChatMessageListVC: /// Updates the collection view data with given `changes`. open func updateMessages(with changes: [ListChange], completion: (() -> Void)? = nil) { - listView.updateMessages(with: changes, completion: completion) + listView.updateMessages( + with: changes, + onMessagesCountUpdate: updateNumberOfMessages, + completion: completion + ) } open func messageForIndexPath(_ indexPath: IndexPath) -> ChatMessage { @@ -563,7 +571,7 @@ open class ChatMessageListVC: } open func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int { - channelController.messages.count + numberOfMessages } open func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell { @@ -588,6 +596,10 @@ open class ChatMessageListVC: return !listView.isLastCellFullyVisible && isMoreContentThanOnePage } + private func updateNumberOfMessages() { + numberOfMessages = channelController.messages.count + } + // MARK: - UIGestureRecognizerDelegate open func gestureRecognizer(_ gestureRecognizer: UIGestureRecognizer, shouldReceive touch: UITouch) -> Bool { diff --git a/Sources/StreamChatUI/ChatMessageList/ChatMessageListView.swift b/Sources/StreamChatUI/ChatMessageList/ChatMessageListView.swift index a6e8c05ab3f..4eab6ff343d 100644 --- a/Sources/StreamChatUI/ChatMessageList/ChatMessageListView.swift +++ b/Sources/StreamChatUI/ChatMessageList/ChatMessageListView.swift @@ -171,6 +171,7 @@ open class ChatMessageListView: UITableView, Customizable, ComponentsProvider { /// Updates the table view data with given `changes`. open func updateMessages( with changes: [ListChange], + onMessagesCountUpdate: () -> Void = {}, completion: (() -> Void)? = nil ) { var shouldScrollToBottom = false @@ -178,6 +179,7 @@ open class ChatMessageListView: UITableView, Customizable, ComponentsProvider { guard let _ = collectionUpdatesMapper.mapToSetsOfIndexPaths( changes: changes, onConflict: { + onMessagesCountUpdate() reloadData() } ) else { return } @@ -210,6 +212,7 @@ open class ChatMessageListView: UITableView, Customizable, ComponentsProvider { self.deleteRows(at: [index], with: .fade) } } + onMessagesCountUpdate() }, completion: { _ in if shouldScrollToBottom { self.scrollToBottomAction = .init { [weak self] in diff --git a/Sources/StreamChatUI/ChatMessageList/ChatThreadVC.swift b/Sources/StreamChatUI/ChatMessageList/ChatThreadVC.swift index 6dad0783986..ee106a72650 100644 --- a/Sources/StreamChatUI/ChatMessageList/ChatThreadVC.swift +++ b/Sources/StreamChatUI/ChatMessageList/ChatThreadVC.swift @@ -28,6 +28,9 @@ open class ChatThreadVC: /// Controller for observing data changes within the parent thread message. open var messageController: ChatMessageController! + + /// We use private property for channels count so we can update it inside `performBatchUpdates` as [documented](https://developer.apple.com/documentation/uikit/uicollectionview/1618045-performbatchupdates#discussion) + private var numberOfMessages = 0 /// Observer responsible for setting the correct offset when keyboard frame is changed open lazy var keyboardObserver = ChatMessageListKeyboardObserver( @@ -79,6 +82,7 @@ open class ChatThreadVC: override open func setUp() { super.setUp() + updateNumberOfMessages() let longPress = UILongPressGestureRecognizer(target: self, action: #selector(handleLongPress)) longPress.minimumPressDuration = 0.33 @@ -92,6 +96,11 @@ open class ChatThreadVC: messageComposerVC.setDelegate(self) messageComposerVC.channelController = channelController messageComposerVC.userSearchController = userSuggestionSearchController + + messageController.delegate = self + messageController.synchronize() + messageController.loadPreviousReplies() + if let message = messageController.message { messageComposerVC.content.threadMessage = message } @@ -101,10 +110,6 @@ open class ChatThreadVC: channelController.setDelegate(self) channelController.synchronize() - messageController.delegate = self - messageController.synchronize() - messageController.loadPreviousReplies() - if let cid = channelController.cid { headerView.channelController = channelController.client.channelController(for: cid) } @@ -235,7 +240,7 @@ open class ChatThreadVC: } open func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int { - messages.count + numberOfMessages } open func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell { @@ -290,7 +295,11 @@ open class ChatThreadVC: /// Updates the collection view data with given `changes`. open func updateMessages(with changes: [ListChange], completion: (() -> Void)? = nil) { - listView.updateMessages(with: changes, completion: completion) + listView.updateMessages( + with: changes, + onMessagesCountUpdate: updateNumberOfMessages, + completion: completion + ) } /// Presents custom actions controller with all possible actions with the selected message. @@ -498,6 +507,10 @@ open class ChatThreadVC: .string(from: messageForIndexPath(indexPath).createdAt) } + private func updateNumberOfMessages() { + numberOfMessages = messages.count + } + // MARK: - UIGestureRecognizerDelegate open func gestureRecognizer(_ gestureRecognizer: UIGestureRecognizer, shouldReceive touch: UITouch) -> Bool {