fix: wait for ledger offscreen iframe load (#26225)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

In order to instantiate a functioning communication between the
offscreen iframe for Ledger and the LedgerKeyring (through
`LedgerOffscreenBridge`), we need to make sure that the iframe is loaded
before sending any message to it.

We currently wait for the offscreen page to load, but the iframe load is
completely async and it will most likely be ready after the rest of the
offscreen page, leaving messages proxied to the iframe hanging forever.

On a higher level, this is dangerous because everytime we try to send a
message to the Ledger iframe the `KeyringController` controller-level
mutex is locked, and any other operation will wait for its release to
proceed - this creates a deadlock situation in the case the iframe does
not respond to a message.

This PR makes Ledger iframe initialization into a Promise, and the
offscreen page will wait for it to be resolved before sending the
"ready" signal back to the extension. To avoid waiting forever, the
Ledger initialisation promise races with a 5s timeout: in case of
timeout, interactions with Ledger accounts will not work during the
entire session (until the offscreen page is re-initialised, and another
attempt is made to init Ledger).

Note that the UI setup is not affected by this change, since the
Offscreen initialisation is awaited only when unlocking the wallet.

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/26225?quickstart=1)

## **Related issues**

Progresses: #26840

## **Manual testing steps**

N/A

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->
 N/A

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

app/scripts/offscreen.js

@@ -1,5 +1,8 @@
 import { captureException } from '@sentry/browser';
-import { OffscreenCommunicationTarget } from '../../shared/constants/offscreen-communication';
+import {
+  OFFSCREEN_LOAD_TIMEOUT,
+  OffscreenCommunicationTarget,
+} from '../../shared/constants/offscreen-communication';
 import { getSocketBackgroundToMocha } from '../../test/e2e/background-socket/socket-background-to-mocha';
 
 /**
@@ -65,7 +68,7 @@ export async function createOffscreen() {
 
   // In case we are in a bad state where the offscreen document is not loading, timeout and let execution continue.
   const timeoutPromise = new Promise((resolve) => {
-    setTimeout(resolve, 5000);
+    setTimeout(resolve, OFFSCREEN_LOAD_TIMEOUT);
   });
 
   await Promise.race([loadPromise, timeoutPromise]);

offscreen/scripts/ledger.ts

@@ -16,15 +16,11 @@ const LEDGER_KEYRING_IFRAME_CONNECTED_EVENT = 'ledger-connection-event';
 
 const callbackProcessor = new CallbackProcessor();
 
-export default function init() {
-  const iframe = document.createElement('iframe');
-  iframe.src = 'https://metamask.github.io/eth-ledger-bridge-keyring';
-  iframe.allow = 'hid';
-  document.body.appendChild(iframe);
+function setupMessageListeners(iframe: HTMLIFrameElement) {
   // This listener receives action responses from the live ledger iframe
   // Then forwards the response to the offscreen bridge
   window.addEventListener('message', ({ origin, data, source }) => {
-    if (origin !== KnownOrigins.ledger || source !== iframe?.contentWindow) {
+    if (origin !== KnownOrigins.ledger || source !== iframe.contentWindow) {
       return;
     }
 
@@ -64,7 +60,7 @@ export default function init() {
         return;
       }
 
-      if (!iframe?.contentWindow) {
+      if (!iframe.contentWindow) {
         const error = new Error('Ledger iframe not present');
         sendResponse({
           success: false,
@@ -87,10 +83,7 @@ export default function init() {
         messageId,
       };
 
-      // It has already been checked that they are not null above, so the
-      // optional chaining here is for compiler typechecking only. This avoids
-      // overriding our non-null assertion rule.
-      iframe?.contentWindow?.postMessage(iframeMsg, KnownOrigins.ledger);
+      iframe.contentWindow.postMessage(iframeMsg, KnownOrigins.ledger);
 
       // This keeps sendResponse function valid after return
       // https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/runtime/onMessage
@@ -99,3 +92,16 @@ export default function init() {
     },
   );
 }
+
+export default async function init() {
+  return new Promise<void>((resolve) => {
+    const iframe = document.createElement('iframe');
+    iframe.src = 'https://metamask.github.io/eth-ledger-bridge-keyring';
+    iframe.allow = 'hid';
+    iframe.onload = () => {
+      setupMessageListeners(iframe);
+      resolve();
+    };
+    document.body.appendChild(iframe);
+  });
+}

offscreen/scripts/offscreen.ts

@@ -1,6 +1,8 @@
 import { BrowserRuntimePostMessageStream } from '@metamask/post-message-stream';
 import { ProxySnapExecutor } from '@metamask/snaps-execution-environments';
+import { isObject } from '@metamask/utils';
 import {
+  OFFSCREEN_LEDGER_INIT_TIMEOUT,
   OffscreenCommunicationEvents,
   OffscreenCommunicationTarget,
 } from '../../shared/constants/offscreen-communication';
@@ -9,41 +11,63 @@ import initLedger from './ledger';
 import initTrezor from './trezor';
 import initLattice from './lattice';
 
-initLedger();
-initTrezor();
-initLattice();
-
 /**
  * Initialize a post message stream with the parent window that is initialized
  * in the metamask-controller (background/serivce worker) process. This will be
  * utilized by snaps for communication with snaps running in the offscreen
  * document.
  */
