shill: vpn: Use DefaultPhysicalServiceEvent in tests of VPNDrivers

Refactors the unit test for OpenVPNDriver and ThirdPartyVpnDriver, and
some other cleanup.

BUG=b:172218910
TEST=unit_tests

Change-Id: I673c36ab357745e05f77dc2df26e2dfff673e0da
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2550965
Tested-by: Jie Jiang <jiejiang@chromium.org>
Reviewed-by: Taoyu Li <taoyl@chromium.org>
Reviewed-by: Hugo Benichi <hugobenichi@google.com>
Commit-Queue: Jie Jiang <jiejiang@chromium.org>
diff --git a/shill/vpn/openvpn_driver.h b/shill/vpn/openvpn_driver.h
index f43572e..7f47ed5 100644
--- a/shill/vpn/openvpn_driver.h
+++ b/shill/vpn/openvpn_driver.h
@@ -23,7 +23,6 @@
 namespace shill {
 
 class CertificateFile;
-class DeviceInfo;
 class Error;
 class OpenVPNManagementServer;
 
@@ -98,6 +97,8 @@
   std::string GetProviderType() const override;
   IfType GetIfType() const override;
   void OnConnectTimeout() override;
+  void OnDefaultPhysicalServiceEvent(
+      DefaultPhysicalServiceEvent event) override;
 
  private:
   friend class OpenVPNDriverTest;
@@ -123,7 +124,7 @@
   FRIEND_TEST(OpenVPNDriverTest, NotifyFail);
   FRIEND_TEST(OpenVPNDriverTest, OnConnectTimeout);
   FRIEND_TEST(OpenVPNDriverTest, OnConnectTimeoutResolve);
-  FRIEND_TEST(OpenVPNDriverTest, OnDefaultServiceChanged);
+  FRIEND_TEST(OpenVPNDriverTest, OnDefaultPhysicalServiceEvent);
   FRIEND_TEST(OpenVPNDriverTest, OnOpenVPNDied);
   FRIEND_TEST(OpenVPNDriverTest, ParseForeignOption);
   FRIEND_TEST(OpenVPNDriverTest, ParseForeignOptions);
@@ -219,9 +220,6 @@
   void Notify(const std::string& reason,
               const std::map<std::string, std::string>& dict) override;
 
-  void OnDefaultPhysicalServiceEvent(
-      DefaultPhysicalServiceEvent event) override;
-
   void ReportConnectionMetrics();
 
   Sockets sockets_;
diff --git a/shill/vpn/openvpn_driver_test.cc b/shill/vpn/openvpn_driver_test.cc
index df323b0..64ebd77 100644
--- a/shill/vpn/openvpn_driver_test.cc
+++ b/shill/vpn/openvpn_driver_test.cc
@@ -1252,30 +1252,25 @@
   EXPECT_EQ(2, driver_->GetCommandLineArgs().size());
 }
 
-TEST_F(OpenVPNDriverTest, OnDefaultServiceChanged) {
+TEST_F(OpenVPNDriverTest, OnDefaultPhysicalServiceEvent) {
   SetService(service_);
 
   // Switch from Online service -> no service.  VPN should be put on hold.
-  ServiceRefPtr null_service;
   EXPECT_CALL(*management_server_, Hold());
-  service_->OnDefaultPhysicalServiceChanged(null_service);
+  driver_->OnDefaultPhysicalServiceEvent(
+      VPNDriver::kDefaultPhysicalServiceDown);
   Mock::VerifyAndClearExpectations(management_server_);
 
   // Switch from no service -> Online.  VPN should release the hold.
-  scoped_refptr<MockService> mock_service(new MockService(&manager_));
-
-  EXPECT_CALL(*mock_service, IsOnline()).WillOnce(Return(true));
   EXPECT_CALL(*management_server_, ReleaseHold());
-  service_->OnDefaultPhysicalServiceChanged(mock_service);
+  driver_->OnDefaultPhysicalServiceEvent(VPNDriver::kDefaultPhysicalServiceUp);
   Mock::VerifyAndClearExpectations(management_server_);
 
   // Switch from Online service -> another Online service.  VPN should restart
   // immediately.
-  scoped_refptr<MockService> mock_service2(new MockService(&manager_));
-
-  EXPECT_CALL(*mock_service2, IsOnline()).WillOnce(Return(true));
   EXPECT_CALL(*management_server_, Restart());
-  service_->OnDefaultPhysicalServiceChanged(mock_service2);
+  driver_->OnDefaultPhysicalServiceEvent(
+      VPNDriver::kDefaultPhysicalServiceChanged);
 }
 
 TEST_F(OpenVPNDriverTest, GetReconnectTimeoutSeconds) {
diff --git a/shill/vpn/third_party_vpn_driver_test.cc b/shill/vpn/third_party_vpn_driver_test.cc
index 5698f58..1653a82 100644
--- a/shill/vpn/third_party_vpn_driver_test.cc
+++ b/shill/vpn/third_party_vpn_driver_test.cc
@@ -117,30 +117,30 @@
   driver_->reconnect_supported_ = true;
 
   // Roam from one Online network to another -> kLinkChanged.
-  scoped_refptr<MockService> mock_service(new NiceMock<MockService>(&manager_));
   EXPECT_CALL(*adaptor_interface_, EmitPlatformMessage(static_cast<uint32_t>(
                                        ThirdPartyVpnDriver::kLinkChanged)));
-  EXPECT_CALL(*mock_service, IsOnline()).WillRepeatedly(Return(true));
-  service_->OnDefaultPhysicalServiceChanged(mock_service);
+  driver_->OnDefaultPhysicalServiceEvent(
+      VPNDriver::kDefaultPhysicalServiceChanged);
 
   // Default physical service is not Online -> kLinkDown.
   EXPECT_CALL(*adaptor_interface_, EmitPlatformMessage(static_cast<uint32_t>(
                                        ThirdPartyVpnDriver::kLinkDown)));
-  service_->OnDefaultPhysicalServiceChanged(nullptr);
+  driver_->OnDefaultPhysicalServiceEvent(
+      VPNDriver::kDefaultPhysicalServiceDown);
 
   // Default physical service comes Online -> kLinkUp.
   EXPECT_CALL(
       *adaptor_interface_,
       EmitPlatformMessage(static_cast<uint32_t>(ThirdPartyVpnDriver::kLinkUp)));
-  EXPECT_CALL(*mock_service, IsOnline()).WillRepeatedly(Return(true));
-  service_->OnDefaultPhysicalServiceChanged(mock_service);
+  driver_->OnDefaultPhysicalServiceEvent(VPNDriver::kDefaultPhysicalServiceUp);
 
   // Default physical service vanishes, but the app doesn't support
   // reconnecting -> kDisconnected.
   driver_->reconnect_supported_ = false;
   EXPECT_CALL(*adaptor_interface_, EmitPlatformMessage(static_cast<uint32_t>(
                                        ThirdPartyVpnDriver::kDisconnected)));
-  service_->OnDefaultPhysicalServiceChanged(nullptr);
+  driver_->OnDefaultPhysicalServiceEvent(
+      VPNDriver::kDefaultPhysicalServiceDown);
 
   driver_->Disconnect();
 }