Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Vpn country selection menu android #21458

Merged
merged 7 commits into from
Jan 17, 2024
Merged

Vpn country selection menu android #21458

merged 7 commits into from
Jan 17, 2024

Conversation

deeppandya
Copy link
Contributor

@deeppandya deeppandya commented Dec 22, 2023 •
edited
Loading

Resolves brave/brave-browser#27014
Resolves brave/brave-browser#34738

Submitter Checklist:

  • I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
  • 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:

@deeppandya deeppandya added CI/skip-macos-x64 Do not run CI builds for macOS x64 CI/skip-windows-x64 Do not run CI builds for Windows x64 unused-CI/skip-linux-x64 Do not run CI builds for Linux x64 labels Dec 22, 2023
@deeppandya deeppandya added this to the 1.63.x - Nightly milestone Dec 22, 2023
@deeppandya deeppandya self-assigned this Dec 22, 2023
@deeppandya deeppandya changed the title Vpn country selection menu android [WIP] Vpn country selection menu android Dec 22, 2023
@deeppandya deeppandya force-pushed the vpn_country_selection_menu_android branch 5 times, most recently from 6194530 to 3120418 Compare January 12, 2024 18:49
@deeppandya deeppandya requested review from sangwoo108, DJAndries and tapanmodh and removed request for sangwoo108, DJAndries and tapanmodh January 12, 2024 18:52
@deeppandya deeppandya changed the title [WIP] Vpn country selection menu android Vpn country selection menu android Jan 12, 2024
@deeppandya deeppandya marked this pull request as ready for review January 12, 2024 18:52
@deeppandya deeppandya added the CI/skip-ios Do not run CI builds for iOS label Jan 12, 2024
@deeppandya deeppandya force-pushed the vpn_country_selection_menu_android branch from 3120418 to a58c8b6 Compare January 12, 2024 22:09
samartnik
samartnik approved these changes Jan 16, 2024
View reviewed changes
Copy link
Contributor

@samartnik samartnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generally lgtm

android/java/res/drawable/ic_checkbox_checked.xml Show resolved Hide resolved
android/java/res/layout/vpn_location_menu_item_action_layout.xml
@@ -0,0 +1,18 @@
<?xml version="1.0" encoding="utf-8"?>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same header related comment applies here too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updating right now.

@deeppandya deeppandya force-pushed the vpn_country_selection_menu_android branch 2 times, most recently from a35dba0 to 25ba104 Compare January 17, 2024 04:49
tapanmodh
tapanmodh approved these changes Jan 17, 2024
View reviewed changes
Copy link
Contributor

@tapanmodh tapanmodh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@deeppandya deeppandya force-pushed the vpn_country_selection_menu_android branch from 25ba104 to efbedc9 Compare January 17, 2024 06:39
@deeppandya
Update UI changes for country selection page
9a2b68a
@deeppandya
Add country flags with unicode emoji
15eb03f
@deeppandya
Add VPN settings with preferences
6b4e3b5
@deeppandya
Add vpn server selection activity
39d259d 
Remove server selection preferences
@deeppandya
Add logic to connect to different server
27900ff
@deeppandya
Update VPN location menu item with flag
f39a7d8 
Update server selection behaviour from menu

Update server selection menu item

Add country flag

Update country selection icon tint

Update behaviour for country selection
@deeppandya
Add changes for upgrade scenario
988110a 
Resolve presubmit issues

Format code changes

Add copyright headers

Resolve OneshotSupplier update

Update manifest with DiscouragedApi
@deeppandya deeppandya force-pushed the vpn_country_selection_menu_android branch from efbedc9 to 988110a Compare January 17, 2024 14:38
@github-actions GitHub Actions
Copy link
Contributor

[puLL-Merge] - brave/brave-core@21458

Description

This pull request introduces changes that affect the Brave browser on Android, specifically the way VPN resources and Java sources are managed and organized.

Changes

Changes

Filename: android/brave_java_resources.gni

  • Added new drawable resource ic_checkbox_checked.xml.
  • Removed the ic_server_selection_check.xml and rounded_filled_bg_radius_12.xml drawable resources.
  • Added new layout resource activity_vpn_server_selection.xml.
  • Removed brave_vpn_server_selection_layout.xml layout resource.

Filename: android/brave_java_sources.gni

  • Removed BraveVpnPreferences.java and BraveVpnServerSelectionPreferences.java Java source files.
  • Added VpnServerSelectionActivity.java Java source file to correspond with the new activity layout.

Filename: android/java/AndroidManifest.xml

  • Added a new <activity> entry for VpnServerSelectionActivity.

Filename: android/java/org/chromium/chrome/browser/app/BraveActivity.java

  • Introduced event handling for new VPN location menu items.
  • Added a call to retrieve all server regions when it is not the first install and the ISO code upgrade hasn't been done yet.

