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.

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

Fixed vpn button still has connected state for not purchased user #20533

Merged
merged 1 commit into from
Oct 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions browser/ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1096,8 +1096,10 @@ source_set("unit_tests") {
sources += [ "views/toolbar/brave_vpn_button_unittest.cc" ]
deps += [
"//base",
"//brave/components/brave_vpn/browser",
"//brave/components/brave_vpn/browser/connection:api",
"//brave/components/brave_vpn/browser/connection/ikev2:sim",
"//brave/components/skus/browser",
"//chrome/test:test_support",
"//skia",
"//testing/gtest",
Expand Down
19 changes: 9 additions & 10 deletions browser/ui/views/toolbar/brave_vpn_button.cc
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ BraveVPNButton::BraveVPNButton(Browser* browser)
service_(brave_vpn::BraveVpnServiceFactory::GetForProfile(
browser_->profile())) {
CHECK(service_);
UpdateButtonState();
Observe(service_);

// Replace ToolbarButton's highlight path generator.
Expand Down Expand Up @@ -211,15 +212,14 @@ void BraveVPNButton::OnConnectionStateChanged(ConnectionState state) {

void BraveVPNButton::UpdateButtonState() {
is_error_state_ = IsConnectError();
is_connected_ = IsConnected();
}

void BraveVPNButton::OnPurchasedStateChanged(
brave_vpn::mojom::PurchasedState state,
const absl::optional<std::string>& description) {
UpdateButtonState();
if (IsPurchased()) {
UpdateColorsAndInsets();
}
UpdateColorsAndInsets();
}

std::unique_ptr<views::Border> BraveVPNButton::GetBorder(
Expand Down Expand Up @@ -256,13 +256,12 @@ void BraveVPNButton::UpdateColorsAndInsets() {
image()->SetBackground(std::make_unique<ConnectErrorIconBackground>(
cp->GetColor(kColorBraveVpnButtonIconErrorInner)));
} else {
const bool is_connected = IsConnected();
SetImage(
views::Button::STATE_NORMAL,
gfx::CreateVectorIcon(
is_connected ? kVpnIndicatorOnIcon : kVpnIndicatorOffIcon,
cp->GetColor(is_connected ? kColorBraveVpnButtonIconConnected
: kColorBraveVpnButtonIconDisconnected)));
SetImage(views::Button::STATE_NORMAL,
gfx::CreateVectorIcon(
is_connected_ ? kVpnIndicatorOnIcon : kVpnIndicatorOffIcon,
cp->GetColor(is_connected_
? kColorBraveVpnButtonIconConnected
: kColorBraveVpnButtonIconDisconnected)));

// Use background for inner color of button image.
// Adjusted border thickness to make invisible to the outside of the icon.
Expand Down
1 change: 1 addition & 0 deletions browser/ui/views/toolbar/brave_vpn_button.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ class BraveVPNButton : public ToolbarButton,
void UpdateButtonState();

bool is_error_state_ = false;
bool is_connected_ = false;
absl::optional<brave_vpn::mojom::ConnectionState>
connection_state_for_testing_;
raw_ptr<Browser> browser_ = nullptr;
Expand Down
48 changes: 46 additions & 2 deletions browser/ui/views/toolbar/brave_vpn_button_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@
#include "base/run_loop.h"
#include "brave/browser/brave_browser_process_impl.h"
#include "brave/browser/brave_vpn/brave_vpn_service_factory.h"
#include "brave/components/brave_vpn/browser/brave_vpn_service.h"
#include "brave/components/brave_vpn/browser/connection/brave_vpn_os_connection_api.h"
#include "brave/components/brave_vpn/browser/connection/ikev2/brave_vpn_ras_connection_api_sim.h"
#include "brave/components/skus/browser/skus_utils.h"
#include "brave/test/base/testing_brave_browser_process.h"
#include "chrome/browser/prefs/browser_prefs.h"
#include "chrome/browser/themes/theme_service_factory.h"
Expand Down Expand Up @@ -78,13 +80,30 @@ class BraveVpnButtonUnitTest : public testing::Test {
profile_.reset();
base::RunLoop().RunUntilIdle();
}

bool IsErrorState() { return button_->IsErrorState(); }
void FireVpnState(brave_vpn::mojom::ConnectionState state) {

void FireVpnState(mojom::ConnectionState state) {
button_->SetVpnConnectionStateForTesting(state);
button_->OnConnectionStateChanged(state);
}

private:
// Give button's visual connection state.
bool DoesButtonHaveConnectedState() const { return button_->is_connected_; }

void SetPurchasedState(const std::string& env, mojom::PurchasedState state) {
button_->service_->SetPurchasedState(env, state);
}

void SetConnectionState(mojom::ConnectionState state) {
button_->service_->connection_api_->SetConnectionStateForTesting(state);
}

bool IsOsVpnConnected() const {
return button_->service_->GetConnectionState() ==
mojom::ConnectionState::CONNECTED;
}

content::BrowserTaskEnvironment task_environment_;
std::unique_ptr<BraveVPNButton> button_;
ChromeLayoutProvider layout_provider_;
Expand Down Expand Up @@ -113,4 +132,29 @@ TEST_F(BraveVpnButtonUnitTest, SkipAttemptsToConnectInFailedState) {
EXPECT_FALSE(IsErrorState());
}

TEST_F(BraveVpnButtonUnitTest, ButtonStateTestWithPurchasedState) {
// Set underlying vpn state as connected state and check button's connect
// state.
std::string env = skus::GetDefaultEnvironment();
SetConnectionState(mojom::ConnectionState::CONNECTED);
EXPECT_TRUE(IsOsVpnConnected());

// Button should have not connected state when not purchased.
SetPurchasedState(env, mojom::PurchasedState::NOT_PURCHASED);
base::RunLoop().RunUntilIdle();
EXPECT_FALSE(DoesButtonHaveConnectedState());
EXPECT_TRUE(IsOsVpnConnected());

// Button should have connected state when changed to purchased.
SetPurchasedState(env, mojom::PurchasedState::PURCHASED);
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(DoesButtonHaveConnectedState());
EXPECT_TRUE(IsOsVpnConnected());

SetPurchasedState(env, mojom::PurchasedState::NOT_PURCHASED);
base::RunLoop().RunUntilIdle();
EXPECT_FALSE(DoesButtonHaveConnectedState());
EXPECT_TRUE(IsOsVpnConnected());
}

} // namespace brave_vpn
6 changes: 6 additions & 0 deletions browser/ui/webui/skus_internals_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
#include "ui/base/clipboard/scoped_clipboard_writer.h"

#if BUILDFLAG(ENABLE_BRAVE_VPN)
#include "brave/browser/brave_vpn/brave_vpn_service_factory.h"
#include "brave/components/brave_vpn/browser/brave_vpn_service.h"
#include "brave/components/brave_vpn/browser/brave_vpn_service_helper.h"
#include "brave/components/brave_vpn/browser/connection/brave_vpn_os_connection_api.h"
#include "brave/components/brave_vpn/common/brave_vpn_utils.h"
Expand Down Expand Up @@ -156,6 +158,10 @@ void SkusInternalsUI::ResetSkusState() {
auto* profile = Profile::FromWebUI(web_ui());
if (brave_vpn::IsBraveVPNEnabled(profile->GetPrefs())) {
brave_vpn::ClearSubscriberCredential(local_state_);
if (auto* service =
brave_vpn::BraveVpnServiceFactory::GetForProfile(profile)) {
service->ReloadPurchasedState();
}
}
#endif

Expand Down
1 change: 1 addition & 0 deletions components/brave_vpn/browser/brave_vpn_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ class BraveVpnService :

private:
friend class BraveVPNServiceTest;
friend class BraveVpnButtonUnitTest;

#if !BUILDFLAG(IS_ANDROID)
friend class ::BraveAppMenuBrowserTest;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ class BraveVPNOSConnectionAPI
net::NetworkChangeNotifier::ConnectionType type) override;

private:
friend class BraveVpnButtonUnitTest;

FRIEND_TEST_ALL_PREFIXES(BraveVPNOSConnectionAPIUnitTest, NeedsConnectTest);
FRIEND_TEST_ALL_PREFIXES(BraveVPNOSConnectionAPIUnitTest, ConnectionInfoTest);
FRIEND_TEST_ALL_PREFIXES(BraveVPNOSConnectionAPIUnitTest,
Expand Down