Commit f35e6d64 authored by Claude van der Merwe's avatar Claude van der Merwe Committed by Commit Bot

Modify FeatureStateManager to use WifiSyncFeatureManager

The following changes are made:
1. WifiSyncFeatureManager is injected into FeatureStateManager.
2. PerformSetFeatureEnabledState relies on WifiSyncFeatureManager to
set enable/disable Wifi Sync, which acts as a global toggle.
3. GetEnabledOrDisabledState relies on WifiSyncFeatureManager to get
the enabled/disabled state for Wifi Sync.
4. Add fakes for WifiSyncFeatureManager to be used in
feature_state_manager_impl unittests

Bug: 1117619
Change-Id: I68e6cad0e9d878d4029dea9cfeedc52dcee4a4b4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2419451
Commit-Queue: Claude van der Merwe <cvandermerwe@google.com>
Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Reviewed-by: default avatarJon Mann <jonmann@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811030}
parent 8d6c9051
......@@ -113,6 +113,8 @@ static_library("test_support") {
"fake_host_status_provider.h",
"fake_host_verifier.cc",
"fake_host_verifier.h",
"fake_wifi_sync_feature_manager.cc",
"fake_wifi_sync_feature_manager.h",
]
deps = [
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chromeos/services/multidevice_setup/fake_wifi_sync_feature_manager.h"
namespace chromeos {
namespace multidevice_setup {
FakeWifiSyncFeatureManager::FakeWifiSyncFeatureManager()
: WifiSyncFeatureManager() {}
FakeWifiSyncFeatureManager::~FakeWifiSyncFeatureManager() = default;
void FakeWifiSyncFeatureManager::SetIsWifiSyncEnabled(bool enabled) {
is_wifi_sync_enabled_ = enabled;
}
bool FakeWifiSyncFeatureManager::IsWifiSyncEnabled() {
return is_wifi_sync_enabled_;
}
} // namespace multidevice_setup
} // namespace chromeos
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef CHROMEOS_SERVICES_MULTIDEVICE_SETUP_FAKE_WIFI_SYNC_FEATURE_MANAGER_H_
#define CHROMEOS_SERVICES_MULTIDEVICE_SETUP_FAKE_WIFI_SYNC_FEATURE_MANAGER_H_
#include <utility>
#include <vector>
#include "base/callback_forward.h"
#include "base/macros.h"
#include "base/optional.h"
#include "chromeos/components/multidevice/remote_device_ref.h"
#include "chromeos/services/multidevice_setup/wifi_sync_feature_manager.h"
namespace chromeos {
namespace multidevice_setup {
// Test WifiSyncFeatureManager implementation.
class FakeWifiSyncFeatureManager : public WifiSyncFeatureManager {
public:
FakeWifiSyncFeatureManager();
~FakeWifiSyncFeatureManager() override;
// WifiSyncFeatureManager:
void SetIsWifiSyncEnabled(bool enabled) override;
bool IsWifiSyncEnabled() override;
private:
bool is_wifi_sync_enabled_ = false;
};
} // namespace multidevice_setup
} // namespace chromeos
#endif // CHROMEOS_SERVICES_MULTIDEVICE_SETUP_FAKE_WIFI_SYNC_FEATURE_MANAGER_H_
......@@ -17,6 +17,7 @@
#include "chromeos/components/multidevice/remote_device_ref.h"
#include "chromeos/components/multidevice/software_feature.h"
#include "chromeos/services/multidevice_setup/public/cpp/prefs.h"
#include "chromeos/services/multidevice_setup/wifi_sync_feature_manager.h"
#include "components/prefs/pref_service.h"
namespace chromeos {
......@@ -271,16 +272,17 @@ std::unique_ptr<FeatureStateManager> FeatureStateManagerImpl::Factory::Create(
PrefService* pref_service,
HostStatusProvider* host_status_provider,
device_sync::DeviceSyncClient* device_sync_client,
AndroidSmsPairingStateTracker* android_sms_pairing_state_tracker) {
AndroidSmsPairingStateTracker* android_sms_pairing_state_tracker,
WifiSyncFeatureManager* wifi_sync_feature_manager) {
if (test_factory_) {
return test_factory_->CreateInstance(pref_service, host_status_provider,
device_sync_client,
android_sms_pairing_state_tracker);
return test_factory_->CreateInstance(
pref_service, host_status_provider, device_sync_client,
android_sms_pairing_state_tracker, wifi_sync_feature_manager);
}
return base::WrapUnique(new FeatureStateManagerImpl(
pref_service, host_status_provider, device_sync_client,
android_sms_pairing_state_tracker));
android_sms_pairing_state_tracker, wifi_sync_feature_manager));
}
// static
......@@ -295,11 +297,13 @@ FeatureStateManagerImpl::FeatureStateManagerImpl(
PrefService* pref_service,
HostStatusProvider* host_status_provider,
device_sync::DeviceSyncClient* device_sync_client,
AndroidSmsPairingStateTracker* android_sms_pairing_state_tracker)
AndroidSmsPairingStateTracker* android_sms_pairing_state_tracker,
WifiSyncFeatureManager* wifi_sync_feature_manager)
: pref_service_(pref_service),
host_status_provider_(host_status_provider),
device_sync_client_(device_sync_client),
android_sms_pairing_state_tracker_(android_sms_pairing_state_tracker),
wifi_sync_feature_manager_(wifi_sync_feature_manager),
feature_to_enabled_pref_name_map_(GenerateFeatureToEnabledPrefNameMap()),
feature_to_allowed_pref_name_map_(GenerateFeatureToAllowedPrefNameMap()),
cached_feature_state_map_(GenerateInitialDefaultCachedStateMap()) {
......@@ -356,14 +360,12 @@ FeatureStateManagerImpl::GetFeatureStates() {
void FeatureStateManagerImpl::PerformSetFeatureEnabledState(
mojom::Feature feature,
bool enabled) {
// TODO(cvandermerwe) Replace wifi_sync_enabled_ once HostBackendDelegate
// tracks Wifi feature state. Wifi sync enabled toggle acts as a 'global'
// toggle which applies to all Chrome OS devices and a connected Android
// phone.
// Wifi sync enabled toggle acts as a global toggle which applies to all
// Chrome OS devices and a connected Android phone.
if (feature == mojom::Feature::kWifiSync) {
wifi_sync_enabled_ = enabled;
wifi_sync_feature_manager_->SetIsWifiSyncEnabled(enabled);
// Need to manually trigger UpdateFeatureStateCache since changes to
// wifi_sync_enabled_ is not observed by |registrar_| and will not invoke
// wifi sync is not observed by |registrar_| and will not invoke
// OnPrefValueChanged
UpdateFeatureStateCache(true /* notify_observers_of_changes */);
return;
......@@ -533,8 +535,18 @@ bool FeatureStateManagerImpl::HasBeenActivatedByPhone(
if (pair.first != feature)
continue;
return host_device.GetSoftwareFeatureState(pair.second) ==
multidevice::SoftwareFeatureState::kEnabled;
multidevice::SoftwareFeatureState feature_state =
host_device.GetSoftwareFeatureState(pair.second);
if (feature_state == multidevice::SoftwareFeatureState::kEnabled) {
return true;
}
// Edge Case: Wifi Sync is considered activated on host when the state is
// kSupported or kEnabled. kEnabled/kSupported correspond to on/off for Wifi
// Sync Host.
return (feature == mojom::Feature::kWifiSync &&
feature_state == multidevice::SoftwareFeatureState::kSupported);
}
NOTREACHED();
......@@ -558,13 +570,13 @@ bool FeatureStateManagerImpl::RequiresFurtherSetup(mojom::Feature feature) {
mojom::FeatureState FeatureStateManagerImpl::GetEnabledOrDisabledState(
mojom::Feature feature) {
// TODO(cvandermerwe) Replace wifi_sync_enabled_ once HostBackendDelegate
// tracks Wifi feature state. Wifi sync enabled toggle acts as a 'global'
// toggle which applies to all Chrome OS devices and a connected Android
// phone.
// WifiSyncFeatureManager is the source of truth for Wifi Sync enabled state.
// It is a global setting that applies to all synced Chrome OS devices and a
// connected Android phone.
if (feature == mojom::Feature::kWifiSync) {
return wifi_sync_enabled_ ? mojom::FeatureState::kEnabledByUser
: mojom::FeatureState::kDisabledByUser;
return (wifi_sync_feature_manager_->IsWifiSyncEnabled()
? mojom::FeatureState::kEnabledByUser
: mojom::FeatureState::kDisabledByUser);
}
if (!base::Contains(feature_to_enabled_pref_name_map_, feature)) {
......
......@@ -12,6 +12,7 @@
#include "chromeos/services/multidevice_setup/host_status_provider.h"
#include "chromeos/services/multidevice_setup/public/cpp/android_sms_pairing_state_tracker.h"
#include "chromeos/services/multidevice_setup/public/mojom/multidevice_setup.mojom.h"
#include "chromeos/services/multidevice_setup/wifi_sync_feature_manager.h"
#include "components/prefs/pref_change_registrar.h"
class PrefService;
......@@ -36,7 +37,8 @@ class FeatureStateManagerImpl : public FeatureStateManager,
PrefService* pref_service,
HostStatusProvider* host_status_provider,
device_sync::DeviceSyncClient* device_sync_client,
AndroidSmsPairingStateTracker* android_sms_pairing_state_tracker);
AndroidSmsPairingStateTracker* android_sms_pairing_state_tracker,
WifiSyncFeatureManager* wifi_sync_feature_manager);
static void SetFactoryForTesting(Factory* test_factory);
protected:
......@@ -45,7 +47,8 @@ class FeatureStateManagerImpl : public FeatureStateManager,
PrefService* pref_service,
HostStatusProvider* host_status_provider,
device_sync::DeviceSyncClient* device_sync_client,
AndroidSmsPairingStateTracker* android_sms_pairing_state_tracker) = 0;
AndroidSmsPairingStateTracker* android_sms_pairing_state_tracker,
WifiSyncFeatureManager* wifi_sync_feature_manager) = 0;
private:
static Factory* test_factory_;
......@@ -58,7 +61,8 @@ class FeatureStateManagerImpl : public FeatureStateManager,
PrefService* pref_service,
HostStatusProvider* host_status_provider,
device_sync::DeviceSyncClient* device_sync_client,
AndroidSmsPairingStateTracker* android_sms_pairing_state_tracker);
AndroidSmsPairingStateTracker* android_sms_pairing_state_tracker,
WifiSyncFeatureManager* wifi_sync_feature_manager);
// FeatureStateManager:
FeatureStatesMap GetFeatureStates() override;
......@@ -91,10 +95,7 @@ class FeatureStateManagerImpl : public FeatureStateManager,
HostStatusProvider* host_status_provider_;
device_sync::DeviceSyncClient* device_sync_client_;
AndroidSmsPairingStateTracker* android_sms_pairing_state_tracker_;
// TODO(cvandermerwe) Remove once HostBackendDelegate tracks Wifi feature
// state. Always initialized to true at the start of each user session for
// now.
bool wifi_sync_enabled_ = true;
WifiSyncFeatureManager* wifi_sync_feature_manager_;
// Map from feature to the pref name which indicates the enabled/disabled
// boolean state for the feature.
......
......@@ -14,6 +14,7 @@
#include "chromeos/services/device_sync/public/cpp/fake_device_sync_client.h"
#include "chromeos/services/multidevice_setup/fake_feature_state_manager.h"
#include "chromeos/services/multidevice_setup/fake_host_status_provider.h"
#include "chromeos/services/multidevice_setup/fake_wifi_sync_feature_manager.h"
#include "chromeos/services/multidevice_setup/public/cpp/fake_android_sms_pairing_state_tracker.h"
#include "chromeos/services/multidevice_setup/public/cpp/prefs.h"
#include "components/sync_preferences/testing_pref_service_syncable.h"
......@@ -107,10 +108,14 @@ class MultiDeviceSetupFeatureStateManagerImplTest : public testing::Test {
std::make_unique<FakeAndroidSmsPairingStateTracker>();
fake_android_sms_pairing_state_tracker_->SetPairingComplete(true);
fake_wifi_sync_feature_manager_ =
std::make_unique<FakeWifiSyncFeatureManager>();
manager_ = FeatureStateManagerImpl::Factory::Create(
test_pref_service_.get(), fake_host_status_provider_.get(),
fake_device_sync_client_.get(),
fake_android_sms_pairing_state_tracker_.get());
fake_android_sms_pairing_state_tracker_.get(),
fake_wifi_sync_feature_manager_.get());
fake_observer_ = std::make_unique<FakeFeatureStateManagerObserver>();
manager_->AddObserver(fake_observer_.get());
......@@ -226,6 +231,9 @@ class MultiDeviceSetupFeatureStateManagerImplTest : public testing::Test {
}
FeatureStateManager* manager() { return manager_.get(); }
FakeWifiSyncFeatureManager* wifi_sync_manager() {
return fake_wifi_sync_feature_manager_.get();
}
private:
multidevice::RemoteDeviceRef test_local_device_;
......@@ -237,6 +245,7 @@ class MultiDeviceSetupFeatureStateManagerImplTest : public testing::Test {
std::unique_ptr<device_sync::FakeDeviceSyncClient> fake_device_sync_client_;
std::unique_ptr<FakeAndroidSmsPairingStateTracker>
fake_android_sms_pairing_state_tracker_;
std::unique_ptr<FakeWifiSyncFeatureManager> fake_wifi_sync_feature_manager_;
std::unique_ptr<FakeFeatureStateManagerObserver> fake_observer_;
......@@ -555,14 +564,16 @@ TEST_F(MultiDeviceSetupFeatureStateManagerImplTest, WifiSync) {
SetSoftwareFeatureState(true /* use_local_device */,
multidevice::SoftwareFeature::kWifiSyncClient,
multidevice::SoftwareFeatureState::kSupported);
VerifyFeatureState(mojom::FeatureState::kNotSupportedByPhone,
wifi_sync_manager()->SetIsWifiSyncEnabled(true);
VerifyFeatureState(mojom::FeatureState::kDisabledByUser,
mojom::Feature::kWifiSync);
VerifyFeatureStateChange(1u /* expected_index */, mojom::Feature::kWifiSync,
mojom::FeatureState::kNotSupportedByPhone);
mojom::FeatureState::kDisabledByUser);
SetSoftwareFeatureState(false /* use_local_device */,
multidevice::SoftwareFeature::kWifiSyncHost,
multidevice::SoftwareFeatureState::kEnabled);
wifi_sync_manager()->SetIsWifiSyncEnabled(false);
VerifyFeatureState(mojom::FeatureState::kEnabledByUser,
mojom::Feature::kWifiSync);
VerifyFeatureStateChange(2u /* expected_index */, mojom::Feature::kWifiSync,
......
......@@ -26,6 +26,7 @@
#include "chromeos/services/multidevice_setup/public/cpp/android_sms_pairing_state_tracker.h"
#include "chromeos/services/multidevice_setup/public/cpp/auth_token_validator.h"
#include "chromeos/services/multidevice_setup/public/cpp/oobe_completion_tracker.h"
#include "chromeos/services/multidevice_setup/wifi_sync_feature_manager_impl.h"
namespace chromeos {
......@@ -116,11 +117,16 @@ MultiDeviceSetupImpl::MultiDeviceSetupImpl(
host_backend_delegate_.get(),
device_sync_client,
pref_service)),
wifi_sync_feature_manager_(WifiSyncFeatureManagerImpl::Factory::Create(
host_status_provider_.get(),
pref_service,
device_sync_client)),
feature_state_manager_(FeatureStateManagerImpl::Factory::Create(
pref_service,
host_status_provider_.get(),
device_sync_client,
android_sms_pairing_state_tracker)),
android_sms_pairing_state_tracker,
wifi_sync_feature_manager_.get())),
host_device_timestamp_manager_(
HostDeviceTimestampManagerImpl::Factory::Create(
host_status_provider_.get(),
......
......@@ -13,6 +13,8 @@
#include "chromeos/services/multidevice_setup/host_status_provider.h"
#include "chromeos/services/multidevice_setup/multidevice_setup_base.h"
#include "chromeos/services/multidevice_setup/public/mojom/multidevice_setup.mojom.h"
#include "chromeos/services/multidevice_setup/wifi_sync_feature_manager.h"
#include "chromeos/services/multidevice_setup/wifi_sync_feature_manager_impl.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/remote_set.h"
#include "url/gurl.h"
......@@ -142,6 +144,7 @@ class MultiDeviceSetupImpl : public MultiDeviceSetupBase,
std::unique_ptr<HostStatusProvider> host_status_provider_;
std::unique_ptr<GrandfatheredEasyUnlockHostDisabler>
grandfathered_easy_unlock_host_disabler_;
std::unique_ptr<WifiSyncFeatureManager> wifi_sync_feature_manager_;
std::unique_ptr<FeatureStateManager> feature_state_manager_;
std::unique_ptr<HostDeviceTimestampManager> host_device_timestamp_manager_;
std::unique_ptr<AccountStatusChangeDelegateNotifier> delegate_notifier_;
......
......@@ -28,6 +28,7 @@
#include "chromeos/services/multidevice_setup/fake_host_status_observer.h"
#include "chromeos/services/multidevice_setup/fake_host_status_provider.h"
#include "chromeos/services/multidevice_setup/fake_host_verifier.h"
#include "chromeos/services/multidevice_setup/fake_wifi_sync_feature_manager.h"
#include "chromeos/services/multidevice_setup/feature_state_manager_impl.h"
#include "chromeos/services/multidevice_setup/grandfathered_easy_unlock_host_disabler.h"
#include "chromeos/services/multidevice_setup/host_backend_delegate_impl.h"
......@@ -40,6 +41,7 @@
#include "chromeos/services/multidevice_setup/public/cpp/fake_auth_token_validator.h"
#include "chromeos/services/multidevice_setup/public/cpp/oobe_completion_tracker.h"
#include "chromeos/services/multidevice_setup/public/mojom/multidevice_setup.mojom.h"
#include "chromeos/services/multidevice_setup/wifi_sync_feature_manager_impl.h"
#include "components/sync_preferences/testing_pref_service_syncable.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -239,6 +241,49 @@ class FakeHostStatusProviderFactory : public HostStatusProviderImpl::Factory {
DISALLOW_COPY_AND_ASSIGN(FakeHostStatusProviderFactory);
};
class FakeWifiSyncFeatureManagerFactory
: public WifiSyncFeatureManagerImpl::Factory {
public:
FakeWifiSyncFeatureManagerFactory(
FakeHostStatusProviderFactory* fake_host_status_provider_factory,
sync_preferences::TestingPrefServiceSyncable*
expected_testing_pref_service,
device_sync::FakeDeviceSyncClient* expected_device_sync_client)
: fake_host_status_provider_factory_(fake_host_status_provider_factory),
expected_testing_pref_service_(expected_testing_pref_service),
expected_device_sync_client_(expected_device_sync_client) {}
~FakeWifiSyncFeatureManagerFactory() override = default;
FakeWifiSyncFeatureManager* instance() { return instance_; }
private:
// WifiSyncFeatureManagerImpl::Factory:
std::unique_ptr<WifiSyncFeatureManager> CreateInstance(
HostStatusProvider* host_status_provider,
PrefService* pref_service,
device_sync::DeviceSyncClient* device_sync_client,
std::unique_ptr<base::OneShotTimer> timer) override {
EXPECT_FALSE(instance_);
EXPECT_EQ(fake_host_status_provider_factory_->instance(),
host_status_provider);
EXPECT_EQ(expected_testing_pref_service_, pref_service);
EXPECT_EQ(expected_device_sync_client_, device_sync_client);
auto instance = std::make_unique<FakeWifiSyncFeatureManager>();
instance_ = instance.get();
return instance;
}
FakeHostStatusProviderFactory* fake_host_status_provider_factory_;
sync_preferences::TestingPrefServiceSyncable* expected_testing_pref_service_;
device_sync::FakeDeviceSyncClient* expected_device_sync_client_;
FakeWifiSyncFeatureManager* instance_ = nullptr;
DISALLOW_COPY_AND_ASSIGN(FakeWifiSyncFeatureManagerFactory);
};
class FakeGrandfatheredEasyUnlockHostDisablerFactory
: public GrandfatheredEasyUnlockHostDisabler::Factory {
public:
......@@ -301,8 +346,8 @@ class FakeFeatureStateManagerFactory : public FeatureStateManagerImpl::Factory {
PrefService* pref_service,
HostStatusProvider* host_status_provider,
device_sync::DeviceSyncClient* device_sync_client,
AndroidSmsPairingStateTracker* android_sms_pairing_state_tracker)
override {
AndroidSmsPairingStateTracker* android_sms_pairing_state_tracker,
WifiSyncFeatureManager* wifi_sync_feature_manager) override {
EXPECT_FALSE(instance_);
EXPECT_EQ(expected_testing_pref_service_, pref_service);
EXPECT_EQ(fake_host_status_provider_factory_->instance(),
......@@ -544,6 +589,13 @@ class MultiDeviceSetupImplTest : public ::testing::TestWithParam<bool> {
HostStatusProviderImpl::Factory::SetFactoryForTesting(
fake_host_status_provider_factory_.get());
fake_wifi_sync_feature_manager_factory_ =
std::make_unique<FakeWifiSyncFeatureManagerFactory>(
fake_host_status_provider_factory_.get(), test_pref_service_.get(),
fake_device_sync_client_.get());
WifiSyncFeatureManagerImpl::Factory::SetFactoryForTesting(
fake_wifi_sync_feature_manager_factory_.get());
fake_grandfathered_easy_unlock_host_disabler_factory_ =
std::make_unique<FakeGrandfatheredEasyUnlockHostDisablerFactory>(
fake_host_backend_delegate_factory_.get(),
......@@ -609,6 +661,7 @@ class MultiDeviceSetupImplTest : public ::testing::TestWithParam<bool> {
DeviceReenroller::Factory::SetFactoryForTesting(nullptr);
AndroidSmsAppInstallingStatusObserver::Factory::SetFactoryForTesting(
nullptr);
WifiSyncFeatureManagerImpl::Factory::SetFactoryForTesting(nullptr);
}
bool IsV1DeviceSyncEnabled() { return GetParam(); }
......@@ -929,6 +982,8 @@ class MultiDeviceSetupImplTest : public ::testing::TestWithParam<bool> {
std::unique_ptr<FakeHostVerifierFactory> fake_host_verifier_factory_;
std::unique_ptr<FakeHostStatusProviderFactory>
fake_host_status_provider_factory_;
std::unique_ptr<FakeWifiSyncFeatureManagerFactory>
fake_wifi_sync_feature_manager_factory_;
std::unique_ptr<FakeGrandfatheredEasyUnlockHostDisablerFactory>
fake_grandfathered_easy_unlock_host_disabler_factory_;
std::unique_ptr<FakeFeatureStateManagerFactory>
......
......@@ -18,6 +18,7 @@
#include "chromeos/services/multidevice_setup/public/cpp/android_sms_app_helper_delegate.h"
#include "chromeos/services/multidevice_setup/public/cpp/android_sms_pairing_state_tracker.h"
#include "chromeos/services/multidevice_setup/public/cpp/prefs.h"
#include "chromeos/services/multidevice_setup/wifi_sync_feature_manager_impl.h"
namespace chromeos {
......@@ -29,6 +30,7 @@ void MultiDeviceSetupService::RegisterProfilePrefs(
HostDeviceTimestampManagerImpl::RegisterPrefs(registry);
AccountStatusChangeDelegateNotifierImpl::RegisterPrefs(registry);
HostBackendDelegateImpl::RegisterPrefs(registry);
WifiSyncFeatureManagerImpl::RegisterPrefs(registry);
HostVerifierImpl::RegisterPrefs(registry);
GrandfatheredEasyUnlockHostDisabler::RegisterPrefs(registry);
RegisterFeaturePrefs(registry);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment