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

Do not enable Wifi Sync V2 if not allowed after multidevice setup

This CL prevents Wifi Sync v2 from being enabled after multidevice
setup if:
1. Wifi Sync V2 is not supported on the host device.
2. Wifi Sync V2 is prohibited by enterprise policy.

The unittests are also updated to test for these cases.

Bug: 1117619
Change-Id: I7e797430f4bc81700281ce74c54b2497a71fe312
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2468916
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@{#817963}
parent 7f910760
......@@ -15,6 +15,7 @@
#include "chromeos/constants/chromeos_features.h"
#include "chromeos/services/device_sync/feature_status_change.h"
#include "chromeos/services/multidevice_setup/host_status_provider.h"
#include "chromeos/services/multidevice_setup/public/cpp/prefs.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/pref_service.h"
......@@ -96,11 +97,8 @@ void WifiSyncFeatureManagerImpl::OnHostStatusChange(
!ShouldEnableOnVerify()) {
ResetPendingWifiSyncHostNetworkRequest();
}
// kHostSetLocallyButWaitingForBackendConfirmation is only possible if the
// setup flow has been completed on the local device.
if (host_status_with_device.host_status() ==
mojom::HostStatus::kHostSetLocallyButWaitingForBackendConfirmation &&
features::IsWifiSyncAndroidEnabled()) {
if (ShouldAttemptToEnableAfterHostVerified()) {
SetPendingWifiSyncHostNetworkRequest(
PendingState::kSetPendingEnableOnVerify);
return;
......@@ -317,6 +315,33 @@ void WifiSyncFeatureManagerImpl::ProcessEnableOnVerifyAttempt() {
SetIsWifiSyncEnabled(true);
}
bool WifiSyncFeatureManagerImpl::ShouldAttemptToEnableAfterHostVerified() {
HostStatusProvider::HostStatusWithDevice host_status_with_device =
host_status_provider_->GetHostWithStatus();
// kHostSetLocallyButWaitingForBackendConfirmation is only possible if the
// setup flow has been completed on the local device.
if (host_status_with_device.host_status() !=
mojom::HostStatus::kHostSetLocallyButWaitingForBackendConfirmation) {
return false;
}
// Check if enterprise policy prohibits Wifi Sync or if feature flag is
// disabled.
if (!IsFeatureAllowed(mojom::Feature::kWifiSync, pref_service_)) {
return false;
}
// Check if wifi sync is supported by host device.
if (host_status_with_device.host_device()->GetSoftwareFeatureState(
multidevice::SoftwareFeature::kWifiSyncHost) ==
multidevice::SoftwareFeatureState::kNotSupported) {
return false;
}
return true;
}
} // namespace multidevice_setup
} // namespace chromeos
......@@ -123,6 +123,7 @@ class WifiSyncFeatureManagerImpl
device_sync::mojom::NetworkRequestResult result_code);
bool ShouldEnableOnVerify();
void ProcessEnableOnVerifyAttempt();
bool ShouldAttemptToEnableAfterHostVerified();
HostStatusProvider* host_status_provider_;
PrefService* pref_service_;
......
......@@ -18,6 +18,7 @@
#include "chromeos/constants/chromeos_features.h"
#include "chromeos/services/device_sync/public/cpp/fake_device_sync_client.h"
#include "chromeos/services/multidevice_setup/fake_host_status_provider.h"
#include "chromeos/services/multidevice_setup/public/cpp/prefs.h"
#include "chromeos/services/multidevice_setup/public/mojom/multidevice_setup.mojom.h"
#include "components/sync_preferences/testing_pref_service_syncable.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -70,7 +71,9 @@ class MultiDeviceSetupWifiSyncFeatureManagerImplTest
test_pref_service_ =
std::make_unique<sync_preferences::TestingPrefServiceSyncable>();
WifiSyncFeatureManagerImpl::RegisterPrefs(test_pref_service_->registry());
// Allow Wifi Sync by policy
test_pref_service_->registry()->RegisterBooleanPref(
kWifiSyncAllowedPrefName, true);
fake_device_sync_client_ =
std::make_unique<device_sync::FakeDeviceSyncClient>();
fake_device_sync_client_->set_synced_devices(test_devices_);
......@@ -793,6 +796,47 @@ TEST_P(MultiDeviceSetupWifiSyncFeatureManagerImplTest,
EXPECT_FALSE(delegate()->IsWifiSyncEnabled());
}
TEST_P(MultiDeviceSetupWifiSyncFeatureManagerImplTest,
SetPendingEnableOnVerify_WifiSyncNotAllowedByPolicy) {
SetFeatureFlags(GetParam() /* use_v1_devicesync */,
true /* enable_wifi_sync */);
// Disable by policy
test_pref_service()->SetBoolean(kWifiSyncAllowedPrefName, false);
CreateDelegate(base::nullopt /* initial_host */);
// kHostSetLocallyButWaitingForBackendConfirmation is only possible if the
// setup flow has been completed on the local device.
SetHostInDeviceSyncClient(test_devices()[0]);
fake_host_status_provider()->SetHostWithStatus(
mojom::HostStatus::kHostSetLocallyButWaitingForBackendConfirmation,
test_devices()[0]);
EXPECT_EQ(
test_pref_service()->GetInteger(kPendingWifiSyncRequestEnabledPrefName),
kPendingNone);
EXPECT_FALSE(delegate()->IsWifiSyncEnabled());
}
TEST_P(MultiDeviceSetupWifiSyncFeatureManagerImplTest,
SetPendingEnableOnVerify_WifiSyncNotSupportedOnHostDevice) {
SetFeatureFlags(GetParam() /* use_v1_devicesync */,
true /* enable_wifi_sync */);
CreateDelegate(base::nullopt /* initial_host */);
GetMutableRemoteDevice(test_devices()[0])
->software_features[multidevice::SoftwareFeature::kWifiSyncHost] =
multidevice::SoftwareFeatureState::kNotSupported;
// kHostSetLocallyButWaitingForBackendConfirmation is only possible if the
// setup flow has been completed on the local device.
SetHostInDeviceSyncClient(test_devices()[0]);
fake_host_status_provider()->SetHostWithStatus(
mojom::HostStatus::kHostSetLocallyButWaitingForBackendConfirmation,
test_devices()[0]);
EXPECT_EQ(
test_pref_service()->GetInteger(kPendingWifiSyncRequestEnabledPrefName),
kPendingNone);
EXPECT_FALSE(delegate()->IsWifiSyncEnabled());
}
TEST_P(MultiDeviceSetupWifiSyncFeatureManagerImplTest,
SetPendingEnableOnVerify_HostRemoved) {
SetFeatureFlags(GetParam() /* use_v1_devicesync */,
......
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