Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: use an interstitial page to load popup.html; load scripts using defered script tags #26555

Merged
merged 4 commits into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/manifest/v2/_base.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
16 changes: 15 additions & 1 deletion app/manifest/v2/firefox.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,19 @@
"id": "[email protected]",
"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
}
2 changes: 1 addition & 1 deletion app/manifest/v3/_base.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
2 changes: 1 addition & 1 deletion app/notification.html
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,6 @@
/>
</div>
<div id="popover-content"></div>
<script src="./scripts/load/ui.ts" async></script>
<script src="./scripts/load/ui.ts" defer></script>
</body>
</html>
39 changes: 39 additions & 0 deletions app/popup-init.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<!DOCTYPE html>
<html dir="ltr">
<head>
<meta http-equiv="refresh" content="0;url=/popup.html">
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
<title>MetaMask</title>
<style>
/**
* We try to match the user's system theme for the background color, but
* since the actual popup will use the theme from the user's extension
* settings we can't get it right 100% of the time. The browser doesn't
* allow a height or width of 0, otherwise we wouldn't bother with the
* background color.
* Unfortunately (and conveniently?), if the user *does* have a
* non-system theme set popup.html will flash the wrong theme, as the
* loading screen itself always uses the system theme. So while this page
* doesn't match popup.html's loading screen, the backgrounds will match,
* so the redirect isn't all that jarring.
*
* Implementing https://github.com/MetaMask/metamask-extension/issues/26545
* would improve the situation.
*
* Porting the loading screen (inlining all assets and CSS) to this init
* page may be a further UX improvement.
*/
html {
width: 357px;
height: 600px;
background: #f2f4f6;
}
@media screen and (prefers-color-scheme: dark) {
html {
background: #24272a;
}
}
</style>
</head>
</html>
26 changes: 0 additions & 26 deletions app/scripts/load-app.js

This file was deleted.

70 changes: 35 additions & 35 deletions development/build/scripts.js
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,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({
Expand All @@ -667,36 +673,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;
}
Expand All @@ -708,14 +717,7 @@ function createFactoredBuild({
browserPlatforms,
shouldIncludeSnow,
applyLavaMoat,
});
renderJavaScriptLoader({
groupSet,
commonSet,
browserPlatforms,
shouldIncludeSnow,
applyLavaMoat,
destinationFileName: 'load-background.js',
scripts,
});
if (isManifestV3) {
const jsBundles = [
Expand Down Expand Up @@ -745,17 +747,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;
}
Expand Down Expand Up @@ -1139,13 +1143,11 @@ async function createBundle(buildConfiguration, { reloadOnChange }) {
}
}

function renderJavaScriptLoader({
function getScriptTags({
groupSet,
commonSet,
browserPlatforms,
shouldIncludeSnow,
applyLavaMoat,
destinationFileName,
}) {
if (applyLavaMoat === undefined) {
throw new Error(
Expand Down Expand Up @@ -1179,17 +1181,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 `<script src="${src}" defer></script>`;
});
}

Expand All @@ -1200,6 +1193,7 @@ function renderHtmlFile({
applyLavaMoat,
isMMI,
isTest,
scripts = [],
}) {
if (applyLavaMoat === undefined) {
throw new Error(
Expand All @@ -1212,7 +1206,12 @@ 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();
Expand All @@ -1223,9 +1222,10 @@ function renderHtmlFile({
.replace('./scripts/load/background.ts', './load-background.js')
.replace(
'<script src="./load-background.js" defer></script>',
'<script src="./load-background.js" defer></script>\n <script src="./chromereload.js" defer></script>',
`${scriptTags}\n <script src="./chromereload.js" async></script>`,
)
.replace('./scripts/load/ui.ts', './load-app.js')
.replace('<script src="./scripts/load/ui.ts" defer></script>', scriptTags)
.replace('<script src="./load-offscreen.js" defer></script>', scriptTags)
.replace('../ui/css/index.scss', './index.css')
.replace('@lavamoat/snow/snow.prod.js', './scripts/snow.js');
browserPlatforms.forEach((platform) => {
Expand Down
9 changes: 0 additions & 9 deletions development/build/static.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,10 +165,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`
Expand All @@ -191,11 +187,6 @@ function getCopyTargets(shouldIncludeLockdown, shouldIncludeSnow) {
dest: `scripts/runtime-lavamoat.js`,
pattern: '',
},
{
src: `./offscreen/`,
pattern: `*.html`,
dest: '',
},
{
src: getPathInsideNodeModules('@blockaid/ppom_release', '/'),
pattern: '*.wasm',
Expand Down
2 changes: 1 addition & 1 deletion offscreen/offscreen.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@
<title>MetaMask Offscreen Page</title>
</head>
<body>
<script src="./load-offscreen.js"></script>
<script src="./load-offscreen.js" defer></script>
</body>
</html>
Loading