Skip to content

Wait for service worker activation before reloading the web editor - #188

Open
snowyukitty wants to merge 3 commits into
Auburn:masterfrom
snowyukitty:fix/web-editor-first-load
Open

snowyukitty wants to merge 3 commits into
Auburn:masterfrom
snowyukitty:fix/web-editor-first-load

Conversation

@snowyukitty

Copy link
Copy Markdown

The cross-origin isolation bootstrap reloads at updatefound, which indicates a newly installing worker rather than readiness to serve the next navigation. In the unmodified Chromium test, isolation required two automatic reloads. Waiting for activation lets the first uncontrolled document reload after an active registration is available.

For a document that starts without a controller, wait for navigator.serviceWorker.ready after successful registration and reload once it resolves. This waits for an active worker even when registration completion arrives late. Capture control state before awaiting registration: a document already loaded through the worker will not gain isolation by reloading again, so avoid looping when browser policy prevents isolation.

Related to #185. I also simulated late delivery of the registration result to exercise the missed-event case: after clients.claim(), the existing active && !controller fallback skips the reload and leaves a black canvas. A naturally occurring black screen was not observed in ordinary fresh-profile tests, so this does not establish the cause of every environment reported there.

Validation:

  • Served the published NodeEditor.js and NodeEditor.wasm locally with the upstream bootstrap script, then repeated with this change. Used fresh Chromium and Firefox profiles and software WebGL rendering.
  • Delayed delivery of the real serviceWorker.register() result by 300 ms to exercise activation before the registration continuation. On the base, the worker controls the page but crossOriginIsolated remains false and the editor never reaches Module.calledRun; a manual refresh recovers it. With the change, the page reloads automatically and the full editor renders in both browsers. Warm reloads also work.
  • Also served Permissions-Policy: cross-origin-isolated=() to exercise a controlled document that cannot become isolated. The bootstrap avoids repeated reloads.
  • Checked the bootstrap lifecycle with Node's built-in VM: first activation, activation before registration completion, existing active worker, controlled but non-isolated page, already-isolated page, and rejected registration. JavaScript syntax check passes.

The browser test uses the published WASM bundle; I did not rebuild the C++ editor or run the native platform build matrix. The initial unisolated document can still log a WASM worker error before the automatic reload.

@Auburn

Auburn commented Oct 4, 2026

Copy link
Copy Markdown
Owner

What does this fix?

@snowyukitty

Copy link
Copy Markdown
Author

This changes first-visit startup: an initially uncontrolled page waits for an active service worker before reloading. It also handles registration completing after the worker has already claimed the page.

In fresh Chromium profiles, the existing script rendered the editor after two automatic reloads; this change rendered it after one. When I deliberately delayed delivery of the real registration result by 300 ms, the existing script needed a manual refresh, while this change recovered automatically in Chromium and Firefox.

I have not reproduced a naturally occurring black screen from #185. This addresses the tested startup ordering, but does not establish the cause of every reported black screen.

@Auburn

Auburn commented Oct 10, 2026

Copy link
Copy Markdown
Owner

I got Claude to do some testing with this, it found other edge cases that your PR doesn't catch. Have a look at #195

@snowyukitty

Copy link
Copy Markdown
Author

Thanks for testing those cases. You're right: this branch skips the reload when the page already has a controller, even if it was claimed after navigation and is still not isolated. It also doesn't guard an unavailable navigator.serviceWorker.

I checked #195 against this branch: it waits for this script's own worker to activate and uses a URL marker to bound the reload, rather than relying on the initial controller state. That covers the gaps you pointed out, so I agree with carrying the fix in #195 rather than adding a competing update here.

This branch has not been deployed

No deployments
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