You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Adds a detectorPerf feature that measures on-device detector execution cost (bot/fraud/adwall utils and config-driven webDetection scans) via a synchronous timeDetector wrapper. Per-page counters (runs, worst single run, per-detector total, combined total) drive threshold-crossing events fired at occurrence through webEvents (at most once per page per event type), with a capped detectorPerf_severe immediate event when a run crosses the highest configured edge. Exact per-detector stats attach to breakage reports. Threshold edges come from remote config (privacy-configuration#5878); nothing is page-observable in production (no performance.mark/measure, no DOM reads). A debug-flag-gated detectorPerfDebugStats CustomEvent feeds live overlays on the detectorPerf test pages (already live).
Testing Steps
Automated (covers all behavior):
cd injected
npm run test-unit # thresholds, at-most-once, severe cap, debug gating, subframe suppression
npx playwright test integration-test/detector-perf.spec.js # 9 end-to-end tests
The integration tests cover: occurrence events fired once per page, severe with exact detector attribution, breakage-report stats (including the fire-and-forget ordering guard), no page-observable timeline entries, debug broadcast on/off, disabled-state emits nothing, and top-frame-only measured.
Manual end-to-end (macOS debug build, optional):
Point the macOS app at this branch's build (SPM: pr-releases/detector-perf-monitoring, see build-branch comment below) and build the Debug scheme.
Serve a config with detectorPerf + eventHub telemetry enabled: check out privacy-configuration#5878, build it, serve the generated macOS config locally, and set it via Debug → Remote Configuration URL. For deterministic severe events, lower singleRunThresholdsMs to [0.001] in the served config.
Enable Debug → Content Scope Scripts Debug State, then relaunch the app.
Visit the live test pages (both render a live per-detector timing overlay, bottom right):
Typical page — baseline: realistic ~1,700-element article; all timings ~1ms, no crossings expected.
Heavy page — stress: timings climb with DOM size; with lowered edges the overlay shows SEVERE entries.
Verify native events: /usr/bin/log stream --process DuckDuckGo --level debug | grep -i eventhub shows period routing for each event and immediate telemetry for severe.
Production invisibility: with the debug flag off, the overlay stays in its waiting state and the page can observe nothing (also covered by integration tests).
Checklist
Please tick all that apply:
I have tested this change locally
I have tested this change locally in all supported browsers
This change will be visible to users
I have added automated tests that cover this change
I have ensured the change is gated by config
This change was covered by a ship review
This change was covered by a tech design
Any dependent config has been merged
Note
Medium Risk
Adds synchronous timing and telemetry on the Web Detection hot path when config enables the feature; failures are swallowed but misconfigured thresholds could increase event volume until capped.
Overview
Introduces a remote-config-gated detectorPerf feature that times synchronous Web Detection runs via timeDetector, emits threshold-crossing detectorPerf_* events through webEvents (including capped immediate detectorPerf_severe), and attaches exact per-detector stats to breakage reports. webDetection wraps evaluateMatch when detectorPerf is present in config; on-demand bot/fraud/YouTube detectors are intentionally not instrumented.
The feature is registered as a platform-specific dependency (still active when site protections are off) and documented in detector-performance.md. Tests add a large unit suite, Playwright detector-perf.spec.js, and small harness fixes (breakage-reporting waits, Playwright console.warning fallback, performanceNow in captured globals).
Reviewed by Cursor Bugbot for commit fb73f8f. Bugbot is set up for automated code reviews on this repo. Configure here.
Times every detector invocation (bot/fraud/adwall utils and config-driven
webDetection scans) with a synchronous fire-and-forget wrapper, accumulates
per-page counters (run count, worst single run, total, combined), and flushes
threshold-crossing events through webEvents when the page is hidden — at most
once per event type per page. Threshold bin edges come from remote config
with code fallbacks. Nothing is emitted per run and no page-observable
performance marks are created.
Co-authored-by: Cursor <cursoragent@cursor.com>
…d exact stats for breakage reports
- Emit measured/ran/threshold events as soon as they first become true
instead of flushing on page-hidden, so killed processes lose nothing
- Add detectorPerf_severe immediate event when a run crosses the highest
configured edge, capped by maxSeverePerPage, with exact detector
attribution (config IDs) in the data payload
- Expose getStats() and attach exact per-detector timings to breakage
report payloads
- Capture performance.now at module load; own-property check on
config-supplied threshold overrides
Co-authored-by: Cursor <cursoragent@cursor.com>
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
File
Lines
Severity
Finding
injected/src/features/detector-perf.js
371–376
info
timeDetector preserves the synchronous detector contract: it returns fn()'s value immediately and only fires reportDuration fire-and-forget. No await on the hot path, so callers that expect sync return values (e.g. breakage-report detector utils) are unchanged.
injected/src/features/web-detection.js
64–78
info
_evaluateMatch keeps the existing try/catch → 'error' behaviour inside the wrapped fn; only timing is added around it.
injected/src/features/detector-perf.js
89–90, 170–243
info
Measurement deliberately avoids DOM/layout reads and performance.mark/measure; integration test (measurement leaves no page-observable performance timeline entries) verifies no detector-specific timeline pollution.
injected/src/features/breakage-reporting.js
62–63, 89
info
timeDetector adds two performance.now reads per wrapped detector call. Cost is negligible relative to DOM-based detectors, but it runs even when detectorPerf is disabled (recording is silently dropped).
injected/src/features/breakage-reporting.js
81–88
warning
Breakage-report getStats ordering relies on fire-and-forget reportDuration completing before await getStats(). This works in practice when detectorPerf._ready is already settled (typical at user-initiated report time) because record runs synchronously after the await. If _ready were still pending, void reportDuration could race getStats and produce an incomplete snapshot. Integration test covers the happy path; the FIFO-on-_ready rationale is fragile if init ordering changes.
injected/src/captured-globals.js
20
info
performanceNow is captured without optional chaining (performance.now.bind(performance)). A missing performance API would throw at module load and break the entire bundle. All target WebViews/extension contexts provide performance; risk is theoretical.
injected/src/features.js
platformSupport
info
detectorPerf is enabled only on apple-isolated, android, and windows — not extension/firefox/chrome builds. Extension detector paths are unaffected.
Security Assessment
File
Lines
Severity
Finding
injected/src/captured-globals.js
20
info
New performanceNow capture follows the established .bind(owner) pattern and is consumed only from detector-perf.js, reducing bypass risk from page-tampered performance.now throwing during detector execution.
injected/src/features/detector-perf.js
174, 195–196, 205
info
hasOwnProperty.call on detectorOverrides blocks prototype-chain keys like constructor. NAME_PATTERN rejects malformed name/detail before they reach event-type construction or severe payloads.
injected/src/features/detector-perf.js
57–61, 159–162
info
parseThresholds and maxSeverePerPage sanitize config input (finite positive numbers only; severe cap floors to positive integer). maxSeverePerPage limits blast radius from a bad threshold push.
injected/src/features/detector-perf.js
333–342
info
_dispatch passes explicit { type } or { type, data } to webEvents.fireEvent — no spread of untrusted objects, so nativeData cannot be forwarded.
injected/src/features/detector-perf.js
96–111
info
Uses uncaptured native Map/Set (not imported from captured-globals.js). At document-start load this is low risk; consistent with many other features but not maximal hardening.
injected/src/features/detector-perf.js
131–138, integration test
info
Feature is remote-config gated (state: disabled emits nothing and omits detectorPerf from breakage payloads). Rollback path exists without code deploy.
injected/src/features/breakage-reporting.js
89–91
info
detectorPerf stats attach only to user-initiated breakage reports via existing breakageData encoding — no new postMessage or cross-frame channel.
Risk Level
Medium Risk — Adds a config-gated telemetry feature and a minimal captured-globals capture, wrapping existing detector call sites with sync timing overhead but no browser API overrides, prototype patches, messaging trust-boundary changes, or message-bridge modifications.
Recommendations
warning — breakage-report ordering: Before getStats, explicitly drain pending recordings (e.g. await reportDuration for breakage-report timeDetector calls, or a detectorPerf.flush() exposed method) instead of relying on _ready microtask ordering.
info — performanceNow capture: Add optional chaining or a safe fallback (e.g. Date.now) so a missing performance API cannot fail module load for the whole bundle.
info — disabled overhead: Optionally skip performanceNow in timeDetector when detectorPerf is disabled/skipped (feature-setting check on the caller side) to avoid unnecessary work on every detector invocation fleet-wide.
info — captured intrinsics: Import Map/Set from captured-globals.js for consistency with global-capture hygiene.
info — tests (already strong): Integration coverage for disabled state, non-observable timeline, severe attribution, and breakage-report payload shape is good; consider one unit test that simulates _ready still pending when timeDetector fires to document/lock the ordering assumption.
When the platform debug flag is set, detectorPerf dispatches a
detectorPerfDebugStats CustomEvent on window after every recorded run,
carrying the exact stats snapshot as a JSON-string detail (primitives
cross isolated-world boundaries; objects do not on Chromium). Test
pages render this as a live per-detector timing overlay for human
testing. Production builds never set the flag, so the branch is inert
and the page-observability invariant holds for users.
Also fixes the integration harness console forwarder to tolerate
Playwright console types that don't exist on Node's console (e.g.
'warning'), which the new production-mode test surfaces.
Co-authored-by: Cursor <cursoragent@cursor.com>
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Injected PR Evaluation: Web Compatibility & Security
Re-assessed at 4ac25bc3 (adds debug-only detectorPerfDebugStats page broadcast). Prior findings on breakage-report ordering and performanceNow capture still apply; new findings below focus on the debug broadcast path.
Web Compatibility Assessment
File
Lines
Severity
Finding
injected/src/features/detector-perf.js
354–366
warning
_debugBroadcast dispatches a page-observable CustomEvent when isDebug is true, partially relaxing the "measurement must never be page-observable" invariant documented at lines 101–105. Integration tests confirm production (debug: false) builds emit nothing, but any platform that sets args.debug in non-test builds would expose live detector timing to page scripts — including hostile pages that register listeners.
injected/src/features/detector-perf.js
426–431
info
timeDetector wraps detector calls with two performanceNow() reads. Overhead is negligible and synchronous; no return-value or Promise contract changes.
injected/src/features/breakage-reporting.js
81–91
warning
getStats snapshot relies on fire-and-forget reportDuration microtasks completing before await callFeatureMethod('detectorPerf', 'getStats'). Microtask FIFO ordering makes this work today, but the _ready-promise comment is misleading — this is fragile if timeDetector ever becomes async or if callFeatureMethod gains concurrent dispatch. Integration test breakage reports carry exact per-detector timing stats provides coverage.
injected/src/features/web-detection.js
64–78
info
_evaluateMatch now routes through timeDetector inside existing try/catch; return type and error handling unchanged.
injected/src/captured-globals.js
20
info
performanceNow binds globalThis.performance.now without optional chaining (unlike dispatchEvent). Safe at document-start in target WebViews, but inconsistent with neighboring captures.
Security Assessment
File
Lines
Severity
Finding
injected/src/features/detector-perf.js
354–366
warning
Debug broadcast uses an unsecreted CustomEvent name (detectorPerfDebugStats) dispatched via captured dispatchEvent/CustomEvent. Unlike message-bridge, there is no messageSecret — acceptable only because isDebug gates it, but a platform misconfiguration setting debug: true in release would let any page script observe per-detector run counts, durations, and severe-crossing attribution. Payload is JSON-stringified (good for isolated-world boundary), but the string is page-readable on Chromium.
injected/src/features/detector-perf.js
3, 426–431
info
Uses captured performanceNow, CustomEvent, dispatchEvent, and hasOwnProperty. Map/Set are instantiated from uncaptured globals — low risk at document-start. No nativeData leakage; telemetry goes through typed webEvents.fireEvent params.
injected/src/features/detector-perf.js
219–221, 735–736
info
Input validation on record() rejects malformed names/durations; hasOwnProperty guards config detectorOverrides against prototype-pollution keys like constructor.
injected/src/features/breakage-reporting.js
89–91
info
detectorPerf stats attached to user-initiated breakage reports only; gracefully omitted when feature disabled or returns CallFeatureMethodError.
injected/src/features.js
41, 77, 92, 122
info
Feature registered on apple/android/windows/extension platform lists — remotely disableable via detectorPerf config state.
Risk Level
Medium Risk — New telemetry feature with config gating and strong test coverage; no API overrides or messaging-boundary changes. The debug-only page broadcast is the main new surface area, but it is inert in production builds.
Recommendations
(warning) Add a brief platform-build comment or assertion near _debugBroadcast documenting that args.debug must never be set in release browser builds — the integration test covers the negative path, but native entry-points should be the source of truth.
(warning) Consider making reportDuration awaitable from the breakage-report path (or snapshot synchronously inside timeDetector before returning) to remove the implicit microtask-ordering dependency for getStats.
(info) Align performanceNow capture with optional chaining: globalThis.performance?.now?.bind(globalThis.performance) for consistency with dispatchEvent.
(info) Import captured Map/Set in detector-perf.js for consistency with global-capture hygiene.
No API shims, message-bridge changes, or load()-time patches detected. Test coverage is thorough: unit tests for threshold/severe/at-most-once logic, integration tests for disabled state, timeline invisibility, severe attribution, breakage payload, and debug-broadcast gating.
The feature initializes in every injected subframe, but detectors are
top-frame-only by default, so each iframe inflated the pages-measured
denominator without ever contributing crossings. Gate the measured emit
to the top frame; recording stays frame-agnostic.
Co-authored-by: Cursor <cursoragent@cursor.com>
…nvariant
- performanceNow captured with optional chaining + Date.now fallback so a
missing performance API cannot throw at module load
- Import Map/Set from captured-globals for capture consistency
- Document the release-builds-must-not-set-debug invariant at _debugBroadcast
- Integration test: a second breakage report must show exactly one more run
for the standalone detectors, locking the getStats fire-and-forget
ordering assumption
Co-authored-by: Cursor <cursoragent@cursor.com>
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Injected PR Evaluation: Web Compatibility & Security
Assessed synchronize commit 3372d543 ("Address detector performance review feedback"). This is a material delta since the prior assessment at 1f03b877 (merge-only sync).
Web Compatibility Assessment
File
Lines
Severity
Finding
injected/src/features/detector-perf.js
136–140, 450–487
info
No browser API overrides, DOM reads, or prototype patches. timeDetector is a synchronous wrapper around detector call sites; when detectorPerf is disabled or absent, wrapped detectors behave identically to unwrapped ones.
injected/src/features/web-detection.js
60–65
info
Error handling preserves prior semantics: exceptions from evaluateMatch are caught and returned as 'error', while timeDetector now records failed runs before the catch. No change to detector return contracts visible to callers.
injected/src/features/breakage-reporting.js
80–84
warning
getStats() follows runDetectors via fire-and-forget reportDuration (void + async callFeatureMethod). Microtask FIFO ordering should drain record() before getStats(), but the explicit ordering-guard integration test was removed in this commit when bot/fraud timing was dropped. Residual risk is telemetry/breakage-report accuracy, not site breakage.
injected/src/features/detector-perf.js
188–199
info
Top-frame-only detectorPerf_measured guard prevents iframe inflation of the page denominator — correct cross-frame behavior.
injected/src/features/breakage-reporting.js
60–66
info
Removing bot/fraud/YouTube from timeDetector instrumentation eliminates extra synchronous work on the breakage-report path — positive for on-demand report latency.
injected/src/features/detector-perf.js
462–466
info
performance.now is captured at module load; page clock poisoning is acknowledged and mitigated by record() input validation (non-finite/negative durations rejected).
Security Assessment
File
Lines
Severity
Finding
injected/src/captured-globals.js
22, 28
info
Map/Set and performanceNow (with Date.now fallback) added to captured globals — correct hygiene for a feature that must not read page-tampered builtins.
injected/src/features/detector-perf.js
395–414
warning
Debug-only detectorPerfDebugStatsCustomEvent is page-observable when isDebug is set, exposing per-detector timing and severe attribution. The INVARIANT comment (L395–398) correctly documents that production builds must never enable args.debug on user pages.
injected/src/features/detector-perf.js
236–243
info
hasOwnProperty.call on detectorOverrides prevents prototype-chain keys (e.g. constructor) from affecting threshold resolution.
injected/src/features/detector-perf.js
34, 87–106
info
Periodic event names are bounded to DETECTOR_PERF_DETECTOR_NAMES via getDetectorPerfEventTypes(); config-driven detector IDs appear only in severe payloads and breakage-report data (not in dynamic event-type strings).
injected/src/features/detector-perf.js
376–386
info
Severe events use explicit { kind, detector, thresholdMs } payloads — no nativeData spread, no page-controlled event type construction.
injected/src/features/detector-perf.js
—
info
Feature is remote-config gated; disabled state is a no-op with graceful callFeatureMethod error handling. No messaging trust-boundary, origin-validation, or postMessage changes.
Risk Level
Medium Risk — New config-gated telemetry feature with cross-feature timing instrumentation and minor captured-globals additions, but no API shims, messaging transport changes, or page-world security-boundary relaxations.
Recommendations
Restore a webDetection ordering guard (info): Re-add an integration test that issues two consecutive breakage reports and asserts detectorPerf.detectors[<id>].runs increments — the bot/fraud variant was removed in 3372d543 but the same fire-and-forget pattern still applies to webDetection.
Privacy-config parity (info): Confirm EventHub source entries are updated for the pooled webDetection event namespace (replacing per-detector bot/fraud types) and that getDetectorPerfEventTypes() is mirrored in privacy-configuration contract tests.
Debug flag enforcement (info): The detectorPerfDebugStats exposure surface is acceptable given the INVARIANT; no code change needed if native release pipelines already guarantee args.debug is never set on user pages.
Assessed at:eb20dd06ac (synchronize, 2026-09-08) Delta since prior review (3372d543): commit eb20dd06ac removes detectorPerf from platformSpecificFeatures, restoring proper remote-config gating.
Web Compatibility Assessment
File
Lines
Severity
Finding
injected/src/features/detector-perf.js
476–487
info
timeDetector is a synchronous wrapper around detector invocations. No browser API overrides, no toString() masking concerns, and no prototype mutations. When detectorPerf is disabled, wrapped calls are behavior-identical to unwrapped ones.
injected/src/features/web-detection.js
60–65
info
Only config-driven webDetection detectors are instrumented. Bot/fraud/YouTube timing was intentionally removed, reducing breakage-report latency and avoiding pooled-label ambiguity.
injected/src/features/detector-perf.js
197–199
info
detectorPerf_measured fires only in the top frame (window.self === window.top), preventing iframe inflation of the page denominator.
injected/src/features/breakage-reporting.js
80–84
warning
getStats() follows await runDetectors(). record() delivery is fire-and-forget via void reportDuration(), so microtask ordering determines whether stats include the just-finished report flow. Integration test validates first-report stats; the prior bot/fraud ordering-guard test was removed when those detectors were de-instrumented. Low user impact (telemetry-only), but worth monitoring.
injected/integration-test/detector-perf.spec.js
188–222
info
Confirms no performance.mark/measure entries from measurement path (framework lifecycle marks excluded). Measurement is not page-observable in production.
Security Assessment
File
Lines
Severity
Finding
injected/unit-test/utils.js
1020–1023
info
Resolved since prior review:detectorPerf is correctly absent from platformSpecificFeatures. The feature now requires explicit remote-config enablement and can be disabled per-domain — matching the config-gated telemetry pattern.
injected/src/features/detector-perf.js
3, 22, 28
info
Captured globals hygiene: Map, Set, performanceNow, CustomEvent, dispatchEvent imported from captured-globals.js. performanceNow uses optional chaining + Date.now fallback so a missing performance API cannot throw at module load.
injected/src/features/detector-perf.js
239
info
hasOwnProperty.call() on detectorOverrides prevents prototype-chain keys like constructor from being treated as overrides.
injected/src/features/detector-perf.js
87–106
info
getDetectorPerfEventTypes() enumerates a bounded, statically-known event set. Config-driven detector IDs are pooled under webDetection for periodic events; exact IDs appear only in breakage reports and severe-event payloads.
injected/src/features/detector-perf.js
403–411
warning
Debug-only detectorPerfDebugStatsCustomEvent exposes timing stats and severe-crossing attribution to any page listener when isDebug is set. INVARIANT documented at L395–398. Production safety depends on native never setting args.debug on user pages.
injected/src/features/detector-perf.js
437–446
info
Event dispatch via callFeatureMethod('webEvents', 'fireEvent', …) — no direct postMessage, no nativeData forwarding, failures silently swallowed.
injected/src/captured-globals.js
22
info
performanceNow added to captured globals (module-load snapshot).
Risk Level
Medium Risk — New config-gated telemetry feature with captured globals and debug-only page-observable events, but no API overrides, messaging transport changes, or message-bridge modifications.
Recommendations
Confirm privacy-config EventHub sources are updated for pooled webDetection event types (getDetectorPerfEventTypes() contract) and that stale detectorPerf_* sources are removed.
Verify native release builds never set args.debug on user pages (sole gate for detectorPerfDebugStats).
Optional: Add an integration assertion that a second breakage report shows incremented webDetection run counts in getStats(), replacing the removed bot/fraud ordering-guard test now that only webDetection is instrumented.
Tests: 44/44 unit specs pass in detector-perf.spec.js.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Injected PR Evaluation: Web Compatibility & Security
Delta since prior review (eb20dd06ac): Commit 2914bc4223 re-adds detectorPerf to platformSpecificFeatures (so telemetry survives user-disabled protections) and introduces an explicit #active gate keyed on hasOwnProperty(featureSettings, 'detectorPerf'). New unit + integration tests cover both the unprotected-domain and absent-from-config paths.
Web Compatibility Assessment
File
Lines
Severity
Finding
injected/src/features/detector-perf.js
L490–500
info
timeDetector is a synchronous wrapper around detector invocations; no browser API overrides, no observable page surface in production.
injected/src/features/detector-perf.js
L207–209
info
detectorPerf_measured fires only in the top frame, preventing iframe inflation of the page denominator.
injected/src/features/breakage-reporting.js
L63–65
info
Bot/fraud/YouTube detectors run without timeDetector instrumentation, reducing breakage-report latency vs. prior bot/fraud timing.
injected/src/features/breakage-reporting.js
L82–84
warning
getStats is awaited after fire-and-forget reportDuration from webDetection; a theoretical ordering race remains for webDetection-only stats. Bot/fraud de-instrumentation mitigates the prior concern.
injected/src/features/detector-perf.js
L196–198
info
When absent from enabled remote config, init() returns before any events or debug broadcast — platform-specific bundling has no page-observable side effects.
Security Assessment
File
Lines
Severity
Finding
injected/src/features/detector-perf.js
L145–151, L197–198, L271, L338
info
#active + hasOwnProperty on featureSettings correctly separates platform-specific bundling from remote-config enablement. Resolves the prior platformSpecificFeatures bypass concern from 3372d543.
injected/src/features/detector-perf.js
L3, L154, L169, L249
info
Captured Map/Set/performanceNow/hasOwnProperty; detectorOverrides uses own-property checks to avoid prototype-chain keys like constructor.
injected/src/features/detector-perf.js
L415–426
warning
Debug-only detectorPerfDebugStatsCustomEvent is page-observable when isDebug. Production invariant is documented; verify native never sets args.debug on user pages.
injected/src/features/detector-perf.js
L87–105, L388–389
info
Event types are bounded via getDetectorPerfEventTypes(); severe emissions capped by maxSeverePerPage.
injected/src/features/breakage-reporting.js
L82–84
info
When inactive, getStats() returns undefined which still passes the CallFeatureMethodError check — harmless (JSON.stringify omits undefined values) but an explicit null guard would be cleaner.
Risk Level
Medium Risk — config-gated internal telemetry using captured globals; no API overrides, messaging boundary changes, or captured-globals.js modifications. Platform-specific bundling is now correctly gated via #active.
Recommendations
Confirm privacy-config EventHub sources cover pooled webDetection event names.
Verify native release builds never set args.debug on user pages.
(Optional) Add explicit detectorPerfStats != null guard in breakage-reporting.js before attaching stats.
(Optional) Add a webDetection-specific ordering-guard integration test for getStats after async record.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Injected PR Evaluation: Web Compatibility & Security
Re-assessed at ebd8f636 (sync delta since 2914bc4223: ordering regression test + getStats null guard + main merge).
Web Compatibility Assessment
File
Lines
Sev
Finding
injected/src/features/detector-perf.js
~490–500
info
timeDetector is a synchronous wrapper around detector evaluation. No browser API overrides; return values and thrown errors are preserved. Overhead is limited to captured performanceNow calls.
injected/src/features/web-detection.js
~60–65, ~205–206
info
_evaluateMatch now routes through timeDetector. Detector semantics unchanged; only timing instrumentation added.
injected/src/features/detector-perf.js
~415–428
warning
_debugBroadcast dispatches a page-observable detectorPerfDebugStatsCustomEvent when isDebug is set. Documented INVARIANT requires native to never enable debug on user pages.
injected/src/features/breakage-reporting.js
~48–49, ~82–84
info
getStats runs after await runDetectors, so record() microtasks from fire-and-forget reportDuration calls drain before the handler continuation reaches getStats. New integration test (af01d912db) guards second-report run counts.
injected/src/features/detector-perf.js
~196–208
info
#active gate (hasOwnProperty on featureSettings) keeps the feature inert when absent/disabled in remote config despite platformSpecificFeatures bundling. Top-frame-only measured prevents iframe denominator inflation.
Security Assessment
File
Lines
Sev
Finding
injected/src/captured-globals.js
~20–22
info
New performanceNow export uses .bind(globalThis.performance) with Date.now fallback — correct capture hygiene.
injected/src/features/detector-perf.js
throughout
info
Uses captured Map, Set, CustomEvent, dispatchEvent, hasOwnProperty. detectorOverrides lookup uses own-property check (blocks prototype-pollution keys like constructor). Event types are bounded/config-derived; no nativeData in outbound payloads.
injected/src/features/detector-perf.js
~388–397
info
maxSeverePerPage caps blast radius from misconfigured near-zero thresholds.
injected/src/features/detector-perf.js
~415–428
warning
Debug-only CustomEvent exposes per-detector timing and severe attribution to any page listener when isDebug is true. Acceptable for test builds; production safety depends on the documented native invariant.
injected/src/features/breakage-reporting.js
~82–84
info
detectorPerfStats != null guard prevents attaching stats when feature is inactive — addresses prior optional recommendation.
Risk Level
Medium Risk — config-gated telemetry with captured globals and no API surface overrides; instrumentation wraps existing synchronous detector paths only.
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jdorweiler. I did some local testing and can see the events firing.
I think the event names need fixing up - currently just showing webDetection.
Also the fn() measurement is a decent approximation for simple detectors, but many of the suggested detectors that will likely come out of my project won't fit the same measurement pattern (they don't just do one chunk of work; they may do repeated work over a period of time, wait for events, etc.) -- we should make it clear at postmortem that this only handles simple detectors and we'll need to revisit if we want to be able to measure other kinds of detection in future.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Injected PR Evaluation: Web Compatibility & Security
Sync delta (0477d4dc): Periodic telemetry now buckets by Web Detection group (adwalls, captcha, autorun, …) instead of a single pooled webDetection label. Breakage reports and single-run severe events still carry exact config IDs via #detectorsDetailed / the detail argument.
46 detector-perf unit specs pass locally (Node 24).
Web Compatibility Assessment
File
Lines
Severity
Finding
injected/src/features/detector-perf.js
488–498
info
timeDetector is a synchronous wrapper: return values and thrown errors are unchanged. Fire-and-forget record() dispatch adds negligible overhead and does not alter detector semantics.
injected/src/features/web-detection.js
61–66
info
Instrumentation passes groupName for periodic events and fullDetectorId as detail for exact attribution — no change to evaluateMatch behaviour or error handling ('error' sentinel preserved).
injected/src/features/detector-perf.js
205–207
info
detectorPerf_measured gated to top frame only — iframes cannot inflate the page denominator.
injected/integration-test/detector-perf.spec.js
402–413
info
Second-report ordering regression test guards the FIFO microtask assumption between fire-and-forget record() and the awaited getStats() call.
injected/src/features/detector-perf.js
413–427
warning
Debug-only detectorPerfDebugStatsCustomEvent is page-observable when isDebug is set. INVARIANT is documented; production builds must never set the debug flag on user pages (integration test covers debug: false).
New performanceNow capture uses optional chaining + Date.now fallback — avoids module-load throw and page-tampered performance.now references.
injected/src/features/detector-perf.js
3, 152, 167
info
Map, Set, hasOwnProperty, CustomEvent, dispatchEvent all sourced from captured globals.
injected/src/features/detector-perf.js
195, 247
info
#active gate uses hasOwnProperty on featureSettings — bundled feature cannot bypass remote-config disable. detectorOverrides lookup uses own-property check, mitigating prototype-pollution via keys like constructor.
injected/src/features/detector-perf.js
270–271
info
NAME_PATTERN rejects malformed group/detail strings before they reach event-type construction — bounds injectable event-name segments.
injected/src/features/detector-perf.js
386–396
info
maxSeverePerPage cap + per-kind guard keys limit blast radius from misconfigured near-zero thresholds.
injected/src/features/breakage-reporting.js
82–84
info
getStats result gated on != null and CallFeatureMethodError — no attachment when feature is disabled/absent.
injected/src/features/detector-perf.js
413–427
warning
Debug broadcast dispatches a page-listenable CustomEvent with timing/attribution data. Acceptable behind isDebug; residual risk is operational (native must not set debug on user pages).
No messaging trust-boundary changes, no nativeData leakage, no API overrides, no uncaptured security-sensitive globals in the hot path.
Risk Level
Medium Risk — Config-gated telemetry with proper captured-global hygiene and no browser API shimming; the 0477d4dc grouping change is a telemetry-contract refinement (not a page-facing behaviour change) but requires coordinated privacy-config EventHub updates.
Recommendations
High — privacy-config contract: Update EventHub source entries from pooled detectorPerf_webDetection_* to per-group types (detectorPerf_adwalls_*, detectorPerf_captcha_*, …). getDetectorPerfEventTypes() and docs now assume build-time enumeration of enabled groups.
Medium — operational gate: Confirm native release builds never set args.debug on user pages.
Low — regression lock: Keep the second-report ordering integration test when touching callFeatureMethod dispatch semantics.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
File
Lines
Severity
Finding
injected/src/features/detector-perf.js
523–534
info
timeDetector is a synchronous wrapper with fire-and-forget reporting. Detector return values and throw semantics are unchanged; measurement cost is excluded from the timed window.
injected/src/features/web-detection.js
61–66
info
Instrumentation passes groupName (periodic telemetry family) and fullDetectorId (exact config ID for severe attribution / breakage reports). No DOM or layout reads added.
injected/src/features/detector-perf.js
212–214, 360–371
info
detectorPerf_measured fires top-frame only; per-frame accumulators prevent iframe inflation of the page denominator.
injected/src/features/detector-perf.js
docs + code
info
Bot/fraud (request-driven) and YouTube interference (recurring sweep) detectors are intentionally excluded — their semantics differ from auto-run Web Detection.
injected/integration-test/detector-perf.spec.js
ordering test
info
Regression test guards callFeatureMethod microtask FIFO so timeDetector reporting cannot reorder relative to other feature calls.
injected/src/features/detector-perf.js
387–409
info
New (d40748b): Configurable singleRunSevereThresholdMs / totalPerPageSevereThresholdMs change which severe edges emit, not detector execution. Combined totals retain highest-edge-only behavior. Highest-first ordering under maxSeverePerPage preserves strongest signals.
injected/src/features/detector-perf.js
448–461
warning
Debug-only detectorPerfDebugStatsCustomEvent is page-observable when isDebug is set. Documented INVARIANT: native release builds must never set args.debug on user pages.
No API overrides, prototype patches, or DOM mutations that could break third-party script compatibility.
Security Assessment
File
Lines
Severity
Finding
injected/src/features/detector-perf.js
3
info
Uses captured Map, Set, performanceNow, CustomEvent, dispatchEvent, hasOwnProperty from captured-globals.js.
injected/src/features/detector-perf.js
202, 272
info
Config gating via hasOwnProperty.call(this.featureSettings, this.name) — disabled/absent feature is inert even when bundled.
injected/src/features/detector-perf.js
272, 295–296
info
hasOwnProperty on detectorOverrides prevents prototype-chain keys (e.g. constructor) from being treated as overrides. NAME_PATTERN rejects malformed detector/group names.
injected/src/features/detector-perf.js
233–249
info
New (d40748b): Severe cutoffs require positive finite numbers; invalid values fall back to highest-edge behavior (no crash, no unbounded emission).
injected/src/features/detector-perf.js
421–430
info
New (d40748b): Dedup guard key now includes thresholdMs; maxSeverePerPage (default 10) caps blast radius for misconfigured thresholds.
injected/src/features/detector-perf.js
430
info
_emitSevere passes explicit { kind, detector, thresholdMs } — no object spread of untrusted input; no nativeData leakage.
injected/src/features/detector-perf.js
293–296
info
record() silently ignores invalid input — recording never throws back into detector call sites.
injected/src/features/detector-perf.js
448–461
warning
Debug CustomEvent exposes detector timing and severe attribution to any page listener when isDebug is true. Sole gate is the platform debug flag.
No message-bridge, origin-validation, or captured-globals boundary changes.
Risk Level
Medium Risk — Config-gated internal telemetry with captured globals and input validation; no browser API overrides or messaging trust-boundary changes. The d40748b severe-threshold configurability is a telemetry-contract refinement (more granular severe pixels, still capped).
Recommendations
Privacy-config coordination: Per-group periodic event types (detectorPerf_<group>_…) and any fleet-wide severe-cutoff rollout should stay aligned with EventHub sources in privacy-configuration.
Debug flag invariant: Confirm native release builds never set args.debug on user pages (the debug CustomEvent is the only page-observable surface).
Severe cutoff rollout: When lowering singleRunSevereThresholdMs / totalPerPageSevereThresholdMs fleet-wide, more detectorPerf_severe pixels can fire per crossing (up to maxSeverePerPage); validate EventHub/immediate-pixel capacity before broad rollout.
Tests: 50 unit specs in detector-perf.spec.js pass locally on Node 24 (includes 4 new severe-cutoff cases from d40748b).
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Injected PR Evaluation: Web Compatibility & Security
Sync delta (fe3f40d1 since f015be61): Adds configurable combinedSevereThresholdMs, aligning combined accumulated severe emissions with the existing single-run and per-group total behavior — every crossed edge at or above the cutoff fires (highest-first, still subject to maxSeverePerPage). Two unit tests cover the new cutoff and invalid-config fallback. No new API overrides, messaging paths, or detector execution changes.
Synchronous wrapper around detector invocations; duration measured before async dispatch so reporting cost does not pollute timing. When detectorPerf is disabled/absent, wrapped calls behave identically to unwrapped ones.
injected/src/features/web-detection.js
info
_evaluateMatch now passes groupName + fullDetectorId into timeDetector. Detector semantics unchanged; only instrumentation added.
injected/src/features/breakage-reporting.js
info
Bot/fraud/YouTube on-demand detectors intentionally excluded from detectorPerf; only config-driven webDetection detectors are timed.
injected/src/features/detector-perf.js (init)
info
detectorPerf_measured fires top-frame only (window.self === window.top) to prevent iframe inflation of page denominator.
combinedSevereThresholdMs (new in this sync) mirrors single/total severe cutoff semantics; invalid values fall back to highest combined edge. Telemetry-contract change only — no detector execution impact.
injected/integration-test/detector-perf.spec.js
info
Ordering integration test guards microtask FIFO for fire-and-forget callFeatureMethod dispatch.
Debug-only CustomEvent (detectorPerfDebugStats) exposes timing/stats to page scripts when isDebug is set. Documented INVARIANT: native release builds must never set args.debug on user pages.
Security Assessment
File
Severity
Finding
injected/src/captured-globals.js
info
Adds performanceNow with optional chaining + Date.now fallback. Captured at module load before page scripts; .bind() applied correctly.
Page-observable CustomEvent when debug flag is set. Sole gate is isDebug; verify native never enables this on production user pages.
injected/src/features/breakage-reporting.js
info
getStats attached to user-initiated breakage reports only; exact timings keyed by config IDs, not page-derived strings.
Risk Level
Medium Risk — New config-gated telemetry feature with captured globals and no browser API overrides; the combinedSevereThresholdMs commit is a telemetry-contract refinement within the same bounded surface.
Recommendations
Privacy-config alignment — Ensure EventHub sources cover all detectorPerf_* event types including per-group thresholds and the new combinedSevereThresholdMs rollout path.
Debug flag audit — Confirm native release builds never set args.debug on user-facing pages (the debug CustomEvent is the only page-observable measurement path).
Severe cutoff rollout — When lowering severe cutoffs fleet-wide, validate EventHub immediate-pixel capacity; maxSeverePerPage caps per-frame emissions but frequent crossings across pages could increase volume.
The reason will be displayed to describe this comment to others. Learn more.
Web Compatibility Assessment
File
Lines
Severity
Finding
injected/src/features/detector-perf.js
534–545
info
timeDetector is synchronous, preserves return values, rethrows errors; fire-and-forget reporting does not alter detector semantics.
injected/src/features/web-detection.js
68–71
info
New in fb73f8f9: early exit bypasses the timing wrapper when detectorPerf is absent/disabled — identical to an unwrapped evaluateMatch call, with lower overhead on the default path. Integration tests confirm detectors still run (webDetectionAutoRun count > 0).
injected/src/features/web-detection.js
52
info
Gating via hasOwnProperty on featureSettings matches the detector-perf.js init guard — consistent enable/disable semantics across both features.
injected/src/features/breakage-reporting.js
63–65
info
Bot/fraud/YouTube on-demand detectors intentionally excluded from timing; detection results in breakage reports are unaffected.
injected/src/features/detector-perf.js
215–217
info
detectorPerf_measured is top-frame-only, preventing iframe subframes from inflating the page denominator.
injected/integration-test/detector-perf.spec.js
203–214
info
FIFO ordering regression test guards the microtask race between fire-and-forget record calls and an immediately following getStats.
injected/src/features/detector-perf.js
459–469
warning
Debug detectorPerfDebugStatsCustomEvent is page-observable when args.debug is set. Documented invariant: production builds must never enable this flag on user pages.
Security Assessment
File
Lines
Severity
Finding
injected/src/captured-globals.js
22, 28
info
performanceNow, Map, Set captured at load; timeDetector uses captured performanceNow so a page-tampered global cannot throw at the call site.
injected/src/features/detector-perf.js
3, 280
info
Uses captured hasOwnProperty, CustomEvent, dispatchEvent, Map, Set; detectorOverrides keyed with own-property check (blocks prototype-pollution via keys like constructor).
injected/src/features/detector-perf.js
31, 303
info
NAME_PATTERN validates detector/group names before emission or accumulation.
injected/src/features/detector-perf.js
205, 302
info
Remote-config gating via hasOwnProperty on featureSettings; #active guard on all record paths.
injected/src/features/detector-perf.js
433–441
info
maxSeverePerPage cap + per-threshold dedup limits blast radius of a misconfigured threshold push.
injected/src/features/detector-perf.js
495–504
info
Outbound webEvents use explicit {type} / {type, data} — no object spread, no nativeData leakage risk.
injected/src/features/detector-perf.js
459–469
warning
Debug CustomEvent exposes timing/stats to any page listener when debug flag is set; relies on native never setting args.debug on user pages.
Risk Level
Medium Risk — Config-gated telemetry instrumentation with captured globals; no API overrides or messaging transport changes. Latest commit (fb73f8f9) adds a safe early-exit on the disabled path.
Recommendations
Privacy-config alignment — Ensure EventHub sources cover per-group event types and configurable severe cutoffs (singleRunSevereThresholdMs, totalPerPageSevereThresholdMs, combinedSevereThresholdMs).
Debug flag invariant — Verify native release builds never set args.debug on user pages (debug CustomEvent is page-observable).
Fleet rollout — Monitor EventHub capacity if severe cutoffs are lowered fleet-wide.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
semver-minorNew feature — triggers minor version bump
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
github-actions
Bot
added
the
semver-minor
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
GuiltyDolphin
left a comment
Asana Task/Github Issue:
Description
Adds a
detectorPerffeature that measures on-device detector execution cost (bot/fraud/adwall utils and config-drivenwebDetectionscans) via a synchronoustimeDetectorwrapper. Per-page counters (runs, worst single run, per-detector total, combined total) drive threshold-crossing events fired at occurrence throughwebEvents(at most once per page per event type), with a cappeddetectorPerf_severeimmediate event when a run crosses the highest configured edge. Exact per-detector stats attach to breakage reports. Threshold edges come from remote config (privacy-configuration#5878); nothing is page-observable in production (noperformance.mark/measure, no DOM reads). A debug-flag-gateddetectorPerfDebugStatsCustomEvent feeds live overlays on the detectorPerf test pages (already live).Testing Steps
Automated (covers all behavior):
The integration tests cover: occurrence events fired once per page, severe with exact detector attribution, breakage-report stats (including the fire-and-forget ordering guard), no page-observable timeline entries, debug broadcast on/off, disabled-state emits nothing, and top-frame-only
measured.Manual end-to-end (macOS debug build, optional):
pr-releases/detector-perf-monitoring, see build-branch comment below) and build the Debug scheme.detectorPerf+ eventHub telemetry enabled: check out privacy-configuration#5878, build it, serve the generated macOS config locally, and set it via Debug → Remote Configuration URL. For deterministic severe events, lowersingleRunThresholdsMsto[0.001]in the served config./usr/bin/log stream --process DuckDuckGo --level debug | grep -i eventhubshows period routing for each event and immediate telemetry for severe.Checklist
Please tick all that apply:
Note
Medium Risk
Adds synchronous timing and telemetry on the Web Detection hot path when config enables the feature; failures are swallowed but misconfigured thresholds could increase event volume until capped.
Overview
Introduces a remote-config-gated
detectorPerffeature that times synchronous Web Detection runs viatimeDetector, emits threshold-crossingdetectorPerf_*events throughwebEvents(including capped immediatedetectorPerf_severe), and attaches exact per-detector stats to breakage reports.webDetectionwrapsevaluateMatchwhendetectorPerfis present in config; on-demand bot/fraud/YouTube detectors are intentionally not instrumented.The feature is registered as a platform-specific dependency (still active when site protections are off) and documented in
detector-performance.md. Tests add a large unit suite, Playwrightdetector-perf.spec.js, and small harness fixes (breakage-reporting waits, Playwrightconsole.warningfallback,performanceNowin captured globals).Reviewed by Cursor Bugbot for commit fb73f8f. Bugbot is set up for automated code reviews on this repo. Configure here.