Procházet zdrojové kódy

Write printer status straight through while the tab is hidden (#2754)

    Removing the requestAnimationFrame wrapper fixed the total stall but left the
    100ms coalescing timer in the path, and a hidden page's timers are clamped to
    once a second at best -- once a minute past five minutes hidden. The reporter
    still saw a tab title at 2% beside a page at 40%.

    The coalescing guards against a render cascade, which a hidden tab cannot
    have, so it is skipped there and kept while visible.

    The existing hidden-tab tests advanced fake timers, which simulates the timer
    the browser was throttling; the new one never advances the clock.
maziggy před 3 týdny
rodič
revize
50399144a1

+ 99 - 9
frontend/src/__tests__/hooks/useWebSocket.test.ts

@@ -623,16 +623,23 @@ describe('useWebSocket hook', () => {
 
   /**
    * #2754 (reporter @mic4rd): live updates froze whenever the tab wasn't in
-   * front, and caught up all at once on switching back. The cache writes ran
-   * inside requestAnimationFrame, and a hidden tab gets no rendering
-   * opportunities — so the browser holds queued frame callbacks indefinitely
-   * rather than merely throttling them.
+   * front, and caught up all at once on switching back.
    *
-   * The stub below is what makes these tests meaningful: it hands back a
-   * handle and never invokes the callback, which is what a real hidden tab
-   * does. `document.hidden` is set alongside it to name the scenario, but the
-   * production code doesn't branch on visibility — it simply no longer defers
-   * to a frame. Reintroduce a rAF wrapper on either path and these fail.
+   * Two causes, fixed in two rounds. First the cache writes ran inside a
+   * requestAnimationFrame, and a hidden tab gets no rendering opportunities —
+   * the browser holds queued frame callbacks indefinitely rather than merely
+   * throttling them. The rAF stub below is what makes those tests meaningful:
+   * it hands back a handle and never invokes the callback, which is what a
+   * real hidden tab does.
+   *
+   * Removing the frame callback did not close the report, because the 100ms
+   * coalescing timer was still in the path and a hidden page's timers are
+   * clamped to at best once a second — once a minute past five minutes hidden.
+   * So the writes must not depend on a timer either while hidden, which is
+   * what `writes without waiting on a timer` pins down. Note it deliberately
+   * never advances the clock: a test that advances fake timers cannot tell a
+   * throttled timer from a prompt one, which is exactly why the original tests
+   * kept passing while the reporter's tab stayed frozen.
    */
   describe('hidden tab (#2754)', () => {
     let rafSpy: ReturnType<typeof vi.fn>;
@@ -690,6 +697,49 @@ describe('useWebSocket hook', () => {
       expect(rafSpy).not.toHaveBeenCalled();
     });
 
+    it('writes without waiting on a timer', async () => {
+      const { useWebSocket } = await import('../../hooks/useWebSocket');
+
+      renderHook(() => useWebSocket(), { wrapper: createWrapper(queryClient) });
+      const ws = await waitForWs();
+      act(() => ws.open());
+
+      act(() => {
+        ws.simulateMessage({
+          type: 'printer_status',
+          printer_id: 1,
+          data: { state: 'RUNNING', progress: 42 },
+        });
+      });
+
+      // No advanceTimersByTime: a hidden tab's timers are throttled to once a
+      // second at best, so anything the title depends on has to have landed
+      // already. Reintroduce the coalescing timer on this path and the cache
+      // is still empty here.
+      expect(queryClient.getQueryData(['printerStatus', 1])).toMatchObject({
+        state: 'RUNNING',
+        progress: 42,
+      });
+    });
+
+    it('applies the newest value when several arrive before a frame would have run', async () => {
+      const { useWebSocket } = await import('../../hooks/useWebSocket');
+
+      renderHook(() => useWebSocket(), { wrapper: createWrapper(queryClient) });
+      const ws = await waitForWs();
+      act(() => ws.open());
+
+      act(() => {
+        ws.simulateMessage({ type: 'printer_status', printer_id: 1, data: { progress: 40 } });
+        ws.simulateMessage({ type: 'printer_status', printer_id: 1, data: { progress: 41 } });
+      });
+
+      // Writing through per message must not resurrect an earlier one: the
+      // pending map is drained on each flush, so a stale entry cannot be
+      // re-applied over the newer value.
+      expect(queryClient.getQueryData(['printerStatus', 1])).toMatchObject({ progress: 41 });
+    });
+
     it('drains queued messages instead of wedging the queue', async () => {
       const { useWebSocket } = await import('../../hooks/useWebSocket');
       const invalidateSpy = vi.spyOn(queryClient, 'invalidateQueries');
@@ -715,6 +765,46 @@ describe('useWebSocket hook', () => {
     });
   });
 
+  describe('visible tab still coalesces (#2754)', () => {
+    /**
+     * The counterpart to the hidden-tab block: the write-through is scoped to
+     * a hidden tab on purpose. A visible one is painting, and the 100ms window
+     * is what stops a burst of status messages turning into a render cascade —
+     * so "just always write through" is not the simplification it looks like.
+     */
+    it('defers the write while the tab is visible', async () => {
+      const { useWebSocket } = await import('../../hooks/useWebSocket');
+
+      const client = new QueryClient({
+        defaultOptions: { queries: { retry: false, gcTime: Infinity } },
+      });
+      vi.useFakeTimers();
+      try {
+        renderHook(() => useWebSocket(), { wrapper: createWrapper(client) });
+        const ws = await waitForWs();
+        act(() => ws.open());
+
+        act(() => {
+          ws.simulateMessage({
+            type: 'printer_status',
+            printer_id: 1,
+            data: { state: 'RUNNING', progress: 42 },
+          });
+        });
+
+        expect(client.getQueryData(['printerStatus', 1])).toBeUndefined();
+
+        await act(async () => {
+          vi.advanceTimersByTime(200);
+        });
+
+        expect(client.getQueryData(['printerStatus', 1])).toMatchObject({ progress: 42 });
+      } finally {
+        vi.useRealTimers();
+      }
+    });
+  });
+
   describe('sendMessage', () => {
     it('sends JSON message when connected', async () => {
       const { useWebSocket } = await import('../../hooks/useWebSocket');

+ 54 - 30
frontend/src/hooks/useWebSocket.ts

@@ -194,45 +194,69 @@ export function useWebSocket() {
     wsRef.current = ws;
   }, [processMessageQueue]);
 
-  // Throttled printer status update - coalesces rapid updates per printer.
+  // Write every pending printer status into the query cache.
   //
-  // #2754: these cache writes used to happen inside a requestAnimationFrame.
-  // A hidden tab gets no rendering opportunities, so the browser *holds*
-  // queued frame callbacks rather than throttling them — every status update
-  // parked in a pending frame and nothing reached the query cache until the
-  // tab was shown again, at which point they all ran at once. That froze the
-  // tab-title progress (usePrintProgressTitle reads this key and nothing
-  // else) and stalled every other live view. The 100ms coalescing below is
-  // what prevented the original render cascade; the frame callback only ever
-  // deferred the write by a frame, so it is gone.
+  // Extracted so the hidden-tab path below can run it inline: both paths share
+  // this one body, so the merge semantics cannot drift apart. Cancels any
+  // scheduled coalescing timer, since everything it was going to write has
+  // just been written and re-running it would re-apply stale data over newer.
+  const flushPrinterStatus = useCallback(() => {
+    if (printerStatusTimeoutRef.current) {
+      clearTimeout(printerStatusTimeoutRef.current);
+      printerStatusTimeoutRef.current = null;
+    }
+
+    const updates = new Map(pendingPrinterStatus.current);
+    pendingPrinterStatus.current.clear();
+
+    updates.forEach((statusData, id) => {
+      queryClient.setQueryData(['printerStatus', id], (old: Record<string, unknown> | undefined) => {
+        const merged = { ...old, ...statusData };
+        if (merged.wifi_signal == null && old?.wifi_signal != null) {
+          merged.wifi_signal = old.wifi_signal;
+        }
+        return merged;
+      });
+    });
+  }, [queryClient]);
+
+  // Printer status update — coalesced while the tab is visible, written
+  // straight through while it is not.
+  //
+  // #2754 (reporter @mic4rd), in two stages. First, these writes ran inside a
+  // requestAnimationFrame: a hidden tab gets no rendering opportunities, so
+  // the browser *holds* queued frame callbacks rather than throttling them,
+  // and nothing reached the cache until the tab was shown again. Removing the
+  // frame callback fixed that total stall but not the report, because a second
+  // timer-shaped dependency was left behind — this 100ms coalescing window.
+  //
+  // Browsers clamp timers in a hidden page to at best once a second, and drop
+  // pages hidden for more than five minutes to roughly one wake-up a minute.
+  // The reporter saw a tab title stuck at 2% beside a page at 40%.
+  //
+  // The coalescing exists to stop rapid messages triggering a render cascade.
+  // A hidden tab is not painting, so there is no cascade to prevent there —
+  // the timer is pure cost, and it is exactly the thing being throttled. So
+  // when hidden, skip it and write immediately.
+  //
+  // Note "hidden", not "unfocused": on Windows a fully-occluded window reports
+  // visibilityState 'hidden' too, which is why the reporter saw this from
+  // merely clicking away rather than only from switching tabs.
   const throttledPrinterStatusUpdate = useCallback((printerId: number, data: Record<string, unknown>) => {
     // Merge with any pending data for this printer
     const existing = pendingPrinterStatus.current.get(printerId) || {};
     pendingPrinterStatus.current.set(printerId, { ...existing, ...data });
 
+    if (document.hidden) {
+      flushPrinterStatus();
+      return;
+    }
+
     // Schedule update if not already scheduled
     if (!printerStatusTimeoutRef.current) {
-      printerStatusTimeoutRef.current = window.setTimeout(() => {
-        const updates = new Map(pendingPrinterStatus.current);
-        pendingPrinterStatus.current.clear();
-        printerStatusTimeoutRef.current = null;
-
-        // Apply all pending updates
-        updates.forEach((statusData, id) => {
-          queryClient.setQueryData(
-            ['printerStatus', id],
-            (old: Record<string, unknown> | undefined) => {
-              const merged = { ...old, ...statusData };
-              if (merged.wifi_signal == null && old?.wifi_signal != null) {
-                merged.wifi_signal = old.wifi_signal;
-              }
-              return merged;
-            }
-          );
-        });
-      }, 100); // Update at most every 100ms
+      printerStatusTimeoutRef.current = window.setTimeout(flushPrinterStatus, 100);
     }
-  }, [queryClient]);
+  }, [flushPrinterStatus]);
 
   // Debounced invalidation helper - coalesces multiple rapid invalidations
   const debouncedInvalidate = useCallback((queryKey: string) => {