msglist: Support outbox messages in message list

This adds some overhead in magnitude of O(1) (where the constant is
the number of outbox messages in a view, expected to be small) on
message event handling.

We add outboxMessages as a list independent from messages on
_MessageSequence.  Because outbox messages are not rendered
(the raw content is shown as plain text), we leave the 1-1
relationship between `messages` and `contents` unchanged.

When computing `items`, we now start to look at `outboxMessages` as
well, with the guarantee that the items related to outbox messages
always come after those for other messages.  Look for places that call
`_processOutboxMessage(int index)` for references, and the changes to
`checkInvariants` on how this affects the message list invariants.

`addOutboxMessage`, similar to `handleMessage`, also look at
`fetched` before adding the outbox message.  However, there is no data
race to prevent — we can totally not clear `outboxMessages` in `_reset`,
and do the initial fetch from `MessageListView.init`, so that outbox
messages do not rely on the fetched state at all.  I decided against it
since it is easy to check `fetched`, and making `fetchInitial` reprocess
`outboxMessages` from a clean state (which is inexpensive) helps
ensuring message list invariants.

Author: PIG208

lib/model/message_list.dart

@@ -63,6 +63,21 @@ class MessageListMessageItem extends MessageListMessageBaseItem {
   });
 }
 
+class MessageListOutboxMessageItem extends MessageListMessageBaseItem {
+  @override
+  final OutboxMessage message;
+  @override
+  final ZulipContent content;
+
+  MessageListOutboxMessageItem(
+    this.message, {
+    required super.showSender,
+    required super.isLastInBlock,
+  }) : content = ZulipContent(nodes: [
+    ParagraphNode(links: [], nodes: [TextNode(message.content)]),
+  ]);
+}
+
 /// Indicates the app is loading more messages at the top.
 // TODO(#80): or loading at the bottom, by adding a [MessageListDirection.newer]
 class MessageListLoadingItem extends MessageListItem {
@@ -90,7 +105,16 @@ mixin _MessageSequence {
   /// See also [contents] and [items].
   final List<Message> messages = [];
 
-  /// Whether [messages] and [items] represent the results of a fetch.
+  /// The messages sent by the self-user.
+  ///
+  /// See also [items].
+  ///
+  /// Usually this should not have that many items, so we do not anticipate
+  /// performance issues with unoptimized O(N) iterations through this list.
+  final List<OutboxMessage> outboxMessages = [];
+
+  /// Whether [messages], [outboxMessages], and [items] represent the results
+  /// of a fetch.
   ///
   /// This allows the UI to distinguish "still working on fetching messages"
   /// from "there are in fact no messages here".
@@ -142,11 +166,12 @@ mixin _MessageSequence {
   /// The messages and their siblings in the UI, in order.
   ///
   /// This has a [MessageListMessageItem] corresponding to each element
-  /// of [messages], in order.  It may have additional items interspersed
-  /// before, between, or after the messages.
+  /// of [messages], followed by each element in [outboxMessages] in order.
+  /// It may have additional items interspersed before, between, or after the
+  /// messages.
   ///
-  /// This information is completely derived from [messages] and
-  /// the flags [haveOldest], [fetchingOlder] and [fetchOlderCoolingDown].
+  /// This information is completely derived from [messages], [outboxMessages]
+  /// and the flags [haveOldest], [fetchingOlder] and [fetchOlderCoolingDown].
   /// It exists as an optimization, to memoize that computation.
   final QueueList<MessageListItem> items = QueueList();
 
@@ -168,9 +193,10 @@ mixin _MessageSequence {
         }
       case MessageListRecipientHeaderItem(:var message):
       case MessageListDateSeparatorItem(:var message):
-        if (message.id == null)                  return 1;  // TODO(#1441): test
+        if (message.id == null)                  return 1;
         return message.id! <= messageId ? -1 : 1;
       case MessageListMessageItem(:var message): return message.id.compareTo(messageId);
+      case MessageListOutboxMessageItem():       return 1;
     }
   }
 
@@ -261,6 +287,23 @@ mixin _MessageSequence {
     return true;
   }
 
+  /// Remove all outbox messages that satisfy [test].
+  ///
+  /// Returns true if any outbox messages were removed, false otherwise.
+  bool _removeOutboxMessagesWhere(bool Function(OutboxMessage) test) {
+    final count = outboxMessages.length;
+    outboxMessages.removeWhere(test);
+    if (outboxMessages.length == count) {
+      return false;
+    }
+    _removeOutboxMessageItems();
+    for (int i = 0; i < outboxMessages.length; i++) {
+      _processOutboxMessage(i);
+    }
+    _updateEndMarkers();
+    return true;
+  }
+
   void _insertAllMessages(int index, Iterable<Message> toInsert) {
     // TODO parse/process messages in smaller batches, to not drop frames.
     //   On a Pixel 5, a batch of 100 messages takes ~15-20ms in _insertAllMessages.
@@ -278,6 +321,7 @@ mixin _MessageSequence {
   void _reset() {
     generation += 1;
     messages.clear();
+    outboxMessages.clear();
     _fetched = false;
     _haveOldest = false;
     _fetchingOlder = false;
@@ -301,7 +345,8 @@ mixin _MessageSequence {
   ///
   /// Returns whether an item has been appended or not.
   ///
-  /// The caller must append a [MessageListMessageBaseItem] after this.
+  /// The caller must append a [MessageListMessageBaseItem] for [message]
+  /// after this.
   bool _maybeAppendAuxillaryItem(MessageBase message, {
     required MessageBase? prevMessage,
   }) {
@@ -338,6 +383,40 @@ mixin _MessageSequence {
       isLastInBlock: true));
   }
 
+  /// Append to [items] based on the index-th outbox message.
+  ///
+  /// All [messages] and previous messages in [outboxMessages] must already have
+  /// been processed.
+  void _processOutboxMessage(int index) {
+    final prevMessage = index == 0 ? messages.lastOrNull : outboxMessages[index - 1];
+    final message = outboxMessages[index];
+
+    final appended = _maybeAppendAuxillaryItem(message, prevMessage: prevMessage);
+    items.add(MessageListOutboxMessageItem(message,
+      showSender: appended || prevMessage?.senderId != message.senderId,
+      isLastInBlock: true));
+  }
+
+  /// Remove items associated with [outboxMessages] from [items].
+  ///
+  /// This is efficient due to the expected small size of [outboxMessages].
+  void _removeOutboxMessageItems() {
+    // This loop relies on the assumption that all [MessageListMessageItem]
+    // items comes before those associated with outbox messages.  If there
+    // is no [MessageListMessageItem] at all, this will end up removing
+    // end markers as well.
+    while (items.isNotEmpty && items.last is! MessageListMessageItem) {
+      items.removeLast();
+    }
+    assert(items.none((e) => e is MessageListOutboxMessageItem));
+
+    if (items.isNotEmpty) {
+      final lastItem = items.last as MessageListMessageItem;
+      lastItem.isLastInBlock = true;
+    }
+    _updateEndMarkers();
+  }
+
   /// Update [items] to include markers at start and end as appropriate.
   void _updateEndMarkers() {
     assert(fetched);
@@ -362,12 +441,16 @@ mixin _MessageSequence {
     }
   }
 
-  /// Recompute [items] from scratch, based on [messages], [contents], and flags.
+  /// Recompute [items] from scratch, based on [messages], [contents],
+  /// [outboxMessages] and flags.
   void _reprocessAll() {
     items.clear();
     for (var i = 0; i < messages.length; i++) {
       _processMessage(i);
     }
+    for (var i = 0; i < outboxMessages.length; i++) {
+      _processOutboxMessage(i);
+    }
     _updateEndMarkers();
   }
 }
@@ -497,7 +580,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {
     // TODO(#80): fetch from anchor firstUnread, instead of newest
     // TODO(#82): fetch from a given message ID as anchor
     assert(!fetched && !haveOldest && !fetchingOlder && !fetchOlderCoolingDown);
-    assert(messages.isEmpty && contents.isEmpty);
+    assert(messages.isEmpty && contents.isEmpty && outboxMessages.isEmpty);
     // TODO schedule all this in another isolate
     final generation = this.generation;
     final result = await getMessages(store.connection,
@@ -515,6 +598,9 @@ class MessageListView with ChangeNotifier, _MessageSequence {
         _addMessage(message);
       }
     }
+    for (final outboxMessage in store.outboxMessages.values) {
+      _maybeAddOutboxMessage(outboxMessage);
+    }
     _fetched = true;
     _haveOldest = result.foundOldest;
     _updateEndMarkers();
@@ -621,15 +707,43 @@ class MessageListView with ChangeNotifier, _MessageSequence {
     }
   }
 
+  /// Add [outboxMessage] if it belongs to the view.
+  ///
+  /// Returns true if the message was added, false otherwise.
+  bool _maybeAddOutboxMessage(OutboxMessage outboxMessage) {
+    assert(outboxMessages.none(
+      (message) => message.localMessageId == outboxMessage.localMessageId));
+    if (!outboxMessage.hidden
+        && narrow.containsMessage(outboxMessage)
+        && _messageVisible(outboxMessage)) {
+      outboxMessages.add(outboxMessage);
+      _processOutboxMessage(outboxMessages.length - 1);
+      return true;
+    }
+    return false;
+  }
+
   void addOutboxMessage(OutboxMessage outboxMessage) {
-    // TODO(#1441) implement this
+    if (!fetched) return;
+    if (_maybeAddOutboxMessage(outboxMessage)) {
+      notifyListeners();
+    }
   }
 
   /// Remove the [outboxMessage] from the view.
   ///
   /// This is a no-op if the message is not found.
   void removeOutboxMessage(OutboxMessage outboxMessage) {
-    // TODO(#1441) implement this
+    final removed = outboxMessages.remove(outboxMessage);
+    if (!removed) {
+      return;
+    }
+
+    _removeOutboxMessageItems();
+    for (int i = 0; i < outboxMessages.length; i++) {
+      _processOutboxMessage(i);
+    }
+    notifyListeners();
   }
 
   void handleUserTopicEvent(UserTopicEvent event) {
@@ -638,10 +752,17 @@ class MessageListView with ChangeNotifier, _MessageSequence {
         return;
 
       case VisibilityEffect.muted:
-        if (_removeMessagesWhere((message) =>
-            (message is StreamMessage
-             && message.streamId == event.streamId
-             && message.topic == event.topicName))) {
+        bool removed = _removeOutboxMessagesWhere((message) =>
+          message is StreamOutboxMessage
+          && message.conversation.streamId == event.streamId
+          && message.conversation.topic == event.topicName);
+
+        removed |= _removeMessagesWhere((message) =>
+          message is StreamMessage
+          && message.streamId == event.streamId
+          && message.topic == event.topicName);
+
+        if (removed) {
           notifyListeners();
         }
 
@@ -667,14 +788,29 @@ class MessageListView with ChangeNotifier, _MessageSequence {
   void handleMessageEvent(MessageEvent event) {
     final message = event.message;
     if (!narrow.containsMessage(message) || !_messageVisible(message)) {
+      assert(event.localMessageId == null || outboxMessages.none((message) =>
+        message.localMessageId == int.parse(event.localMessageId!, radix: 10)));
       return;
     }
     if (!_fetched) {
       // TODO mitigate this fetch/event race: save message to add to list later
       return;
     }
+    // We always remove all outbox message items
+    // to ensure that message items come before them.
+    _removeOutboxMessageItems();
     // TODO insert in middle instead, when appropriate
     _addMessage(message);
+    if (event.localMessageId != null) {
+      final localMessageId = int.parse(event.localMessageId!);
+      // [outboxMessages] is epxected to be short, so removing the corresponding
+      // outbox message and reprocessing them all in linear time is efficient.
+      outboxMessages.removeWhere(
+        (message) => message.localMessageId == localMessageId);
+    }
+    for (int i = 0; i < outboxMessages.length; i++) {
+      _processOutboxMessage(i);
+    }
     notifyListeners();
   }
 
@@ -795,7 +931,11 @@ class MessageListView with ChangeNotifier, _MessageSequence {
 
   /// Notify listeners if the given outbox message is present in this view.
   void notifyListenersIfOutboxMessagePresent(int localMessageId) {
-    // TODO(#1441) implement this
+    final isAnyPresent =
+      outboxMessages.any((message) => message.localMessageId == localMessageId);
+    if (isAnyPresent) {
+      notifyListeners();
+    }
   }
 
   /// Called when the app is reassembled during debugging, e.g. for hot reload.

lib/widgets/message_list.dart

@@ -686,6 +686,11 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
           key: ValueKey(data.message.id),
           header: header,
           item: data);
+      case MessageListOutboxMessageItem():
+        final header = RecipientHeader(message: data.message, narrow: widget.narrow);
+        return MessageItem(
+          header: header,
+          item: data);
     }
   }
 }
@@ -979,6 +984,7 @@ class MessageItem extends StatelessWidget {
       child: Column(children: [
         switch (item) {
           MessageListMessageItem() => MessageWithPossibleSender(item: item),
+          MessageListOutboxMessageItem() => OutboxMessageWithPossibleSender(item: item),
         },
       ]));
     if (item case MessageListMessageItem(:final message)) {
@@ -1332,7 +1338,7 @@ final _kMessageTimestampFormat = DateFormat('h:mm aa', 'en_US');
 class _SenderRow extends StatelessWidget {
   const _SenderRow({required this.message, required this.showTimestamp});
 
-  final Message message;
+  final MessageBase message;
   final bool showTimestamp;
 
   @override
@@ -1362,7 +1368,9 @@ class _SenderRow extends StatelessWidget {
                     userId: message.senderId),
                   const SizedBox(width: 8),
                   Flexible(
-                    child: Text(message.senderFullName, // TODO(#716): use `store.senderDisplayName`
+                    child: Text(message is Message
+                        ? store.senderDisplayName(message as Message)
+                        : store.userDisplayName(message.senderId),
                       style: TextStyle(
                         fontSize: 18,
                         height: (22 / 18),
@@ -1461,3 +1469,30 @@ class MessageWithPossibleSender extends StatelessWidget {
         ])));
   }
 }
+
+/// A placeholder for Zulip message sent by the self-user.
+///
+/// See also [OutboxMessage].
+class OutboxMessageWithPossibleSender extends StatelessWidget {
+  const OutboxMessageWithPossibleSender({super.key, required this.item});
+
+  final MessageListOutboxMessageItem item;
+
+  @override
+  Widget build(BuildContext context) {
+    final message = item.message;
+    return Padding(
+      padding: const EdgeInsets.symmetric(vertical: 4),
+      child: Column(children: [
+        if (item.showSender) _SenderRow(message: message, showTimestamp: false),
+        Padding(
+          padding: const EdgeInsets.symmetric(horizontal: 16),
+          // This is adapated from [MessageContent].
+          // TODO(#576): Offer InheritedMessage ancestor once we are ready
+          //   to support local echoing images and lightbox.
+          child: DefaultTextStyle(
+            style: ContentTheme.of(context).textStylePlainParagraph,
+            child: BlockContentList(nodes: item.content.nodes))),
+      ]));
+  }
+}

test/api/model/model_checks.dart

@@ -34,6 +34,10 @@ extension StreamConversationChecks on Subject<StreamConversation> {
   Subject<String?> get displayRecipient => has((x) => x.displayRecipient, 'displayRecipient');
 }
 
+extension DmConversationChecks on Subject<DmConversation> {
+  Subject<List<int>> get allRecipientIds => has((x) => x.allRecipientIds, 'allRecipientIds');
+}
+
 extension MessageBaseChecks<T extends Conversation> on Subject<MessageBase<T>> {
   Subject<int?> get id => has((e) => e.id, 'id');
   Subject<int> get senderId => has((e) => e.senderId, 'senderId');