Skip to content

Commit

Permalink
Revert "[CrOS Network] Refactor ESimManager to use CellularESimInstal…
Browse files Browse the repository at this point in the history
…ler"

This reverts commit cfd49cd.

Reason for revert: Suspicious about causing test failures of EuiccTest.InstallProfileFromActivationCode
https://ci.chromium.org/ui/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests/24408/test-results

Original change's description:
> [CrOS Network] Refactor ESimManager to use CellularESimInstaller
>
> This CL refactors the ESimManager to use the CellularESimInstaller for
> installing profile from activation code.
>
> Bug: 1231305
> Change-Id: I0360afbb2bc3ab28179a44118004232a63006cdd
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3055531
> Reviewed-by: Azeem Arshad <azeemarshad@chromium.org>
> Commit-Queue: Jason Zhang <jiajunz@google.com>
> Cr-Commit-Position: refs/heads/master@{#906467}

Bug: 1231305
Change-Id: I1a677b34f6abb5e2d38a28f637975d8dddae6728
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3056328
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Owners-Override: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#906625}
  • Loading branch information
yuki3 authored and Chromium LUCI CQ committed Jul 29, 2021
1 parent 0dd251c commit 2f7ea17
Show file tree
Hide file tree
Showing 9 changed files with 236 additions and 121 deletions.
17 changes: 1 addition & 16 deletions chromeos/network/test_cellular_esim_profile_handler.cc
Expand Up @@ -10,9 +10,7 @@

namespace chromeos {

TestCellularESimProfileHandler::TestCellularESimProfileHandler()
: enable_notify_profile_list_update_(true),
has_pending_notify_list_update_(false) {}
TestCellularESimProfileHandler::TestCellularESimProfileHandler() = default;

TestCellularESimProfileHandler::~TestCellularESimProfileHandler() = default;

Expand All @@ -27,15 +25,6 @@ void TestCellularESimProfileHandler::SetHasRefreshedProfilesForEuicc(
refreshed_eids_.erase(eid);
}

void TestCellularESimProfileHandler::SetEnableNotifyProfileListUpdate(
bool enable_notify_profile_list_update) {
enable_notify_profile_list_update_ = enable_notify_profile_list_update;
if (enable_notify_profile_list_update_ && has_pending_notify_list_update_) {
NotifyESimProfileListUpdated();
has_pending_notify_list_update_ = false;
}
}

std::vector<CellularESimProfile>
TestCellularESimProfileHandler::GetESimProfiles() {
return esim_profile_states_;
Expand All @@ -57,10 +46,6 @@ void TestCellularESimProfileHandler::OnHermesPropertiesUpdated() {
esim_profile_states_ = new_profile_states;

network_state_handler()->SyncStubCellularNetworks();
if (!enable_notify_profile_list_update_) {
has_pending_notify_list_update_ = true;
return;
}
NotifyESimProfileListUpdated();
}

Expand Down
9 changes: 0 additions & 9 deletions chromeos/network/test_cellular_esim_profile_handler.h
Expand Up @@ -22,13 +22,6 @@ class TestCellularESimProfileHandler : public CellularESimProfileHandler {
void SetHasRefreshedProfilesForEuicc(const std::string& eid,
bool has_refreshed);

// Enables or disables profile list update notification.
// When set to false, this class will disable triggering the
// NotifyESimProfileListUpdated() and when the next time it's set to true, it
// will call the NotifyESimProfileListUpdated() to fire any pending list
// update notification.
void SetEnableNotifyProfileListUpdate(bool enable_notify_profile_list_update);

// CellularESimProfileHandler:
std::vector<CellularESimProfile> GetESimProfiles() override;
bool HasRefreshedProfilesForEuicc(const std::string& eid) override;
Expand All @@ -38,8 +31,6 @@ class TestCellularESimProfileHandler : public CellularESimProfileHandler {
private:
std::vector<CellularESimProfile> esim_profile_states_;
base::flat_set<std::string> refreshed_eids_;
bool enable_notify_profile_list_update_;
bool has_pending_notify_list_update_;
};

} // namespace chromeos
Expand Down
3 changes: 0 additions & 3 deletions chromeos/services/cellular_setup/esim_manager.cc
Expand Up @@ -52,7 +52,6 @@ std::string ESimManager::GetRootSmdsAddress() {

ESimManager::ESimManager()
: ESimManager(NetworkHandler::Get()->cellular_connection_handler(),
NetworkHandler::Get()->cellular_esim_installer(),
NetworkHandler::Get()->cellular_esim_profile_handler(),
NetworkHandler::Get()->cellular_esim_uninstall_handler(),
NetworkHandler::Get()->cellular_inhibitor(),
Expand All @@ -61,14 +60,12 @@ ESimManager::ESimManager()

ESimManager::ESimManager(
CellularConnectionHandler* cellular_connection_handler,
CellularESimInstaller* cellular_esim_installer,
CellularESimProfileHandler* cellular_esim_profile_handler,
CellularESimUninstallHandler* cellular_esim_uninstall_handler,
CellularInhibitor* cellular_inhibitor,
NetworkConnectionHandler* network_connection_handler,
NetworkStateHandler* network_state_handler)
: cellular_connection_handler_(cellular_connection_handler),
cellular_esim_installer_(cellular_esim_installer),
cellular_esim_profile_handler_(cellular_esim_profile_handler),
cellular_esim_uninstall_handler_(cellular_esim_uninstall_handler),
cellular_inhibitor_(cellular_inhibitor),
Expand Down
7 changes: 0 additions & 7 deletions chromeos/services/cellular_setup/esim_manager.h
Expand Up @@ -22,7 +22,6 @@ class ObjectPath;
namespace chromeos {

class CellularConnectionHandler;
class CellularESimInstaller;
class CellularESimUninstallHandler;
class CellularInhibitor;
class NetworkConnectionHandler;
Expand All @@ -46,7 +45,6 @@ class ESimManager : public mojom::ESimManager,

ESimManager();
ESimManager(CellularConnectionHandler* cellular_connection_handler,
CellularESimInstaller* cellular_esim_installer,
CellularESimProfileHandler* cellular_esim_profile_handler,
CellularESimUninstallHandler* cellular_esim_uninstall_handler,
CellularInhibitor* cellular_inhibitor,
Expand Down Expand Up @@ -80,10 +78,6 @@ class ESimManager : public mojom::ESimManager,
// Notifies observers of changes to ESimProfile Lists.
void NotifyESimProfileListChanged(Euicc* euicc);

CellularESimInstaller* cellular_esim_installer() {
return cellular_esim_installer_;
}

CellularESimProfileHandler* cellular_esim_profile_handler() {
return cellular_esim_profile_handler_;
}
Expand Down Expand Up @@ -117,7 +111,6 @@ class ESimManager : public mojom::ESimManager,
bool CreateEuiccIfNew(const dbus::ObjectPath& euicc_path);

CellularConnectionHandler* cellular_connection_handler_;
CellularESimInstaller* cellular_esim_installer_;
CellularESimProfileHandler* cellular_esim_profile_handler_;
CellularESimUninstallHandler* cellular_esim_uninstall_handler_;
CellularInhibitor* cellular_inhibitor_;
Expand Down
8 changes: 1 addition & 7 deletions chromeos/services/cellular_setup/esim_test_base.cc
Expand Up @@ -12,7 +12,6 @@
#include "chromeos/dbus/shill/shill_clients.h"
#include "chromeos/dbus/shill/shill_manager_client.h"
#include "chromeos/network/cellular_connection_handler.h"
#include "chromeos/network/cellular_esim_installer.h"
#include "chromeos/network/cellular_esim_uninstall_handler.h"
#include "chromeos/network/cellular_inhibitor.h"
#include "chromeos/network/fake_network_connection_handler.h"
Expand Down Expand Up @@ -64,10 +63,6 @@ void ESimTestBase::SetUp() {
cellular_connection_handler_->Init(network_state_handler_.get(),
cellular_inhibitor_.get(),
cellular_esim_profile_handler_.get());
cellular_esim_installer_ = std::make_unique<CellularESimInstaller>();
cellular_esim_installer_->Init(
cellular_connection_handler_.get(), cellular_inhibitor_.get(),
network_connection_handler_.get(), network_state_handler_.get());
cellular_esim_uninstall_handler_ =
std::make_unique<CellularESimUninstallHandler>();
cellular_esim_uninstall_handler_->Init(
Expand All @@ -76,8 +71,7 @@ void ESimTestBase::SetUp() {
network_state_handler_.get());

esim_manager_ = std::make_unique<ESimManager>(
cellular_connection_handler_.get(), cellular_esim_installer_.get(),
cellular_esim_profile_handler_.get(),
cellular_connection_handler_.get(), cellular_esim_profile_handler_.get(),
cellular_esim_uninstall_handler_.get(), cellular_inhibitor_.get(),
network_connection_handler_.get(), network_state_handler_.get());
observer_ = std::make_unique<ESimManagerTestObserver>();
Expand Down
21 changes: 7 additions & 14 deletions chromeos/services/cellular_setup/esim_test_base.h
Expand Up @@ -13,15 +13,14 @@

namespace chromeos {

class CellularConnectionHandler;
class CellularESimInstaller;
class CellularESimUninstallHandler;
class CellularInhibitor;
class FakeNetworkConnectionHandler;
class NetworkConfigurationHandler;
class NetworkDeviceHandler;
class NetworkStateHandler;
class TestCellularESimProfileHandler;
class NetworkDeviceHandler;
class NetworkConfigurationHandler;
class FakeNetworkConnectionHandler;
class CellularConnectionHandler;
class CellularESimUninstallHandler;
class CellularESimProfileHandler;

namespace cellular_setup {

Expand Down Expand Up @@ -63,19 +62,13 @@ class ESimTestBase : public testing::Test {
return network_state_handler_.get();
}

TestCellularESimProfileHandler* cellular_esim_profile_handler() {
return cellular_esim_profile_handler_.get();
}

base::test::TaskEnvironment* task_environment() { return &task_environment_; }

private:
std::unique_ptr<NetworkStateHandler> network_state_handler_;
std::unique_ptr<NetworkDeviceHandler> network_device_handler_;
std::unique_ptr<CellularInhibitor> cellular_inhibitor_;
std::unique_ptr<CellularESimInstaller> cellular_esim_installer_;
std::unique_ptr<TestCellularESimProfileHandler>
cellular_esim_profile_handler_;
std::unique_ptr<CellularESimProfileHandler> cellular_esim_profile_handler_;
std::unique_ptr<NetworkConfigurationHandler> network_configuration_handler_;
std::unique_ptr<FakeNetworkConnectionHandler> network_connection_handler_;
std::unique_ptr<CellularESimUninstallHandler>
Expand Down

0 comments on commit 2f7ea17

Please sign in to comment.