Skip to content

fix: harden popup lifecycle against hangs and duplicates#39

Merged
BitHighlander merged 2 commits intodevelopfrom
fix/popup-lifecycle-hardening
Apr 21, 2026
Merged

fix: harden popup lifecycle against hangs and duplicates#39
BitHighlander merged 2 commits intodevelopfrom
fix/popup-lifecycle-hardening

Conversation

@BitHighlander
Copy link
Copy Markdown
Collaborator

Summary

Fixes three recurring popup bugs:

  1. Hangs open forever after a signing flow completes.
  2. Opens multiple windows for a single pending request.
  3. Approval promise never resolves when the user X's the popup (dapp's RPC call hangs until its own timeout).

Root causes

Hang: Chain handlers broadcast `signature_complete` / `transaction_complete` without any event identifier. The popup's `Transaction.tsx` closes the window on ANY such message — including messages destined for a different queued request — while the corresponding `requireApproval()` promise keeps waiting for the original response.

Duplicates: `methods.ts` tracks "popup open" with an in-memory flag. In MV3 the service worker is torn down frequently, so the flag resets to `false` while the popup window is still visible; the next request opens a second window. A parallel, unused `openPopup` in `ethereumHandler.ts` had no dedup at all.

X → hang: `requireApproval()` only resolves when an `eth_sign_response` message arrives. Closing the popup sent no such message, so the promise never settled; the background handler stayed awaiting and the dapp's RPC call blocked until its own timeout.

Changes

`chrome-extension/src/background/methods.ts`

  • Replace the `isPopupOpen` flag with an async check via `chrome.windows.getAll({windowTypes:['popup']})` matched by URL. Survives service worker restarts — no more second windows after a wake-up.
  • Register `chrome.windows.onRemoved` exactly once at module load and fan out to per-request subscribers. Previously a new listener was added on every `openPopup` call and leaked.
  • `requireApproval()` now subscribes to the popup-close stream and resolves `{success:false}` if the window closes before an approval response arrives.

All chain handlers (`ethereum`, `solana`, `bitcoin`, `bitcoincash`, `dogecoin`, `litecoin`, `dash`, `thorchain`, `cosmos`, `osmosis`, `maya`, `ripple`)

  • Every `transaction_complete` / `signature_complete` / `transaction_error` payload now carries `eventId`.
  • Ethereum threads the event id through `processApprovedEvent` → `signMessage` / `signTypedData`.
  • Solana's `buildEvent` now mutates `requestInfo.id` so it's the same id everywhere (it was previously possible for the event to be stored under a fresh UUID that `requireApproval` never saw).

`pages/popup/src/components/Transaction.tsx`

  • Early-returns from `handleMessage` if the incoming `eventId` doesn't match the current event. Unscoped legacy messages still flow through for backward compatibility.

`chrome-extension/src/background/chains/ethereumHandler.ts`

  • Deletes the dead `openPopup` / `requireUnlock` pair (not called anywhere) that lacked dedup and would have been a future foot-gun.

Test plan

  • Single sign: `eth_signTypedData_v4` → popup opens → approve → popup closes, no hang in background.
  • User cancel: open popup, close the window with the X → dapp receives "User denied" promptly (no hang).
  • Duplicate protection: rapid-fire two requests → single popup (focused), not two windows.
  • Service worker wake: use the extension, leave it idle until SW terminates, fire a request → still single popup.
  • Solana `signMessage` + `signTransaction` + `signAndSendTransaction` complete as expected.
  • UTXO chain (BTC/LTC/DOGE/DASH/BCH) transfer completes, TxidPage shows hash.
  • Cosmos-family (THOR/Maya/Osmosis/Cosmos) + XRP transfers complete.
  • Existing installs still close the popup on legacy messages that don't carry an `eventId`.

🤖 Generated with Claude Code

Addresses three symptoms: popup hanging open after completion, multiple
popup windows opening per request, and the approval promise never
resolving when the user X's the popup.

methods.ts
- Replace in-memory isPopupOpen flag with chrome.windows.getAll lookup
  that survives service worker restarts. Focuses an existing popup
  (matched by URL) instead of opening a second one.
- Register chrome.windows.onRemoved exactly once at module load and
  fan out to per-request subscribers, instead of adding a new listener
  on every openPopup() call (previously leaked forever).
- requireApproval now resolves {success:false} when the popup closes
  without an eth_sign_response, so chain handlers no longer hang and
  the dapp's RPC call terminates.

chain handlers
- Every transaction_complete / signature_complete / transaction_error
  message now carries eventId so the popup can match it to the right
  in-flight request. ethereum threads the id through signMessage /
  signTypedData; solana's buildEvent mutates requestInfo.id so the
  match works end-to-end.

Transaction.tsx
- Ignores completion/error messages whose eventId doesn't match the
  current event. Previously any signature_complete would close the
  popup, slamming the window on unrelated queued requests.

ethereumHandler.ts
- Removes dead duplicate openPopup / requireUnlock (never called,
  would have bypassed dedup if it had been).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The outer catch in handleWalletRequest sends transaction_error to the
popup but was the one remaining site missing the eventId scope tag.
Combined with Transaction.tsx's backward-compat rule that accepts
unscoped messages, a thrown chain handler would surface its error on
the wrong event if two events were queued — exactly the
cross-contamination this PR is fixing everywhere else.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@BitHighlander BitHighlander merged commit 14311ba into develop Apr 21, 2026
3 of 4 checks passed
@BitHighlander BitHighlander deleted the fix/popup-lifecycle-hardening branch April 21, 2026 00:13
@BitHighlander BitHighlander mentioned this pull request Apr 21, 2026
4 tasks
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