Uplift Reconnect when network restored (uplift of #18755)

components/brave_vpn/browser/connection/ikev2/brave_vpn_ras_connection_api_base.cc

@@ -18,8 +18,6 @@
 #include "brave/components/brave_vpn/common/brave_vpn_utils.h"
 #include "brave/components/brave_vpn/common/pref_names.h"
 #include "components/prefs/pref_service.h"
-#include "third_party/abseil-cpp/absl/types/optional.h"
-
 namespace brave_vpn {
 
 using ConnectionState = mojom::ConnectionState;
@@ -229,12 +227,40 @@ void BraveVPNOSConnectionAPIBase::OnConnectFailed() {
   UpdateAndNotifyConnectionStateChange(ConnectionState::CONNECT_FAILED);
 }
 
-void BraveVPNOSConnectionAPIBase::OnDisconnected() {
-  UpdateAndNotifyConnectionStateChange(ConnectionState::DISCONNECTED);
+bool BraveVPNOSConnectionAPIBase::MaybeReconnect() {
+  VLOG(2) << __func__;
 
-  if (needs_connect_) {
+  if (!needs_connect_) {
+    VLOG(2) << "Should be called only when reconnect expected";
+    return false;
+  }
+  if (GetConnectionState() != ConnectionState::DISCONNECTED) {
+    VLOG(2) << "For reconnection we expect DISCONNECTED status";
+    return false;
+  }
+  if (IsPlatformNetworkAvailable()) {
     needs_connect_ = false;
     Connect();
+    return true;
+  }
+  return false;
+}
+
+void BraveVPNOSConnectionAPIBase::OnNetworkChanged(
+    net::NetworkChangeNotifier::ConnectionType type) {
+  if (needs_connect_ && MaybeReconnect()) {
+    VLOG(2) << "Network is live, reconnecting";
+    return;
+  }
+  BraveVPNOSConnectionAPI::OnNetworkChanged(type);
+}
+
+void BraveVPNOSConnectionAPIBase::OnDisconnected() {
+  UpdateAndNotifyConnectionStateChange(ConnectionState::DISCONNECTED);
+  // Sometimes disconnected event happens before network state restored,
+  // we postpone reconnection in this cases.
+  if (needs_connect_ && !MaybeReconnect()) {
+    VLOG(2) << "Network is down, will be reconnected when connection restored";
   }
 }
 

components/brave_vpn/browser/connection/ikev2/brave_vpn_ras_connection_api_base.h

@@ -21,6 +21,7 @@
 #include "brave/components/brave_vpn/common/mojom/brave_vpn.mojom.h"
 #include "net/base/network_change_notifier.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
+#include "third_party/abseil-cpp/absl/types/optional.h"
 
 class PrefService;
 
@@ -48,6 +49,8 @@ class BraveVPNOSConnectionAPIBase : public BraveVPNOSConnectionAPI {
   void UpdateAndNotifyConnectionStateChange(
       mojom::ConnectionState state) override;
   void SetSelectedRegion(const std::string& name) override;
+  void OnNetworkChanged(
+      net::NetworkChangeNotifier::ConnectionType type) override;
 
  protected:
   BraveVPNOSConnectionAPIBase(
@@ -61,6 +64,7 @@ class BraveVPNOSConnectionAPIBase : public BraveVPNOSConnectionAPI {
   virtual void ConnectImpl(const std::string& name) = 0;
   virtual void DisconnectImpl(const std::string& name) = 0;
   virtual void CheckConnectionImpl(const std::string& name) = 0;
+  virtual bool IsPlatformNetworkAvailable() = 0;
 
   // Subclass should call below callbacks whenever corresponding event happens.
   void OnCreated();
@@ -70,6 +74,7 @@ class BraveVPNOSConnectionAPIBase : public BraveVPNOSConnectionAPI {
   void OnConnectFailed();
   void OnDisconnected();
   void OnIsDisconnecting();
+  bool MaybeReconnect();
 
   std::string target_vpn_entry_name() const { return target_vpn_entry_name_; }
 
@@ -94,7 +99,6 @@ class BraveVPNOSConnectionAPIBase : public BraveVPNOSConnectionAPI {
                            IgnoreDisconnectedStateWhileConnecting);
   FRIEND_TEST_ALL_PREFIXES(BraveVPNOSConnectionAPIUnitTest,
                            ClearLastConnectionErrorWhenNewConnectionStart);
-
   void CreateVPNConnection();
   std::string GetCurrentEnvironment() const;
   void FetchHostnamesForRegion(const std::string& name);
@@ -112,7 +116,6 @@ class BraveVPNOSConnectionAPIBase : public BraveVPNOSConnectionAPI {
   bool needs_connect_ = false;
   bool prevent_creation_ = false;
   std::string target_vpn_entry_name_;
-
   BraveVPNConnectionInfo connection_info_;
   raw_ptr<PrefService> local_prefs_ = nullptr;
   std::unique_ptr<Hostname> hostname_;

components/brave_vpn/browser/connection/ikev2/brave_vpn_ras_connection_api_sim.cc

@@ -150,4 +150,15 @@ void BraveVPNOSConnectionAPISim::OnIsDisconnecting(const std::string& name) {
   BraveVPNOSConnectionAPIBase::OnIsDisconnecting();
 }
 
+void BraveVPNOSConnectionAPISim::SetNetworkAvailableForTesting(bool value) {
+  network_available_ = value;
+}
+
+bool BraveVPNOSConnectionAPISim::IsPlatformNetworkAvailable() {
+  if (network_available_.has_value()) {
+    return network_available_.value();
+  }
+  return true;
+}
+
 }  // namespace brave_vpn

components/brave_vpn/browser/connection/ikev2/brave_vpn_ras_connection_api_sim.h

@@ -28,6 +28,7 @@ class BraveVPNOSConnectionAPISim : public BraveVPNOSConnectionAPIBase {
 
   bool IsConnectionCreated() const;
   bool IsConnectionChecked() const;
+  void SetNetworkAvailableForTesting(bool value);
 
  protected:
   friend class base::NoDestructor<BraveVPNOSConnectionAPISim>;
@@ -45,6 +46,7 @@ class BraveVPNOSConnectionAPISim : public BraveVPNOSConnectionAPIBase {
   void ConnectImpl(const std::string& name) override;
   void DisconnectImpl(const std::string& name) override;
   void CheckConnectionImpl(const std::string& name) override;
+  bool IsPlatformNetworkAvailable() override;
 
  private:
   friend class BraveVPNServiceTest;
@@ -69,6 +71,7 @@ class BraveVPNOSConnectionAPISim : public BraveVPNOSConnectionAPIBase {
   bool connection_created_ = false;
   bool check_connection_called_ = false;
 
+  absl::optional<bool> network_available_;
   base::WeakPtrFactory<BraveVPNOSConnectionAPISim> weak_factory_{this};
 };
 

components/brave_vpn/browser/connection/ikev2/brave_vpn_ras_connection_api_unittest.cc

@@ -27,6 +27,7 @@
 namespace brave_vpn {
 
 namespace {
+
 const char kProfileCredentialData[] = R"(
         {
           "eap-username": "brave-user",
@@ -409,6 +410,21 @@ TEST_F(BraveVPNOSConnectionAPIUnitTest, NeedsConnectTest) {
   test_api->OnDisconnected();
   EXPECT_FALSE(test_api->needs_connect_);
   EXPECT_EQ(mojom::ConnectionState::CONNECTING, test_api->GetConnectionState());
+
+  test_api->connection_state_ = mojom::ConnectionState::CONNECTED;
+  test_api->Connect();
+  EXPECT_TRUE(test_api->needs_connect_);
+  EXPECT_EQ(mojom::ConnectionState::DISCONNECTING,
+            test_api->GetConnectionState());
+  static_cast<BraveVPNOSConnectionAPISim*>(test_api)
+      ->SetNetworkAvailableForTesting(false);
+  test_api->OnDisconnected();
+  EXPECT_TRUE(test_api->needs_connect_);
+  static_cast<BraveVPNOSConnectionAPISim*>(test_api)
+      ->SetNetworkAvailableForTesting(true);
+  test_api->OnNetworkChanged(net::NetworkChangeNotifier::CONNECTION_ETHERNET);
+  EXPECT_FALSE(test_api->needs_connect_);
+  EXPECT_EQ(mojom::ConnectionState::CONNECTING, test_api->GetConnectionState());
 }
 
 TEST_F(BraveVPNOSConnectionAPIUnitTest,

components/brave_vpn/browser/connection/ikev2/mac/brave_vpn_ras_connection_api_mac.h

@@ -29,6 +29,7 @@ class BraveVPNOSConnectionAPIMac : public BraveVPNOSConnectionAPIBase {
   void ConnectImpl(const std::string& name) override;
   void DisconnectImpl(const std::string& name) override;
   void CheckConnectionImpl(const std::string& name) override;
+  bool IsPlatformNetworkAvailable() override;
   void ObserveVPNConnectionChange();
 
   id vpn_observer_ = nil;

components/brave_vpn/browser/connection/ikev2/mac/brave_vpn_ras_connection_api_mac.mm

@@ -8,11 +8,14 @@
 #include <memory>
 
 #import <NetworkExtension/NetworkExtension.h>
+#include <SystemConfiguration/SystemConfiguration.h>
+#include <netinet/in.h>
 
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/mac/bundle_locations.h"
 #include "base/mac/foundation_util.h"
+#include "base/mac/scoped_cftyperef.h"
 #include "base/notreached.h"
 #include "base/strings/sys_string_conversions.h"
 
@@ -330,4 +333,23 @@ OSStatus StorePassword(const NSString* password, std::string* error_str) {
               }];
 }
 
+bool BraveVPNOSConnectionAPIMac::IsPlatformNetworkAvailable() {
+  // 0.0.0.0 is a special token that causes reachability to monitor the general
+  // routing status of the device, both IPv4 and IPv6.
+  struct sockaddr_in addr = {0};
+  addr.sin_len = sizeof(addr);
+  addr.sin_family = AF_INET;
+  base::ScopedCFTypeRef<SCNetworkReachabilityRef> reachability(
+      SCNetworkReachabilityCreateWithAddress(
+          kCFAllocatorDefault, reinterpret_cast<struct sockaddr*>(&addr)));
+  SCNetworkReachabilityFlags flags;
+  BOOL success = SCNetworkReachabilityGetFlags(reachability, &flags);
+  if (!success) {
+    return false;
+  }
+  BOOL isReachable = flags & kSCNetworkReachabilityFlagsReachable;
+  BOOL needsConnection = flags & kSCNetworkReachabilityFlagsConnectionRequired;
+  return isReachable && !needsConnection;
+}
+
 }  // namespace brave_vpn

components/brave_vpn/browser/connection/ikev2/win/brave_vpn_ras_connection_api_win.cc

@@ -7,8 +7,9 @@
 
 #include <windows.h>
 
+#include <netlistmgr.h>  // For CLSID_NetworkListManager
 #include <ras.h>
-
+#include <wrl/client.h>
 #include <memory>
 
 #include "base/logging.h"
@@ -17,7 +18,6 @@
 #include "base/task/thread_pool.h"
 #include "brave/components/brave_vpn/browser/connection/ikev2/win/ras_utils.h"
 #include "brave/components/brave_vpn/common/brave_vpn_constants.h"
-
 // Most of Windows implementations are based on Brian Clifton
 // (brian@clifton.me)'s work (https://github.com/bsclifton/winvpntool).
 
@@ -174,4 +174,28 @@ void BraveVPNOSConnectionAPIWin::StartVPNConnectionChangeMonitoring() {
       event_handle_for_connected_disconnected_, this);
 }
 
+bool BraveVPNOSConnectionAPIWin::IsPlatformNetworkAvailable() {
+  // If any errors occur, return that internet connection is available.
+  Microsoft::WRL::ComPtr<INetworkListManager> manager;
+  HRESULT hr = ::CoCreateInstance(CLSID_NetworkListManager, nullptr, CLSCTX_ALL,
+                                  IID_PPV_ARGS(&manager));
+  if (FAILED(hr)) {
+    LOG(ERROR) << "CoCreateInstance(NetworkListManager) hr=" << std::hex << hr;
+    return true;
+  }
+
+  VARIANT_BOOL is_connected;
+  hr = manager->get_IsConnectedToInternet(&is_connected);
+  if (FAILED(hr)) {
+    LOG(ERROR) << "get_IsConnectedToInternet failed hr=" << std::hex << hr;
+    return true;
+  }
+
+  // Normally VARIANT_TRUE/VARIANT_FALSE are used with the type VARIANT_BOOL
+  // but in this case the docs explicitly say to use FALSE.
+  // https://docs.microsoft.com/en-us/windows/desktop/api/Netlistmgr/
+  //     nf-netlistmgr-inetworklistmanager-get_isconnectedtointernet
+  return is_connected != FALSE;
+}
+
 }  // namespace brave_vpn

components/brave_vpn/browser/connection/ikev2/win/brave_vpn_ras_connection_api_win.h

@@ -38,6 +38,7 @@ class BraveVPNOSConnectionAPIWin : public BraveVPNOSConnectionAPIBase,
   void ConnectImpl(const std::string& name) override;
   void DisconnectImpl(const std::string& name) override;
   void CheckConnectionImpl(const std::string& name) override;
+  bool IsPlatformNetworkAvailable() override;
 
   // base::win::ObjectWatcher::Delegate overrides:
   void OnObjectSignaled(HANDLE object) override;