perf(user_status): skip heartbeats another tab already sent

apps/user_status/src/services/heartbeatScheduler.spec.ts

@@ -3,16 +3,21 @@
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
 
+import { getBuilder } from '@nextcloud/browser-storage'
 import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
 import {
 	AWAY_TIMEOUT,
 	HEARTBEAT_INTERVAL,
+	HEARTBEAT_THROTTLE,
 	MOUSE_MOVE_DEBOUNCE,
 	startHeartbeat,
 } from './heartbeatScheduler.ts'
 
 const HOUR = 60 * 60 * 1000
 
+// The same scoped store the scheduler writes to, so seeding uses the real key
+const storage = getBuilder('user_status').clearOnLogout().persist().build()
+
 let stop: (() => void) | undefined
 
 /**
@@ -37,6 +42,7 @@ describe('heartbeat scheduler', () => {
 	beforeEach(() => {
 		vi.clearAllTimers()
 		vi.resetAllMocks()
+		localStorage.clear()
 	})
 
 	afterEach(() => {
@@ -118,4 +124,70 @@ describe('heartbeat scheduler', () => {
 		expect(beat).toHaveBeenCalledTimes(1)
 		expect(vi.getTimerCount()).toBe(0)
 	})
+
+	it('skips the start heartbeat when another tab reported recently', () => {
+		storage.setItem('lastHeartbeat', String(Date.now() - HEARTBEAT_THROTTLE + 1000))
+		const beat = vi.fn()
+		stop = startHeartbeat(beat)
+
+		expect(beat).not.toHaveBeenCalled()
+	})
+
+	it('sends the start heartbeat once the throttle window has passed', () => {
+		storage.setItem('lastHeartbeat', String(Date.now() - HEARTBEAT_THROTTLE - 1000))
+		const beat = vi.fn()
+		stop = startHeartbeat(beat)
+
+		expect(beat).toHaveBeenCalledTimes(1)
+	})
+
+	it.each([
+		['a timestamp from the future', () => String(Date.now() + HOUR)],
+		['an unparseable timestamp', () => 'not a number'],
+	])('sends the start heartbeat despite %s', (_label, stored) => {
+		storage.setItem('lastHeartbeat', stored())
+		const beat = vi.fn()
+		stop = startHeartbeat(beat)
+
+		expect(beat).toHaveBeenCalledTimes(1)
+	})
+
+	it('sends one heartbeat in total for two schedulers on the same page', () => {
+		const first = vi.fn()
+		const second = vi.fn()
+		stop = startHeartbeat(first)
+		const stopSecond = startHeartbeat(second)
+
+		expect(first).toHaveBeenCalledTimes(1)
+		expect(second).not.toHaveBeenCalled()
+		stopSecond()
+	})
+
+	it('never throttles the user coming back from away', async () => {
+		const beat = vi.fn()
+		stop = startHeartbeat(beat)
+
+		window.dispatchEvent(new MouseEvent('mousemove'))
+		await vi.advanceTimersByTimeAsync(AWAY_TIMEOUT + MOUSE_MOVE_DEBOUNCE)
+		beat.mockClear()
+
+		// Well inside the throttle window
+		window.dispatchEvent(new MouseEvent('mousemove'))
+
+		expect(beat).toHaveBeenCalledTimes(1)
+	})
+
+	it('waits for the first reveal before announcing a background tab', () => {
+		const visibility = vi.spyOn(document, 'visibilityState', 'get').mockReturnValue('hidden')
+		const beat = vi.fn()
+		stop = startHeartbeat(beat)
+
+		expect(beat).not.toHaveBeenCalled()
+
+		visibility.mockReturnValue('visible')
+		document.dispatchEvent(new Event('visibilitychange'))
+
+		expect(beat).toHaveBeenCalledTimes(1)
+		visibility.mockRestore()
+	})
 })

apps/user_status/src/services/heartbeatScheduler.ts

@@ -3,15 +3,21 @@
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
 
+import { getBuilder } from '@nextcloud/browser-storage'
 import debounce from 'debounce'
 
+const browserStorage = getBuilder('user_status').clearOnLogout().persist().build()
+
 /** Has to stay below the server margin between `StatusService::REFRESH_STATUS_THRESHOLD` and `StatusService::INVALIDATE_STATUS_THRESHOLD`. */
 export const HEARTBEAT_INTERVAL = 5 * 60 * 1000
 
 export const AWAY_TIMEOUT = 2 * 60 * 1000
 
 export const MOUSE_MOVE_DEBOUNCE = 2 * 1000
 
+/** Below `HEARTBEAT_INTERVAL`, so a lone tab is never suppressed and its gap to the server never grows. */
+export const HEARTBEAT_THROTTLE = 4 * 60 * 1000
+
 /**
  * Send heartbeats on a fixed interval, and once more whenever the user comes back from being away.
  *
@@ -21,6 +27,18 @@ export const MOUSE_MOVE_DEBOUNCE = 2 * 1000
 export function startHeartbeat(beat: (isAway: boolean) => void): () => void {
 	let isAway = false
 	let awayTimeout: ReturnType<typeof setTimeout> | undefined
+	let onVisible: (() => void) | undefined
+
+	const announce = (force = false) => {
+		// NaN (missing or unparseable) and a negative age (future timestamp)
+		// both fail this test, so both send
+		const age = Date.now() - Number.parseInt(browserStorage.getItem('lastHeartbeat') ?? '', 10)
+		if (!force && age >= 0 && age < HEARTBEAT_THROTTLE) {
+			return
+		}
+		browserStorage.setItem('lastHeartbeat', String(Date.now()))
+		beat(isAway)
+	}
 
 	const onMouseMove = debounce(() => {
 		const wasAway = isAway
@@ -32,22 +50,38 @@ export function startHeartbeat(beat: (isAway: boolean) => void): () => void {
 		}, AWAY_TIMEOUT)
 
 		if (wasAway) {
-			beat(isAway)
+			// Coming back is real signal, so it is never throttled
+			announce(true)
 		}
 	}, MOUSE_MOVE_DEBOUNCE, { immediate: true })
 
-	const interval = setInterval(() => beat(isAway), HEARTBEAT_INTERVAL)
+	const interval = setInterval(() => announce(), HEARTBEAT_INTERVAL)
 	window.addEventListener('mousemove', onMouseMove, {
 		capture: true,
 		passive: true,
 	})
 
-	beat(isAway)
+	if (document.visibilityState === 'hidden') {
+		// A tab opened in the background has nothing to report until it is looked at
+		onVisible = () => {
+			if (document.visibilityState === 'hidden') {
+				return
+			}
+			document.removeEventListener('visibilitychange', onVisible!)
+			announce()
+		}
+		document.addEventListener('visibilitychange', onVisible)
+	} else {
+		announce()
+	}
 
 	return () => {
 		clearInterval(interval)
 		clearTimeout(awayTimeout)
 		onMouseMove.clear()
 		window.removeEventListener('mousemove', onMouseMove, { capture: true })
+		if (onVisible) {
+			document.removeEventListener('visibilitychange', onVisible)
+		}
 	}
 }