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

A crash on checking for a badge #23604

Closed
SergeyZhukovsky opened this issue Jun 22, 2022 · 5 comments · Fixed by brave/brave-core#13896
Closed

A crash on checking for a badge #23604

SergeyZhukovsky opened this issue Jun 22, 2022 · 5 comments · Fixed by brave/brave-core#13896
Assignees
Labels
crash feature/web3/wallet/dapps feature/web3/wallet Integrating Ethereum+ wallet support OS/Android Fixes related to Android browser functionality priority/P1 A very extremely bad problem. We might push a hotfix for it. QA Pass - Android ARM QA Pass - Android x86 QA/Yes release-notes/include

Comments

@SergeyZhukovsky
Copy link
Member

SergeyZhukovsky commented Jun 22, 2022

java.lang.NullPointerException: 
  at org.chromium.brave_wallet.mojom.BraveWalletService_Internal$BraveWalletServiceGetPendingSignMessageRequestsResponseParams.run (BraveWalletService_Internal.java:405)
  at android.os.Handler.handleCallback (Handler.java:883)
  at android.os.Handler.dispatchMessage (Handler.java:100)
  at android.os.Looper.loop (Looper.java:214)
  at android.app.ActivityThread.main (ActivityThread.java:7386)
  at java.lang.reflect.Method.invoke (Native Method)
  at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:492)
  at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:980)

There is a crash I see in a Google Play Console. I cannot replicate it, but it looks like it's related to showing a badge on a wallet icon. It's possible that a call to mojo services are done to get any pending requests when native isn't ready yet.
So to fix it I'm going to do:

  1. Call it only from a place when we are 100% that native is ready
  2. Call it only when a wallet icon is visible as now it's called all the time on onResume.

Test Plan

Trigger badge on wallet icon

Test Case 1:

  • Visit https://chainlist.org and connect wallet
  • Click on Add network
  • Close the wallet popup, badge is shown on wallet
  • Background the browser and relaunch, ensure no browser crash

Test Case 2:

  • Visit https://app.skiff.com
  • Click on Login with Brave Wallet
  • Continue with account permission
  • Close the panel popup when the sign message is shown
  • Wallet should have a badge shown
  • Background the browser and relaunch, ensure no browser crash
  • Quit the browser and restart, wallet shouldn't display the badge (pending transactions are kept in memory which is cleared when exiting the browser)

Test Case 3:

  • Visit https://metamask.github.io/test-dapp
  • Click on Connect and connect the wallet
  • Click on Get Encryption key
  • Close the panel popup and close the browser
  • Quit the browser and restart, wallet shouldn't display the badge (pending transactions are kept in memory which is cleared when exiting the browser)
@LaurenWags
Copy link
Member

LaurenWags commented Jun 22, 2022

Verified on Asus Zenfone (x86) with Android 6 running 1.40.106

Test Case 1 - PASSED

  • Create wallet, lock wallet
  • Visit https://chainlist.org and connect wallet
  • Scroll to something down on list (like Moonriver) and click on Add to Brave
  • When "Allow this site to add a network?" popup shows up, use Android "back" button to close popup
  • Confirm badge is shown on wallet icon
  • Background the browser and relaunch, ensure no browser crash

Test Case 2 - PASSED

  • Visit https://app.skiff.com
  • Click on Login with Brave Wallet
  • Continue with account permission
  • Close the panel popup (using Android back button) when the sign message is shown
  • Wallet should have a badge shown
  • Background the browser and relaunch, ensure no browser crash and wallet icon still shows badge
  • Quit the browser and restart, no crash when relaunching the browser (no wallet icon displayed, no badge displayed)

Test Case 3 - PASSED

  • Visit https://metamask.github.io/test-dapp
  • Click on Connect and connect the wallet
  • Click on Get Encryption key
  • Close the panel popup (using Android back button) and background the browser
  • Reopen the browser, wallet icon should be shown with badge and no crash when bringing the browser to foreground
  • Quit the browser and restart, no crash when relaunching the browser (wallet icon displayed, no badge displayed)

@LaurenWags
Copy link
Member

LaurenWags commented Jun 22, 2022

Verified on Samsung J7 Neo with Android 7 running 1.40.106

Test Case 1 - PASSED

  • Create wallet, lock wallet
  • Visit https://chainlist.org and connect wallet
  • Scroll to something down on list (like Moonriver) and click on Add to Brave
  • When "Allow this site to add a network?" popup shows up, use Android "back" button to close popup
  • Confirm badge is shown on wallet icon
  • Background the browser and relaunch, ensure no browser crash

