Skip to content

Commit aa1b311

Browse files
supermassivesupermassive
authored
Fix DanglingUntriaged pointers in wallet (#27952)
1 parent 0c0b725 commit aa1b311

33 files changed

+455
-419
lines changed

‎browser/brave_content_browser_client.cc

+6-68
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313

1414
#include "base/command_line.h"
1515
#include "base/functional/bind.h"
16-
#include "base/json/json_reader.h"
1716
#include "base/strings/strcat.h"
1817
#include "base/system/sys_info.h"
1918
#include "brave/browser/ai_chat/ai_chat_service_factory.h"
@@ -28,6 +27,7 @@
2827
#include "brave/browser/brave_wallet/brave_wallet_context_utils.h"
2928
#include "brave/browser/brave_wallet/brave_wallet_provider_delegate_impl.h"
3029
#include "brave/browser/brave_wallet/brave_wallet_service_factory.h"
30+
#include "brave/browser/brave_wallet/brave_wallet_tab_helper.h"
3131
#include "brave/browser/cosmetic_filters/cosmetic_filters_tab_helper.h"
3232
#include "brave/browser/debounce/debounce_service_factory.h"
3333
#include "brave/browser/ephemeral_storage/ephemeral_storage_service_factory.h"
@@ -76,8 +76,6 @@
7676
#include "brave/components/brave_wallet/browser/brave_wallet_p3a_private.h"
7777
#include "brave/components/brave_wallet/browser/brave_wallet_service.h"
7878
#include "brave/components/brave_wallet/browser/brave_wallet_utils.h"
79-
#include "brave/components/brave_wallet/browser/ethereum_provider_impl.h"
80-
#include "brave/components/brave_wallet/browser/solana_provider_impl.h"
8179
#include "brave/components/brave_wallet/common/brave_wallet.mojom.h"
8280
#include "brave/components/brave_wallet/common/common_utils.h"
8381
#include "brave/components/brave_webtorrent/browser/buildflags/buildflags.h"
@@ -368,65 +366,6 @@ void MaybeBindWalletP3A(
368366
}
369367
}
370368