Filename: android/java/org/chromium/chrome/browser/app/appmenu/BraveAppMenuPropertiesDelegateImpl.java and android/java/org/chromium/chrome/browser/app/appmenu/BraveTabbedAppMenuPropertiesDelegate.java

  • Updated the app menus to handle new VPN location menu items.
  • Modified coloring of the VPN location icon.

Filename: android/java/org/chromium/chrome/browser/settings/BraveVpnServerSelectionPreferences.java

  • This Java source file has been deleted.

Filename: android/java/org/chromium/chrome/browser/vpn/BraveVpnNativeWorker.java

  • Synchronized lock object has been renamed from mLock to sLock.
  • Ensured singleton pattern through thread-safe synchronized code block.

Filename: android/java/org/chromium/chrome/browser/vpn/DisconnectVpnBroadcastReceiver.java

  • Removed a log entry which seemed unnecessary.

Filename: android/java/org/chromium/chrome/browser/vpn/activities/BraveVpnParentActivity.java

  • Introduced an invalidation timer for credentials after changing the VPN server location.

Filename: android/java/org/chromium/chrome/browser/vpn/activities/BraveVpnPlansActivity.java and other activity files

  • Removed createProfileProvider as it seems to no longer be necessary.

Filename: b/android/java/org/chromium/chrome/browser/vpn/activities/VpnServerSelectionActivity.java

  • New Java class for VPN server selection activity.

Filename: android/java/org/chromium/chrome/browser/vpn/adapters/BraveVpnServerSelectionAdapter.java

  • Updated the server location to include the country flag emoji using BraveVpnUtils.countryCodeToEmoji.

Filename: android/java/org/chromium/chrome/browser/vpn/models/BraveVpnPrefModel.java

  • Added countryIsoCode to BraveVpnServerRegion.

Filename: android/java/org/chromium/chrome/browser/vpn/settings/BraveVpnPreferences.java

  • Updated to include new VPN preference locations and remove outdated code.

Filename: android/java/org/chromium/chrome/browser/vpn/settings/VpnCalloutPreference.java

  • Removed an unnecessary requestLayout call which was redundantly triggering a layout pass.

Filename: android/java/org/chromium/chrome/browser/vpn/utils/BraveVpnApiResponseUtils.java

  • Removed the code that used to trigger a VPN region change.

Filename: android/java/org/chromium/chrome/browser/vpn/utils/BraveVpnPrefUtils.java

  • Updated shared preference access to use ChromeSharedPreferences.

Filename: android/java/org/chromium/chrome/browser/vpn/utils/BraveVpnUtils.java

  • Added a new utility method countryCodeToEmoji to convert country codes to flag emojis.

Drawable Resource Changes:

  • ic_checkbox_checked.xml: New drawable added.
  • ic_server_selection_check.xml: Drawable resource removed.
  • rounded_filled_bg_radius_12.xml: Drawable resource removed.

Layout Resource Changes:

  • activity_advance_tx_setting.xml: Included vpn_location_menu_item_action_layout.
  • brave_vpn_server_selection_layout.xml (deleted)
  • activity_vpn_server_selection.xml: New layout added based on deleted layout.
  • brave_vpn_server_selection_item_layout.xml: Text color changed.
  • vpn_location_menu_item_action_layout.xml: New layout added for the VPN location menu item.

Value Resource Changes:

  • brave_colors.xml: Removed rounded_filled_background.
  • brave_ids.xml: Added IDs for new VPN location menu items.
  • brave_main_preferences.xml: Changed the fragment used for VPN settings to the new one.
  • brave_vpn_preferences.xml: Removed the old VPN server selection preference.

Preprocessing Changes:

  • menu_main_menu_xml.py: Modified to add new VPN location menu items.

Security Hotspots

None of the introduced changes raise immediate security concerns. Most changes are related to UI elements, resource handling, and internal code restructuring, rather than critical logic that would impact the security of the application.

@deeppandya deeppandya merged commit 3f8aa31 into master Jan 17, 2024
18 checks passed
@deeppandya deeppandya deleted the vpn_country_selection_menu_android branch January 17, 2024 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tapanmodh tapanmodh tapanmodh approved these changes

@samartnik samartnik samartnik approved these changes

@sangwoo108 sangwoo108 Awaiting requested review from sangwoo108

@DJAndries DJAndries Awaiting requested review from DJAndries

Assignees

@deeppandya deeppandya

Labels
CI/skip-ios Do not run CI builds for iOS CI/skip-macos-x64 Do not run CI builds for macOS x64 CI/skip-windows-x64 Do not run CI builds for Windows x64 puLL-Merge unused-CI/skip-linux-x64 Do not run CI builds for Linux x64
Projects
None yet
Milestone
1.63.x - Release
3 participants
@deeppandya @tapanmodh @samartnik