Skip to content

Commit c400617

Browse files
committed
message [nfc]: Add _disposed flag; check it
This change should have no user-facing effect. The one spot where we have an `if (_disposed)` check in editMessage prevents a state update and a rebuild from happening. This only applies if the store is disposed before the edit request fails, but the MessageListView with the edited message should get rebuilt anyway (through onNewStore) when that happens.
1 parent 77ab930 commit c400617

File tree

1 file changed

+15
-0
lines changed

1 file changed

+15
-0
lines changed

lib/model/message.dart

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,12 +94,16 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore {
9494

9595
@override
9696
void registerMessageList(MessageListView view) {
97+
assert(!_disposed);
9798
final added = _messageListViews.add(view);
9899
assert(added);
99100
}
100101

101102
@override
102103
void unregisterMessageList(MessageListView view) {
104+
// TODO: Add `assert(!_disposed);` here once we ensure [PerAccountStore] is
105+
// only disposed after [MessageListView]s with references to it are
106+
// disposed. See [dispose] for details.
103107
final removed = _messageListViews.remove(view);
104108
assert(removed);
105109
}
@@ -122,6 +126,8 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore {
122126
}
123127
}
124128

129+
bool _disposed = false;
130+
125131
void dispose() {
126132
// Not disposing the [MessageListView]s here, because they are owned by
127133
// (i.e., they get [dispose]d by) the [_MessageListState], including in the
@@ -137,10 +143,14 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore {
137143
// [InheritedNotifier] to rebuild in the next frame) before the owner's
138144
// `dispose` or `onNewStore` is called. Discussion:
139145
// https://chat.zulip.org/#narrow/channel/243-mobile-team/topic/MessageListView.20lifecycle/near/2086893
146+
147+
assert(!_disposed);
148+
_disposed = true;
140149
}
141150

142151
@override
143152
Future<void> sendMessage({required MessageDestination destination, required String content}) {
153+
assert(!_disposed);
144154
// TODO implement outbox; see design at
145155
// https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/.23M3881.20Sending.20outbox.20messages.20is.20fraught.20with.20issues/near/1405739
146156
return _apiSendMessage(connection,
@@ -152,6 +162,7 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore {
152162

153163
@override
154164
void reconcileMessages(List<Message> messages) {
165+
assert(!_disposed);
155166
// What to do when some of the just-fetched messages are already known?
156167
// This is common and normal: in particular it happens when one message list
157168
// overlaps another, e.g. a stream and a topic within it.
@@ -185,6 +196,7 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore {
185196
required String originalRawContent,
186197
required String newContent,
187198
}) async {
199+
assert(!_disposed);
188200
if (_editMessageRequests.containsKey(messageId)) {
189201
throw StateError('an edit request is already in progress');
190202
}
@@ -202,6 +214,8 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore {
202214
} catch (e) {
203215
// TODO(log) if e is something unexpected
204216

217+
if (_disposed) return;
218+
205219
final status = _editMessageRequests[messageId];
206220
if (status == null) {
207221
// The event actually arrived before this request failed
@@ -216,6 +230,7 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore {
216230

217231
@override
218232
({String originalRawContent, String newContent}) takeFailedMessageEdit(int messageId) {
233+
assert(!_disposed);
219234
final status = _editMessageRequests.remove(messageId);
220235
_notifyMessageListViewsForOneMessage(messageId);
221236
if (status == null) {

0 commit comments

Comments
 (0)