Address security concern etld dapps #13385
Conversation
| getWalletService().geteTldPlusOneFromOrigin(Utils.getCurrentMojomOrigin(), origin -> { | ||
| siteTv.setText(Utils.geteTLDFromGRUL(origin.eTldPlusOne)); | ||
| }); | ||
| getFavIcon(siteUrl.getOrigin()); |
There was a problem hiding this comment.
nit: rename to showFavIcon or something as getFavIcon returns void
| if (activity != null) { | ||
|
|
||
| FaviconImageCallback imageCallback = (bitmap, | ||
| iconUrl) -> AddSwitchChainNetworkFragment.this.onFaviconAvailable(url, bitmap); |
There was a problem hiding this comment.
nit: is it required AddSwitchChainNetworkFragment.this.?
| } | ||
| } | ||
|
|
||
| private void onFaviconAvailable(GURL pageUrl, Bitmap iconBitmap) { |
There was a problem hiding this comment.
nit: since it's not a callback method so would be better to rename to showFavIcon or similar for convention and per title case use Icon in FavIcon
...oid/java/org/chromium/chrome/browser/crypto_wallet/fragments/dapps/DAppsMessageFragment.java
Show resolved
Hide resolved
| </message> | ||
| <message name="IDS_BRAVE_WALLET_PROVIDE_ENCRYPTION_KEY_DESCRIPTION" desc="Provide Encryption Key Panel Details">is requesting your wallets public encryption key. If you consent to providing this key, the site will be able to compose encrypted messages to you.</message> |
There was a problem hiding this comment.
nit: description doesn't have to be in title case, seems like we have done this before so kindly ignore some existing examples or fix them to avoid the influence.
| private BraveWalletService getWalletService() { | ||
| assert getActivity() instanceof BraveWalletBaseActivity; | ||
| return ((BraveWalletBaseActivity) getActivity()).getBraveWalletService(); | ||
| } |
There was a problem hiding this comment.
redundant, remove this, BaseDAppsFragment already has the getBraveWalletService.
This is one of the side effect of this approach, better start using the domain layer or anything better to improve code quality.
|
@qamarngr I don't think CI is running btw if you work off your fork. As I see lint format isn't applied as well. Could you please work directly from brave-core repository? |
| @@ -112,7 +112,9 @@ public void onClick(View v) { | |||
| mRecyclerView = view.findViewById(R.id.accounts_list); | |||
| mFavicon = view.findViewById(R.id.favicon); | |||
|
|
|||
| mWebSite.setText(getCurrentHostHttpAddress().getSpec()); | |||
| getBraveWalletService().geteTldPlusOneFromOrigin(Utils.getCurrentMojomOrigin(), origin -> { | |||
| mWebSite.setText(Utils.geteTLDFromGRUL(origin.eTldPlusOne)); | |||
There was a problem hiding this comment.
oh sorry, I refactored it, I was previously passing GURL, later on I removed it.
| if (mBraveWalletService != null) { | ||
| return; | ||
| } | ||
| mBraveWalletService = BraveWalletServiceFactory.getInstance().getBraveWalletService(this); |
There was a problem hiding this comment.
we need to close the service in DisconnectMojoServices
|
Duplicate of #13398 |
Resolves brave/brave-browser#21798
Submitter Checklist:
QA/YesorQA/No;release-notes/includeorrelease-notes/exclude;OS/...) to the associated issuenpm run test -- brave_browser_tests,npm run test -- brave_unit_tests,npm run lint,npm run gn_check,npm run tslintgit rebase master(if needed)Reviewer Checklist:
gnAfter-merge Checklist:
changes has landed on
Test Plan:
Same as
#12701