store [nfc]: Add and use PerAccountStoreAwareStateMixin

lib/model/autocomplete.dart

@@ -192,27 +192,30 @@ class MentionAutocompleteView extends ChangeNotifier {
   final PerAccountStore store;
   final Narrow narrow;
 
-  MentionAutocompleteQuery? _currentQuery;
-  set query(MentionAutocompleteQuery query) {
-    _currentQuery = query;
-    _startSearch(query);
+  MentionAutocompleteQuery? get query => _query;
+  MentionAutocompleteQuery? _query;
+  set query(MentionAutocompleteQuery? query) {
+    _query = query;
+    if (query != null) {
+      _startSearch(query);
+    }
   }
 
   /// Recompute user results for the current query, if any.
   ///
   /// Called in particular when we get a [RealmUserEvent].
   void refreshStaleUserResults() {
-    if (_currentQuery != null) {
-      _startSearch(_currentQuery!);
+    if (_query != null) {
+      _startSearch(_query!);
     }
   }
 
   /// Called when the app is reassembled during debugging, e.g. for hot reload.
   ///
   /// This will redo the search from scratch for the current query, if any.
   void reassemble() {
-    if (_currentQuery != null) {
-      _startSearch(_currentQuery!);
+    if (_query != null) {
+      _startSearch(_query!);
     }
   }
 
@@ -251,7 +254,7 @@ class MentionAutocompleteView extends ChangeNotifier {
       // CPU perf: End this task; enqueue a new one for resuming this work
       await Future(() {});
 
-      if (query != _currentQuery || !hasListeners) { // false if [dispose] has been called.
+      if (query != _query || !hasListeners) { // false if [dispose] has been called.
         return null;
       }
 

lib/widgets/autocomplete.dart

@@ -26,15 +26,21 @@ class ComposeAutocomplete extends StatefulWidget {
   State<ComposeAutocomplete> createState() => _ComposeAutocompleteState();
 }
 
-class _ComposeAutocompleteState extends State<ComposeAutocomplete> {
+class _ComposeAutocompleteState extends State<ComposeAutocomplete> with PerAccountStoreAwareStateMixin<ComposeAutocomplete> {
   MentionAutocompleteView? _viewModel; // TODO different autocomplete view types
 
+  void _initViewModel() {
+    final store = PerAccountStoreWidget.of(context);
+    _viewModel = MentionAutocompleteView.init(store: store, narrow: widget.narrow)
+      ..addListener(_viewModelChanged);
+  }
+
   void _composeContentChanged() {
     final newAutocompleteIntent = widget.controller.autocompleteIntent();
     if (newAutocompleteIntent != null) {
-      final store = PerAccountStoreWidget.of(context);
-      _viewModel ??= MentionAutocompleteView.init(store: store, narrow: widget.narrow)
-        ..addListener(_viewModelChanged);
+      if (_viewModel == null) {
+        _initViewModel();
+      }
       _viewModel!.query = newAutocompleteIntent.query;
     } else {
       if (_viewModel != null) {
@@ -51,6 +57,16 @@ class _ComposeAutocompleteState extends State<ComposeAutocomplete> {
     widget.controller.addListener(_composeContentChanged);
   }
 
+  @override
+  void onNewStore() {
+    if (_viewModel != null) {
+      final query = _viewModel!.query;
+      _viewModel!.dispose();
+      _initViewModel();
+      _viewModel!.query = query;
+    }
+  }
+
   @override
   void didUpdateWidget(covariant ComposeAutocomplete oldWidget) {
     super.didUpdateWidget(oldWidget);

lib/widgets/message_list.dart

@@ -107,7 +107,7 @@ class MessageList extends StatefulWidget {
   State<StatefulWidget> createState() => _MessageListState();
 }
 
-class _MessageListState extends State<MessageList> {
+class _MessageListState extends State<MessageList> with PerAccountStoreAwareStateMixin<MessageList> {
   MessageListView? model;
   final ScrollController scrollController = ScrollController();
   final ValueNotifier<bool> _scrollToBottomVisibleValue = ValueNotifier<bool>(false);
@@ -119,16 +119,9 @@ class _MessageListState extends State<MessageList> {
   }
 
   @override
-  void didChangeDependencies() {
-    super.didChangeDependencies();
-    final store = PerAccountStoreWidget.of(context);
-    if (model != null && model!.store == store) {
-      // We already have a model, and it's for the right store.
-      return;
-    }
-    // Otherwise, set up the model.  Dispose of any old model.
+  void onNewStore() {
     model?.dispose();
-    _initModel(store);
+    _initModel(PerAccountStoreWidget.of(context));
   }
 
   @override

lib/widgets/store.dart

@@ -140,6 +140,9 @@ class PerAccountStoreWidget extends StatefulWidget {
   ///  * [GlobalStoreWidget.of], for the app's data beyond that of a
   ///    particular account.
   ///  * [InheritedNotifier], which provides the "dependency" mechanism.
+  // TODO(#185): Explain in dartdoc that the returned [PerAccountStore] might
+  //   differ from one call to the next, and to handle that with
+  //   [PerAccountStoreAwareStateMixin].
   static PerAccountStore of(BuildContext context) {
     final widget = context.dependOnInheritedWidgetOfExactType<_PerAccountStoreInheritedWidget>();
     assert(widget != null, 'No PerAccountStoreWidget ancestor');
@@ -246,3 +249,41 @@ class LoadingPage extends StatelessWidget {
     return const Center(child: CircularProgressIndicator());
   }
 }
+
+/// A [State] that uses the ambient [PerAccountStore].
+///
+/// The ambient [PerAccountStore] can be replaced in some circumstances,
+/// such as when an event queue expires. See [PerAccountStoreWidget.of].
+/// When that happens, stateful widgets should
+/// - remove listeners on the old [PerAccountStore], and
+/// - add listeners on the new one.
+///
+/// Use this mixin, overriding [onNewStore], to do that concisely.
+// TODO(#185): Until #185, I think [PerAccountStoreWidget.of] never actually
+//   returns a different [PerAccountStore] from one call to the next.
+//   But it will, and when it does, we want our [StatefulWidgets] to handle it.
+mixin PerAccountStoreAwareStateMixin<T extends StatefulWidget> on State<T> {
+  PerAccountStore? _store;
+
+  /// Called when there is a new ambient [PerAccountStore].
+  ///
+  /// Specifically this is called when this element is first inserted into the tree
+  /// (so that it has an ambient [PerAccountStore] for the first time),
+  /// and again whenever dependencies change so that [PerAccountStoreWidget.of]
+  /// would return a different store from previously.
+  ///
+  /// In this, remove any listeners on the old store
+  /// and add them on the new store.
+  void onNewStore();
+
+  @override
+  void didChangeDependencies() {
+    super.didChangeDependencies();
+
+    final storeNow = PerAccountStoreWidget.of(context);
+    if (_store != storeNow) {
+      _store = storeNow;
+      onNewStore();
+    }
+  }
+}

test/flutter_checks.dart

@@ -2,14 +2,19 @@
 import 'dart:ui';
 
 import 'package:checks/checks.dart';
-import 'package:flutter/foundation.dart';
-import 'package:flutter/painting.dart';
 import 'package:flutter/services.dart';
+import 'package:flutter/widgets.dart';
 
 extension ClipboardDataChecks on Subject<ClipboardData> {
   Subject<String?> get text => has((d) => d.text, 'text');
 }
 
+extension GlobalKeyChecks<T extends State<StatefulWidget>> on Subject<GlobalKey<T>> {
+  Subject<BuildContext?> get currentContext => has((k) => k.currentContext, 'currentContext');
+  Subject<Widget?> get currentWidget => has((k) => k.currentWidget, 'currentWidget');
+  Subject<T?> get currentState => has((k) => k.currentState, 'currentState');
+}
+
 extension ValueNotifierChecks<T> on Subject<ValueNotifier<T>> {
   Subject<T> get value => has((c) => c.value, 'value');
 }

test/widgets/store_test.dart

@@ -4,10 +4,47 @@ import 'package:flutter_test/flutter_test.dart';
 import 'package:zulip/model/store.dart';
 import 'package:zulip/widgets/store.dart';
 
+import '../flutter_checks.dart';
 import '../model/binding.dart';
 import '../example_data.dart' as eg;
 import '../model/store_checks.dart';
 
+/// A widget whose state uses [PerAccountStoreAwareStateMixin].
+class MyWidgetWithMixin extends StatefulWidget {
+  const MyWidgetWithMixin({super.key});
+
+  @override
+  State<MyWidgetWithMixin> createState() => MyWidgetWithMixinState();
+}
+
+class MyWidgetWithMixinState extends State<MyWidgetWithMixin> with PerAccountStoreAwareStateMixin<MyWidgetWithMixin> {
+  int anyDepChangeCounter = 0;
+  int storeChangeCounter = 0;
+
+  @override
+  void onNewStore() {
+    storeChangeCounter++;
+  }
+
+  @override
+  void didChangeDependencies() {
+    super.didChangeDependencies();
+    anyDepChangeCounter++;
+  }
+
+  @override
+  Widget build(BuildContext context) {
+    final brightness = Theme.of(context).brightness;
+    final accountId = PerAccountStoreWidget.of(context).account.id;
+    return Text('brightness: $brightness; accountId: $accountId');
+  }
+}
+
+extension MyWidgetWithMixinStateChecks on Subject<MyWidgetWithMixinState> {
+  Subject<int> get anyDepChangeCounter => has((w) => w.anyDepChangeCounter, 'anyDepChangeCounter');
+  Subject<int> get storeChangeCounter => has((w) => w.storeChangeCounter, 'storeChangeCounter');
+}
+
 void main() {
   TestZulipBinding.ensureInitialized();
 
@@ -113,4 +150,65 @@ void main() {
     check(tester.any(find.textContaining('found store'))).isTrue();
     tester.widget(find.text('found store, account: ${eg.selfAccount.id}'));
   });
+
+  testWidgets('PerAccountStoreAwareStateMixin', (tester) async {
+    final widgetWithMixinKey = GlobalKey<MyWidgetWithMixinState>();
+    final accountId = eg.selfAccount.id;
+
+    await TestZulipBinding.instance.globalStore.add(eg.selfAccount, eg.initialSnapshot());
+
+    Future<void> pumpWithParams({required bool light, required int accountId}) async {
+      await tester.pumpWidget(
+        MaterialApp(
+          theme: light ? ThemeData.light() : ThemeData.dark(),
+          home: GlobalStoreWidget(
+            child: PerAccountStoreWidget(
+              accountId: accountId,
+              child: MyWidgetWithMixin(key: widgetWithMixinKey)))));
+    }
+
+    // [onNewStore] called initially
+    await pumpWithParams(light: true, accountId: accountId);
+    await tester.pump(); // global store
+    await tester.pump(); // per-account store
+    check(widgetWithMixinKey).currentState.isNotNull()
+      ..anyDepChangeCounter.equals(1)
+      ..storeChangeCounter.equals(1);
+
+    // [onNewStore] not called on unrelated dependency change
+    await pumpWithParams(light: false, accountId: accountId);
+    await tester.pumpAndSettle(kThemeAnimationDuration);
+    check(widgetWithMixinKey).currentState.isNotNull()
+      ..anyDepChangeCounter.equals(2)
+      ..storeChangeCounter.equals(1);
+
+    // [onNewStore] called when store changes
+    //
+    // TODO: Trigger `store` change by simulating an event-queue renewal,
+    //   instead of with this hack where we change the UI's backing Account
+    //   out from under it. (That account-swapping would be suspicious in
+    //   production code, where we could reasonably add an assert against it.
+    //   If forced, we could let this test code proceed despite such an assert…)
+    // hack; the snapshot probably corresponds to selfAccount, not otherAccount.
+    await TestZulipBinding.instance.globalStore.add(eg.otherAccount, eg.initialSnapshot());
+    await pumpWithParams(light: false, accountId: eg.otherAccount.id);
+    // Nudge PerAccountStoreWidget to send its updated store to MyWidgetWithMixin.
+    //
+    // A change in PerAccountStoreWidget's [accountId] field doesn't by itself
+    // prompt dependent widgets (those using PerAccountStoreWidget.of) to update,
+    // even though it holds a new store. (See its state's didUpdateWidget,
+    // or lack thereof.) That's reasonable, since such a change is never expected;
+    // see TODO above.
+    //
+    // But when PerAccountStoreWidget gets a notification from the GlobalStore,
+    // it checks at that time whether it has a new PerAccountStore to distribute
+    // (as it will when widget.accountId has changed), and if so,
+    // it will notify dependent widgets. (See its state's didChangeDependencies.)
+    // So, take advantage of that.
+    TestZulipBinding.instance.globalStore.notifyListeners();
+    await tester.pumpAndSettle();
+    check(widgetWithMixinKey).currentState.isNotNull()
+      ..anyDepChangeCounter.equals(3)
+      ..storeChangeCounter.equals(2);
+  });
 }