fix(reader): stop per-chapter listener leak that degrades paragraph mode (#4735)

The annotator's foliate `load` handler (onLoad) attached a renderer `scroll` listener and, on Android, a global `native-touch` dispatcher listener on every section load. Both the renderer and the eventDispatcher outlive individual sections — and foliate fires `load` for preloaded neighbour sections too — so these listeners accumulated without bound, one set per chapter. Each renderer `scroll` (fired on every paragraph-mode `goTo`) then ran all of them, and on Android the scroll/native-touch handlers do real work. Reading a long book (e.g. a 3000-chapter web novel) in paragraph mode slowed down steadily after a few chapters and only an app restart cleared it. Register these listeners once per view via a new `useRendererInputListeners` hook with cleanup, instead of once per section load. The native-touch handler now resolves the CURRENT primary section's doc/index at fire time rather than capturing a (possibly off-screen, preloaded) section's. The redundant `scroll` → `repositionPopups` listener is dropped — a dedicated effect already repositions popups on scroll. Doc-scoped listeners stay in onLoad, since they die with the section's iframe. Add useRendererInputListeners unit tests covering register-once-per-view, no-accumulation-across-re-renders, latest-handler routing, Android gating, and unmount cleanup. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-23 00:12:39 +08:00
parent 140b71ee30
commit bc9b8b23e6
3 changed files with 289 additions and 27 deletions
@@ -0,0 +1,173 @@
+import { afterEach, describe, expect, it, vi } from 'vitest';
+import { cleanup, renderHook } from '@testing-library/react';
+
+// A controllable stand-in for the global eventDispatcher: it tracks the
+// `native-touch` listeners so a test can assert they never accumulate (the bug
+// was that each section load added one and none were ever removed).
+const nativeTouchListeners = new Set<(e: CustomEvent) => void>();
+vi.mock('@/utils/event', () => ({
+  eventDispatcher: {
+    on: vi.fn((event: string, cb: (e: CustomEvent) => void) => {
+      if (event === 'native-touch') nativeTouchListeners.add(cb);
+    }),
+    off: vi.fn((event: string, cb: (e: CustomEvent) => void) => {
+      if (event === 'native-touch') nativeTouchListeners.delete(cb);
+    }),
+  },
+}));
+
+import { useRendererInputListeners } from '@/app/reader/hooks/useRendererInputListeners';
+import type { FoliateView } from '@/types/view';
+
+// Stand-in for the long-lived foliate paginator (an EventTarget). It records its
+// `scroll` listeners so a test can prove they stay at exactly one per view.
+class MockRenderer extends EventTarget {
+  scrollListeners = new Set<EventListenerOrEventListenerObject>();
+  override addEventListener(type: string, cb: EventListenerOrEventListenerObject, opts?: unknown) {
+    if (type === 'scroll') this.scrollListeners.add(cb);
+    super.addEventListener(type, cb, opts as AddEventListenerOptions);
+  }
+  override removeEventListener(
+    type: string,
+    cb: EventListenerOrEventListenerObject,
+    opts?: unknown,
+  ) {
+    if (type === 'scroll') this.scrollListeners.delete(cb);
+    super.removeEventListener(type, cb, opts as EventListenerOptions);
+  }
+}
+
+const makeView = (renderer: MockRenderer) => ({ renderer }) as unknown as FoliateView;
+
+const noopOpts = {
+  onRendererScroll: () => {},
+  enableNativeTouch: false,
+  listenToNativeTouchEvents: () => {},
+};
+
+afterEach(() => {
+  cleanup();
+  nativeTouchListeners.clear();
+  vi.clearAllMocks();
+});
+
+describe('useRendererInputListeners (paragraph-mode scroll/touch leak)', () => {
+  it('registers exactly one renderer scroll listener and never accumulates across re-renders', () => {
+    const renderer = new MockRenderer();
+    const view = makeView(renderer);
+    const { rerender } = renderHook(
+      ({ s }) => useRendererInputListeners(view, { ...noopOpts, onRendererScroll: s }),
+      { initialProps: { s: vi.fn() } },
+    );
+
+    expect(renderer.scrollListeners.size).toBe(1);
+
+    // A long reading session re-renders the annotator many times (every section
+    // load, progress update, popup toggle…). None of those may add a listener.
+    for (let i = 0; i < 20; i++) rerender({ s: vi.fn() });
+    expect(renderer.scrollListeners.size).toBe(1);
+  });
+
+  it('routes scroll events to the latest handler without re-subscribing', () => {
+    const renderer = new MockRenderer();
+    const view = makeView(renderer);
+    const first = vi.fn();
+    const { rerender } = renderHook(
+      ({ s }) => useRendererInputListeners(view, { ...noopOpts, onRendererScroll: s }),
+      { initialProps: { s: first } },
+    );
+
+    const second = vi.fn();
+    rerender({ s: second });
+    renderer.dispatchEvent(new Event('scroll'));
+
+    expect(first).not.toHaveBeenCalled();
+    expect(second).toHaveBeenCalledTimes(1);
+    expect(renderer.scrollListeners.size).toBe(1);
+  });
+
+  it('removes the renderer scroll listener on unmount', () => {
+    const renderer = new MockRenderer();
+    const { unmount } = renderHook(() => useRendererInputListeners(makeView(renderer), noopOpts));
+
+    expect(renderer.scrollListeners.size).toBe(1);
+    unmount();
+    expect(renderer.scrollListeners.size).toBe(0);
+  });
+
+  it('wires the Android native-touch bridge exactly once and tears it down on unmount', () => {
+    const renderer = new MockRenderer();
+    const view = makeView(renderer);
+    const listenToNativeTouchEvents = vi.fn();
+    const { rerender, unmount } = renderHook(
+      ({ t }) =>
+        useRendererInputListeners(view, {
+          onRendererScroll: () => {},
+          onNativeTouch: t,
+          enableNativeTouch: true,
+          listenToNativeTouchEvents,
+        }),
+      { initialProps: { t: vi.fn() } },
+    );
+
+    expect(nativeTouchListeners.size).toBe(1);
+    expect(listenToNativeTouchEvents).toHaveBeenCalledTimes(1);
+
+    for (let i = 0; i < 20; i++) rerender({ t: vi.fn() });
+    expect(nativeTouchListeners.size).toBe(1);
+    expect(listenToNativeTouchEvents).toHaveBeenCalledTimes(1);
+
+    unmount();
+    expect(nativeTouchListeners.size).toBe(0);
+  });
+
+  it('does not wire native-touch when not on Android', () => {
+    const renderer = new MockRenderer();
+    const listenToNativeTouchEvents = vi.fn();
+    renderHook(() =>
+      useRendererInputListeners(makeView(renderer), {
+        onRendererScroll: () => {},
+        onNativeTouch: vi.fn(),
+        enableNativeTouch: false,
+        listenToNativeTouchEvents,
+      }),
+    );
+
+    expect(nativeTouchListeners.size).toBe(0);
+    expect(listenToNativeTouchEvents).not.toHaveBeenCalled();
+  });
+
+  it('delivers native-touch events to the latest handler with the live event detail', () => {
+    const renderer = new MockRenderer();
+    const view = makeView(renderer);
+    const first = vi.fn();
+    const { rerender } = renderHook(
+      ({ t }) =>
+        useRendererInputListeners(view, {
+          onRendererScroll: () => {},
+          onNativeTouch: t,
+          enableNativeTouch: true,
+          listenToNativeTouchEvents: () => {},
+        }),
+      { initialProps: { t: first } },
+    );
+
+    const second = vi.fn();
+    rerender({ t: second });
+    const detail = { type: 'touchmove', x: 1, y: 2 };
+    nativeTouchListeners.forEach((cb) => cb(new CustomEvent('native-touch', { detail })));
+
+    expect(first).not.toHaveBeenCalled();
+    expect(second).toHaveBeenCalledWith(detail);
+  });
+
+  it('does not register anything until a view exists', () => {
+    const { rerender } = renderHook(({ v }) => useRendererInputListeners(v, noopOpts), {
+      initialProps: { v: null as FoliateView | null },
+    });
+    // No view yet → no throw, nothing wired.
+    const renderer = new MockRenderer();
+    rerender({ v: makeView(renderer) });
+    expect(renderer.scrollListeners.size).toBe(1);
+  });
+});
@@ -22,6 +22,7 @@ import { useTranslation } from '@/hooks/useTranslation';
 import { useResponsiveSize } from '@/hooks/useResponsiveSize';
 import { useDeviceControlStore } from '@/store/deviceStore';
 import { useFoliateEvents } from '../../hooks/useFoliateEvents';
+import { useRendererInputListeners } from '../../hooks/useRendererInputListeners';
 import { useNotesSync } from '../../hooks/useNotesSync';
 import { useReadwiseSync } from '../../hooks/useReadwiseSync';
 import { useHardcoverSync } from '../../hooks/useHardcoverSync';
@@ -356,35 +357,16 @@ const Annotator: React.FC<{ bookKey: string; contentInsets: Insets }> = ({
      handleTouchMove(ev);
    };

-    const handleNativeTouch = (event: CustomEvent) => {
-      const ev = event.detail as NativeTouchEventType;
-      if (ev.type === 'touchstart') {
-        androidTouchEndRef.current = false;
-        beginGesture(deferredQuickActionRef.current);
-        handleTouchStart();
-      } else if (ev.type === 'touchmove') {
-        // The Android pointer engagement signal (throttled in MainActivity.kt).
-        handleNativeTouchMove(ev.x, ev.y, doc);
-      } else if (ev.type === 'touchend') {
-        androidTouchEndRef.current = true;
-        handleTouchEnd();
-        handlePointerUp(doc, index);
-        flushDeferredAction(deferredQuickActionRef.current);
-      }
-    };
-
-    if (appService?.isAndroidApp) {
-      listenToNativeTouchEvents();
-      eventDispatcher.on('native-touch', handleNativeTouch);
-    }
-
    // Attach generic selection listeners for all formats, including PDF.
    // For PDF we only guarantee Copy & Translate; highlight/annotate may be limited by CFI support.
-    view?.renderer?.addEventListener('scroll', handleScroll);
-    // Reposition popups on scroll to keep them in view
-    view?.renderer?.addEventListener('scroll', () => {
-      repositionPopups();
-    });
+    //
+    // The renderer `scroll` listener and the Android `native-touch` bridge are
+    // NOT attached here: onLoad fires for every (pre)loaded section, but those
+    // listeners live on the renderer / global dispatcher, which outlive sections.
+    // Attaching them per load leaked one set per chapter and degraded paragraph
+    // mode over a long session. They are registered once per view via
+    // useRendererInputListeners below. Popup repositioning on scroll is already
+    // handled by the dedicated effect further down.
    const opts = { passive: false };
    detail.doc?.addEventListener('touchstart', handleTouchStart, opts);
    detail.doc?.addEventListener('touchmove', handleTouchmove, opts);
@@ -592,6 +574,41 @@ const Annotator: React.FC<{ bookKey: string; contentInsets: Insets }> = ({

  useFoliateEvents(view, { onLoad, onCreateOverlay, onDrawAnnotation, onShowAnnotation });

+  // Android native-touch handler (the per-gesture engagement signal bridged from
+  // MainActivity.kt). Registered once per view by useRendererInputListeners; it
+  // resolves the CURRENT primary section's doc/index at fire time rather than
+  // capturing them at load time, because foliate also fires `load` for preloaded
+  // neighbour sections, whose doc/index would be off-screen.
+  const handleNativeTouch = (ev: NativeTouchEventType) => {
+    const contents = view?.renderer?.getContents?.() ?? [];
+    const content = contents.find((c) => c.index === view?.renderer?.primaryIndex) ?? contents[0];
+    const doc = content?.doc;
+    const index = content?.index;
+    if (!doc || index === undefined) return;
+    if (ev.type === 'touchstart') {
+      androidTouchEndRef.current = false;
+      beginGesture(deferredQuickActionRef.current);
+      handleTouchStart();
+    } else if (ev.type === 'touchmove') {
+      handleNativeTouchMove(ev.x, ev.y, doc);
+    } else if (ev.type === 'touchend') {
+      androidTouchEndRef.current = true;
+      handleTouchEnd();
+      handlePointerUp(doc, index);
+      flushDeferredAction(deferredQuickActionRef.current);
+    }
+  };
+
+  // Register the renderer `scroll` listener and (on Android) the `native-touch`
+  // bridge once per view, with cleanup — see the hook for why attaching these in
+  // onLoad leaked listeners and degraded paragraph mode over a long session.
+  useRendererInputListeners(view, {
+    onRendererScroll: handleScroll,
+    onNativeTouch: handleNativeTouch,
+    enableNativeTouch: !!appService?.isAndroidApp,
+    listenToNativeTouchEvents,
+  });
+
  // Word Lens: open the dictionary popup for a tapped glossed word. The tap is
  // detected in the iframe click handler (iframeEventHandlers.ts), which sends
  // the gloss <ruby> element here. We synthesize a selection over the base word
@@ -0,0 +1,72 @@
+import { useEffect, useRef } from 'react';
+import { FoliateView } from '@/types/view';
+import { NativeTouchEventType } from '@/types/system';
+import { eventDispatcher } from '@/utils/event';
+
+interface RendererInputListenersOptions {
+  /** Fired on every renderer `scroll` event (the long-lived paginator). */
+  onRendererScroll: () => void;
+  /** Fired on each Android `native-touch` event; ignored when not on Android. */
+  onNativeTouch?: (ev: NativeTouchEventType) => void;
+  /** Whether to wire the Android native-touch bridge for this view. */
+  enableNativeTouch: boolean;
+  /** Starts the native touch-event bridge (safe to call once per view). */
+  listenToNativeTouchEvents: () => void;
+}
+
+/**
+ * Registers the input listeners that must live as long as the book view does —
+ * the renderer `scroll` listener and (on Android) the global `native-touch`
+ * dispatcher listener — exactly ONCE per view, with cleanup.
+ *
+ * These used to be attached inside the foliate `load` handler (the annotator's
+ * `onLoad`), which runs on every section load — including foliate's *preloaded*
+ * neighbour sections. Both the renderer and the global eventDispatcher outlive
+ * individual sections, so those listeners accumulated without bound: every
+ * chapter transition added more, and each renderer `scroll` (fired on every
+ * paragraph-mode `goTo`) then ran all of them. Reading a long book — especially
+ * in paragraph mode on Android, where the scroll/native-touch handlers do real
+ * work — degraded steadily until the app was restarted.
+ *
+ * The handlers are read through refs, so a re-render never re-subscribes; the
+ * effect re-runs only when the view (or the Android gate) changes.
+ */
+export const useRendererInputListeners = (
+  view: FoliateView | null,
+  {
+    onRendererScroll,
+    onNativeTouch,
+    enableNativeTouch,
+    listenToNativeTouchEvents,
+  }: RendererInputListenersOptions,
+) => {
+  const onRendererScrollRef = useRef(onRendererScroll);
+  onRendererScrollRef.current = onRendererScroll;
+  const onNativeTouchRef = useRef(onNativeTouch);
+  onNativeTouchRef.current = onNativeTouch;
+
+  useEffect(() => {
+    const renderer = view?.renderer;
+    if (!renderer) return;
+
+    const handleScroll = () => onRendererScrollRef.current();
+    renderer.addEventListener('scroll', handleScroll);
+
+    let offNativeTouch: (() => void) | undefined;
+    if (enableNativeTouch) {
+      listenToNativeTouchEvents();
+      const handleNativeTouch = (event: CustomEvent) =>
+        onNativeTouchRef.current?.(event.detail as NativeTouchEventType);
+      eventDispatcher.on('native-touch', handleNativeTouch);
+      offNativeTouch = () => eventDispatcher.off('native-touch', handleNativeTouch);
+    }
+
+    return () => {
+      renderer.removeEventListener('scroll', handleScroll);
+      offNativeTouch?.();
+    };
+    // `listenToNativeTouchEvents` is a stable store action and the handlers are
+    // routed through refs, so the effect only depends on the view + the gate.
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [view, enableNativeTouch]);
+};