-
Notifications
You must be signed in to change notification settings - Fork 884
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
Remove SafetyNet changes android #26719
base: master
Are you sure you want to change the base?
Conversation
1bbcf67
to
af4cdc5
Compare
What about removing |
Sorry i meant to delete the whole directory. updating now. |
af4cdc5
to
3d1f26b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@@ -618,7 +591,6 @@ void RegisterPrefsForBraveReferralsService(PrefRegistrySimple* registry) { | |||
registry->RegisterIntegerPref(kReferralAttemptCount, 0); | |||
#if BUILDFLAG(IS_ANDROID) | |||
registry->RegisterTimePref(kReferralAndroidFirstRunTimestamp, base::Time()); | |||
registry->RegisterStringPref(kSafetynetStatus, std::string()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whenever deprecates prefs, we need to clear its existing value from preferences.
You can refer kNewTabPageShowGemini
how it's cleared after its usage is deleted.
1cfdefa
to
cec20e0
Compare
cec20e0
to
d039b5d
Compare
d039b5d
to
fc8b8de
Compare
fc8b8de
to
4c8295d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++
android/java/org/chromium/chrome/browser/appmenu/BraveTabbedAppMenuPropertiesDelegate.java
Show resolved
Hide resolved
android/java/org/chromium/chrome/browser/ntp_background_images/NTPBackgroundImagesBridge.java
Show resolved
Hide resolved
&& braveRewardsNativeWorker.isSupported() | ||
&& BravePrefServiceBridge.getInstance() | ||
.getSafetynetCheckFailed()) { | ||
&& braveRewardsNativeWorker.isSupported()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need BraveUpgradeJobIntentServiceImpl
class at all. The whole it's purpose was to re-perform the SafetyNet check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SergeyZhukovsky we are still checking for braveRewardsNativeWorker.isSupported()
to decide if we need to show the rewrads icon. i.e. it would be useful for not showing rewards icon for OFAC countries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to do that on every upgrade?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can remove the change on upgrade as we are already checking for rewards icon here :
brave-core/android/java/org/chromium/chrome/browser/toolbar/top/BraveToolbarLayoutImpl.java
Line 451 in 4c8295d
&& mBraveRewardsNativeWorker.isSupported() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i am going to remove in a follow up commit.
4c8295d
to
d0de924
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++
Chromium major version is behind target branch (131.0.6778.85 vs 132.0.6834.15). Please rebase. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rewards 👍
Resolve presubmit issue Remove safetynet directory
Register kSafetynetStatus in brave_prodfile_prefs
d0de924
to
f89dae0
Compare
[puLL-Merge] - brave/brave-core@26719 Here's my review of the pull request: DescriptionThis PR removes SafetyNet checks from the Brave Android app. It eliminates the SafetyNet-related code, including the ChangesChanges
This sequence diagram illustrates the high-level changes made by removing SafetyNet checks from the Brave Android app. The app removes SafetyNet-related preferences, eliminates SafetyNet checks in the Brave Rewards system, updates the UI to remove SafetyNet-related elements, and displays the updated interface without SafetyNet restrictions.```mermaid
|
Resolves brave/brave-browser#42499
Submitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
wikinpm run presubmit
wiki,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan: