Add support for Web Telemetry counter

[GuiltyDolphin]

Task/Issue URL: https://app.asana.com/1/137249556945/task/1212827841082637

Description

Adds support for adwall (aggregate) counting pixels via web telemetry.

Steps to test this PR

Testing steps are covered in https://github.com/duckduckgo/ddg-workflow/blob/gd-detector-telemetry/technical-designs/web-detection-framework/eventhub-android-testing-plan.md

---

Note

Medium Risk
Adds a new backgrounded/foregrounded lifecycle-driven telemetry pipeline that persists state in Room and fires pixels on timers, which could impact analytics correctness and scheduling behavior. The feature is gated by a remote toggle but touches WebView messaging and app lifecycle hooks.

Overview
Introduces a new eventHub telemetry feature that ingests webEvents/webEvent messages from Content Scope Scripts, aggregates counter-based metrics over configurable periods, and fires bucketed pixels with an attributionPeriod parameter.

Adds a new event-hub-impl module with remote-config parsing, per-pixel state persistence (Room), deduping per webViewId/navigation, scheduling to fire at period end, and plugins/hooks for privacy-config updates and app lifecycle foreground/background transitions. Updates content-scope messaging to inject nativeData.webViewId into webEvent messages, wires the module into app/build.gradle, and adds pixel definitions for daily/weekly adwall detection telemetry.

^{Written by Cursor Bugbot for commit d880c78. This will update automatically on new commits. Configure here.}

[cmonfortep]

@GuiltyDolphin there are a few thing we need to review here. One is all the manual handling of the privacy config (which I would like to understand if you can use Toggle instead). There's also the parsing on every event you receive, I'd like to know if intended or can be improved. Then some tidy up of the structure.

Didn't comment on this, but if you can also group classes on more clear package inside impl module (so it's more clear the scope of them).

[cmonfortep]

Forgot to comment on this, you are relying on each pixel definition that lives inside settings (including the state). You should also support the enabled state present at the top level, which should serve as kill switch for all of them, and also support minSupportedVersion in case we need to take actions that affect all pixels.

[GuiltyDolphin]

Forgot to comment on this, you are relying on each pixel definition that lives inside settings (including the state). You should also support the enabled state present at the top level, which should serve as kill switch for all of them, and also support minSupportedVersion in case we need to take actions that affect all pixels.

Right, the global enabled/disabled state of the feature serves as a kill switch. This was manually modeled, but your suggestion of using Toggle simplifies this :)

[GuiltyDolphin]

Couple more comments to get to, will do in the morning 🙂

[cursor]

Cursor Bugbot has reviewed your changes and found 6 potential issues.

Autofix Details

Bugbot Autofix prepared fixes for all 6 issues found in the latest run.

• ✅ Fixed: Dedup survives period reset
  • Added dedupSeen.removeAll { it.startsWith("${pixelConfig.name}:") } in fireTelemetry to clear per-pixel dedup keys when a period ends, and dedupSeen.clear() in onConfigChanged when feature is disabled.
• ✅ Fixed: Timer map can keep stale entries
  • Added scheduledTimers.remove(pixelName) before each early return@launch in the timer coroutine so the map entry is cleaned up when the coroutine exits without calling fireTelemetry.
• ✅ Fixed: Concurrent scheduling can double-fire pixels
  • Wrapped the scheduleFireTelemetry body in synchronized(this) to make the containsKey check and job creation atomic, preventing concurrent callers from creating duplicate timer jobs.
• ✅ Fixed: Invalid periods can crash attribution calculation
  • Added if (period.periodSeconds <= 0) return null after computing the period total in toTelemetryPixelConfig, rejecting configs with negative or zero computed period before they can cause division by zero.
• ✅ Fixed: Unsynchronized state updates can lose events
  • Wrapped checkPixels, onConfigChanged, and the timer coroutine's state-reading/firing block in synchronized(this) to use the same lock as handleWebEvent, preventing interleaved repository reads and writes.
• ✅ Fixed: WebView dedup cache never pruned
  • Added a MAX_TRACKED_WEBVIEWS size check in onNavigationStarted that clears both webViewCurrentUrl and dedupSeen when the cache exceeds 100 entries, and also clears both maps in onConfigChanged when the feature is disabled.

Or push these changes by commenting:

@cursor push 322ac0610a

Preview (322ac0610a)

diff --git a/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/EventHubConfigParser.kt b/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/EventHubConfigParser.kt
--- a/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/EventHubConfigParser.kt
+++ b/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/EventHubConfigParser.kt
@@ -122,6 +122,7 @@
             hours = periodJson.hours,
             days = periodJson.days,
         )
