Remove duplicate subscription

Issue fixed: duplicate subscription

  • Before: Two subscriptions to activity_onBurnComplete (one in
BatchedActivityService, one in ProtectionsProvider)
  • After: Only one subscription exists (in BatchedActivityService). We
listen to the custom event it dispatches instead of creating a
duplicate.

  Implementation details

  1. No duplicate subscription: Only BatchedActivityService subscribes
to activity_onBurnComplete (line 93 in batched-activity.service.js).
  2. Event listener setup: useBlockedCount listens to the custom event
from BatchedActivityService.burns EventTarget (lines 122-137).
  3. Conditional setup: The listener is set up only when
ActivityProvider is mounted (when feed is 'activity'). This is correct
because:
    • Burns only occur when viewing the activity feed
    • The listener is set up when activityService becomes available
    • The effect dependency array includes activityService, so it
re-runs when the service becomes available
  4. Cleanup: The cleanup function captures the burns reference directly
(line 130), ensuring proper cleanup even if activityService is
destroyed/unmounted.
  5. Shared state: burnCompleteTimeRef is shared via
BurnCompleteTimeContext, so all useBlockedCount hooks share the same
burn complete time.

  Edge cases handled

  1. Feed switching: When switching from 'activity' to 'privacy-stats',
ActivityProvider unmounts, the service is destroyed, and the listener is
cleaned up.
  2. Initial render: If feed is not 'activity', activityService is {}
(default), the effect returns early, and no listener is set up
(correct).
  3. Late mount: If ActivityProvider mounts after useBlockedCount runs,
the effect re-runs when activityService becomes available and sets up
the listener.
  4. Service destruction: When BatchedActivityService is destroyed,
burns is set to null, and the cleanup uses optional chaining to handle
this safely.

Author: jsoningram

special-pages/pages/new-tab/app/protections/components/ProtectionsProvider.js

@@ -4,6 +4,7 @@ import { useMessaging } from '../../types.js';
 import { reducer, useConfigSubscription, useInitialDataAndConfig } from '../../service.hooks.js';
 import { ProtectionsService } from '../protections.service.js';
 import { useSignal, useSignalEffect } from '@preact/signals';
+import { ActivityServiceContext } from '../../activity/ActivityProvider.js';
 
 /**
  * @typedef {import('../../../types/new-tab.js').ProtectionsData} ProtectionsData
@@ -60,18 +61,10 @@ export function ProtectionsProvider(props) {
     // subscribe to config updates
     useConfigSubscription({ dispatch, service });
 
-    // Set up a single subscription to activity_onBurnComplete at the Provider level
-    // This prevents duplicate subscription errors when multiple useBlockedCount hooks are used
-    const ntp = useMessaging();
+    // Create a shared ref for burn complete time
+    // This will be updated by listening to the custom event from BatchedActivityService
+    // instead of creating a duplicate subscription
     const burnCompleteTimeRef = useRef(/** @type {number | null} */ (null));
-    
-    useEffect(() => {
-        if (!ntp) return;
-        return ntp.messaging.subscribe('activity_onBurnComplete', () => {
-            // Mark that we should skip animation if next update goes to 0
-            burnCompleteTimeRef.current = Date.now();
-        });
-    }, [ntp]);
 
     // expose a fn for sync toggling
     const toggle = useCallback(() => {
@@ -120,9 +113,29 @@ export function useService() {
 export function useBlockedCount(initial) {
     const service = useContext(ProtectionsServiceContext);
     const burnCompleteTimeRef = useContext(BurnCompleteTimeContext);
+    const activityService = useContext(ActivityServiceContext);
     const signal = useSignal(initial);
     const skipAnimationSignal = useSignal(false);
 
+    // Listen to the custom event from BatchedActivityService instead of creating a duplicate subscription
+    // BatchedActivityService already subscribes to activity_onBurnComplete and dispatches this event
+    useEffect(() => {
+        if (!activityService?.burns) return;
+        
+        const handleBurnComplete = () => {
+            // Mark that we should skip animation if next update goes to 0
+            burnCompleteTimeRef.current = Date.now();
+        };
+        
+        const burns = activityService.burns;
+        burns.addEventListener('activity_onBurnComplete', handleBurnComplete);
+        return () => {
+            // Use the captured burns reference to ensure we remove from the correct EventTarget
+            // even if activityService is destroyed/unmounted
+            burns?.removeEventListener('activity_onBurnComplete', handleBurnComplete);
+        };
+    }, [activityService, burnCompleteTimeRef]);
+    
     // @todo jingram possibly refactor to include full object
     useSignalEffect(() => {
         return service?.onData((evt) => {