Improved region change process while connected

fix brave/brave-browser#26857 Network state could be unavailable shortly right after vpn disconnected. In this case, connect should go ahead if we want to connect after disconnected. BraveVpnAPIRequest will handle network failure by retrying.
brave · Nov 23, 2022 · b99e198 · b99e198
1 parent fc5f308
commit b99e198
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 8 deletions.
diff --git a/components/brave_vpn/brave_vpn_os_connection_api.cc b/components/brave_vpn/brave_vpn_os_connection_api.cc
@@ -67,7 +67,7 @@ void BraveVPNOSConnectionAPI::CreateVPNConnection() {
   CreateVPNConnectionImpl(connection_info_);
 }
 
-void BraveVPNOSConnectionAPI::Connect() {
+void BraveVPNOSConnectionAPI::Connect(bool ignore_network_state) {
   if (IsInProgress()) {
     VLOG(2) << __func__ << ": Current state: " << connection_state_
             << " : prevent connecting while previous operation is in-progress";
@@ -88,7 +88,7 @@ void BraveVPNOSConnectionAPI::Connect() {
   VLOG(2) << __func__ << " : start connecting!";
   UpdateAndNotifyConnectionStateChange(ConnectionState::CONNECTING);
 
-  if (!IsNetworkAvailable()) {
+  if (!ignore_network_state && !IsNetworkAvailable()) {
     VLOG(2) << __func__ << ": Network is not available, failed to connect";
     UpdateAndNotifyConnectionStateChange(ConnectionState::CONNECT_FAILED);
     return;
@@ -224,9 +224,10 @@ void BraveVPNOSConnectionAPI::OnDisconnected() {
 
   if (needs_connect_) {
     needs_connect_ = false;
-
-    if (connection_state_ == ConnectionState::DISCONNECTED)
-      Connect();
+    // Right after disconnected, network could be unavailable shortly.
+    // In this situation, connect process should go ahead because
+    // because BraveVpnAPIRequest could handle network failure by retrying.
+    Connect(true /* ignore_network_state */);
   }
 }
 

diff --git a/components/brave_vpn/brave_vpn_os_connection_api.h b/components/brave_vpn/brave_vpn_os_connection_api.h
@@ -68,7 +68,7 @@ class BraveVPNOSConnectionAPI : public base::PowerSuspendObserver,
   void SetConnectionState(mojom::ConnectionState state);
   bool IsInProgress() const;
 
-  void Connect();
+  void Connect(bool ignore_network_state = false);
   void Disconnect();
   void ToggleConnection();
   void RemoveVPNConnection();

diff --git a/components/brave_vpn/brave_vpn_unittest.cc b/components/brave_vpn/brave_vpn_unittest.cc
@@ -868,17 +868,34 @@ TEST_F(BraveVPNServiceTest, NeedsConnectTest) {
   OnDisconnected();
   EXPECT_FALSE(needs_connect());
   EXPECT_EQ(ConnectionState::CONNECTING, connection_state());
+}
 
-  // Handle connect after disconnect current connection.
-  connection_state() = ConnectionState::CONNECTED;
+TEST_F(BraveVPNServiceTest, OnDisconnectedWithoutNetwork) {
+  std::string env = skus::GetDefaultEnvironment();
+  // Connection state can be changed with purchased.
+  SetPurchasedState(env, PurchasedState::PURCHASED);
+
+  SetDeviceRegion("eu-es");
   auto network_change_notifier = net::NetworkChangeNotifier::CreateIfNeeded();
   net::test::ScopedMockNetworkChangeNotifier mock_notifier;
   mock_notifier.mock_network_change_notifier()->SetConnectionType(
       net::NetworkChangeNotifier::CONNECTION_NONE);
   EXPECT_EQ(net::NetworkChangeNotifier::CONNECTION_NONE,
             net::NetworkChangeNotifier::GetConnectionType());
+
+  // Handle connect after disconnect current connection.
+  // When we need another connect, it should make next connect call
+  // even if network is not available.
+  connection_state() = ConnectionState::CONNECTED;
+  needs_connect() = true;
   OnDisconnected();
   EXPECT_FALSE(needs_connect());
+  EXPECT_EQ(ConnectionState::CONNECTING, connection_state());
+
+  // Set connect failed when we don't want another connect.
+  connection_state() = ConnectionState::CONNECTED;
+  needs_connect() = false;
+  OnDisconnected();
   EXPECT_EQ(ConnectionState::CONNECT_FAILED, connection_state());
 }