371-
void MaybeBindEthereumProvider(
372-
content::RenderFrameHost* const frame_host,
373-
mojo::PendingReceiver<brave_wallet::mojom::EthereumProvider> receiver) {
374-
auto* brave_wallet_service =
375-
brave_wallet::BraveWalletServiceFactory::GetServiceForContext(
376-
frame_host->GetBrowserContext());
377-
if (!brave_wallet_service) {
378-
return;
379-
}
380-
381-
content::WebContents* web_contents =
382-
content::WebContents::FromRenderFrameHost(frame_host);
383-
mojo::MakeSelfOwnedReceiver(
384-
std::make_unique<brave_wallet::EthereumProviderImpl>(
385-
HostContentSettingsMapFactory::GetForProfile(
386-
Profile::FromBrowserContext(frame_host->GetBrowserContext())),
387-
brave_wallet_service,
388-
std::make_unique<brave_wallet::BraveWalletProviderDelegateImpl>(
389-
web_contents, frame_host),
390-
user_prefs::UserPrefs::Get(web_contents->GetBrowserContext())),
391-
std::move(receiver));
392-
}
393-
394-
void MaybeBindSolanaProvider(
395-
content::RenderFrameHost* const frame_host,
396-
mojo::PendingReceiver<brave_wallet::mojom::SolanaProvider> receiver) {
397-
auto* brave_wallet_service =
398-
brave_wallet::BraveWalletServiceFactory::GetServiceForContext(
399-
frame_host->GetBrowserContext());
400-
if (!brave_wallet_service) {
401-
return;
402-
}
403-
404-
auto* json_rpc_service = brave_wallet_service->json_rpc_service();
405-
CHECK(json_rpc_service);
406-
407-
auto* keyring_service = brave_wallet_service->keyring_service();
408-
CHECK(keyring_service);
409-
410-
auto* tx_service = brave_wallet_service->tx_service();
411-
CHECK(tx_service);
412-
413-
auto* host_content_settings_map =
414-
HostContentSettingsMapFactory::GetForProfile(
415-
frame_host->GetBrowserContext());
416-
if (!host_content_settings_map) {
417-
return;
418-
}
419-
420-
content::WebContents* web_contents =
421-
content::WebContents::FromRenderFrameHost(frame_host);
422-
mojo::MakeSelfOwnedReceiver(
423-
std::make_unique<brave_wallet::SolanaProviderImpl>(
424-
*host_content_settings_map, brave_wallet_service,
425-
std::make_unique<brave_wallet::BraveWalletProviderDelegateImpl>(
426-
web_contents, frame_host)),
427-
std::move(receiver));
428-
}
429-
430369
void BindBraveSearchFallbackHost(
431370
content::ChildProcessId process_id,
432371
mojo::PendingReceiver<brave_search::mojom::BraveSearchFallback> receiver) {
@@ -842,10 +781,10 @@ void BraveContentBrowserClient::RegisterBrowserInterfaceBindersForFrame(
842781
if (brave_wallet::IsAllowedForContext(
843782
render_frame_host->GetBrowserContext())) {
844783
if (brave_wallet::IsNativeWalletEnabled()) {
845-
map->Add<brave_wallet::mojom::EthereumProvider>(
846-
base::BindRepeating(&MaybeBindEthereumProvider));
847-
map->Add<brave_wallet::mojom::SolanaProvider>(
848-
base::BindRepeating(&MaybeBindSolanaProvider));
784+
map->Add<brave_wallet::mojom::EthereumProvider>(base::BindRepeating(
785+
&brave_wallet::BraveWalletTabHelper::BindEthereumProvider));
786+
map->Add<brave_wallet::mojom::SolanaProvider>(base::BindRepeating(
787+
&brave_wallet::BraveWalletTabHelper::BindSolanaProvider));
849788
}
850789
}
851790

@@ -1060,8 +999,7 @@ void BraveContentBrowserClient::WillCreateURLLoaderFactory(
1060999
scoped_refptr<base::SequencedTaskRunner> navigation_response_task_runner) {
10611000
// TODO(iefremov): Skip proxying for certain requests?
10621001
BraveProxyingURLLoaderFactory::MaybeProxyRequest(
1063-
browser_context, frame,
1064-
factory_builder, navigation_response_task_runner);
1002+
browser_context, frame, factory_builder, navigation_response_task_runner);
10651003

10661004
ChromeContentBrowserClient::WillCreateURLLoaderFactory(
10671005
browser_context, frame, render_process_id, type, request_initiator,

‎browser/brave_wallet/BUILD.gn

+10-2
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,6 @@ source_set("brave_wallet") {
5858

5959
source_set("brave_wallet_delegate") {
6060
sources = [
61-
"brave_wallet_provider_delegate_impl.cc",
62-
"brave_wallet_provider_delegate_impl.h",
6361
"brave_wallet_service_delegate_base.cc",
6462
"brave_wallet_service_delegate_base.h",
6563
"brave_wallet_service_delegate_helper.cc",
@@ -136,18 +134,28 @@ source_set("external_wallets_importer") {
136134

137135
source_set("tab_helper") {
138136
sources = [
137+
"brave_wallet_provider_delegate_impl.cc",
138+
"brave_wallet_provider_delegate_impl.h",
139139
"brave_wallet_tab_helper.cc",
140140
"brave_wallet_tab_helper.h",
141141
]
142142

143143
deps = [
144+
"//brave/browser/brave_wallet:brave_wallet",
144145
"//brave/common",
146+
"//brave/components/brave_wallet/browser:browser",
147+
"//brave/components/brave_wallet/browser:constants",
145148
"//brave/components/brave_wallet/browser:permission_utils",
149+
"//brave/components/brave_wallet/browser:utils",
150+
"//chrome/browser/content_settings:content_settings_factory",
146151
"//components/permissions",
147152
"//components/sessions",
153+
"//components/user_prefs:user_prefs",
148154
"//content/public/browser",
149155
]
150156

157+
public_deps = [ "//brave/components/brave_wallet/common:mojom" ]
158+
151159
if (!is_android) {
152160
deps += [ "//brave/browser/ui/brave_wallet" ]
153161
}

‎browser/brave_wallet/brave_wallet_event_emitter_browsertest.cc

+13-12
Original file line numberDiff line numberDiff line change
@@ -95,11 +95,6 @@ class BraveWalletEventEmitterTest : public InProcessBrowserTest {
9595
test_data_dir = test_data_dir.AppendASCII(kEmbeddedTestServerDirectory);
9696
https_server_->ServeFilesFromDirectory(test_data_dir);
9797

98-
brave_wallet_service_ =
99-
brave_wallet::BraveWalletServiceFactory::GetServiceForContext(
100-
browser()->profile());
101-
keyring_service_ = brave_wallet_service_->keyring_service();
102-
10398
ASSERT_TRUE(https_server_->Start());
10499
}
105100

@@ -113,7 +108,7 @@ class BraveWalletEventEmitterTest : public InProcessBrowserTest {
113108
mojo::Remote<brave_wallet::mojom::JsonRpcService> GetJsonRpcService() {
114109
if (!json_rpc_service_) {
115110
mojo::PendingRemote<brave_wallet::mojom::JsonRpcService> remote;
116-
brave_wallet_service_->json_rpc_service()->Bind(
111+
brave_wallet_service()->json_rpc_service()->Bind(
117112
remote.InitWithNewPipeAndPassReceiver());
118113
json_rpc_service_.Bind(std::move(remote));
119114
}
@@ -128,27 +123,33 @@ class BraveWalletEventEmitterTest : public InProcessBrowserTest {
128123
return browser()->tab_strip_model()->GetActiveWebContents();
129124
}
130125

126+
BraveWalletService* brave_wallet_service() {
127+
return BraveWalletServiceFactory::GetServiceForContext(
128+
browser()->profile());
129+
}
130+
131+
KeyringService* keyring_service() {
132+
return brave_wallet_service()->keyring_service();
133+
}
134+
131135
url::Origin GetLastCommitedOrigin() {
132136
return url::Origin::Create(web_contents()->GetLastCommittedURL());
133137
}
134138

135-
AccountUtils GetAccountUtils() { return AccountUtils(keyring_service_); }
139+
AccountUtils GetAccountUtils() { return AccountUtils(keyring_service()); }
136140

137141
void RestoreWallet() {
138-
ASSERT_TRUE(keyring_service_->RestoreWalletSync(
142+
ASSERT_TRUE(keyring_service()->RestoreWalletSync(
139143
kMnemonicDripCaution, kTestWalletPassword, false));
140144
}
141145

142146
void SetSelectedAccount(const mojom::AccountIdPtr& account_id) {
143-
ASSERT_TRUE(keyring_service_->SetSelectedAccountSync(account_id.Clone()));
147+
ASSERT_TRUE(keyring_service()->SetSelectedAccountSync(account_id.Clone()));
144148
}
145149

146150
private:
147151
content::ContentMockCertVerifier mock_cert_verifier_;
148152
mojo::Remote<brave_wallet::mojom::JsonRpcService> json_rpc_service_;
149-
raw_ptr<BraveWalletService, DanglingUntriaged> brave_wallet_service_ =
150-
nullptr;
151-
raw_ptr<KeyringService, DanglingUntriaged> keyring_service_ = nullptr;
152153
std::unique_ptr<net::EmbeddedTestServer> https_server_;
153154
base::test::ScopedFeatureList feature_list_;
154155
};

‎browser/brave_wallet/brave_wallet_service_browsertest.cc

+7-5
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,14 @@
33
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
44
* You can obtain one at https://mozilla.org/MPL/2.0/. */
55

6+
#include "brave/components/brave_wallet/browser/brave_wallet_service.h"
7+
68
#include "base/memory/raw_ptr.h"
79
#include "base/path_service.h"
810
#include "base/test/bind.h"
911
#include "base/test/mock_callback.h"
1012
#include "base/test/scoped_feature_list.h"
1113
#include "brave/browser/brave_wallet/brave_wallet_service_factory.h"
12-
#include "brave/components/brave_wallet/browser/brave_wallet_service.h"
1314
#include "brave/components/brave_wallet/browser/brave_wallet_service_observer_base.h"
1415
#include "brave/components/brave_wallet/browser/brave_wallet_utils.h"
1516
#include "brave/components/brave_wallet/common/brave_wallet.mojom.h"
@@ -84,8 +85,6 @@ class BraveWalletServiceTest : public InProcessBrowserTest {
8485
https_server_.ServeFilesFromDirectory(test_data_dir);
8586
EXPECT_TRUE(https_server_.Start());
8687
host_resolver()->AddRule("*", "127.0.0.1");
87-
wallet_service_ = brave_wallet::BraveWalletServiceFactory::GetInstance()
88-
->GetServiceForContext(browser()->profile());
8988
}
9089

9190
void TestIsPrivateWindow(BraveWalletService* wallet_service,
@@ -96,7 +95,11 @@ class BraveWalletServiceTest : public InProcessBrowserTest {
9695
wallet_service->IsPrivateWindow(callback.Get());
9796
}
9897

99-
BraveWalletService* wallet_service() { return wallet_service_; }
98+
BraveWalletService* wallet_service() {
99+
return BraveWalletServiceFactory::GetServiceForContext(
100+
browser()->profile());
101+
}
102+
100103
BraveWalletService* incognito_wallet_service() {
101104
return brave_wallet::BraveWalletServiceFactory::GetInstance()
102105
->GetServiceForContext(
@@ -105,7 +108,6 @@ class BraveWalletServiceTest : public InProcessBrowserTest {
105108
const net::EmbeddedTestServer* https_server() const { return &https_server_; }
106109

107110
private:
108-
raw_ptr<BraveWalletService, DanglingUntriaged> wallet_service_ = nullptr;
109111
net::EmbeddedTestServer https_server_;
110112
base::test::ScopedFeatureList feature_list_;
111113
};

‎browser/brave_wallet/brave_wallet_sign_message_browsertest.cc

+13-12
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,6 @@ class BraveWalletSignMessageBrowserTest : public InProcessBrowserTest {
9292
test_data_dir = test_data_dir.AppendASCII("brave-wallet");
9393
https_server_.ServeFilesFromDirectory(test_data_dir);
9494
ASSERT_TRUE(https_server()->Start());
95-
96-
brave_wallet_service_ =
97-
brave_wallet::BraveWalletServiceFactory::GetServiceForContext(
98-
browser()->profile());
99-
keyring_service_ = brave_wallet_service_->keyring_service();
10095
}
10196

10297
content::WebContents* web_contents() {
@@ -106,7 +101,7 @@ class BraveWalletSignMessageBrowserTest : public InProcessBrowserTest {
106101
net::EmbeddedTestServer* https_server() { return &https_server_; }
107102

108103
void RestoreWallet() {
109-
ASSERT_TRUE(keyring_service_->RestoreWalletSync(
104+
ASSERT_TRUE(keyring_service()->RestoreWalletSync(
110105
kMnemonicDripCaution, kTestWalletPassword, false));
111106
}
112107
void UserGrantPermission(bool granted) {
@@ -139,9 +134,16 @@ class BraveWalletSignMessageBrowserTest : public InProcessBrowserTest {
139134
https_server()->GetURL(uri, "/sign_message.html").spec());
140135
}
141136

137+
BraveWalletService* brave_wallet_service() {
138+
return BraveWalletServiceFactory::GetServiceForContext(
139+
browser()->profile());
140+
}
141+
142+
KeyringService* keyring_service() {
143+
return brave_wallet_service()->keyring_service();
144+
}
145+
142146
protected:
143-
raw_ptr<BraveWalletService, DanglingUntriaged> brave_wallet_service_ =
144-
nullptr;
145147
std::vector<std::string> methods_{"signMessage", "signMessageViaSend",
146148
"signMessageViaSend2",
147149
"signMessageViaSendAsync"};
@@ -150,7 +152,6 @@ class BraveWalletSignMessageBrowserTest : public InProcessBrowserTest {
150152
content::ContentMockCertVerifier mock_cert_verifier_;
151153
base::test::ScopedFeatureList scoped_feature_list_;
152154
net::test_server::EmbeddedTestServer https_server_;
153-
raw_ptr<KeyringService, DanglingUntriaged> keyring_service_ = nullptr;
154155
};
155156

156157
IN_PROC_BROWSER_TEST_F(BraveWalletSignMessageBrowserTest, UserApprovedRequest) {
@@ -172,7 +173,7 @@ IN_PROC_BROWSER_TEST_F(BraveWalletSignMessageBrowserTest, UserApprovedRequest) {
172173
// Wait for EthereumProviderImpl::ContinueSignMessage
173174
base::RunLoop().RunUntilIdle();
174175
EXPECT_TRUE(WaitForWalletBubble(web_contents()));
175-
brave_wallet_service_->NotifySignMessageRequestProcessed(
176+
brave_wallet_service()->NotifySignMessageRequestProcessed(
176177
true, request_index++, nullptr, std::nullopt);
177178
EXPECT_EQ(EvalJs(web_contents(), "getSignMessageResult()").ExtractString(),
178179
"0x670651c072cac2a3f93cb862a17378f6849c66b4516e5d5a30210868a2840e"
@@ -200,7 +201,7 @@ IN_PROC_BROWSER_TEST_F(BraveWalletSignMessageBrowserTest, UserRejectedRequest) {
200201
// Wait for EthereumProviderImpl::ContinueSignMessage
201202
base::RunLoop().RunUntilIdle();
202203
EXPECT_TRUE(WaitForWalletBubble(web_contents()));
203-
brave_wallet_service_->NotifySignMessageRequestProcessed(
204+
brave_wallet_service()->NotifySignMessageRequestProcessed(
204205
false, request_index++, nullptr, std::nullopt);
205206
EXPECT_EQ(EvalJs(web_contents(), "getSignMessageResult()").ExtractString(),
206207
l10n_util::GetStringUTF8(IDS_WALLET_USER_REJECTED_REQUEST));
@@ -338,7 +339,7 @@ IN_PROC_BROWSER_TEST_F(BraveWalletSignMessageBrowserTest, SIWE) {
338339
// Wait for EthereumProviderImpl::ContinueSignMessage
339340
base::RunLoop().RunUntilIdle();
340341
EXPECT_TRUE(WaitForWalletBubble(web_contents()));
341-
brave_wallet_service_->NotifySignMessageRequestProcessed(
342+
brave_wallet_service()->NotifySignMessageRequestProcessed(
342343
true, request_index++, nullptr, std::nullopt);
343344
// port is dynamic
344345
EXPECT_TRUE(EvalJs(web_contents(), "getSignMessageResult()")

0 commit comments

Comments
 (0)