+        if (period.periodSeconds <= 0) return null
 
         val parameters = mutableMapOf<String, TelemetryParameterConfig>()
         for ((paramName, paramJson) in json.parameters) {

diff --git a/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/RealEventHubPixelManager.kt b/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/RealEventHubPixelManager.kt
--- a/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/RealEventHubPixelManager.kt
+++ b/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/RealEventHubPixelManager.kt
@@ -64,6 +64,10 @@
 
     override fun onNavigationStarted(webViewId: String, url: String) {
         if (webViewId.isEmpty() || url.isEmpty()) return
+        if (webViewCurrentUrl.size > MAX_TRACKED_WEBVIEWS) {
+            webViewCurrentUrl.clear()
+            dedupSeen.clear()
+        }
         val previousUrl = webViewCurrentUrl.put(webViewId, url)
         if (previousUrl != null && previousUrl != url) {
             logcat(VERBOSE) { "EventHub: navigation detected for tab $webViewId ($previousUrl -> $url), clearing dedup" }
@@ -136,21 +140,23 @@
     override fun checkPixels() {
         if (!isFeatureEnabled()) return
 
-        val nowMillis = timeProvider.currentTimeMillis()
+        synchronized(this) {
+            val nowMillis = timeProvider.currentTimeMillis()
 
-        for (state in repository.getAllPixelStates()) {
-            val storedConfig = EventHubConfigParser.parseSinglePixelConfig(state.pixelName, state.configJson) ?: continue
+            for (state in repository.getAllPixelStates()) {
+                val storedConfig = EventHubConfigParser.parseSinglePixelConfig(state.pixelName, state.configJson) ?: continue
 
-            if (nowMillis >= state.periodEndMillis) {
-                fireTelemetry(storedConfig, state)
-            } else {
-                scheduleFireTelemetry(state.pixelName, state.periodEndMillis - nowMillis)
+                if (nowMillis >= state.periodEndMillis) {
+                    fireTelemetry(storedConfig, state)
+                } else {
+                    scheduleFireTelemetry(state.pixelName, state.periodEndMillis - nowMillis)
+                }
             }
-        }
 
-        for (pixelConfig in getTelemetryConfigs()) {
-            if (repository.getPixelState(pixelConfig.name) == null) {
-                startNewPeriod(pixelConfig)
+            for (pixelConfig in getTelemetryConfigs()) {
+                if (repository.getPixelState(pixelConfig.name) == null) {
+                    startNewPeriod(pixelConfig)
+                }
             }
         }
     }
@@ -158,41 +164,59 @@
     override fun onConfigChanged() {
         cachedTelemetryConfigs = null
 
-        if (!isFeatureEnabled()) {
-            logcat(DEBUG) { "EventHub: feature disabled, clearing all pixel states" }
-            cancelAllTimers()
-            repository.deleteAllPixelStates()
-            return
-        }
+        synchronized(this) {
+            if (!isFeatureEnabled()) {
+                logcat(DEBUG) { "EventHub: feature disabled, clearing all pixel states" }
+                cancelAllTimers()
+                repository.deleteAllPixelStates()
+                dedupSeen.clear()
+                webViewCurrentUrl.clear()
+                return
+            }
 
-        val telemetry = getTelemetryConfigs()
-        logcat(DEBUG) { "EventHub: onConfigChanged — feature enabled, ${telemetry.size} telemetry pixel(s) in config" }
-        for (pixelConfig in telemetry) {
-            if (repository.getPixelState(pixelConfig.name) == null) {
-                startNewPeriod(pixelConfig)
+            val telemetry = getTelemetryConfigs()
+            logcat(DEBUG) { "EventHub: onConfigChanged — feature enabled, ${telemetry.size} telemetry pixel(s) in config" }
+            for (pixelConfig in telemetry) {
+                if (repository.getPixelState(pixelConfig.name) == null) {
+                    startNewPeriod(pixelConfig)
+                }
             }
         }
     }
 
     fun scheduleFireTelemetry(pixelName: String, delayMillis: Long) {
-        if (scheduledTimers.containsKey(pixelName)) {
-            logcat(VERBOSE) { "EventHub: timer already scheduled for $pixelName, skipping" }
-            return
-        }
+        synchronized(this) {
+            if (scheduledTimers.containsKey(pixelName)) {
+                logcat(VERBOSE) { "EventHub: timer already scheduled for $pixelName, skipping" }
+                return
+            }
 
-        logcat(VERBOSE) { "EventHub: scheduling fire for $pixelName in ${delayMillis}ms" }
-        val job = appCoroutineScope.launch(dispatcherProvider.io()) {
-            delay(delayMillis)
-            ensureActive()
+            logcat(VERBOSE) { "EventHub: scheduling fire for $pixelName in ${delayMillis}ms" }
+            val job = appCoroutineScope.launch(dispatcherProvider.io()) {
+                delay(delayMillis)
+                ensureActive()
 
-            if (!isFeatureEnabled()) return@launch
+                if (!isFeatureEnabled()) {
+                    scheduledTimers.remove(pixelName)
+                    return@launch
+                }
 
-            val state = repository.getPixelState(pixelName) ?: return@launch
-            val storedConfig = EventHubConfigParser.parseSinglePixelConfig(state.pixelName, state.configJson) ?: return@launch
-
-            fireTelemetry(storedConfig, state)
+                synchronized(this@RealEventHubPixelManager) {
+                    val state = repository.getPixelState(pixelName)
+                    if (state == null) {
+                        scheduledTimers.remove(pixelName)
+                        return@launch
+                    }
+                    val storedConfig = EventHubConfigParser.parseSinglePixelConfig(state.pixelName, state.configJson)
+                    if (storedConfig == null) {
+                        scheduledTimers.remove(pixelName)
+                        return@launch
+                    }
+                    fireTelemetry(storedConfig, state)
+                }
+            }
+            scheduledTimers[pixelName] = job
         }
-        scheduledTimers[pixelName] = job
     }
 
     fun hasScheduledTimer(pixelName: String): Boolean = scheduledTimers.containsKey(pixelName)
@@ -214,6 +238,7 @@
 
     private fun fireTelemetry(pixelConfig: TelemetryPixelConfig, state: EventHubPixelStateEntity) {
         cancelScheduledFire(pixelConfig.name)
+        dedupSeen.removeAll { it.startsWith("${pixelConfig.name}:") }
 
         val pixelData = buildPixel(pixelConfig, state)
 
@@ -287,6 +312,7 @@
 
     companion object {
         const val PARAM_ATTRIBUTION_PERIOD = "attributionPeriod"
+        private const val MAX_TRACKED_WEBVIEWS = 100
 
         fun calculateAttributionPeriod(periodStartMillis: Long, period: TelemetryPeriodConfig): Long {
             return toStartOfInterval(periodStartMillis, period.periodSeconds)

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Autofix Details

Bugbot Autofix prepared fixes for both issues found in the latest run.

• ✅ Fixed: Missing synchronization in onConfigChanged causes race conditions
  • Wrapped repository operations in onConfigChanged with synchronized(this) to match the locking pattern used by handleWebEvent, checkPixels, and the scheduled timer coroutine.
• ✅ Fixed: Lifecycle observer ordering race with foreground state
  • Added a skipForegroundCheck parameter to startNewPeriod and set it to true when called from checkPixels, since checkPixels is inherently a foreground-only operation and should not depend on observer ordering.

Or push these changes by commenting:

@cursor push 00c5f9a066

Preview (00c5f9a066)

diff --git a/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/EventHubPixelManager.kt b/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/EventHubPixelManager.kt
--- a/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/EventHubPixelManager.kt
+++ b/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/EventHubPixelManager.kt
@@ -151,7 +151,7 @@
 
             for (pixelConfig in getTelemetryConfigs()) {
                 if (repository.getPixelState(pixelConfig.name) == null) {
-                    startNewPeriod(pixelConfig)
+                    startNewPeriod(pixelConfig, skipForegroundCheck = true)
                 }
             }
         }
@@ -163,15 +163,19 @@
         if (!isFeatureEnabled()) {
             logcat(DEBUG) { "EventHub: feature disabled, clearing all pixel states" }
             cancelAllTimers()
-            repository.deleteAllPixelStates()
+            synchronized(this) {
+                repository.deleteAllPixelStates()
+            }
             return
         }
 
         val telemetry = getTelemetryConfigs()
         logcat(DEBUG) { "EventHub: onConfigChanged — feature enabled, ${telemetry.size} telemetry pixel(s) in config" }
-        for (pixelConfig in telemetry) {
-            if (repository.getPixelState(pixelConfig.name) == null) {
-                startNewPeriod(pixelConfig)
+        synchronized(this) {
+            for (pixelConfig in telemetry) {
+                if (repository.getPixelState(pixelConfig.name) == null) {
+                    startNewPeriod(pixelConfig)
+                }
             }
         }
     }
@@ -251,8 +255,8 @@
         }
     }
 
-    private fun startNewPeriod(pixelConfig: TelemetryPixelConfig) {
-        if (!foregroundStateProvider.isInForeground || !isFeatureEnabled() || !pixelConfig.isEnabled) {
+    private fun startNewPeriod(pixelConfig: TelemetryPixelConfig, skipForegroundCheck: Boolean = false) {
+        if ((!skipForegroundCheck && !foregroundStateProvider.isInForeground) || !isFeatureEnabled() || !pixelConfig.isEnabled) {
             logcat(VERBOSE) { "EventHub: skipping startNewPeriod for ${pixelConfig.name}" }
             return
         }

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

• ✅ Fixed: Stale timer coroutine can double-fire pixel after cancellation
  • Added expectedPeriodEndMillis parameter to scheduleFireTelemetry and a guard check inside the synchronized block that skips firing when the retrieved pixel state's periodEndMillis doesn't match, preventing stale timer coroutines from firing pixels belonging to a newly created period.

Or push these changes by commenting:

@cursor push 1bd83c3aa6

Preview (1bd83c3aa6)

diff --git a/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManager.kt b/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManager.kt
--- a/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManager.kt
+++ b/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManager.kt
@@ -146,7 +146,7 @@
                 if (nowMillis >= pixelState.periodEndMillis) {
                     fireTelemetry(pixelState)
                 } else {
-                    scheduleFireTelemetry(pixelState.pixelName, pixelState.periodEndMillis - nowMillis)
+                    scheduleFireTelemetry(pixelState.pixelName, pixelState.periodEndMillis - nowMillis, pixelState.periodEndMillis)
                 }
             }
 
@@ -179,7 +179,7 @@
         }
     }
 
-    fun scheduleFireTelemetry(pixelName: String, delayMillis: Long) {
+    fun scheduleFireTelemetry(pixelName: String, delayMillis: Long, expectedPeriodEndMillis: Long) {
         if (scheduledTimers.containsKey(pixelName)) {
             logcat(VERBOSE) { "EventHub: timer already scheduled for $pixelName, skipping" }
             return
@@ -201,6 +201,10 @@
                     scheduledTimers.remove(pixelName)
                     return@launch
                 }
+                if (pixelState.periodEndMillis != expectedPeriodEndMillis) {
+                    logcat(VERBOSE) { "EventHub: stale timer for $pixelName, period changed, skipping" }
+                    return@launch
+                }
                 fireTelemetry(pixelState)
             }
         }
@@ -273,7 +277,7 @@
             ),
         )
 
-        scheduleFireTelemetry(pixelConfig.name, periodMillis)
+        scheduleFireTelemetry(pixelConfig.name, periodMillis, nowMillis + periodMillis)
     }
 
     private fun buildPixel(pixelState: PixelState): Map<String, String> {

diff --git a/event-hub/event-hub-impl/src/test/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManagerTest.kt b/event-hub/event-hub-impl/src/test/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManagerTest.kt
--- a/event-hub/event-hub-impl/src/test/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManagerTest.kt
+++ b/event-hub/event-hub-impl/src/test/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManagerTest.kt
@@ -2012,16 +2012,16 @@
 
     @Test
     fun `scheduleFireTelemetry does not double-schedule same pixel`() {
-        manager.scheduleFireTelemetry("test_pixel", 5000L)
+        manager.scheduleFireTelemetry("test_pixel", 5000L, 0L)
         assertTrue(manager.hasScheduledTimer("test_pixel"))
 
-        manager.scheduleFireTelemetry("test_pixel", 10000L)
+        manager.scheduleFireTelemetry("test_pixel", 10000L, 0L)
         assertTrue(manager.hasScheduledTimer("test_pixel"))
     }
 
     @Test
     fun `cancelScheduledFire removes pending timer`() {
-        manager.scheduleFireTelemetry("test_pixel", 60_000L)
+        manager.scheduleFireTelemetry("test_pixel", 60_000L, 0L)
         assertTrue(manager.hasScheduledTimer("test_pixel"))
 
         manager.cancelScheduledFire("test_pixel")
@@ -2036,8 +2036,8 @@
 
     @Test
     fun `onConfigChanged cancels all timers when feature disabled`() {
-        manager.scheduleFireTelemetry("pixel_a", 60_000L)
-        manager.scheduleFireTelemetry("pixel_b", 120_000L)
+        manager.scheduleFireTelemetry("pixel_a", 60_000L, 0L)
+        manager.scheduleFireTelemetry("pixel_b", 120_000L, 0L)
         assertTrue(manager.hasScheduledTimer("pixel_a"))
         assertTrue(manager.hasScheduledTimer("pixel_b"))
 
@@ -2057,7 +2057,7 @@
         val state = pixelState("webTelemetry_testPixel1", mapOf("count" to 3), periodStart = periodStart, periodEnd = periodEnd)
         stubPixelStates(state)
 
-        manager.scheduleFireTelemetry("webTelemetry_testPixel1", 1000L)
+        manager.scheduleFireTelemetry("webTelemetry_testPixel1", 1000L, periodEnd)
         assertTrue(manager.hasScheduledTimer("webTelemetry_testPixel1"))
 
         manager.checkPixels()
@@ -2176,7 +2176,7 @@
 
     @Test
     fun `timer clears scheduledTimers entry when feature disabled`() {
-        manager.scheduleFireTelemetry("webTelemetry_testPixel1", 1000L)
+        manager.scheduleFireTelemetry("webTelemetry_testPixel1", 1000L, 0L)
         assertTrue(manager.hasScheduledTimer("webTelemetry_testPixel1"))
 
         whenever(selfToggle.isEnabled()).thenReturn(false)
@@ -2189,7 +2189,7 @@
 
     @Test
     fun `timer clears scheduledTimers entry when pixel state missing`() {
-        manager.scheduleFireTelemetry("webTelemetry_testPixel1", 1000L)
+        manager.scheduleFireTelemetry("webTelemetry_testPixel1", 1000L, 0L)
         assertTrue(manager.hasScheduledTimer("webTelemetry_testPixel1"))
 
         whenever(repository.getPixelState("webTelemetry_testPixel1")).thenReturn(null)
@@ -2202,7 +2202,7 @@
 
     @Test
     fun `timer cleanup allows rescheduling after feature disable and re-enable`() {
-        manager.scheduleFireTelemetry("webTelemetry_testPixel1", 1000L)
+        manager.scheduleFireTelemetry("webTelemetry_testPixel1", 1000L, 0L)
         assertTrue(manager.hasScheduledTimer("webTelemetry_testPixel1"))
 
         whenever(selfToggle.isEnabled()).thenReturn(false)
@@ -2211,7 +2211,7 @@
         assertFalse(manager.hasScheduledTimer("webTelemetry_testPixel1"))
 
         whenever(selfToggle.isEnabled()).thenReturn(true)
-        manager.scheduleFireTelemetry("webTelemetry_testPixel1", 5000L)
+        manager.scheduleFireTelemetry("webTelemetry_testPixel1", 5000L, 0L)
         assertTrue(manager.hasScheduledTimer("webTelemetry_testPixel1"))
     }
 
@@ -2287,7 +2287,7 @@
 
     @Test
     fun `timer cancelled between delay and fire does not execute fireTelemetry`() {
-        manager.scheduleFireTelemetry("webTelemetry_testPixel1", 5000L)
+        manager.scheduleFireTelemetry("webTelemetry_testPixel1", 5000L, 0L)
         assertTrue(manager.hasScheduledTimer("webTelemetry_testPixel1"))
 
         // Advance time so delay completes, but cancel before the scheduler runs the continuation

[GuiltyDolphin]

@cmonfortep that should be all the existing comments addressed! Switched to using Toggle and avoided the re-parsing on each event. Tidied up the module structure.

Had a few rounds of bugbot feedback. Some false positives, caught a couple of valid, if unlikely, race conditions.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

• ✅ Fixed: Cache invalidation race outside synchronized block
  • Moved cachedTelemetryConfigs = null inside the synchronized(this) block in onConfigChanged() so cache invalidation and re-population are atomic, preventing concurrent threads from repopulating the cache with stale config.

Or push these changes by commenting:

@cursor push 577186fe2f

Preview (577186fe2f)

diff --git a/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManager.kt b/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManager.kt
--- a/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManager.kt
+++ b/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManager.kt
@@ -159,9 +159,8 @@
     }
 
     override fun onConfigChanged() {
-        cachedTelemetryConfigs = null
-
         synchronized(this) {
+            cachedTelemetryConfigs = null
             if (!isFeatureEnabled()) {
                 logcat(DEBUG) { "EventHub: feature disabled, clearing all pixel states" }
                 cancelAllTimers()

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

• ✅ Fixed: Lifecycle observer ordering may prevent new period startup
  • Added an assumeForeground flag threaded through checkPixels() -> fireTelemetry() -> startNewPeriod() so that the foreground-triggered path bypasses the racy isInForeground guard, while the timer-fired path retains the guard.

Or push these changes by commenting:

@cursor push 43116cd0b9

Preview (43116cd0b9)

diff --git a/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManager.kt b/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManager.kt
--- a/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManager.kt
+++ b/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManager.kt
@@ -144,7 +144,7 @@
         synchronized(this) {
             for (pixelState in repository.getAllPixelStates()) {
                 if (nowMillis >= pixelState.periodEndMillis) {
-                    fireTelemetry(pixelState)
+                    fireTelemetry(pixelState, assumeForeground = true)
                 } else {
                     scheduleFireTelemetry(pixelState.pixelName, pixelState.periodEndMillis - nowMillis)
                 }
@@ -152,7 +152,7 @@
 
             for (pixelConfig in getTelemetryConfigs()) {
                 if (repository.getPixelState(pixelConfig.name) == null) {
-                    startNewPeriod(pixelConfig)
+                    startNewPeriod(pixelConfig, assumeForeground = true)
                 }
             }
         }
@@ -228,7 +228,7 @@
         scheduledTimers.clear()
     }
 
-    private fun fireTelemetry(pixelState: PixelState) {
+    private fun fireTelemetry(pixelState: PixelState, assumeForeground: Boolean = false) {
         cancelScheduledFire(pixelState.pixelName)
 
         val pixelData = buildPixel(pixelState)
@@ -254,12 +254,12 @@
 
         val latestPixelConfig = getTelemetryConfigs().find { it.name == pixelState.pixelName }
         if (latestPixelConfig != null) {
-            startNewPeriod(latestPixelConfig)
+            startNewPeriod(latestPixelConfig, assumeForeground = assumeForeground)
         }
     }
 
-    private fun startNewPeriod(pixelConfig: TelemetryPixelConfig) {
-        if (!foregroundStateProvider.isInForeground || !isFeatureEnabled() || !pixelConfig.isEnabled) {
+    private fun startNewPeriod(pixelConfig: TelemetryPixelConfig, assumeForeground: Boolean = false) {
+        if (!(assumeForeground || foregroundStateProvider.isInForeground) || !isFeatureEnabled() || !pixelConfig.isEnabled) {
             logcat(VERBOSE) { "EventHub: skipping startNewPeriod for ${pixelConfig.name}" }
             return
         }

diff --git a/event-hub/event-hub-impl/src/test/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManagerTest.kt b/event-hub/event-hub-impl/src/test/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManagerTest.kt
--- a/event-hub/event-hub-impl/src/test/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManagerTest.kt
+++ b/event-hub/event-hub-impl/src/test/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManagerTest.kt
@@ -2357,7 +2357,7 @@
     // --- foreground-gated cycles ---
 
     @Test
-    fun `timer fires while backgrounded - pixel enqueued but no new period started`() {
+    fun `checkPixels fires elapsed pixel and starts new period even if foreground state not yet set`() {
         foregroundState.isInForeground = false
 
         val periodStart = 1000L
@@ -2382,7 +2382,10 @@
             type = eq(Count),
         )
         verify(repository).deletePixelState("webTelemetry_testPixel1")
-        verify(repository, never()).savePixelState(any())
+        val captor = argumentCaptor<PixelState>()
+        verify(repository).savePixelState(captor.capture())
+        assertEquals("webTelemetry_testPixel1", captor.firstValue.pixelName)
+        assertEquals(timeProvider.time, captor.firstValue.periodStartMillis)
     }
 
     @Test
@@ -2464,7 +2467,7 @@
     }
 
     @Test
-    fun `full background-foreground lifecycle - fire in background, recover on foreground`() {
+    fun `full background-foreground lifecycle - checkPixels fires and starts new period immediately`() {
         foregroundState.isInForeground = false
 
         val periodStart = 1000L
@@ -2484,20 +2487,6 @@
 
         verify(pixel).enqueueFire(any<String>(), any(), any(), any())
         verify(repository).deletePixelState("webTelemetry_testPixel1")
-        verify(repository, never()).savePixelState(any())
-
-        org.mockito.Mockito.reset(repository, pixel, selfToggle)
-        whenever(eventHubFeature.self()).thenReturn(selfToggle)
-        whenever(selfToggle.isEnabled()).thenReturn(true)
-        whenever(selfToggle.getSettings()).thenReturn(fullConfig)
-        whenever(repository.getAllPixelStates()).thenReturn(emptyList())
-        whenever(repository.getPixelState("webTelemetry_testPixel1")).thenReturn(null)
-
-        foregroundState.isInForeground = true
-        timeProvider.time = periodEnd + 5000L
-
-        manager.checkPixels()
-
         val captor = argumentCaptor<PixelState>()
         verify(repository).savePixelState(captor.capture())
         assertEquals("webTelemetry_testPixel1", captor.firstValue.pixelName)

[cmonfortep]

There's still lot of code to fix here, main focus is to revisit those Synchronized which should be avoided.

Haven't tested this as we are still iterating on the implementation. Please also revisit what can be tested, I see a lot of classes in the impl module but only 3 test files. Ensure what can be tested has a test. Also try to use the test rules we have, and other infra we use in other parts of the code.

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Autofix Details

Bugbot Autofix prepared fixes for both issues found in the latest run.

• ✅ Fixed: Lifecycle observer ordering race with foreground state
  • Added skipForegroundCheck parameter to startNewPeriod and passed true from checkPixels(), since checkPixels is only called on foreground and the foreground state provider may not have been notified yet due to non-deterministic multibinding ordering.
• ✅ Fixed: Redundant coroutine launch wrapping checkPixels call
  • Removed the redundant outer appCoroutineScope.launch(IO) wrapper and its unused dependencies from EventHubLifecycleObserver.onStart, calling checkPixels() directly since it already internally launches on IO.

Or push these changes by commenting:

@cursor push bbfd638b6d

Preview (bbfd638b6d)

diff --git a/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/EventHubLifecycleObserver.kt b/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/EventHubLifecycleObserver.kt
--- a/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/EventHubLifecycleObserver.kt
+++ b/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/EventHubLifecycleObserver.kt
@@ -18,14 +18,10 @@
 
 import androidx.annotation.UiThread
 import androidx.lifecycle.LifecycleOwner
-import com.duckduckgo.app.di.AppCoroutineScope
 import com.duckduckgo.app.lifecycle.MainProcessLifecycleObserver
-import com.duckduckgo.common.utils.DispatcherProvider
 import com.duckduckgo.di.scopes.AppScope
 import com.duckduckgo.eventhub.impl.pixels.EventHubPixelManager
 import com.squareup.anvil.annotations.ContributesMultibinding
-import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.launch
 import logcat.LogPriority.VERBOSE
 import logcat.logcat
 import javax.inject.Inject
@@ -39,15 +35,11 @@
 )
 class EventHubLifecycleObserver @Inject constructor(
     private val pixelManager: EventHubPixelManager,
-    private val dispatcherProvider: DispatcherProvider,
-    @AppCoroutineScope private val appCoroutineScope: CoroutineScope,
 ) : MainProcessLifecycleObserver {
 
     @UiThread
     override fun onStart(owner: LifecycleOwner) {
         logcat(VERBOSE) { "EventHub: app foregrounded, checking pixels" }
-        appCoroutineScope.launch(dispatcherProvider.io()) {
-            pixelManager.checkPixels()
-        }
+        pixelManager.checkPixels()
     }
 }

diff --git a/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManager.kt b/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManager.kt
--- a/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManager.kt
+++ b/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManager.kt
@@ -159,7 +159,7 @@
 
                 for (pixelConfig in getTelemetryConfigs()) {
                     if (repository.getPixelState(pixelConfig.name) == null) {
-                        startNewPeriod(pixelConfig)
+                        startNewPeriod(pixelConfig, skipForegroundCheck = true)
                     }
                 }
             }
@@ -267,8 +267,8 @@
         }
     }
 
-    private fun startNewPeriod(pixelConfig: TelemetryPixelConfig) {
-        if (!foregroundStateProvider.isInForeground || !isFeatureEnabled() || !pixelConfig.isEnabled) {
+    private fun startNewPeriod(pixelConfig: TelemetryPixelConfig, skipForegroundCheck: Boolean = false) {
+        if ((!skipForegroundCheck && !foregroundStateProvider.isInForeground) || !isFeatureEnabled() || !pixelConfig.isEnabled) {
             logcat(VERBOSE) { "EventHub: skipping startNewPeriod for ${pixelConfig.name}" }
             return
         }

[cmonfortep]

Nothing major to flag. All good code wise.

I'm doing testing, and has been working well using your config.

I want to do another round to test enabled/disabled state, but nothing else apart from that. 👍

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Bugbot Autofix prepared fixes for both issues found in the latest run.

• ✅ Fixed: Inconsistent boundary check for period end comparison
  • Changed > to >= on line 132 in handleWebEvent so that the period end boundary check is consistent with processPixelStates which uses >=.
• ✅ Fixed: Dedup side-effect persists even when counter doesn't increment
  • Moved isDuplicateEvent call after the shouldStopCounting check so the dedup set side-effect only occurs when the event will actually be counted.

Or push these changes by commenting:

@cursor push 0045d9e558

Preview (0045d9e558)

diff --git a/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManager.kt b/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManager.kt
--- a/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManager.kt
+++ b/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/pixels/EventHubPixelManager.kt
@@ -129,7 +129,7 @@
             val nowMillis = timeProvider.currentTimeMillis()
 
             for (pixelState in repository.getAllPixelStates()) {
-                if (nowMillis > pixelState.periodEndMillis) continue
+                if (nowMillis >= pixelState.periodEndMillis) continue
 
                 val updatedParams = pixelState.params.toMutableMap()
                 var changed = false
@@ -138,15 +138,17 @@
                     if (paramConfig.isCounter && paramConfig.source == eventType) {
                         val paramState = updatedParams[paramName] ?: ParamState(0)
                         if (paramState.stopCounting) continue
-                        if (isDuplicateEvent(pixelState.pixelName, paramName, eventType, webViewId)) continue
 
-                        changed = true
                         if (BucketCounter.shouldStopCounting(paramState.value, paramConfig.buckets)) {
+                            changed = true
                             updatedParams[paramName] = paramState.copy(stopCounting = true)
                             logcat(VERBOSE) { "EventHub: ${pixelState.pixelName}.$paramName already at max bucket, stopCounting" }
                             continue
                         }
 
+                        if (isDuplicateEvent(pixelState.pixelName, paramName, eventType, webViewId)) continue
+
+                        changed = true
                         val newValue = paramState.value + 1
                         updatedParams[paramName] = paramState.copy(value = newValue)
                         logcat(VERBOSE) { "EventHub: ${pixelState.pixelName}.$paramName incremented to $newValue" }

[cmonfortep]

Just one last comment about an ANR risk. Please fix that one.

Tested this with the provided config, haven't seen anything wrong.
Tested disabling, and correctly stop sending / scheduling.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix prepared a fix for the issue found in the latest run.

• ✅ Fixed: Asynchronous config store races persisted callback
  • Replaced async appCoroutineScope.launch with runBlocking so setWebEventsConfigJson completes synchronously before store() returns, matching the PrivacyFeaturePlugin contract.

Or push these changes by commenting:

@cursor push d9fc5e8fac

Preview (d9fc5e8fac)

diff --git a/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/webevents/WebEventsFeaturePlugin.kt b/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/webevents/WebEventsFeaturePlugin.kt
--- a/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/webevents/WebEventsFeaturePlugin.kt
+++ b/event-hub/event-hub-impl/src/main/java/com/duckduckgo/eventhub/impl/webevents/WebEventsFeaturePlugin.kt
@@ -16,25 +16,22 @@
 
 package com.duckduckgo.eventhub.impl.webevents
 
-import com.duckduckgo.app.di.AppCoroutineScope
 import com.duckduckgo.common.utils.DispatcherProvider
 import com.duckduckgo.di.scopes.AppScope
 import com.duckduckgo.privacy.config.api.PrivacyFeaturePlugin
 import com.squareup.anvil.annotations.ContributesMultibinding
-import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.launch
+import kotlinx.coroutines.runBlocking
 import javax.inject.Inject
 
 @ContributesMultibinding(AppScope::class)
 class WebEventsFeaturePlugin @Inject constructor(
     private val webEventsDataStore: WebEventsDataStore,
-    @AppCoroutineScope private val appCoroutineScope: CoroutineScope,
     private val dispatcherProvider: DispatcherProvider,
 ) : PrivacyFeaturePlugin {
 
     override fun store(featureName: String, jsonString: String): Boolean {
         if (featureName == this.featureName) {
-            appCoroutineScope.launch(dispatcherProvider.io()) {
+            runBlocking(dispatcherProvider.io()) {
                 webEventsDataStore.setWebEventsConfigJson(jsonString)
             }
             return true

diff --git a/event-hub/event-hub-impl/src/test/java/com/duckduckgo/eventhub/impl/webevents/WebEventsFeaturePluginTest.kt b/event-hub/event-hub-impl/src/test/java/com/duckduckgo/eventhub/impl/webevents/WebEventsFeaturePluginTest.kt
--- a/event-hub/event-hub-impl/src/test/java/com/duckduckgo/eventhub/impl/webevents/WebEventsFeaturePluginTest.kt
+++ b/event-hub/event-hub-impl/src/test/java/com/duckduckgo/eventhub/impl/webevents/WebEventsFeaturePluginTest.kt
@@ -39,7 +39,6 @@
 
     private val plugin = WebEventsFeaturePlugin(
         webEventsDataStore = dataStore,
-        appCoroutineScope = coroutineTestRule.testScope,
         dispatcherProvider = coroutineTestRule.testDispatcherProvider,
     )