Skip to content

Revert "Attach shared workers through browser-level auto-attach (#424)" - #432

Merged
dcruzeneil2 merged 1 commit into
mainfrom
hypeship/revert-shared-worker-auto-attach
Oct 1, 2026
Merged

dcruzeneil2 merged 1 commit into
mainfrom
hypeship/revert-shared-worker-auto-attach

Conversation

@dcruzeneil2

@dcruzeneil2 dcruzeneil2 commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Reverts #424 (7cf9c46).

Why

#424 made the telemetry monitor send a browser-level Target.setAutoAttach (filtered to shared_worker, waitForDebuggerOnStart: false) in every browser. Chromium treats any client with browser-level auto-attach as a debugger that wants new popups paused: TargetHandler::ShouldThrottlePopups() returns auto_attach_ without consulting the filter or the wait flag. Every window.open popup is then created with wait_for_debugger, and the renderer sits in a nested pause loop (WebDevToolsAgentImpl::WaitForDebuggerWhenShown) until a session attached to the popup sends Runtime.runIfWaitingForDebugger.

The monitor never attaches to popups, so it never resumes them. Clients that auto-attach and resume themselves (Playwright, Puppeteer) still work; anything else — live view input, the computer API, Selenium, raw CDP — gets a popup frozen at about:blank and an opener frozen with the "Debugger paused in another tab" infobar, since the pause disables input for the whole browsing context group.

Reproduced on a fresh headful browser with no CDP client attached: one OS-level click on a sign-in-with-Google button, and also a plain window.open('https://example.com'). A single Runtime.runIfWaitingForDebugger on the popup target unfreezes it. target="_blank" links are unaffected.

What this changes

Straight git revert: the monitor goes back to Target.attachToTarget for shared workers, and the regression test and CI entry from #424 are removed with it, since they asserted the reverted behaviour. The crash #424 targeted (a new client's auto-attach walking an ended shared worker's DevTools host) remains covered by patch 0035 in the Chromium fork.

Tests

  • go build ./..., go vet, and go test ./lib/browsersurface ./lib/cdpmonitor pass.
  • With the reverted tracker, a local real-Chromium check (monitor attached, no other client, opener calls window.open) has the popup navigate normally; on main the same check hangs inside window.open.

Note

Medium Risk
Changes CDP target attachment for telemetry shared workers and reintroduces the ended-worker DevTools host issue #424 addressed (mitigated separately in the Chromium fork).

Overview
Reverts browser-level Target.setAutoAttach for shared_worker targets introduced in #424. Shared workers are tracked again through normal discovery and Target.attachToTarget, alongside service workers and OOPIFs; only dedicated worker targets stay parent/auto-attach only.

The Chrome E2E regression TestNewClientAutoAttachAfterMonitoredSharedWorkerEnds and its CI filter entry are removed, and browsersurface unit tests no longer assert the shared-worker auto-attach path. cdpmonitor README is updated to match.

This trades the #424 crash workaround for fixing a Chromium side effect where any browser-level auto-attach throttles window.open popups (frozen at about:blank until another client resumes them), which broke live view, computer API, and non-Playwright CDP clients.

Reviewed by Cursor Bugbot for commit 3b77c93. Bugbot is set up for automated code reviews on this repo. Configure here.

@dcruzeneil2
dcruzeneil2 marked this pull request as ready for review October 1, 2026 21:17
@dcruzeneil2
dcruzeneil2 merged commit 27c0089 into main Oct 1, 2026
12 checks passed
@dcruzeneil2
dcruzeneil2 deleted the hypeship/revert-shared-worker-auto-attach branch October 1, 2026 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants