Timeline: When scrolling far into history, discard newest messages

Co-authored-by: Scott Nonnenberg <scott@signal.org>

ts/components/conversation/Timeline.tsx

@@ -33,12 +33,12 @@ import type { PropsType as SmartContactSpoofingReviewDialogPropsType } from '../
 import type { GroupNameCollisionsWithIdsByTitle } from '../../util/groupMemberNameCollisions';
 import { hasUnacknowledgedCollisions } from '../../util/groupMemberNameCollisions';
 import { TimelineFloatingHeader } from './TimelineFloatingHeader';
-import type { TimelineMessageLoadingState } from '../../util/timelineUtil';
 import {
-  ScrollAnchor,
-  UnreadIndicatorPlacement,
   getScrollAnchorBeforeUpdate,
   getWidthBreakpoint,
+  ScrollAnchor,
+  TimelineMessageLoadingState,
+  UnreadIndicatorPlacement,
 } from '../../util/timelineUtil';
 import {
   getScrollBottom,
@@ -53,6 +53,7 @@ const AT_BOTTOM_DETECTOR_STYLE = { height: AT_BOTTOM_THRESHOLD };
 
 const MIN_ROW_HEIGHT = 18;
 const SCROLL_DOWN_BUTTON_THRESHOLD = 8;
+const LOAD_NEWER_THRESHOLD = 5;
 
 export type WarningType =
   | {
@@ -109,7 +110,13 @@ type PropsHousekeepingType = {
   contactSpoofingReview?: ContactSpoofingReviewPropType;
 
   discardMessages: (
-    _: Readonly<{ conversationId: string; numberToKeepAtBottom: number }>
+    _: Readonly<
+      | {
+          conversationId: string;
+          numberToKeepAtBottom: number;
+        }
+      | { conversationId: string; numberToKeepAtTop: number }
+    >
   ) => void;
   getTimestampForMessage: (messageId: string) => undefined | number;
   getPreferredBadge: PreferredBadgeSelectorType;
@@ -487,10 +494,15 @@ export class Timeline extends React.Component<
         if (newestBottomVisibleMessageId) {
           this.markNewestBottomVisibleMessageRead();
 
+          const rowIndex = getRowIndexFromElement(newestBottomVisible);
+          const maxRowIndex = items.length - 1;
+
           if (
             !messageLoadingState &&
             !haveNewest &&
-            newestBottomVisibleMessageId === last(items)
+            isNumber(rowIndex) &&
+            maxRowIndex >= 0 &&
+            rowIndex >= maxRowIndex - LOAD_NEWER_THRESHOLD
           ) {
             loadNewerMessages(newestBottomVisibleMessageId);
           }
@@ -633,8 +645,16 @@ export class Timeline extends React.Component<
     _prevState: Readonly<StateType>,
     snapshot: Readonly<SnapshotType>
   ): void {
-    const { items: oldItems } = prevProps;
-    const { discardMessages, id, items: newItems } = this.props;
+    const {
+      items: oldItems,
+      messageLoadingState: previousMessageLoadingState,
+    } = prevProps;
+    const {
+      discardMessages,
+      id,
+      items: newItems,
+      messageLoadingState,
+    } = this.props;
 
     const containerEl = this.containerRef.current;
     if (containerEl && snapshot) {
@@ -662,12 +682,33 @@ export class Timeline extends React.Component<
       this.updateIntersectionObserver();
 
       // This condition is somewhat arbitrary.
+      const numberToKeepAtBottom = this.maxVisibleRows * 2;
       const shouldDiscardOlderMessages: boolean =
-        this.isAtBottom() && newItems.length >= this.maxVisibleRows * 1.5;
+        this.isAtBottom() && newItems.length > numberToKeepAtBottom;
       if (shouldDiscardOlderMessages) {
         discardMessages({
           conversationId: id,
-          numberToKeepAtBottom: this.maxVisibleRows,
+          numberToKeepAtBottom,
+        });
+      }
+
+      const loadingStateThatJustFinished:
+        | undefined
+        | TimelineMessageLoadingState =
+        !messageLoadingState && previousMessageLoadingState
+          ? previousMessageLoadingState
+          : undefined;
+      const numberToKeepAtTop = this.maxVisibleRows * 5;
+      const shouldDiscardNewerMessages: boolean =
+        !this.isAtBottom() &&
+        loadingStateThatJustFinished ===
+          TimelineMessageLoadingState.LoadingOlderMessages &&
+        newItems.length > numberToKeepAtTop;
+
+      if (shouldDiscardNewerMessages) {
+        discardMessages({
+          conversationId: id,
+          numberToKeepAtTop,
         });
       }
     } else {
@@ -1195,6 +1236,14 @@ function getMessageIdFromElement(
   return element instanceof HTMLElement ? element.dataset.messageId : undefined;
 }
 
+function getRowIndexFromElement(
+  element: undefined | Element
+): undefined | number {
+  return element instanceof HTMLElement && element.dataset.itemIndex
+    ? parseInt(element.dataset.itemIndex, 10)
+    : undefined;
+}
+
 function showDebugLog() {
   window.showDebugLog();
 }

ts/state/ducks/conversations.ts

@@ -80,6 +80,7 @@ import type { NoopActionType } from './noop';
 import { conversationJobQueue } from '../../jobs/conversationJobQueue';
 import type { TimelineMessageLoadingState } from '../../util/timelineUtil';
 import { isGroup } from '../../util/whatTypeOfConversation';
+import { missingCaseError } from '../../util/missingCaseError';
 
 // State
 
@@ -465,7 +466,13 @@ type CustomColorRemovedActionType = {
 };
 type DiscardMessagesActionType = {
   type: typeof DISCARD_MESSAGES;
-  payload: { conversationId: string; numberToKeepAtBottom: number };
+  payload: Readonly<
+    | {
+        conversationId: string;
+        numberToKeepAtBottom: number;
+      }
+    | { conversationId: string; numberToKeepAtTop: number }
+  >;
 };
 type SetPreJoinConversationActionType = {
   type: 'SET_PRE_JOIN_CONVERSATION';
@@ -2195,35 +2202,70 @@ export function reducer(
   }
 
   if (action.type === DISCARD_MESSAGES) {
-    const { conversationId, numberToKeepAtBottom } = action.payload;
+    const { conversationId } = action.payload;
+    if ('numberToKeepAtBottom' in action.payload) {
+      const { numberToKeepAtBottom } = action.payload;
+      const conversationMessages = getOwn(
+        state.messagesByConversation,
+        conversationId
+      );
+      if (!conversationMessages) {
+        return state;
+      }
 
-    const conversationMessages = getOwn(
-      state.messagesByConversation,
-      conversationId
-    );
-    if (!conversationMessages) {
-      return state;
-    }
+      const { messageIds: oldMessageIds } = conversationMessages;
+      if (oldMessageIds.length <= numberToKeepAtBottom) {
+        return state;
+      }
 
-    const { messageIds: oldMessageIds } = conversationMessages;
-    if (oldMessageIds.length <= numberToKeepAtBottom) {
-      return state;
-    }
+      const messageIdsToRemove = oldMessageIds.slice(0, -numberToKeepAtBottom);
+      const messageIdsToKeep = oldMessageIds.slice(-numberToKeepAtBottom);
 
-    const messageIdsToRemove = oldMessageIds.slice(0, -numberToKeepAtBottom);
-    const messageIdsToKeep = oldMessageIds.slice(-numberToKeepAtBottom);
-
-    return {
-      ...state,
-      messagesLookup: omit(state.messagesLookup, messageIdsToRemove),
-      messagesByConversation: {
-        ...state.messagesByConversation,
-        [conversationId]: {
-          ...conversationMessages,
-          messageIds: messageIdsToKeep,
+      return {
+        ...state,
+        messagesLookup: omit(state.messagesLookup, messageIdsToRemove),
+        messagesByConversation: {
+          ...state.messagesByConversation,
+          [conversationId]: {
+            ...conversationMessages,
+            messageIds: messageIdsToKeep,
+          },
         },
-      },
-    };
+      };
+    }
+
+    if ('numberToKeepAtTop' in action.payload) {
+      const { numberToKeepAtTop } = action.payload;
+      const conversationMessages = getOwn(
+        state.messagesByConversation,
+        conversationId
+      );
+      if (!conversationMessages) {
+        return state;
+      }
+
+      const { messageIds: oldMessageIds } = conversationMessages;
+      if (oldMessageIds.length <= numberToKeepAtTop) {
+        return state;
+      }
+
+      const messageIdsToRemove = oldMessageIds.slice(numberToKeepAtTop);
+      const messageIdsToKeep = oldMessageIds.slice(0, numberToKeepAtTop);
+
+      return {
+        ...state,
+        messagesLookup: omit(state.messagesLookup, messageIdsToRemove),
+        messagesByConversation: {
+          ...state.messagesByConversation,
+          [conversationId]: {
+            ...conversationMessages,
+            messageIds: messageIdsToKeep,
+          },
+        },
+      };
+    }
+
+    throw missingCaseError(action.payload);
   }
 
   if (action.type === 'SET_PRE_JOIN_CONVERSATION') {

ts/test-electron/state/ducks/conversations_test.ts

@@ -54,6 +54,7 @@ const {
   closeRecommendedGroupSizeModal,
   conversationStoppedByMissingVerification,
   createGroup,
+  discardMessages,
   openConversationInternal,
   repairNewestMessage,
   repairOldestMessage,
@@ -1306,6 +1307,52 @@ describe('both/state/ducks/conversations', () => {
       });
     });
 
+    describe('DISCARD_MESSAGES', () => {
+      const startState: ConversationsStateType = {
+        ...getEmptyState(),
+        messagesLookup: {
+          [messageId]: getDefaultMessage(messageId),
+          [messageIdTwo]: getDefaultMessage(messageIdTwo),
+          [messageIdThree]: getDefaultMessage(messageIdThree),
+        },
+        messagesByConversation: {
+          [conversationId]: {
+            metrics: {
+              totalUnseen: 0,
+            },
+            scrollToMessageCounter: 0,
+            messageIds: [messageId, messageIdTwo, messageIdThree],
+          },
+        },
+      };
+
+      it('eliminates older messages', () => {
+        const toDiscard = {
+          conversationId,
+          numberToKeepAtBottom: 2,
+        };
+        const state = reducer(startState, discardMessages(toDiscard));
+
+        assert.deepEqual(
+          state.messagesByConversation[conversationId]?.messageIds,
+          [messageIdTwo, messageIdThree]
+        );
+      });
+
+      it('eliminates newer messages', () => {
+        const toDiscard = {
+          conversationId,
+          numberToKeepAtTop: 2,
+        };
+        const state = reducer(startState, discardMessages(toDiscard));
+
+        assert.deepEqual(
+          state.messagesByConversation[conversationId]?.messageIds,
+          [messageId, messageIdTwo]
+        );
+      });
+    });
+
     describe('SET_PRE_JOIN_CONVERSATION', () => {
       const startState = {
         ...getEmptyState(),