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

Do not parse JSON files in wallet component if wallet isn't created #33757

Closed
yrliou opened this issue Oct 19, 2023 · 4 comments · Fixed by brave/brave-core#20741
Closed

Do not parse JSON files in wallet component if wallet isn't created #33757

yrliou opened this issue Oct 19, 2023 · 4 comments · Fixed by brave/brave-core#20741
Assignees
Labels
feature/web3/wallet/core feature/web3/wallet Integrating Ethereum+ wallet support OS/Android Fixes related to Android browser functionality OS/Desktop perf priority/P2 A bad problem. We might uplift this to the next planned release. QA Pass - Android ARM QA Pass-Win64 QA/Yes release-notes/exclude

Comments

@yrliou
Copy link
Member

yrliou commented Oct 19, 2023

As this affects everyone's startup performance.

As investigated in a thread we parse the Wallet component even if there is no wallet created.

  • no wallet created => no startup work to read/parse json
  • (the best option) don't even register the component. This could create extra slow down on the first load, need to check.
  • add a (browser) test to verify that to prevent future regression.
@yrliou yrliou added priority/P2 A bad problem. We might uplift this to the next planned release. perf feature/web3/wallet Integrating Ethereum+ wallet support OS/Desktop labels Oct 19, 2023
@yrliou yrliou self-assigned this Oct 19, 2023
@bbondy
Copy link
Member

bbondy commented Oct 20, 2023

Maybe do this by not even getting the component until a wallet is being created? I'm pretty sure we did this for Android at least recently cc @SergeyZhukovsky

@SergeyZhukovsky
Copy link
Member

SergeyZhukovsky commented Oct 20, 2023

Yeah @AlexeyBarabash made it happen(download component on demand only) on Android in that PR brave/brave-core#19086. The main goal for Android was to not occupy space on devices unless wallet is created. The code could be re-used to share it with desktop as Java handlers got triggered from c++ code anyways.

@srirambv
Copy link
Contributor

srirambv commented Dec 8, 2023

Verification passed on

Brave 1.62.99 Chromium: 120.0.6099.62 (Official Build) beta (64-bit)
Revision 0f3e892de210168e788b3418961f94c4d0c5942a
OS Windows 11 Version 22H2 (Build 22621.2792)
  • Verified steps from brave/brave-core#20741
  • Verified wallet component is only downloaded after the wallet is created
  • Verified when wallet component is being downloaded loading state is shown
Clean Install MM Import Brave Wallet Import
33757-Clean.Wallet.mp4
33757-MM.Import.mp4
33757-Import.Wallet.mp4

@yrliou yrliou added the OS/Android Fixes related to Android browser functionality label Dec 8, 2023
@srirambv
Copy link
Contributor

Verification passed on Google Pixel 8 with Android 14 running 1.62.114 x64 Beta build

  • Verified steps from brave/brave-core#20741
  • Verified wallet component is only downloaded after the wallet is created
  • Verified when wallet component is being downloaded loading state is shown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/web3/wallet/core feature/web3/wallet Integrating Ethereum+ wallet support OS/Android Fixes related to Android browser functionality OS/Desktop perf priority/P2 A bad problem. We might uplift this to the next planned release. QA Pass - Android ARM QA Pass-Win64 QA/Yes release-notes/exclude
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants