wificond: Move enable/disable Supplicant

Move enable/disable Supplicant to top level wificond Aidl object since
this is used for both STA & AP operations now.

Bug: 69319369
Test: Turn on wifi/softap succesfully.
Test: ./system/connectivity/wificond/runtests.sh
Change-Id: I3c2463f8c2bcb081503103a46188e4ec3e3e6b3e

15 files changed, 84 insertions, 120 deletions

diff --git a/aidl/android/net/wifi/IClientInterface.aidl b/aidl/android/net/wifi/IClientInterface.aidl
index 701a705..f8ba2b1 100644
--- a/aidl/android/net/wifi/IClientInterface.aidl
+++ b/aidl/android/net/wifi/IClientInterface.aidl
@@ -21,15 +21,6 @@ import android.net.wifi.IWifiScannerImpl;
 // IClientInterface represents a network interface that can be used to connect
 // to access points and obtain internet connectivity.
 interface IClientInterface {
-
-  // Enable a wpa_supplicant instance running against this interface.
-  // Returns true if supplicant was successfully enabled, or is already enabled.
-  boolean enableSupplicant();
-
-  // Remove this interface from wpa_supplicant's control.
-  // Returns true if removal was successful.
-  boolean disableSupplicant();
-
   // Get packet counters for this interface.
   // First element in array is the number of successfully transmitted packets.
   // Second element in array is the number of tramsmission failure.
diff --git a/aidl/android/net/wifi/IWificond.aidl b/aidl/android/net/wifi/IWificond.aidl
index 47ee257..49f5217 100644
--- a/aidl/android/net/wifi/IWificond.aidl
+++ b/aidl/android/net/wifi/IWificond.aidl
@@ -53,6 +53,16 @@ interface IWificond {
     // Returrns null on failure.
     @nullable int[] getAvailableDFSChannels();
 
+    // Enable wpa_supplicant.
+    // Returns true if supplicant was successfully enabled,
+    // or is already enabled.
+    boolean enableSupplicant();
+
+    // Disable wpa_supplicant.
+    // Returns true if supplicant was successfully disabled,
+    // or is already disabled.
+    boolean disableSupplicant();
+
     // Register a callback to receive interface status updates.
     //
     // Multiple callbacks can be registered simultaneously.
diff --git a/client_interface_binder.cpp b/client_interface_binder.cpp
index 70e1347..984fcf5 100644
--- a/client_interface_binder.cpp
+++ b/client_interface_binder.cpp
@@ -36,16 +36,6 @@ ClientInterfaceBinder::ClientInterfaceBinder(ClientInterfaceImpl* impl)
 ClientInterfaceBinder::~ClientInterfaceBinder() {
 }
 
-Status ClientInterfaceBinder::enableSupplicant(bool* success) {
-  *success = impl_ && impl_->EnableSupplicant();
-  return Status::ok();
-}
-
-Status ClientInterfaceBinder::disableSupplicant(bool* success) {
-  *success = impl_ && impl_->DisableSupplicant();
-  return Status::ok();
-}
-
 Status ClientInterfaceBinder::getPacketCounters(
     vector<int32_t>* out_packet_counters) {
   if (impl_ == nullptr) {
diff --git a/client_interface_binder.h b/client_interface_binder.h
index 2cdea7f..1dc1200 100644
--- a/client_interface_binder.h
+++ b/client_interface_binder.h
@@ -37,8 +37,6 @@ class ClientInterfaceBinder : public android::net::wifi::BnClientInterface {
   // by remote processes are possible.
   void NotifyImplDead() { impl_ = nullptr; }
 
-  ::android::binder::Status enableSupplicant(bool* success) override;
-  ::android::binder::Status disableSupplicant(bool* success) override;
   ::android::binder::Status getPacketCounters(
       std::vector<int32_t>* out_packet_counters) override;
   ::android::binder::Status signalPoll(
diff --git a/client_interface_impl.cpp b/client_interface_impl.cpp
index 4838bc9..72edd97 100644
--- a/client_interface_impl.cpp
+++ b/client_interface_impl.cpp
@@ -19,7 +19,6 @@
 #include <vector>
 
 #include <android-base/logging.h>
-#include <wifi_system/supplicant_manager.h>
 
 #include "wificond/client_interface_binder.h"
 #include "wificond/net/mlme_event.h"
@@ -33,7 +32,6 @@ using android::net::wifi::IClientInterface;
 using com::android::server::wifi::wificond::NativeScanResult;
 using android::sp;
 using android::wifi_system::InterfaceTool;
-using android::wifi_system::SupplicantManager;
 
 using std::endl;
 using std::string;
@@ -101,7 +99,6 @@ ClientInterfaceImpl::ClientInterfaceImpl(
     uint32_t interface_index,
     const std::vector<uint8_t>& interface_mac_addr,
     InterfaceTool* if_tool,
-    SupplicantManager* supplicant_manager,
     NetlinkUtils* netlink_utils,
     ScanUtils* scan_utils)
     : wiphy_index_(wiphy_index),
@@ -109,7 +106,6 @@ ClientInterfaceImpl::ClientInterfaceImpl(
       interface_index_(interface_index),
       interface_mac_addr_(interface_mac_addr),
       if_tool_(if_tool),
-      supplicant_manager_(supplicant_manager),
       netlink_utils_(netlink_utils),
       scan_utils_(scan_utils),
       offload_service_utils_(new OffloadServiceUtils()),
@@ -138,7 +134,6 @@ ClientInterfaceImpl::ClientInterfaceImpl(
 ClientInterfaceImpl::~ClientInterfaceImpl() {
   binder_->NotifyImplDead();
   scanner_->Invalidate();
-  DisableSupplicant();
   netlink_utils_->UnsubscribeMlmeEvent(interface_index_);
   if_tool_->SetUpState(interface_name_.c_str(), false);
 }
@@ -170,14 +165,6 @@ void ClientInterfaceImpl::Dump(std::stringstream* ss) const {
   *ss << "------- Dump End -------" << endl;
 }
 
-bool ClientInterfaceImpl::EnableSupplicant() {
-  return supplicant_manager_->StartSupplicant();
-}
-
-bool ClientInterfaceImpl::DisableSupplicant() {
-  return supplicant_manager_->StopSupplicant();
-}
-
 bool ClientInterfaceImpl::GetPacketCounters(vector<int32_t>* out_packet_counters) {
   StationInfo station_info;
   if (!netlink_utils_->GetStationInfo(interface_index_,
diff --git a/client_interface_impl.h b/client_interface_impl.h
index 651141e..2d4ee5b 100644
--- a/client_interface_impl.h
+++ b/client_interface_impl.h
@@ -22,7 +22,6 @@
 #include <android-base/macros.h>
 #include <utils/StrongPointer.h>
 #include <wifi_system/interface_tool.h>
-#include <wifi_system/supplicant_manager.h>
 
 #include "android/net/wifi/IClientInterface.h"
 #include "wificond/net/mlme_event_handler.h"
@@ -65,7 +64,6 @@ class ClientInterfaceImpl {
       uint32_t interface_index,
       const std::vector<uint8_t>& interface_mac_addr,
       android::wifi_system::InterfaceTool* if_tool,
-      android::wifi_system::SupplicantManager* supplicant_manager,
       NetlinkUtils* netlink_utils,
       ScanUtils* scan_utils);
   virtual ~ClientInterfaceImpl();
@@ -73,8 +71,6 @@ class ClientInterfaceImpl {
   // Get a pointer to the binder representing this ClientInterfaceImpl.
   android::sp<android::net::wifi::IClientInterface> GetBinder() const;
 
-  bool EnableSupplicant();
-  bool DisableSupplicant();
   bool GetPacketCounters(std::vector<int32_t>* out_packet_counters);
   bool SignalPoll(std::vector<int32_t>* out_signal_poll_results);
   const std::vector<uint8_t>& GetMacAddress();
@@ -91,7 +87,6 @@ class ClientInterfaceImpl {
   const uint32_t interface_index_;
   const std::vector<uint8_t> interface_mac_addr_;
   android::wifi_system::InterfaceTool* const if_tool_;
-  android::wifi_system::SupplicantManager* const supplicant_manager_;
   NetlinkUtils* const netlink_utils_;
   ScanUtils* const scan_utils_;
   const std::shared_ptr<OffloadServiceUtils> offload_service_utils_;
diff --git a/server.cpp b/server.cpp
index 3f0a2ae..0cc89c4 100644
--- a/server.cpp
+++ b/server.cpp
@@ -127,7 +127,6 @@ Status Server::createClientInterface(const std::string& iface_name,
       interface.index,
       interface.mac_address,
       if_tool_.get(),
-      supplicant_manager_.get(),
       netlink_utils_,
       scan_utils_));
   *created_interface = client_interface->GetBinder();
@@ -155,6 +154,16 @@ Status Server::tearDownInterfaces() {
   return Status::ok();
 }
 
+Status Server::enableSupplicant(bool* success) {
+  *success = supplicant_manager_->StartSupplicant();
+  return Status::ok();
+}
+
+Status Server::disableSupplicant(bool* success) {
+  *success = supplicant_manager_->StopSupplicant();
+  return Status::ok();
+}
+
 Status Server::GetClientInterfaces(vector<sp<IBinder>>* out_client_interfaces) {
   vector<sp<android::IBinder>> client_interfaces_binder;
   for (auto& it : client_interfaces_) {
diff --git a/server.h b/server.h
index 23e6a70..7552af8 100644
--- a/server.h
+++ b/server.h
@@ -23,6 +23,7 @@
 
 #include <android-base/macros.h>
 #include <wifi_system/interface_tool.h>
+#include <wifi_system/supplicant_manager.h>
 
 #include "android/net/wifi/BnWificond.h"
 #include "android/net/wifi/IApInterface.h"
@@ -77,6 +78,8 @@ class Server : public android::net::wifi::BnWificond {
           created_interface) override;
 
   android::binder::Status tearDownInterfaces() override;
+  android::binder::Status enableSupplicant(bool* success) override;
+  android::binder::Status disableSupplicant(bool* success) override;
 
   android::binder::Status GetClientInterfaces(
       std::vector<android::sp<android::IBinder>>* out_client_ifs) override;
diff --git a/tests/client_interface_impl_unittest.cpp b/tests/client_interface_impl_unittest.cpp
index df6884e..b3b6f4c 100644
--- a/tests/client_interface_impl_unittest.cpp
+++ b/tests/client_interface_impl_unittest.cpp
@@ -19,9 +19,7 @@
 
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
-#include <wifi_system/supplicant_manager.h>
 #include <wifi_system_test/mock_interface_tool.h>
-#include <wifi_system_test/mock_supplicant_manager.h>
 
 #include "wificond/client_interface_impl.h"
 #include "wificond/tests/mock_netlink_manager.h"
@@ -29,8 +27,6 @@
 #include "wificond/tests/mock_scan_utils.h"
 
 using android::wifi_system::MockInterfaceTool;
-using android::wifi_system::MockSupplicantManager;
-using android::wifi_system::SupplicantManager;
 using std::unique_ptr;
 using std::vector;
 using testing::NiceMock;
@@ -59,7 +55,6 @@ class ClientInterfaceImplTest : public ::testing::Test {
         kTestInterfaceIndex,
         vector<uint8_t>{0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
         if_tool_.get(),
-        supplicant_manager_.get(),
         netlink_utils_.get(),
         scan_utils_.get()});
   }
@@ -67,14 +62,10 @@ class ClientInterfaceImplTest : public ::testing::Test {
   void TearDown() override {
     EXPECT_CALL(*netlink_utils_,
                 UnsubscribeMlmeEvent(kTestInterfaceIndex));
-    EXPECT_CALL(*supplicant_manager_, StopSupplicant())
-        .WillOnce(Return(false));
   }
 
   unique_ptr<NiceMock<MockInterfaceTool>> if_tool_{
       new NiceMock<MockInterfaceTool>};
-  unique_ptr<NiceMock<MockSupplicantManager>> supplicant_manager_{
-      new NiceMock<MockSupplicantManager>};
   unique_ptr<NiceMock<MockNetlinkManager>> netlink_manager_{
       new NiceMock<MockNetlinkManager>()};
   unique_ptr<NiceMock<MockNetlinkUtils>> netlink_utils_{
@@ -86,29 +77,5 @@ class ClientInterfaceImplTest : public ::testing::Test {
 
 }  // namespace
 
-TEST_F(ClientInterfaceImplTest, ShouldReportEnableFailure) {
-  EXPECT_CALL(*supplicant_manager_, StartSupplicant())
-      .WillOnce(Return(false));
-  EXPECT_FALSE(client_interface_->EnableSupplicant());
-}
-
-TEST_F(ClientInterfaceImplTest, ShouldReportEnableSuccess) {
-  EXPECT_CALL(*supplicant_manager_, StartSupplicant())
-      .WillOnce(Return(true));
-  EXPECT_TRUE(client_interface_->EnableSupplicant());
-}
-
-TEST_F(ClientInterfaceImplTest, ShouldReportDisableFailure) {
-  EXPECT_CALL(*supplicant_manager_, StopSupplicant())
-      .WillOnce(Return(false));
-  EXPECT_FALSE(client_interface_->DisableSupplicant());
-}
-
-TEST_F(ClientInterfaceImplTest, ShouldReportDisableSuccess) {
-  EXPECT_CALL(*supplicant_manager_, StopSupplicant())
-      .WillOnce(Return(true));
-  EXPECT_TRUE(client_interface_->DisableSupplicant());
-}
-
 }  // namespace wificond
 }  // namespace android
diff --git a/tests/integration/client_interface_test.cpp b/tests/integration/client_interface_test.cpp
index dc8ecd5..aeadfa3 100644
--- a/tests/integration/client_interface_test.cpp
+++ b/tests/integration/client_interface_test.cpp
@@ -40,9 +40,6 @@ namespace wificond {
 namespace {
 
 const char kInterfaceName[] = "wlan0";
-constexpr int kSupplicantStartupTimeoutSeconds = 3;
-constexpr int kSupplicantDeathTimeoutSeconds = 3;
-
 }  // namespace
 
 TEST(ClientInterfaceTest, CanCreateClientInterfaces) {
@@ -77,37 +74,6 @@ TEST(ClientInterfaceTest, CanCreateClientInterfaces) {
   EXPECT_FALSE(if_tool.GetUpState(if_name.c_str()));
 }
 
-TEST(ClientInterfaceTest, CanStartStopSupplicant) {
-  ScopedDevModeWificond dev_mode;
-  sp<IWificond> service = dev_mode.EnterDevModeOrDie();
-  sp<IClientInterface> client_interface;
-  EXPECT_TRUE(service->createClientInterface(
-      kInterfaceName, &client_interface).isOk());
-  ASSERT_NE(nullptr, client_interface.get());
-
-  for (int iteration = 0; iteration < 4; iteration++) {
-    bool supplicant_started = false;
-    EXPECT_TRUE(client_interface->enableSupplicant(&supplicant_started).isOk());
-    EXPECT_TRUE(supplicant_started);
-
-    EXPECT_TRUE(WaitForTrue(SupplicantIsRunning,
-                            kSupplicantStartupTimeoutSeconds))
-        << "Failed on iteration " << iteration;
-
-    // We look for supplicant so quickly that we miss when it dies on startup
-    sleep(1);
-    EXPECT_TRUE(SupplicantIsRunning()) << "Failed on iteration " << iteration;
-
-    bool supplicant_stopped = false;
-    EXPECT_TRUE(
-        client_interface->disableSupplicant(&supplicant_stopped).isOk());
-    EXPECT_TRUE(supplicant_stopped);
-
-    EXPECT_TRUE(WaitForTrue(SupplicantIsDead, kSupplicantDeathTimeoutSeconds))
-        << "Failed on iteration " << iteration;
-  }
-}
-
 TEST(ClientInterfaceTest, CanGetMacAddress) {
   ScopedDevModeWificond dev_mode;
   sp<IWificond> service = dev_mode.EnterDevModeOrDie();
diff --git a/tests/integration/service_test.cpp b/tests/integration/service_test.cpp
index 136cedc..1711339 100644
--- a/tests/integration/service_test.cpp
+++ b/tests/integration/service_test.cpp
@@ -37,8 +37,6 @@ namespace wificond {
 namespace {
 
 constexpr int kTimeoutSeconds = 3;
-const char kInterfaceName[] = "wlan0";
-
 }  // namespace
 
 TEST(ServiceTest, ShouldTearDownSystemOnStartup) {
@@ -46,12 +44,8 @@ TEST(ServiceTest, ShouldTearDownSystemOnStartup) {
   ScopedDevModeWificond dev_mode;
   sp<IWificond> service = dev_mode.EnterDevModeOrDie();
 
-  sp<IClientInterface> client_interface;
-  EXPECT_TRUE(service->createClientInterface(
-       kInterfaceName, &client_interface).isOk());
-
   bool supplicant_started = false;
-  EXPECT_TRUE(client_interface->enableSupplicant(&supplicant_started).isOk());
+  EXPECT_TRUE(service->enableSupplicant(&supplicant_started).isOk());
   EXPECT_TRUE(supplicant_started);
 
   EXPECT_TRUE(WaitForTrue(SupplicantIsRunning, kTimeoutSeconds));
@@ -68,5 +62,32 @@ TEST(ServiceTest, ShouldTearDownSystemOnStartup) {
   EXPECT_TRUE(WaitForTrue(SupplicantIsDead, kTimeoutSeconds));
 }
 
+TEST(ServiceTest, CanStartStopSupplicant) {
+  ScopedDevModeWificond dev_mode;
+  sp<IWificond> service = dev_mode.EnterDevModeOrDie();
+
+  for (int iteration = 0; iteration < 4; iteration++) {
+    bool supplicant_started = false;
+    EXPECT_TRUE(service->enableSupplicant(&supplicant_started).isOk());
+    EXPECT_TRUE(supplicant_started);
+
+    EXPECT_TRUE(WaitForTrue(SupplicantIsRunning,
+                            kTimeoutSeconds))
+        << "Failed on iteration " << iteration;
+
+    // We look for supplicant so quickly that we miss when it dies on startup
+    sleep(1);
+    EXPECT_TRUE(SupplicantIsRunning()) << "Failed on iteration " << iteration;
+
+    bool supplicant_stopped = false;
+    EXPECT_TRUE(
+        service->disableSupplicant(&supplicant_stopped).isOk());
+    EXPECT_TRUE(supplicant_stopped);
+
+    EXPECT_TRUE(WaitForTrue(SupplicantIsDead, kTimeoutSeconds))
+        << "Failed on iteration " << iteration;
+  }
+}
+
 }  // namespace wificond
 }  // namespace android
diff --git a/tests/mock_client_interface_impl.cpp b/tests/mock_client_interface_impl.cpp
index 775eb1c..ba88f1c 100644
--- a/tests/mock_client_interface_impl.cpp
+++ b/tests/mock_client_interface_impl.cpp
@@ -34,7 +34,6 @@ const uint32_t kTestWiphyIndex = 2;
 
 MockClientInterfaceImpl::MockClientInterfaceImpl(
       android::wifi_system::InterfaceTool* interface_tool,
-      android::wifi_system::SupplicantManager* supplicant_manager,
       NetlinkUtils* netlink_utils,
       ScanUtils* scan_utils)
     : ClientInterfaceImpl(
@@ -45,7 +44,6 @@ MockClientInterfaceImpl::MockClientInterfaceImpl(
             kTestInterfaceMacAddress,
             kTestInterfaceMacAddress + arraysize(kTestInterfaceMacAddress)),
         interface_tool,
-        supplicant_manager,
         netlink_utils,
         scan_utils) {}
 
diff --git a/tests/mock_client_interface_impl.h b/tests/mock_client_interface_impl.h
index 05cf686..ea1f4cd 100644
--- a/tests/mock_client_interface_impl.h
+++ b/tests/mock_client_interface_impl.h
@@ -29,7 +29,6 @@ class MockClientInterfaceImpl : public ClientInterfaceImpl {
  public:
   MockClientInterfaceImpl(
       android::wifi_system::InterfaceTool*,
-      android::wifi_system::SupplicantManager*,
       NetlinkUtils*,
       ScanUtils*);
   ~MockClientInterfaceImpl() override = default;
diff --git a/tests/scanner_unittest.cpp b/tests/scanner_unittest.cpp
index 7dcb0c2..bce2788 100644
--- a/tests/scanner_unittest.cpp
+++ b/tests/scanner_unittest.cpp
@@ -34,7 +34,6 @@
 
 using ::android::binder::Status;
 using ::android::wifi_system::MockInterfaceTool;
-using ::android::wifi_system::MockSupplicantManager;
 using ::com::android::server::wifi::wificond::SingleScanSettings;
 using ::com::android::server::wifi::wificond::PnoSettings;
 using ::com::android::server::wifi::wificond::NativeScanResult;
@@ -122,9 +121,8 @@ class ScannerTest : public ::testing::Test {
   NiceMock<MockNetlinkUtils> netlink_utils_{&netlink_manager_};
   NiceMock<MockScanUtils> scan_utils_{&netlink_manager_};
   NiceMock<MockInterfaceTool> if_tool_;
-  NiceMock<MockSupplicantManager> supplicant_manager_;
   NiceMock<MockClientInterfaceImpl> client_interface_impl_{
-      &if_tool_, &supplicant_manager_, &netlink_utils_, &scan_utils_};
+      &if_tool_, &netlink_utils_, &scan_utils_};
   shared_ptr<NiceMock<MockOffloadServiceUtils>> offload_service_utils_{
       new NiceMock<MockOffloadServiceUtils>()};
   shared_ptr<NiceMock<MockOffloadScanCallbackInterfaceImpl>>
diff --git a/tests/server_unittest.cpp b/tests/server_unittest.cpp
index c52e8e3..3c9f94a 100644
--- a/tests/server_unittest.cpp
+++ b/tests/server_unittest.cpp
@@ -154,5 +154,37 @@ TEST_F(ServerTest, CanDestroyInterfaces) {
   EXPECT_TRUE(server_.createApInterface(kFakeInterfaceName, &ap_if).isOk());
 }
 
+TEST_F(ServerTest, ShouldReportEnableFailure) {
+  EXPECT_CALL(*supplicant_manager_, StartSupplicant())
+      .WillOnce(Return(false));
+  bool success;
+  EXPECT_TRUE(server_.enableSupplicant(&success).isOk());
+  EXPECT_FALSE(success);
+}
+
+TEST_F(ServerTest, ShouldReportenableSuccess) {
+  EXPECT_CALL(*supplicant_manager_, StartSupplicant())
+      .WillOnce(Return(true));
+  bool success;
+  EXPECT_TRUE(server_.enableSupplicant(&success).isOk());
+  EXPECT_TRUE(success);
+}
+
+TEST_F(ServerTest, ShouldReportDisableFailure) {
+  EXPECT_CALL(*supplicant_manager_, StopSupplicant())
+      .WillOnce(Return(false));
+  bool success;
+  EXPECT_TRUE(server_.disableSupplicant(&success).isOk());
+  EXPECT_FALSE(success);
+}
+
+TEST_F(ServerTest, ShouldReportDisableSuccess) {
+  EXPECT_CALL(*supplicant_manager_, StopSupplicant())
+      .WillOnce(Return(true));
+  bool success;
+  EXPECT_TRUE(server_.disableSupplicant(&success).isOk());
+  EXPECT_TRUE(success);
+}
+
 }  // namespace wificond
 }  // namespace android