Skip to content

chore(firefox): remove the WebSocket handshake Worker workaround - #42455

Open
Devin Rousso (dcrousso) wants to merge 1 commit into
microsoft:mainfrom
dcrousso:firefox-fix-missing-WebSocket-request-workaround
Open

chore(firefox): remove the WebSocket handshake Worker workaround#42455
Devin Rousso (dcrousso) wants to merge 1 commit into
microsoft:mainfrom
dcrousso:firefox-fix-missing-WebSocket-request-workaround

Conversation

@dcrousso

Copy link
Copy Markdown
Contributor

D310690 preserves the original LoadInfo when Firefox creates the handshake channel, which keeps the worker browsing context associated with its WebSocket request

require requestData again in FrameManager.onWebSocketRequest and assert that _onWebSocketOpened receives it

remove the Firefox guards for the worker locale and setExtraHTTPHeaders coverage because the association now reaches both handshake paths

this requires the Firefox 154 roll that first includes D310690 #42445

`D310690` preserves the original `LoadInfo` when Firefox creates the handshake channel, which keeps the worker browsing context associated with its `WebSocket` request

require `requestData` again in `FrameManager.onWebSocketRequest` and assert that `_onWebSocketOpened` receives it

remove the Firefox guards for the worker `locale` and `setExtraHTTPHeaders` coverage because the association now reaches both handshake paths

this requires the Firefox `154` roll that first includes `D310690`
@github-actions

Copy link
Copy Markdown
Contributor

Test results for "tests 1"

4 failed
❌ [firefox-library] › library/browsercontext-locale.spec.ts:226 › should send Accept-Language header on WebSocket handshake @firefox-ubuntu-22.04-node20
❌ [firefox-library] › library/browsercontext-locale.spec.ts:241 › should send Accept-Language header on WebSocket handshake from a worker @firefox-ubuntu-22.04-node20
❌ [firefox-library] › library/web-socket.spec.ts:200 › should not tear down the page when a WebSocket is opened inside a worker @firefox-ubuntu-22.04-node20
❌ [firefox-library] › library/web-socket.spec.ts:260 › should send extra HTTP headers on WebSocket handshake from a worker @firefox-ubuntu-22.04-node20

51268 passed, 1237 skipped


Merge workflow run.

@github-actions

Copy link
Copy Markdown
Contributor

Test results for "MCP"

8259 passed, 1361 skipped


Merge workflow run.

@github-actions

Copy link
Copy Markdown
Contributor

🔴 The failures are caused by this PR

Hi, I'm the Playwright bot and I took a first look at the CI results.

All four failures are Firefox worker/handshake WebSocket tests that this PR directly rewrites, and they fail for the reason the PR description already calls out: this needs the Firefox 154 roll with D310690, which CI isn't running yet. Until that lands, the worker WebSocket handshake isn't associated with its browsing context, so the newly-required requestData never arrives and the removed guards no longer protect these tests.

Details

Overall: not flakes, not infra. The diff reaches every failing test — it removes the Firefox worker workaround in ffPage.ts::_onWebSocketOpened, makes requestData required in FrameManager.onWebSocketRequest (frames.ts), and deletes the it.fixme(browserName === 'firefox' && !isBidi, …) skips on three of these exact tests. All four fail only on Firefox, which is exactly where the change applies.

Caused by this PR

  • [firefox-library] › library/browsercontext-locale.spec.ts:226 › should send Accept-Language header on WebSocket handshake and :241 › … from a worker — the PR removes the firefox && !isBidi it.fixme on these, so they now run on Firefox. Without the D310690 roll, Firefox still doesn't send the emulated Accept-Language on the (worker) handshake, so the assertion fails.
  • [firefox-library] › library/web-socket.spec.ts:260 › should send extra HTTP headers on WebSocket handshake from a worker — same story: the firefox && !isBidi skip is removed, but the worker handshake still lacks the extra headers on the current Firefox build.
  • [firefox-library] › library/web-socket.spec.ts:200 › should not tear down the page when a WebSocket is opened inside a worker — the workaround that returned early when request/response were missing is gone and _onWebSocketOpened now does assert(request) / assert(response). On a Firefox without D310690, the worker WebSocket has no handshake metadata, so the assert throws instead of the old graceful path — hence the failure.

No separate flake check was needed: the change removes these tests' Firefox guards and rewrites the exact code path they exercise, so the causation is direct.

What unblocks it: land the Firefox 154 roll that includes D310690 first (or rebase onto it), then this PR should go green on Firefox.

Triaged by the Playwright bot - agent run

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.

1 participant