diff --git a/lib/model/message.dart b/lib/model/message.dart index f228d6da91..47f6189612 100644 --- a/lib/model/message.dart +++ b/lib/model/message.dart @@ -60,14 +60,12 @@ class MessageStoreImpl with MessageStore { } } - void dispose() { - // When a MessageListView is disposed, it removes itself from the Set - // `MessageStoreImpl._messageListViews`. Instead of iterating on that Set, - // iterate on a copy, to avoid concurrent modifications. - for (final view in _messageListViews.toList()) { - view.dispose(); - } - } + // No `dispose` method, because there's nothing for it to do. + // The [MessageListView]s are owned by (i.e., they get [dispose]d by) + // the [_MessageListState], including in the case where the [PerAccountStore] + // is replaced. Discussion: + // https://chat.zulip.org/#narrow/channel/243-mobile-team/topic/.60MentionAutocompleteView.2Edispose.60/near/2083074 + // void dispose() { … } @override void reconcileMessages(List messages) { diff --git a/lib/model/store.dart b/lib/model/store.dart index 7603c7f452..322367c432 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -557,7 +557,6 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess assert(!_disposed); recentDmConversationsView.dispose(); unreads.dispose(); - _messages.dispose(); typingStatus.dispose(); typingNotifier.dispose(); updateMachine?.dispose(); diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index 4263041dbc..d827e144ab 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -483,6 +483,7 @@ class _MessageListState extends State with PerAccountStoreAwareStat @override void onNewStore() { // TODO(#464) try to keep using old model until new one gets messages + model?.dispose(); _initModel(PerAccountStoreWidget.of(context)); } diff --git a/test/model/message_test.dart b/test/model/message_test.dart index 43f17be61a..9c9a940d42 100644 --- a/test/model/message_test.dart +++ b/test/model/message_test.dart @@ -77,18 +77,6 @@ void main() { checkNotified(count: messageList.fetched ? messages.length : 0); } - test('disposing multiple registered MessageListView instances', () async { - // Regression test for: https://github.com/zulip/zulip-flutter/issues/810 - await prepare(narrow: const MentionsNarrow()); - MessageListView.init(store: store, narrow: const StarredMessagesNarrow()); - check(store.debugMessageListViews).length.equals(2); - - // When disposing, the [MessageListView]s are expected to unregister - // themselves from the message store. - store.dispose(); - check(store.debugMessageListViews).isEmpty(); - }); - group('reconcileMessages', () { test('from empty', () async { await prepare(); diff --git a/test/model/store_test.dart b/test/model/store_test.dart index c8c6b4c266..bc393d6d6f 100644 --- a/test/model/store_test.dart +++ b/test/model/store_test.dart @@ -12,8 +12,6 @@ import 'package:zulip/api/model/model.dart'; import 'package:zulip/api/route/events.dart'; import 'package:zulip/api/route/messages.dart'; import 'package:zulip/api/route/realm.dart'; -import 'package:zulip/model/message_list.dart'; -import 'package:zulip/model/narrow.dart'; import 'package:zulip/log.dart'; import 'package:zulip/model/store.dart'; import 'package:zulip/notifications/receive.dart'; @@ -824,25 +822,6 @@ void main() { checkReload(prepareHandleEventError); }); - test('expired queue disposes registered MessageListView instances', () => awaitFakeAsync((async) async { - // Regression test for: https://github.com/zulip/zulip-flutter/issues/810 - await preparePoll(); - - // Make sure there are [MessageListView]s in the message store. - MessageListView.init(store: store, narrow: const MentionsNarrow()); - MessageListView.init(store: store, narrow: const StarredMessagesNarrow()); - check(store.debugMessageListViews).length.equals(2); - - // Let the server expire the event queue. - prepareExpiredEventQueue(); - updateMachine.debugAdvanceLoop(); - async.elapse(Duration.zero); - - // The old store's [MessageListView]s have been disposed. - // (And no exception was thrown; that was #810.) - check(store.debugMessageListViews).isEmpty(); - })); - group('report error', () { String? lastReportedError; String? takeLastReportedError() { diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index b3cc208463..204993cc76 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -12,6 +12,7 @@ import 'package:zulip/api/model/initial_snapshot.dart'; import 'package:zulip/api/model/model.dart'; import 'package:zulip/api/model/narrow.dart'; import 'package:zulip/api/route/messages.dart'; +import 'package:zulip/model/actions.dart'; import 'package:zulip/model/localizations.dart'; import 'package:zulip/model/narrow.dart'; import 'package:zulip/model/store.dart'; @@ -130,6 +131,26 @@ void main() { final state = MessageListPage.ancestorOf(tester.element(find.text("a message"))); check(state.composeBoxController).isNull(); }); + + testWidgets('dispose MessageListView when logged out', (tester) async { + addTearDown(testBinding.reset); + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + (store.connection as FakeApiConnection).prepare(json: eg.newestGetMessagesResult( + foundOldest: true, messages: [eg.streamMessage()]).toJson()); + await tester.pumpWidget(TestZulipApp( + accountId: eg.selfAccount.id, + skipAssertAccountExists: true, + child: MessageListPage(initNarrow: const CombinedFeedNarrow()))); + await tester.pump(); + await tester.pump(); + check(store.debugMessageListViews).single; + + final future = logOutAccount(testBinding.globalStore, eg.selfAccount.id); + await tester.pump(TestGlobalStore.removeAccountDuration); + await future; + check(store.debugMessageListViews).isEmpty(); + }); }); group('app bar', () {