Revise the behavior of "Extensions (Brave Wallet fallback)" setting #18172

cypt4 · 2023-04-20T20:06:25Z

Disable brave wallet script injection if wallet is not created or MM is installed
Resolves brave/brave-browser#29853

Submitter Checklist:

I confirm that no security/privacy review is needed, or that I have requested one
There is a ticket for my issue
Used Github auto-closing keywords in the PR description above
Wrote a good PR/commit description
Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
Checked the PR locally:
- npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
- npm run lint, npm run presubmit wiki, npm run gn_check, npm run tslint
Ran git rebase master (if needed)

Reviewer Checklist:

A security review is not needed, or a link to one is included in the PR description
New files have MPL-2.0 license header
Adequate test coverage exists to prevent regressions
Major classes, functions and non-trivial code blocks are well-commented
Changes in component dependencies are properly reflected in gn
Code follows the style guide
Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

Extensions (Brave Wallet fallback) should be selected as default wallets for ETH and SOL

for ETH brave wallet should not be used on pages when wallet is not created or metamask extension is installed
for SOL only wallet creation status is taken into account

Steps:

Open clean browser.
Ensure "Extensions (Brave Wallet fallback)" is used for ETH and SOL in settings
Open new page.
Open developer console and type window.ethereum
Make sure that window.ethereum is undefined
Type window.solana
Make sure that window.solana is undefined
Create wallet
Open new page
Make sure window.ethereum and window.solana are not undefined and have property isBraveWallet == true
Install MetaMask extension
Open new page
Ensure window.ethereum.isBraveWallet == false, ensure window.solana.isBraveWallet is still true.

browser/profiles/brave_renderer_updater.cc

renderer/test/js_ethereum_provider_browsertest.cc

browser/brave_wallet/solana_provider_renderer_browsertest.cc

browser/profiles/brave_renderer_updater.cc

browser/brave_wallet/solana_provider_renderer_browsertest.cc

browser/profiles/brave_renderer_updater.cc

browser/brave_wallet/solana_provider_renderer_browsertest.cc

browser/profiles/brave_renderer_updater.h

renderer/test/js_ethereum_provider_browsertest.cc

browser/brave_wallet/solana_provider_renderer_browsertest.cc

renderer/test/js_ethereum_provider_browsertest.cc

…is installed Resolves brave/brave-browser#29853

darkdh

++, please squash before merging

browser/profiles/brave_renderer_updater.cc

darkdh

Lgtm

srirambv · 2023-04-28T07:20:12Z

Verification passed on

Brave	1.53.2 Chromium: 113.0.5672.53 (Official Build) nightly (64-bit)
Revision	`12f5dac35d12e8f4e72d7dd11df557ef93bc046f-refs/branch-heads/5672@{#703}`
OS	Linux

Verified steps from PR
Verified when no wallet is created window.ethereum and window.solana shows as undefined
Verified when Brave Wallet is created window.ethereum and window.solana shows isBraveWallet: true
Verified when MetaMask is installed window.solana continues to show isBraveWallet: true
Verified when MetaMask is installed window.ethereum doesn't show isBraveWallet: true but instead shows isMetaMask: true

18172.mp4

