Skip to content

Commit

Permalink
fix: Handle error when offscreen document already exists (#25138)
Browse files Browse the repository at this point in the history
<!--
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**

A `try..catch` block has been added to catch errors about the Offscreen
document already existing. This circumstance is not a bug, it's expected
to happen sometimes due to race conditions between `hasDocument` and the
creation step. The code in question is just meant to create the document
if it doesn't exist, so it's OK if we get this error.

The `hasDocument` call has been removed as well, since it's now
redundant. This function wasn't safe to call anyway; it's an
undocumented private function.

See here for more information:
* https://stackoverflow.com/a/7725839
*
https://groups.google.com/a/chromium.org/g/chromium-extensions/c/D5Jg2ukyvUc4

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

## **Related issues**

Fixes #25118

## **Manual testing steps**

I do not know how to test this unfortunately.

## **Screenshots/Recordings**

N/A

## **Pre-merge author checklist**

- [x] 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).
- [x] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] 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.
  • Loading branch information
Gudahtt authored Jul 18, 2024
1 parent e7660c3 commit f434c0f
Showing 1 changed file with 33 additions and 10 deletions.
43 changes: 33 additions & 10 deletions app/scripts/offscreen.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { captureException } from '@sentry/browser';
import { OffscreenCommunicationTarget } from '../../shared/constants/offscreen-communication';

/**
Expand All @@ -9,29 +10,51 @@ import { OffscreenCommunicationTarget } from '../../shared/constants/offscreen-c
*/
export async function createOffscreen() {
const { chrome } = globalThis;
if (!chrome.offscreen || (await chrome.offscreen.hasDocument())) {
if (!chrome.offscreen) {
return;
}

let offscreenDocumentLoadedListener;
const loadPromise = new Promise((resolve) => {
const messageListener = (msg) => {
offscreenDocumentLoadedListener = (msg) => {
if (
msg.target === OffscreenCommunicationTarget.extensionMain &&
msg.isBooted
) {
chrome.runtime.onMessage.removeListener(messageListener);
chrome.runtime.onMessage.removeListener(
offscreenDocumentLoadedListener,
);
resolve();
}
};
chrome.runtime.onMessage.addListener(messageListener);
chrome.runtime.onMessage.addListener(offscreenDocumentLoadedListener);
});

await chrome.offscreen.createDocument({
url: './offscreen.html',
reasons: ['IFRAME_SCRIPTING'],
justification:
'Used for Hardware Wallet and Snaps scripts to communicate with the extension.',
});
try {
await chrome.offscreen.createDocument({
url: './offscreen.html',
reasons: ['IFRAME_SCRIPTING'],
justification:
'Used for Hardware Wallet and Snaps scripts to communicate with the extension.',
});
} catch (error) {
if (offscreenDocumentLoadedListener) {
chrome.runtime.onMessage.removeListener(offscreenDocumentLoadedListener);
}
if (
error?.message?.startsWith(
'Only a single offscreen document may be created',
)
) {
console.debug('Offscreen document already exists; skipping creation');
} else {
// Report unrecongized errors without halting wallet initialization
// Failures to create the offscreen document does not compromise wallet data integrity or
// core functionality, it's just needed for specific features.
captureException(error);
}
return;
}

// 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) => {
Expand Down

0 comments on commit f434c0f

Please sign in to comment.