Skip to content

Commit

Permalink
[CIS-1087] Fix data races in message list's message count (#1347)
Browse files Browse the repository at this point in the history
  • Loading branch information
dmigach authored Aug 10, 2021
1 parent 08b7477 commit e657a92
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
16 changes: 14 additions & 2 deletions Sources/StreamChatUI/ChatMessageList/ChatMessageListVC.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -317,7 +321,11 @@ open class ChatMessageListVC:

/// Updates the collection view data with given `changes`.
open func updateMessages(with changes: [ListChange<ChatMessage>], completion: (() -> Void)? = nil) {
listView.updateMessages(with: changes, completion: completion)
listView.updateMessages(
with: changes,
onMessagesCountUpdate: updateNumberOfMessages,
completion: completion
)
}

open func messageForIndexPath(_ indexPath: IndexPath) -> ChatMessage {
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,13 +171,15 @@ open class ChatMessageListView: UITableView, Customizable, ComponentsProvider {
/// Updates the table view data with given `changes`.
open func updateMessages(
with changes: [ListChange<ChatMessage>],
onMessagesCountUpdate: () -> Void = {},
completion: (() -> Void)? = nil
) {
var shouldScrollToBottom = false

guard let _ = collectionUpdatesMapper.mapToSetsOfIndexPaths(
changes: changes,
onConflict: {
onMessagesCountUpdate()
reloadData()
}
) else { return }
Expand Down Expand Up @@ -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
Expand Down
25 changes: 19 additions & 6 deletions Sources/StreamChatUI/ChatMessageList/ChatThreadVC.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand All @@ -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
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -290,7 +295,11 @@ open class ChatThreadVC:

/// Updates the collection view data with given `changes`.
open func updateMessages(with changes: [ListChange<ChatMessage>], 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.
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit e657a92

Please sign in to comment.