…uplift to 1.51.x) (#18228) Uplift of #18172 (squashed) to beta

…uplift to 1.50.x) (#18229) Uplift of #18172 (squashed) to release

commit c98ff5e Author: brave-builds <brave-builds+devops@brave.com> Date: Fri Apr 28 08:29:10 2023 +0000 1.51.107 commit ce813ef Author: Max <41635752+mkarolin@users.noreply.github.com> Date: Fri Apr 28 04:25:41 2023 -0400 Upgrade from Chromium 113.0.5672.53 to Chromium 113.0.5672.63 (1.51.x). (#18269) * Upgrade from Chromium 113.0.5672.53 to Chromium 113.0.5672.63 * Upgrade patches from Chromium 113.0.5672.53 to Chromium 113.0.5672.63 * Update pins list timestamp * Temporarily disables WindowClosingConfirmBrowserTest.TestWithDownload on MacOS. The test fails because in Chromium Private/Incognito profiles now ignore the "Ask for download location" setting and always prompt for the location. We will fix that in brave/brave-browser#29823, but until then let's disable this test on MacOS. --------- Co-authored-by: brave-builds <brave-builds+devops@brave.com> commit b18aeae Author: brave-builds <45370463+brave-builds@users.noreply.github.com> Date: Fri Apr 28 10:15:10 2023 +0200 Disable site affiliation fetcher (uplift to 1.51.x) (#18286) Uplift of #18153 (squashed) to release commit 6b1db58 Author: brave-builds <45370463+brave-builds@users.noreply.github.com> Date: Fri Apr 28 09:47:20 2023 +0200 Revise the behavior of "Extensions (Brave Wallet fallback)" setting (uplift to 1.51.x) (#18228) Uplift of #18172 (squashed) to beta commit f7f8042 Author: brave-builds <45370463+brave-builds@users.noreply.github.com> Date: Fri Apr 28 09:42:41 2023 +0200 Send kUAModel and kUAPlatformVersion CHs when requested. (uplift to 1.51.x) (#18263) Uplift of #18154 (squashed) to beta commit 545fa57 Author: Max <41635752+mkarolin@users.noreply.github.com> Date: Fri Apr 28 03:07:51 2023 -0400 Don't call GetSessionRoute() when media router is disabled (#18245) (1.51.x). (#18264) Don't call GetSessionRoute() when media router is disabled (#18245) Otherwise, it would end in reaching NOTREACHED() Co-authored-by: Sangwoo Ko <sko@brave.com> commit 0fbe592 Author: Arthur Edelstein <aedelstein@brave.com> Date: Thu Apr 27 17:44:23 2023 -0700 Uplift HTTPS First Mode v2 Migration (#17856) and HTTP error code fallback (#18141) (#18179) * Migrate to HttpsFirstMode v2 (#17856) Migrate HTTPS by Default feature to use HttpsFirstMode v2 * Force HTTPS Upgrader to fall back to HTTP if we have an HTTP error code (#18141) commit e369d71 Author: Aleksey Khoroshilov <5928869+goodov@users.noreply.github.com> Date: Fri Apr 28 03:34:51 2023 +0700 Disable flaky upstream tests. (Uplift to 1.51.x) (#18274) Update nft-details.tsx

Hide network icon commit c98ff5e Author: brave-builds <brave-builds+devops@brave.com> Date: Fri Apr 28 08:29:10 2023 +0000 1.51.107 commit ce813ef Author: Max <41635752+mkarolin@users.noreply.github.com> Date: Fri Apr 28 04:25:41 2023 -0400 Upgrade from Chromium 113.0.5672.53 to Chromium 113.0.5672.63 (1.51.x). (#18269) * Upgrade from Chromium 113.0.5672.53 to Chromium 113.0.5672.63 * Upgrade patches from Chromium 113.0.5672.53 to Chromium 113.0.5672.63 * Update pins list timestamp * Temporarily disables WindowClosingConfirmBrowserTest.TestWithDownload on MacOS. The test fails because in Chromium Private/Incognito profiles now ignore the "Ask for download location" setting and always prompt for the location. We will fix that in brave/brave-browser#29823, but until then let's disable this test on MacOS. --------- Co-authored-by: brave-builds <brave-builds+devops@brave.com> commit b18aeae Author: brave-builds <45370463+brave-builds@users.noreply.github.com> Date: Fri Apr 28 10:15:10 2023 +0200 Disable site affiliation fetcher (uplift to 1.51.x) (#18286) Uplift of #18153 (squashed) to release commit 6b1db58 Author: brave-builds <45370463+brave-builds@users.noreply.github.com> Date: Fri Apr 28 09:47:20 2023 +0200 Revise the behavior of "Extensions (Brave Wallet fallback)" setting (uplift to 1.51.x) (#18228) Uplift of #18172 (squashed) to beta commit f7f8042 Author: brave-builds <45370463+brave-builds@users.noreply.github.com> Date: Fri Apr 28 09:42:41 2023 +0200 Send kUAModel and kUAPlatformVersion CHs when requested. (uplift to 1.51.x) (#18263) Uplift of #18154 (squashed) to beta commit 545fa57 Author: Max <41635752+mkarolin@users.noreply.github.com> Date: Fri Apr 28 03:07:51 2023 -0400 Don't call GetSessionRoute() when media router is disabled (#18245) (1.51.x). (#18264) Don't call GetSessionRoute() when media router is disabled (#18245) Otherwise, it would end in reaching NOTREACHED() Co-authored-by: Sangwoo Ko <sko@brave.com> commit 0fbe592 Author: Arthur Edelstein <aedelstein@brave.com> Date: Thu Apr 27 17:44:23 2023 -0700 Uplift HTTPS First Mode v2 Migration (#17856) and HTTP error code fallback (#18141) (#18179) * Migrate to HttpsFirstMode v2 (#17856) Migrate HTTPS by Default feature to use HttpsFirstMode v2 * Force HTTPS Upgrader to fall back to HTTP if we have an HTTP error code (#18141) commit e369d71 Author: Aleksey Khoroshilov <5928869+goodov@users.noreply.github.com> Date: Fri Apr 28 03:34:51 2023 +0700 Disable flaky upstream tests. (Uplift to 1.51.x) (#18274) Update nft-details.tsx

bridiver · 2024-07-18T16:34:28Z

browser/profiles/brave_renderer_updater.cc

@@ -9,8 +9,11 @@

 #include "base/functional/bind.h"
 #include "brave/browser/brave_wallet/brave_wallet_context_utils.h"
+#include "brave/browser/brave_wallet/keyring_service_factory.h"
+#include "brave/browser/ethereum_remote_client/ethereum_remote_client_constants.h"


this is adding another dep that prevents us from fixing check_includes = false. ethereum_remote_client_constants.h should go in a separate target to avoid this. keyring_service_factory.h is also a problem here, but that has since been removed

actually looks like maybe this is ok now after other changes

github-actions bot assigned cypt4 Apr 20, 2023

cypt4 requested a review from bbondy April 20, 2023 20:41

darkdh reviewed Apr 20, 2023

View reviewed changes

browser/profiles/brave_renderer_updater.cc Outdated Show resolved Hide resolved

browser/profiles/brave_renderer_updater.cc Outdated Show resolved Hide resolved

darkdh reviewed Apr 20, 2023

View reviewed changes

browser/profiles/brave_renderer_updater.cc Outdated Show resolved Hide resolved

darkdh reviewed Apr 20, 2023

View reviewed changes

browser/profiles/brave_renderer_updater.cc Outdated Show resolved Hide resolved

cypt4 force-pushed the brave_29853 branch from 0308b90 to 15ebef5 Compare April 21, 2023 11:43

cypt4 requested a review from a team as a code owner April 21, 2023 11:43

darkdh reviewed Apr 21, 2023

View reviewed changes

renderer/test/js_ethereum_provider_browsertest.cc Outdated Show resolved Hide resolved

browser/brave_wallet/solana_provider_renderer_browsertest.cc Outdated Show resolved Hide resolved

browser/profiles/brave_renderer_updater.cc Outdated Show resolved Hide resolved

cypt4 force-pushed the brave_29853 branch 2 times, most recently from 956c601 to 433fc99 Compare April 24, 2023 13:36

darkdh reviewed Apr 24, 2023

View reviewed changes

cypt4 added 2 commits April 25, 2023 01:31

Disable brave wallet script injection if wallet is not created or MM …

2828774

…is installed Resolves brave/brave-browser#29853

Review fix

c624e8e

cypt4 force-pushed the brave_29853 branch from 433fc99 to c624e8e Compare April 24, 2023 19:11

darkdh approved these changes Apr 24, 2023

View reviewed changes

darkdh reviewed Apr 24, 2023

View reviewed changes

browser/profiles/brave_renderer_updater.cc Outdated Show resolved Hide resolved

darkdh requested changes Apr 24, 2023

View reviewed changes

browser/profiles/brave_renderer_updater.cc Outdated Show resolved Hide resolved

Don't check metamask installation for Solana

253b40c

cypt4 requested a review from darkdh April 24, 2023 20:04

darkdh approved these changes Apr 24, 2023

View reviewed changes

cypt4 merged commit 639dd64 into master Apr 25, 2023

cypt4 deleted the brave_29853 branch April 25, 2023 15:41

github-actions bot added the potential-layer-violation-fixes This PR touches a BUILD.gn file with check_includes=false label Apr 25, 2023

github-actions bot added this to the 1.52.x - Nightly milestone Apr 25, 2023

brave-builds added a commit that referenced this pull request Apr 25, 2023

Uplift of #18172 (squashed) to beta

24455f1

brave-builds mentioned this pull request Apr 25, 2023

Revise the behavior of "Extensions (Brave Wallet fallback)" setting (uplift to 1.51.x) #18228

Merged

7 tasks

brave-builds added a commit that referenced this pull request Apr 25, 2023

Uplift of #18172 (squashed) to release

4e512cb

brave-builds mentioned this pull request Apr 25, 2023

Revise the behavior of "Extensions (Brave Wallet fallback)" setting (uplift to 1.50.x) #18229

Merged

7 tasks

kjozwiak mentioned this pull request Apr 25, 2023

Revise the behavior of "Extensions (Brave Wallet fallback)" setting brave/brave-browser#29853

Closed

kjozwiak pushed a commit that referenced this pull request Apr 28, 2023

Revise the behavior of "Extensions (Brave Wallet fallback)" setting (…

6b1db58

…uplift to 1.51.x) (#18228) Uplift of #18172 (squashed) to beta

kjozwiak pushed a commit that referenced this pull request Apr 28, 2023

Revise the behavior of "Extensions (Brave Wallet fallback)" setting (…

471d261

…uplift to 1.50.x) (#18229) Uplift of #18172 (squashed) to release

darkdh mentioned this pull request Jul 21, 2023

Implement eip-6963 #18751

Merged

25 tasks

darkdh mentioned this pull request Feb 5, 2024

Install Solana provider object even when wallet is not created #21908

Merged

24 tasks

bridiver reviewed Jul 18, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revise the behavior of "Extensions (Brave Wallet fallback)" setting #18172

Revise the behavior of "Extensions (Brave Wallet fallback)" setting #18172

cypt4 commented Apr 20, 2023 •

edited

Loading

darkdh left a comment

darkdh left a comment

srirambv commented Apr 28, 2023

bridiver Jul 18, 2024

bridiver Jul 18, 2024

Revise the behavior of "Extensions (Brave Wallet fallback)" setting #18172

Revise the behavior of "Extensions (Brave Wallet fallback)" setting #18172

Conversation

cypt4 commented Apr 20, 2023 • edited Loading

Submitter Checklist:

Reviewer Checklist:

After-merge Checklist:

Test Plan:

darkdh left a comment

Choose a reason for hiding this comment

darkdh left a comment

Choose a reason for hiding this comment

srirambv commented Apr 28, 2023

bridiver Jul 18, 2024

Choose a reason for hiding this comment

bridiver Jul 18, 2024

Choose a reason for hiding this comment

cypt4 commented Apr 20, 2023 •

edited

Loading