Add dapp detection #2678
Merged
Add dapp detection #2678
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
simonhong
force-pushed
the
dapp_detection
branch
2 times, most recently
from
f80d7d2
to
dddde19
Compare
simonhong
force-pushed
the
dapp_detection
branch
3 times, most recently
from
4b459f8
to
80faaca
Compare
simonhong
force-pushed
the
dapp_detection
branch
from
80faaca
to
3c8cf93
Compare
|
@simonhong is this ready for review? (wasn't sure when I saw the |
|
@bsclifton Yup, it's ready. I added more description about TODOs above :) |
simonhong
commented
Jun 17, 2019
simonhong
force-pushed
the
dapp_detection
branch
from
78f51b6
to
cc3b9de
Compare
simonhong
commented
Jun 27, 2019
components/brave_extension/extension/brave_extension/content.ts
Outdated
Show resolved
Hide resolved
bbondy
force-pushed
the
dapp_detection
branch
7 times, most recently
from
29b7191
to
e492cc2
Compare
bridiver
reviewed
Jul 9, 2019
components/brave_extension/extension/brave_extension/background/events/dappDetectionEvents.ts
Outdated
Show resolved
Hide resolved
bridiver
reviewed
Jul 9, 2019
simonhong
commented
Jul 16, 2019
contents script in brave extension detects whether window.web3 is accessed or not from current tab and sends a message to background script to notify to browser. When browser gets "braveShields.dappAvailable", it shows wallet extension install guide when dapp detection pref is enabled.
Simply loading brave://wallet will install the extension if it's not already installed
Since the permission prompt is properly scoped within the tab, we don't need to check active. And we don't want to check it. The check for the frameId is just to future proof to make sure a DOS attack is not possible subframes
Thi is so that it can continue to be loaded in the future
It is more clear for what is being done
This is different than chrome.braveWallet being available in the first place
bbondy
force-pushed
the
dapp_detection
branch
2 times, most recently
from
b629cac
to
26afc11
Compare
bridiver
reviewed
Jul 17, 2019
bridiver
reviewed
Jul 17, 2019
bridiver
reviewed
Jul 17, 2019
bridiver
previously approved these changes
Jul 17, 2019
|
Passes CI on official and non official build runs. Since this feature is gated on a build flag that's only enabled for non official builds, it was needed to check both. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
contents script in brave extension detects whether window.web3 is accessed or not
from current tab and sends a message to background script to notify to browser.
When browser gets "braveWallet.dappAvailable", it shows wallet extension install
prompt.
dapp detection originally existed in muon and is similar to https://github.com/brave/browser-laptop/blob/master/app/extensions/brave/content/scripts/dappListener.js
Fix brave/brave-browser#718
Submitter Checklist:
npm test brave_unit_tests && npm test brave_browser_tests && npm run test-security) onnpm run lint)git rebase master(if needed).git rebase -ito squash commits (if needed).Test Plan:
yarn test brave_browser_tests --filter=BraveShieldsAPIBrowserTest.DappDetectionTestReviewer Checklist:
After-merge Checklist:
changes has landed on.