Test Case 2 - PASSED

  • Visit https://app.skiff.com
  • Click on Login with Brave Wallet
  • Continue with account permission
  • Close the panel popup (using Android back button) when the sign message is shown
  • Wallet should have a badge shown
  • Background the browser and relaunch, ensure no browser crash and wallet icon still shows badge
  • Quit the browser and restart, no crash when relaunching the browser (no wallet icon displayed, no badge displayed)

Test Case 3 - PASSED

  • Visit https://metamask.github.io/test-dapp
  • Click on Connect and connect the wallet
  • Click on Get Encryption key
  • Close the panel popup (using Android back button) and background the browser
  • Reopen the browser, wallet icon should be shown with badge and no crash when bringing the browser to foreground
  • Quit the browser and restart, no crash when relaunching the browser (wallet icon displayed, no badge displayed)

@stephendonner
Copy link

stephendonner commented Jun 22, 2022

Verified PASSED using

Brave 1.40.106, Chromium 103.0.5060.53 on a Google Pixel XL (arm64) running Android 9.0.

Test Case 1 - PASSED

  1. Create wallet, lock wallet
  2. Visit https://chainlist.org and connect wallet
  3. Scroll to something down on list (like Moonriver) and click on Add to Brave
  4. When "Allow this site to add a network?" popup shows up, use Android "back" button to close popup
  5. Confirm badge is shown on wallet icon
  6. Background the browser and relaunch, ensure no browser crash
20220622_135446.mp4

Test Case 2 - PASSED

  1. Visit https://app.skiff.com
  2. Click on Login with Brave Wallet
  3. Continue with account permission
  4. Close the panel popup (using Android back button) when the sign message is shown
  5. Wallet should have a badge shown
  6. Background the browser and relaunch, ensure no browser crash and wallet icon still shows badge
  7. Quit the browser and restart, no crash when relaunching the browser (no wallet icon displayed, no badge displayed)
20220622_140226.mp4

Test Case 3 - PASSED

  1. Visit https://metamask.github.io/test-dapp
  2. Click on Connect and connect the wallet
  3. Click on Get Encryption key
  4. Close the panel popup (using Android back button) and background the browser
  5. Reopen the browser, wallet icon should be shown with badge and no crash when bringing the browser to foreground
  6. Quit the browser and restart, no crash when relaunching the browser (no wallet icon displayed, no badge displayed)
20220622_140524.mp4

@Uni-verse
Copy link
Contributor

Uni-verse commented Jun 22, 2022

Verified on Samsung GS 21 5G & Tab S7 using

Brave	1.40.106 Chromium: 103.0.5060.53 (Official Build) (64-bit) 
Revision	a1711811edd74ff1cf2150f36ffa3b0dae40b17f-refs/branch-heads/5060@{#853}
OS	Android 12; Build/SP1A.210812.016
  • Confirmed crash is not reproducible for all test cases in A crash on checking for a badge #23604 (comment)
  • Confirmed badge is being displayed when there are wallet requests active
  • Confirmed wallet icon and badge is not displayed when restarting browser unless requests are not sent in UI (TC3, icon always shown)
  • Confirmed user is able to cancel pending request and error is displayed in UI
  • Confirmed new wallet flow is launched if connecting to dApp without wallet.
wallet_TC1.mp4
wallet_TC2.mp4
wallet_TC3.mp4

@kjozwiak
Copy link
Member

kjozwiak commented Jun 22, 2022

Verification PASSED on Pixel 6 running Android 13 using the following build(s):

Brave | 1.40.106 Chromium: 103.0.5060.53 (Official Build) (64-bit)
--- | ---
Revision | a1711811edd74ff1cf2150f36ffa3b0dae40b17f-refs/branch-heads/5060@{#853}
OS | Android 13; Build/TPB3.220610.004

Went through the STR/Cases outlined via #23604 (comment) and ensured that the badge on the wallet icon was still visible when backgrounding and then launching Brave. Also ensured there wasn't any crashes/issues.

  • Test Case #1 - PASSED
  • Test Case #2 - PASSED
  • Test Case #3 - PASSED

Examples of the above working as expected:

screen-20220622-172242.mp4
screen-20220622-172347.mp4
screen-20220622-172632.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crash feature/web3/wallet/dapps feature/web3/wallet Integrating Ethereum+ wallet support OS/Android Fixes related to Android browser functionality priority/P1 A very extremely bad problem. We might push a hotfix for it. QA Pass - Android ARM QA Pass - Android x86 QA/Yes release-notes/include
Projects
Archived in project
6 participants