From 880bc3eb490ac23335e87450360d7d47a40b9735 Mon Sep 17 00:00:00 2001 From: David Murdoch <187813+davidmurdoch@users.noreply.github.com> Date: Wed, 28 Aug 2024 11:07:51 -0400 Subject: [PATCH] perf: use an interstitial page to load `popup.html`; load scripts using `defer`ed script tags (#26555) This PR removes `load-*.js` helpers and load scripts using the `defer` property instead. This change drastically improves the performance of the extension but greatly reduces perceived performance because the browser won't display the popup until the DOMContentLoaded event. But improve perceived performance is mostly regained by initially loading an asset-less HTML file, `popup-init.html`, that then redirects to the slower (but much faster than before this PR) `popup.html`. This was initially authored by @Gudahtt, I've just updated and optimized it to work with the both build systems. One change I did not make was moving scripts to the `head`. I don't think putting the scripts in the head does anything in our case[^1] other than potentially require that we wait for `DOMContentLoaded` before querying for the app container. Note: In Firefox we continue to use `popup.html`. Firefox is very slow to render the `popup-init.html` redirect, and it renders the page only partially anyway, making the UX feel very janky. It also doesn't delay opening the popup until `DOMContentLoaded`, like Chrome does, so the issue `popup-init.html` solves doesn't do anything anyway. --- Here is side-by-side video comparison of `develop` vs `popup-defer-scripts`. This video displays the elapsed time from clicking the MetaMask Fox to Lock Screen render. The end of the video is a frame-by-frame comparison of the new "jank" this PR introduces. The develop branch renders the fox immediately, whereas the popup-defer-scripts branch renders an empty page for about 15 milliseconds. These stats aren't representative of real world performance, but are intended to illustrate relative performance and perceived performance. Both builds were created by running `yarn build dist`. https://github.com/user-attachments/assets/38b856ca-d269-48f7-b305-19f4b9c6dce8 --- 1. build with `yarn dist` 2. install into your browser and go through onboarding 3. open MetaMask 4. Marvel at how much faster it is --- Closes [#25721](https://github.com/MetaMask/metamask-extension/issues/25721) [^1]: our HTML files are very tiny, and thus will be loaded all in one go, we don't have to worry about packets being lost and retransmitted over the network, delaying the browser's preload scanner. Co-authored-by: Mark Stacey --- app/background.html | 2 +- app/home.html | 2 +- app/manifest/v2/_base.json | 2 +- app/manifest/v2/firefox.json | 16 ++++++- app/manifest/v3/_base.json | 2 +- app/notification.html | 2 +- app/popup-init.html | 39 +++++++++++++++++ app/popup.html | 2 +- app/scripts/load-app.js | 26 ----------- development/build/scripts.js | 85 ++++++++++++++++++++---------------- development/build/static.js | 9 ---- offscreen/offscreen.html | 2 +- 12 files changed, 108 insertions(+), 81 deletions(-) create mode 100644 app/popup-init.html delete mode 100644 app/scripts/load-app.js diff --git a/app/background.html b/app/background.html index c0068295d730..7200acf50495 100644 --- a/app/background.html +++ b/app/background.html @@ -5,6 +5,6 @@ - + diff --git a/app/home.html b/app/home.html index b94800b13d78..11cfed10242e 100644 --- a/app/home.html +++ b/app/home.html @@ -16,6 +16,6 @@
- + diff --git a/app/manifest/v2/_base.json b/app/manifest/v2/_base.json index 9843a20529de..31b1c82224fd 100644 --- a/app/manifest/v2/_base.json +++ b/app/manifest/v2/_base.json @@ -15,7 +15,7 @@ "512": "images/icon-512.png" }, "default_title": "MetaMask", - "default_popup": "popup.html" + "default_popup": "popup-init.html" }, "commands": { "_execute_browser_action": { diff --git a/app/manifest/v2/firefox.json b/app/manifest/v2/firefox.json index 6b0a122cfd9e..b2858bb8612f 100644 --- a/app/manifest/v2/firefox.json +++ b/app/manifest/v2/firefox.json @@ -4,5 +4,19 @@ "id": "webextension@metamask.io", "strict_min_version": "89.0" } - } + }, + "browser_action": { + "default_icon": { + "16": "images/icon-16.png", + "19": "images/icon-19.png", + "32": "images/icon-32.png", + "38": "images/icon-38.png", + "64": "images/icon-64.png", + "128": "images/icon-128.png", + "512": "images/icon-512.png" + }, + "default_title": "MetaMask", + "default_popup": "popup.html" + }, + "manifest_version": 2 } diff --git a/app/manifest/v3/_base.json b/app/manifest/v3/_base.json index f3ffbf07c173..71d083208b55 100644 --- a/app/manifest/v3/_base.json +++ b/app/manifest/v3/_base.json @@ -10,7 +10,7 @@ "512": "images/icon-512.png" }, "default_title": "MetaMask", - "default_popup": "popup.html" + "default_popup": "popup-init.html" }, "author": "https://metamask.io", "background": { diff --git a/app/notification.html b/app/notification.html index 70f02e9ab421..0356d2278800 100644 --- a/app/notification.html +++ b/app/notification.html @@ -50,6 +50,6 @@ />
- + diff --git a/app/popup-init.html b/app/popup-init.html new file mode 100644 index 000000000000..2fb79fefcfde --- /dev/null +++ b/app/popup-init.html @@ -0,0 +1,39 @@ + + + + + + + MetaMask + + + diff --git a/app/popup.html b/app/popup.html index 296b0ceae711..3cb985b27a25 100644 --- a/app/popup.html +++ b/app/popup.html @@ -12,6 +12,6 @@
- + diff --git a/app/scripts/load-app.js b/app/scripts/load-app.js deleted file mode 100644 index 4b92255a5bc2..000000000000 --- a/app/scripts/load-app.js +++ /dev/null @@ -1,26 +0,0 @@ -// eslint-disable-next-line import/unambiguous -'use strict'; - -setTimeout(() => { - // eslint-disable-next-line spaced-comment - const scriptsToLoad = [ - /* SCRIPTS */ - ]; - - const loadScript = (src) => { - const script = document.createElement('script'); - script.type = 'text/javascript'; - script.async = true; - script.onload = loadNext; - script.src = src; - document.body.appendChild(script); - }; - - loadNext(); - - function loadNext() { - if (scriptsToLoad.length) { - loadScript(scriptsToLoad.shift()); - } - } -}, 10); diff --git a/development/build/scripts.js b/development/build/scripts.js index c87197bfaf1c..5d8eeac900f7 100644 --- a/development/build/scripts.js +++ b/development/build/scripts.js @@ -661,6 +661,12 @@ function createFactoredBuild({ const isTest = buildTarget === BUILD_TARGETS.TEST || buildTarget === BUILD_TARGETS.TEST_DEV; + const scripts = getScriptTags({ + groupSet, + commonSet, + shouldIncludeSnow, + applyLavaMoat, + }); switch (groupLabel) { case 'ui': { renderHtmlFile({ @@ -669,36 +675,39 @@ function createFactoredBuild({ shouldIncludeSnow, applyLavaMoat, isMMI: buildType === 'mmi', + scripts, }); renderHtmlFile({ htmlName: 'popup', browserPlatforms, shouldIncludeSnow, applyLavaMoat, + scripts, }); renderHtmlFile({ - htmlName: 'notification', + htmlName: 'popup-init', browserPlatforms, shouldIncludeSnow, applyLavaMoat, - isMMI: buildType === 'mmi', - isTest, + scripts, }); renderHtmlFile({ - htmlName: 'home', + htmlName: 'notification', browserPlatforms, shouldIncludeSnow, applyLavaMoat, isMMI: buildType === 'mmi', isTest, + scripts, }); - renderJavaScriptLoader({ - groupSet, - commonSet, + renderHtmlFile({ + htmlName: 'home', browserPlatforms, shouldIncludeSnow, applyLavaMoat, - destinationFileName: 'load-app.js', + isMMI: buildType === 'mmi', + isTest, + scripts, }); break; } @@ -710,14 +719,7 @@ function createFactoredBuild({ browserPlatforms, shouldIncludeSnow, applyLavaMoat, - }); - renderJavaScriptLoader({ - groupSet, - commonSet, - browserPlatforms, - shouldIncludeSnow, - applyLavaMoat, - destinationFileName: 'load-background.js', + scripts, }); if (isEnableMV3) { const jsBundles = [ @@ -747,17 +749,19 @@ function createFactoredBuild({ browserPlatforms, shouldIncludeSnow, applyLavaMoat: false, + scripts, }); break; } case 'offscreen': { - renderJavaScriptLoader({ + renderHtmlFile({ + htmlName: 'offscreen', groupSet, commonSet, browserPlatforms, shouldIncludeSnow, applyLavaMoat, - destinationFileName: 'load-offscreen.js', + scripts, }); break; } @@ -1140,13 +1144,11 @@ async function createBundle(buildConfiguration, { reloadOnChange }) { } } -function renderJavaScriptLoader({ +function getScriptTags({ groupSet, commonSet, - browserPlatforms, shouldIncludeSnow, applyLavaMoat, - destinationFileName, }) { if (applyLavaMoat === undefined) { throw new Error( @@ -1180,17 +1182,8 @@ function renderJavaScriptLoader({ ...jsBundles, ]; - browserPlatforms.forEach((platform) => { - const appLoadFilePath = './app/scripts/load-app.js'; - const appLoadContents = readFileSync(appLoadFilePath, 'utf8'); - - const scriptDest = `./dist/${platform}/${destinationFileName}`; - const scriptOutput = appLoadContents.replace( - '/* SCRIPTS */', - `...${JSON.stringify(requiredScripts)}`, - ); - - writeFileSync(scriptDest, scriptOutput); + return requiredScripts.map((src) => { + return ``; }); } @@ -1201,6 +1194,7 @@ function renderHtmlFile({ applyLavaMoat, isMMI, isTest, + scripts = [], }) { if (applyLavaMoat === undefined) { throw new Error( @@ -1213,15 +1207,30 @@ function renderHtmlFile({ ); } - const htmlFilePath = `./app/${htmlName}.html`; + const scriptTags = scripts.join('\n '); + + const htmlFilePath = + htmlName === 'offscreen' + ? `./offscreen/${htmlName}.html` + : `./app/${htmlName}.html`; const htmlTemplate = readFileSync(htmlFilePath, 'utf8'); const eta = new Eta(); - const htmlOutput = eta.renderString(htmlTemplate, { - isMMI, - isTest, - shouldIncludeSnow, - }); + const htmlOutput = eta + .renderString(htmlTemplate, { + isMMI, + isTest, + shouldIncludeSnow, + }) + // these replacements are added to support the webpack build's automatic + // compilation of html files, which the gulp-based process doesn't support. + .replace('', scriptTags) + .replace('', scriptTags) + .replace( + '', + `${scriptTags}\n `, + ) + .replace('', scriptTags); browserPlatforms.forEach((platform) => { const dest = `./dist/${platform}/${htmlName}.html`; // we dont have a way of creating async events atm diff --git a/development/build/static.js b/development/build/static.js index cf0cd1078423..ff14657b621b 100644 --- a/development/build/static.js +++ b/development/build/static.js @@ -166,10 +166,6 @@ function getCopyTargets(shouldIncludeLockdown, shouldIncludeSnow) { src: './app/scripts/init-globals.js', dest: 'scripts/init-globals.js', }, - { - src: './app/scripts/load-app.js', - dest: 'scripts/load-app.js', - }, { src: shouldIncludeLockdown ? `./app/scripts/lockdown-run.js` @@ -192,11 +188,6 @@ function getCopyTargets(shouldIncludeLockdown, shouldIncludeSnow) { dest: `scripts/runtime-lavamoat.js`, pattern: '', }, - { - src: `./offscreen/`, - pattern: `*.html`, - dest: '', - }, { src: getPathInsideNodeModules('@blockaid/ppom_release', '/'), pattern: '*.wasm', diff --git a/offscreen/offscreen.html b/offscreen/offscreen.html index ea5ad76dd7a7..7137f2406213 100644 --- a/offscreen/offscreen.html +++ b/offscreen/offscreen.html @@ -5,6 +5,6 @@ MetaMask Offscreen Page - +