From 4f047d5f4367738ba4ee17ac1807115e817c2abd Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Wed, 26 Mar 2025 17:52:43 -0230 Subject: [PATCH 01/12] WIP --- app/scripts/app-init.js | 2 -- app/scripts/background.js | 48 +++++++++++++++++++++------------------ 2 files changed, 26 insertions(+), 24 deletions(-) diff --git a/app/scripts/app-init.js b/app/scripts/app-init.js index 9da5414ff015..e34a10de051a 100644 --- a/app/scripts/app-init.js +++ b/app/scripts/app-init.js @@ -187,8 +187,6 @@ const registerInPageContentScript = async () => { chrome.runtime.onInstalled.addListener(function (details) { if (details.reason === 'install') { chrome.storage.session.set({ isFirstTimeInstall: true }); - } else if (details.reason === 'update') { - chrome.storage.session.set({ isFirstTimeInstall: false }); } }); diff --git a/app/scripts/background.js b/app/scripts/background.js index 96f16cd95a4e..f5190f629ca5 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -141,20 +141,10 @@ const PHISHING_WARNING_PAGE_TIMEOUT = ONE_SECOND_IN_MILLISECONDS; // Event emitter for state persistence export const statePersistenceEvents = new EventEmitter(); -if (isFirefox) { +if (isFirefox || !isManifestV3) { browser.runtime.onInstalled.addListener(function (details) { if (details.reason === 'install') { browser.storage.session.set({ isFirstTimeInstall: true }); - } else if (details.reason === 'update') { - browser.storage.session.set({ isFirstTimeInstall: false }); - } - }); -} else if (!isManifestV3) { - browser.runtime.onInstalled.addListener(function (details) { - if (details.reason === 'install') { - global.sessionStorage.setItem('isFirstTimeInstall', true); - } else if (details.reason === 'update') { - global.sessionStorage.setItem('isFirstTimeInstall', false); } }); } @@ -505,12 +495,6 @@ async function initialize() { await sendReadyMessageToTabs(); log.info('MetaMask initialization complete.'); - if (isManifestV3 || isFirefox) { - browser.storage.session.set({ isFirstTimeInstall: false }); - } else { - global.sessionStorage.setItem('isFirstTimeInstall', false); - } - resolveInitialization(); } catch (error) { rejectInitialization(error); @@ -1286,18 +1270,38 @@ const addAppInstalledEvent = () => { }, 500); }; -// On first install, open a new tab with MetaMask -async function onInstall() { +async function checkOnFirstInstall() { + log.debug('Checking whether the extension was just installed'); + let firstInstallTriggered = false; + const onSessionUpdate = (changes) => { + if (changes?.isFirstTimeInstall?.newValue) { + firstInstallTriggered = true; + browser.storage.session.onChanged.removeListener(onSessionUpdate); + onFirstInstall(); + } + }; + browser.storage.session.onChanged.addListener(onSessionUpdate); const sessionData = isManifestV3 || isFirefox ? await browser.storage.session.get(['isFirstTimeInstall']) : await global.sessionStorage.getItem('isFirstTimeInstall'); - const isFirstTimeInstall = sessionData?.isFirstTimeInstall; + if (firstInstallTriggered) { + return; + } + if (sessionData?.isFirstTimeInstall) { + firstInstallTriggered = true; + browser.storage.session.onChanged.removeListener(onSessionUpdate); + onFirstInstall(); + } +} +// On first install, open a new tab with MetaMask +function onFirstInstall() { + log.debug('First install detected'); if (process.env.IN_TEST) { addAppInstalledEvent(); - } else if (!isFirstTimeInstall && !process.env.METAMASK_DEBUG) { + } else if (!process.env.METAMASK_DEBUG) { // If storeAlreadyExisted is true then this is a fresh installation // and an app installed event should be tracked. addAppInstalledEvent(); @@ -1334,7 +1338,7 @@ function setupSentryGetStateGlobal(store) { } async function initBackground() { - await onInstall(); + checkOnFirstInstall(); try { await initialize(); if (process.env.IN_TEST) { From 4cb786348781584ae18f0f1c1bbb12a599e239e2 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Thu, 27 Mar 2025 18:53:36 -0230 Subject: [PATCH 02/12] Use globals rather than session storage to track first install --- app/scripts/app-init.js | 6 +++++- app/scripts/background.js | 33 +++++---------------------------- 2 files changed, 10 insertions(+), 29 deletions(-) diff --git a/app/scripts/app-init.js b/app/scripts/app-init.js index e34a10de051a..0a853b4d58cf 100644 --- a/app/scripts/app-init.js +++ b/app/scripts/app-init.js @@ -186,7 +186,11 @@ const registerInPageContentScript = async () => { chrome.runtime.onInstalled.addListener(function (details) { if (details.reason === 'install') { - chrome.storage.session.set({ isFirstTimeInstall: true }); + if (globalThis.__metamaskTriggerOnInstall) { + globalThis.__metamaskTriggerOnInstall(); + } else { + globalThis.__metamaskWasJustInstalled = true; + } } }); diff --git a/app/scripts/background.js b/app/scripts/background.js index f5190f629ca5..812bb4cc56f9 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -144,9 +144,13 @@ export const statePersistenceEvents = new EventEmitter(); if (isFirefox || !isManifestV3) { browser.runtime.onInstalled.addListener(function (details) { if (details.reason === 'install') { - browser.storage.session.set({ isFirstTimeInstall: true }); + onFirstInstall(); } }); +} else if (globalThis.__metamaskWasJustInstalled) { + onFirstInstall(); +} else { + globalThis.__metamaskTriggerOnInstall = () => onFirstInstall(); } /** @@ -1270,32 +1274,6 @@ const addAppInstalledEvent = () => { }, 500); }; -async function checkOnFirstInstall() { - log.debug('Checking whether the extension was just installed'); - let firstInstallTriggered = false; - const onSessionUpdate = (changes) => { - if (changes?.isFirstTimeInstall?.newValue) { - firstInstallTriggered = true; - browser.storage.session.onChanged.removeListener(onSessionUpdate); - onFirstInstall(); - } - }; - browser.storage.session.onChanged.addListener(onSessionUpdate); - const sessionData = - isManifestV3 || isFirefox - ? await browser.storage.session.get(['isFirstTimeInstall']) - : await global.sessionStorage.getItem('isFirstTimeInstall'); - - if (firstInstallTriggered) { - return; - } - if (sessionData?.isFirstTimeInstall) { - firstInstallTriggered = true; - browser.storage.session.onChanged.removeListener(onSessionUpdate); - onFirstInstall(); - } -} - // On first install, open a new tab with MetaMask function onFirstInstall() { log.debug('First install detected'); @@ -1338,7 +1316,6 @@ function setupSentryGetStateGlobal(store) { } async function initBackground() { - checkOnFirstInstall(); try { await initialize(); if (process.env.IN_TEST) { From deef53953ce3dd0e1ab56755a964f1c1d8c34e6e Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Thu, 27 Mar 2025 19:03:06 -0230 Subject: [PATCH 03/12] Add comments, cleanup global namespace --- app/scripts/app-init.js | 9 +++++++++ app/scripts/background.js | 24 ++++++++++++++++++------ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/app/scripts/app-init.js b/app/scripts/app-init.js index 0a853b4d58cf..746bf3e5f704 100644 --- a/app/scripts/app-init.js +++ b/app/scripts/app-init.js @@ -184,10 +184,19 @@ const registerInPageContentScript = async () => { } }; +// On MV3 builds we must listen for this event in `app-init`, otherwise we found that the listener +// is never called. +// For MV2 builds, the listener is added in `background.js` instead. chrome.runtime.onInstalled.addListener(function (details) { if (details.reason === 'install') { + // This condition is for when `background.js` was loaded before the `onInstalled` listener was + // called. if (globalThis.__metamaskTriggerOnInstall) { globalThis.__metamaskTriggerOnInstall(); + // Delete just to clean up global namespace + delete globalThis.__metamaskTriggerOnInstall; + // This condition is for when the `onInstalled` listener in `app-init` was called before + // `background.js` was loaded. } else { globalThis.__metamaskWasJustInstalled = true; } diff --git a/app/scripts/background.js b/app/scripts/background.js index 812bb4cc56f9..8e2d3e67f014 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -141,16 +141,26 @@ const PHISHING_WARNING_PAGE_TIMEOUT = ONE_SECOND_IN_MILLISECONDS; // Event emitter for state persistence export const statePersistenceEvents = new EventEmitter(); -if (isFirefox || !isManifestV3) { +// On MV3 builds we must listen for this event in `app-init`, otherwise we found that the listener +// is never called. +// There is no `app-init` file on MV2 builds, so we add a listener here instead. +if (!isManifestV3) { browser.runtime.onInstalled.addListener(function (details) { if (details.reason === 'install') { - onFirstInstall(); + onInstall(); } }); + + // This condition is for when the `onInstalled` listener in `app-init` was called before + // `background.js` was loaded. } else if (globalThis.__metamaskWasJustInstalled) { - onFirstInstall(); + onInstall(); + // Delete just to clean up global namespace + delete globalThis.__metamaskWasJustInstalled; + // This condition is for when `background.js` was loaded before the `onInstalled` listener was + // called. } else { - globalThis.__metamaskTriggerOnInstall = () => onFirstInstall(); + globalThis.__metamaskTriggerOnInstall = () => onInstall(); } /** @@ -1274,8 +1284,10 @@ const addAppInstalledEvent = () => { }, 500); }; -// On first install, open a new tab with MetaMask -function onFirstInstall() { +/** + * Trigger actions that should happen only upon initial install (e.g. open tab for onboarding). + */ +function onInstall() { log.debug('First install detected'); if (process.env.IN_TEST) { addAppInstalledEvent(); From a4f850c78dde59269be26b9448e0ab39930e5146 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Thu, 27 Mar 2025 19:27:31 -0230 Subject: [PATCH 04/12] Add missing onNavigate step to background init --- app/scripts/background.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/scripts/background.js b/app/scripts/background.js index 8e2d3e67f014..b9519c063b26 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -1289,15 +1289,13 @@ const addAppInstalledEvent = () => { */ function onInstall() { log.debug('First install detected'); - if (process.env.IN_TEST) { - addAppInstalledEvent(); - } else if (!process.env.METAMASK_DEBUG) { + addAppInstalledEvent(); + if (!process.env.METAMASK_DEBUG) { // If storeAlreadyExisted is true then this is a fresh installation // and an app installed event should be tracked. addAppInstalledEvent(); platform.openExtensionInBrowser(); } - onNavigateToTab(); } function onNavigateToTab() { @@ -1328,6 +1326,7 @@ function setupSentryGetStateGlobal(store) { } async function initBackground() { + onNavigateToTab(); try { await initialize(); if (process.env.IN_TEST) { From 1893fd85d9219574cbad7aec46006f9316dec219 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Thu, 27 Mar 2025 19:40:00 -0230 Subject: [PATCH 05/12] Prevent opening tab on install in e2e tests --- app/scripts/background.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/background.js b/app/scripts/background.js index b9519c063b26..58f0020e3fef 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -1290,7 +1290,7 @@ const addAppInstalledEvent = () => { function onInstall() { log.debug('First install detected'); addAppInstalledEvent(); - if (!process.env.METAMASK_DEBUG) { + if (!process.env.IN_TEST && !process.env.METAMASK_DEBUG) { // If storeAlreadyExisted is true then this is a fresh installation // and an app installed event should be tracked. addAppInstalledEvent(); From 5f8828712f76519668ea4f6794ceeac1d6904af3 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Fri, 28 Mar 2025 14:18:28 -0230 Subject: [PATCH 06/12] Remove duplicate event and obsolete comment --- app/scripts/background.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/scripts/background.js b/app/scripts/background.js index 58f0020e3fef..b016ea383c67 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -1291,9 +1291,6 @@ function onInstall() { log.debug('First install detected'); addAppInstalledEvent(); if (!process.env.IN_TEST && !process.env.METAMASK_DEBUG) { - // If storeAlreadyExisted is true then this is a fresh installation - // and an app installed event should be tracked. - addAppInstalledEvent(); platform.openExtensionInBrowser(); } } From 769b7e179f11ee63054c380c11991f7db2541266 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Fri, 28 Mar 2025 14:28:14 -0230 Subject: [PATCH 07/12] Remove install listener after install event --- app/scripts/app-init.js | 20 +++++++++++++++----- app/scripts/background.js | 20 +++++++++++++++----- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/app/scripts/app-init.js b/app/scripts/app-init.js index 746bf3e5f704..6e66ec789fdd 100644 --- a/app/scripts/app-init.js +++ b/app/scripts/app-init.js @@ -184,10 +184,17 @@ const registerInPageContentScript = async () => { } }; -// On MV3 builds we must listen for this event in `app-init`, otherwise we found that the listener -// is never called. -// For MV2 builds, the listener is added in `background.js` instead. -chrome.runtime.onInstalled.addListener(function (details) { +/** + * `onInstalled` event handler. + * + * On MV3 builds we must listen for this event in `app-init`, otherwise we found that the listener + * is never called. + * For MV2 builds, the listener is added in `background.js` instead. + * + * @param {object} details - Event details. + * @param {string} details.reason - The reason that this event was dispatched. + */ +function onInstalledListener(details) { if (details.reason === 'install') { // This condition is for when `background.js` was loaded before the `onInstalled` listener was // called. @@ -200,7 +207,10 @@ chrome.runtime.onInstalled.addListener(function (details) { } else { globalThis.__metamaskWasJustInstalled = true; } + chrome.runtime.onInstalled.removeListener(onInstalledListener); } -}); +} + +chrome.runtime.onInstalled.addListener(onInstalledListener); registerInPageContentScript(); diff --git a/app/scripts/background.js b/app/scripts/background.js index b016ea383c67..8d50d15c385d 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -141,15 +141,25 @@ const PHISHING_WARNING_PAGE_TIMEOUT = ONE_SECOND_IN_MILLISECONDS; // Event emitter for state persistence export const statePersistenceEvents = new EventEmitter(); -// On MV3 builds we must listen for this event in `app-init`, otherwise we found that the listener -// is never called. -// There is no `app-init` file on MV2 builds, so we add a listener here instead. if (!isManifestV3) { - browser.runtime.onInstalled.addListener(function (details) { + /** + * `onInstalled` event handler. + * + * On MV3 builds we must listen for this event in `app-init`, otherwise we found that the listener + * is never called. + * There is no `app-init` file on MV2 builds, so we add a listener here instead. + * + * @param {object} details - Event details. + * @param {string} details.reason - The reason that this event was dispatched. + */ + const onInstalledListener = (details) => { if (details.reason === 'install') { onInstall(); + browser.runtime.onInstalled.removeListener(onInstalledListener); } - }); + }; + + browser.runtime.onInstalled.addListener(onInstalledListener); // This condition is for when the `onInstalled` listener in `app-init` was called before // `background.js` was loaded. From 57b406bbd1643c69ff72cc861583a3e6fc954781 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Fri, 28 Mar 2025 14:32:02 -0230 Subject: [PATCH 08/12] ADd type declaration for globals --- types/global.d.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/types/global.d.ts b/types/global.d.ts index f18db580d6eb..6aee0bcd27d3 100644 --- a/types/global.d.ts +++ b/types/global.d.ts @@ -263,6 +263,10 @@ export declare global { var stateHooks: StateHooks; + // Used in `app-init.js` and `background.js` to trigger install event + var __metamaskWasJustInstalled: boolean | undefined; + var __metamaskTriggerOnInstall: (() => void) | undefined; + namespace jest { // The interface is being used for declaration merging, which is an acceptable exception to this rule. // eslint-disable-next-line @typescript-eslint/consistent-type-definitions From 75e47430962feb15099e169903d81157f0f2a8a9 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Fri, 28 Mar 2025 14:34:07 -0230 Subject: [PATCH 09/12] Improve comments --- types/global.d.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/types/global.d.ts b/types/global.d.ts index 6aee0bcd27d3..e9574babb8a0 100644 --- a/types/global.d.ts +++ b/types/global.d.ts @@ -263,8 +263,15 @@ export declare global { var stateHooks: StateHooks; - // Used in `app-init.js` and `background.js` to trigger install event + /** + * This is set in `app-init.js` to communicate that MetaMask was just installed, and is read in + * `background.js`. + */ var __metamaskWasJustInstalled: boolean | undefined; + /** + * This is set in `background.js` so that `app-init.js` can trigger "on install" actions when + * the `onInstalled` listener is called. + */ var __metamaskTriggerOnInstall: (() => void) | undefined; namespace jest { From 6c277eeb3e1e71f5d9409cc84c72f4b19f628884 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Fri, 28 Mar 2025 15:31:58 -0230 Subject: [PATCH 10/12] Migrate globals to stateHooks to pass through to root container --- app/scripts/app-init.js | 10 ++++++---- app/scripts/background.js | 6 +++--- types/global.d.ts | 21 ++++++++++----------- 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/app/scripts/app-init.js b/app/scripts/app-init.js index 6e66ec789fdd..dbf5c2b8f44c 100644 --- a/app/scripts/app-init.js +++ b/app/scripts/app-init.js @@ -6,6 +6,8 @@ let scriptsLoadInitiated = false; const { chrome } = globalThis; const testMode = process.env.IN_TEST; +globalThis.stateHooks = globalThis.stateHooks || {}; + const loadTimeLogs = []; // eslint-disable-next-line import/unambiguous function tryImport(...fileNames) { @@ -198,14 +200,14 @@ function onInstalledListener(details) { if (details.reason === 'install') { // This condition is for when `background.js` was loaded before the `onInstalled` listener was // called. - if (globalThis.__metamaskTriggerOnInstall) { - globalThis.__metamaskTriggerOnInstall(); + if (globalThis.stateHooks.metamaskTriggerOnInstall) { + globalThis.stateHooks.metamaskTriggerOnInstall(); // Delete just to clean up global namespace - delete globalThis.__metamaskTriggerOnInstall; + delete globalThis.stateHooks.metamaskTriggerOnInstall; // This condition is for when the `onInstalled` listener in `app-init` was called before // `background.js` was loaded. } else { - globalThis.__metamaskWasJustInstalled = true; + globalThis.stateHooks.metamaskWasJustInstalled = true; } chrome.runtime.onInstalled.removeListener(onInstalledListener); } diff --git a/app/scripts/background.js b/app/scripts/background.js index 8d50d15c385d..7859d210508b 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -163,14 +163,14 @@ if (!isManifestV3) { // This condition is for when the `onInstalled` listener in `app-init` was called before // `background.js` was loaded. -} else if (globalThis.__metamaskWasJustInstalled) { +} else if (globalThis.stateHooks.metamaskWasJustInstalled) { onInstall(); // Delete just to clean up global namespace - delete globalThis.__metamaskWasJustInstalled; + delete globalThis.stateHooks.metamaskWasJustInstalled; // This condition is for when `background.js` was loaded before the `onInstalled` listener was // called. } else { - globalThis.__metamaskTriggerOnInstall = () => onInstall(); + globalThis.stateHooks.metamaskTriggerOnInstall = () => onInstall(); } /** diff --git a/types/global.d.ts b/types/global.d.ts index e9574babb8a0..dfdadbe8fc65 100644 --- a/types/global.d.ts +++ b/types/global.d.ts @@ -250,6 +250,16 @@ type StateHooks = { metamaskGetState?: () => Promise; throwTestBackgroundError?: (msg?: string) => Promise; throwTestError?: (msg?: string) => void; + /** + * This is set in `app-init.js` to communicate that MetaMask was just installed, and is read in + * `background.js`. + */ + metamaskWasJustInstalled?: boolean; + /** + * This is set in `background.js` so that `app-init.js` can trigger "on install" actions when + * the `onInstalled` listener is called. + */ + metamaskTriggerOnInstall?: () => void; }; export declare global { @@ -263,17 +273,6 @@ export declare global { var stateHooks: StateHooks; - /** - * This is set in `app-init.js` to communicate that MetaMask was just installed, and is read in - * `background.js`. - */ - var __metamaskWasJustInstalled: boolean | undefined; - /** - * This is set in `background.js` so that `app-init.js` can trigger "on install" actions when - * the `onInstalled` listener is called. - */ - var __metamaskTriggerOnInstall: (() => void) | undefined; - namespace jest { // The interface is being used for declaration merging, which is an acceptable exception to this rule. // eslint-disable-next-line @typescript-eslint/consistent-type-definitions From 7e06ea8faf70eb5153b439b615ef3d910165f076 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Fri, 28 Mar 2025 15:40:49 -0230 Subject: [PATCH 11/12] Improve JSDoc type specificity --- app/scripts/app-init.js | 3 +-- app/scripts/background.js | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/app/scripts/app-init.js b/app/scripts/app-init.js index dbf5c2b8f44c..dab6cf4724e2 100644 --- a/app/scripts/app-init.js +++ b/app/scripts/app-init.js @@ -193,8 +193,7 @@ const registerInPageContentScript = async () => { * is never called. * For MV2 builds, the listener is added in `background.js` instead. * - * @param {object} details - Event details. - * @param {string} details.reason - The reason that this event was dispatched. + * @param {chrome.runtime.InstalledDetails} details - Event details. */ function onInstalledListener(details) { if (details.reason === 'install') { diff --git a/app/scripts/background.js b/app/scripts/background.js index 7859d210508b..4ab0b6ff486f 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -149,8 +149,7 @@ if (!isManifestV3) { * is never called. * There is no `app-init` file on MV2 builds, so we add a listener here instead. * - * @param {object} details - Event details. - * @param {string} details.reason - The reason that this event was dispatched. + * @param {import('webextension-polyfill').Runtime.OnInstalledDetailsType} details - Event details. */ const onInstalledListener = (details) => { if (details.reason === 'install') { From 18e2d1ba5cd58be9dab9ba76b25ef6a7ca6053e3 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Fri, 28 Mar 2025 16:10:05 -0230 Subject: [PATCH 12/12] Fix lint error by annotating stateHooks assignment with type --- app/scripts/app-init.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/scripts/app-init.js b/app/scripts/app-init.js index dab6cf4724e2..7abad9ad9957 100644 --- a/app/scripts/app-init.js +++ b/app/scripts/app-init.js @@ -6,6 +6,9 @@ let scriptsLoadInitiated = false; const { chrome } = globalThis; const testMode = process.env.IN_TEST; +/** + * @type {globalThis.stateHooks} + */ globalThis.stateHooks = globalThis.stateHooks || {}; const loadTimeLogs = [];