msglist: Ensure sole ownership of MessageListView

PerAccountStore shouldn't be an owner of the MessageListView elements.

Its relationship to MessageListView is similar to that of
AutocompleteViewManager to MentionAutocompleteView (zulip#645).

With two owners, the MessageListView can be disposed twice:

1. before the frame is rendered, `removeAccount` disposes the
  `PerAccountStoreWidget`, which disposes the `MessageListView`;
   `_MessageListState` is not yet disposed;

2. during build, because `store` is set to `null`,
  `PerAccountStoreWidget` gets rebuilt. `_MessageListState`,
  a descendent of it, is no longer in the render tree;

3. during finalization, `_MessageListState` tries to dispose the
   `MessageListView`.

This removes regression tests added for zulip#810, because
`MessageStoreImpl.dispose` no longer exists.  `MessageListView` does not
get disposed unless there is a `_MessageListState` owner.

Signed-off-by: Zixuan James Li <[email protected]>

Author: PIG208

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<Message> messages) {

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();

lib/widgets/message_list.dart

@@ -483,6 +483,7 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
 
   @override
   void onNewStore() { // TODO(#464) try to keep using old model until new one gets messages
+    model?.dispose();
     _initModel(PerAccountStoreWidget.of(context));
   }
 

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();

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() {

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', () {