-const parentStream = new BrowserRuntimePostMessageStream({
-  name: 'child',
-  target: 'parent',
-});
-
-ProxySnapExecutor.initialize(parentStream, './snaps/index.html');
-
-if (process.env.IN_TEST) {
-  chrome.runtime.onMessage.addListener((message) => {
-    if (
-      message &&
-      typeof message === 'object' &&
-      message.event === OffscreenCommunicationEvents.metamaskBackgroundReady &&
-      message.target === OffscreenCommunicationTarget.extension
-    ) {
-      window.document?.documentElement?.classList?.add('controller-loaded');
-    }
+function initializePostMessageStream() {
+  const parentStream = new BrowserRuntimePostMessageStream({
+    name: 'child',
+    target: 'parent',
   });
+
+  ProxySnapExecutor.initialize(parentStream, './snaps/index.html');
 }
 
-chrome.runtime.sendMessage({
-  target: OffscreenCommunicationTarget.extensionMain,
-  isBooted: true,
+/**
+ * Initialize the ledger, trezor, and lattice keyring connections, and the
+ * post message stream for the Snaps environment.
+ */
+async function init(): Promise<void> {
+  initializePostMessageStream();
+  initTrezor();
+  initLattice();
 
-  // This message is being sent from the Offscreen Document to the Service Worker.
-  // The Service Worker has no way to query `navigator.webdriver`, so we send it here.
-  webdriverPresent: navigator.webdriver === true,
+  try {
+    const ledgerInitTimeout = new Promise((_, reject) => {
+      setTimeout(() => {
+        reject(new Error('Ledger initialization timed out'));
+      }, OFFSCREEN_LEDGER_INIT_TIMEOUT);
+    });
+    await Promise.race([initLedger(), ledgerInitTimeout]);
+  } catch (error) {
+    console.error('Ledger initialization failed:', error);
+  }
+}
+
+init().then(() => {
+  if (process.env.IN_TEST) {
+    chrome.runtime.onMessage.addListener((message) => {
+      if (
+        message &&
+        isObject(message) &&
+        message.event ===
+          OffscreenCommunicationEvents.metamaskBackgroundReady &&
+        message.target === OffscreenCommunicationTarget.extension
+      ) {
+        window.document?.documentElement?.classList?.add('controller-loaded');
+      }
+    });
+  }
+
+  chrome.runtime.sendMessage({
+    target: OffscreenCommunicationTarget.extensionMain,
+    isBooted: true,
+
+    // This message is being sent from the Offscreen Document to the Service Worker.
+    // The Service Worker has no way to query `navigator.webdriver`, so we send it here.
+    webdriverPresent: navigator.webdriver === true,
+  });
 });

shared/constants/offscreen-communication.ts

@@ -1,3 +1,6 @@
+export const OFFSCREEN_LEDGER_INIT_TIMEOUT = 4000;
+export const OFFSCREEN_LOAD_TIMEOUT = OFFSCREEN_LEDGER_INIT_TIMEOUT + 1000;
+
 /**
  * Defines legal targets for offscreen communication. These values are used to
  * filter and route messages to the correct target.