Commit 5d7b5565 authored by Alex Chau's avatar Alex Chau Committed by Commit Bot

Add UnidoOnSignIn to fieldtrial_testing_config.json

- Disabled ClickToCall/SharedClipboard SyncTurnedOff browser test when
  UnidoOnSignIn is on as it no longer applies
- Disabled Sharing registration when device is in local sync mode if
  deriving VAPID key as derived key is not availalbe in local sync mode
- Add DeviceInfo to allowed type in transport mode in
  sync_integration_tests when UnidoOnSignIn is on

Bug: 1009516
Change-Id: I59a48d6feb2aee941ae6adb867c3a72c1ce70f98
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1905946
Commit-Queue: Alex Chau <alexchau@chromium.org>
Reviewed-by: default avatarSteven Holte <holte@chromium.org>
Reviewed-by: default avatarMikel Astiz <mastiz@chromium.org>
Reviewed-by: default avatarMichael van Ouwerkerk <mvanouwerkerk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#714467}
parent e6c876ab
...@@ -27,6 +27,7 @@ ...@@ -27,6 +27,7 @@
#include "components/policy/policy_constants.h" #include "components/policy/policy_constants.h"
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
#include "components/sync/driver/profile_sync_service.h" #include "components/sync/driver/profile_sync_service.h"
#include "components/sync/driver/sync_driver_switches.h"
#include "components/ukm/test_ukm_recorder.h" #include "components/ukm/test_ukm_recorder.h"
#include "services/metrics/public/cpp/ukm_builders.h" #include "services/metrics/public/cpp/ukm_builders.h"
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
...@@ -130,6 +131,14 @@ IN_PROC_BROWSER_TEST_F(ClickToCallBrowserTest, ContextMenu_NoDevicesAvailable) { ...@@ -130,6 +131,14 @@ IN_PROC_BROWSER_TEST_F(ClickToCallBrowserTest, ContextMenu_NoDevicesAvailable) {
IN_PROC_BROWSER_TEST_F(ClickToCallBrowserTest, IN_PROC_BROWSER_TEST_F(ClickToCallBrowserTest,
ContextMenu_DevicesAvailable_SyncTurnedOff) { ContextMenu_DevicesAvailable_SyncTurnedOff) {
if (base::FeatureList::IsEnabled(kSharingUseDeviceInfo) &&
base::FeatureList::IsEnabled(kSharingDeriveVapidKey) &&
base::FeatureList::IsEnabled(switches::kSyncDeviceInfoInTransportMode)) {
// Turning off sync will have no effect when Click to Call is available on
// sign-in.
return;
}
Init(sync_pb::SharingSpecificFields::CLICK_TO_CALL, Init(sync_pb::SharingSpecificFields::CLICK_TO_CALL,
sync_pb::SharingSpecificFields::UNKNOWN); sync_pb::SharingSpecificFields::UNKNOWN);
auto devices = sharing_service()->GetDeviceCandidates( auto devices = sharing_service()->GetDeviceCandidates(
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "chrome/app/chrome_command_ids.h" #include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/renderer_context_menu/render_view_context_menu_test_util.h" #include "chrome/browser/renderer_context_menu/render_view_context_menu_test_util.h"
#include "chrome/browser/sharing/features.h"
#include "chrome/browser/sharing/shared_clipboard/feature_flags.h" #include "chrome/browser/sharing/shared_clipboard/feature_flags.h"
#include "chrome/browser/sharing/sharing_browsertest.h" #include "chrome/browser/sharing/sharing_browsertest.h"
#include "chrome/browser/sharing/sharing_constants.h" #include "chrome/browser/sharing/sharing_constants.h"
...@@ -18,6 +19,7 @@ ...@@ -18,6 +19,7 @@
#include "chrome/browser/sharing/sharing_sync_preference.h" #include "chrome/browser/sharing/sharing_sync_preference.h"
#include "chrome/browser/sync/test/integration/sessions_helper.h" #include "chrome/browser/sync/test/integration/sessions_helper.h"
#include "components/sync/driver/profile_sync_service.h" #include "components/sync/driver/profile_sync_service.h"
#include "components/sync/driver/sync_driver_switches.h"
#include "url/gurl.h" #include "url/gurl.h"
namespace { namespace {
...@@ -126,6 +128,14 @@ IN_PROC_BROWSER_TEST_F(SharedClipboardBrowserTest, ContextMenu_NoDevices) { ...@@ -126,6 +128,14 @@ IN_PROC_BROWSER_TEST_F(SharedClipboardBrowserTest, ContextMenu_NoDevices) {
} }
IN_PROC_BROWSER_TEST_F(SharedClipboardBrowserTest, ContextMenu_SyncTurnedOff) { IN_PROC_BROWSER_TEST_F(SharedClipboardBrowserTest, ContextMenu_SyncTurnedOff) {
if (base::FeatureList::IsEnabled(kSharingUseDeviceInfo) &&
base::FeatureList::IsEnabled(kSharingDeriveVapidKey) &&
base::FeatureList::IsEnabled(switches::kSyncDeviceInfoInTransportMode)) {
// Turning off sync will have no effect when Shared Clipboard is available
// on sign-in.
return;
}
Init(sync_pb::SharingSpecificFields::SHARED_CLIPBOARD, Init(sync_pb::SharingSpecificFields::SHARED_CLIPBOARD,
sync_pb::SharingSpecificFields::UNKNOWN); sync_pb::SharingSpecificFields::UNKNOWN);
auto devices = sharing_service()->GetDeviceCandidates( auto devices = sharing_service()->GetDeviceCandidates(
......
...@@ -25,6 +25,7 @@ ...@@ -25,6 +25,7 @@
#include "chrome/browser/sharing/sharing_fcm_sender.h" #include "chrome/browser/sharing/sharing_fcm_sender.h"
#include "chrome/browser/sharing/sharing_message_handler.h" #include "chrome/browser/sharing/sharing_message_handler.h"
#include "chrome/browser/sharing/sharing_metrics.h" #include "chrome/browser/sharing/sharing_metrics.h"
#include "chrome/browser/sharing/sharing_service_factory.h"
#include "chrome/browser/sharing/sharing_sync_preference.h" #include "chrome/browser/sharing/sharing_sync_preference.h"
#include "chrome/browser/sharing/sharing_utils.h" #include "chrome/browser/sharing/sharing_utils.h"
#include "chrome/browser/sharing/sms/sms_fetch_request_handler.h" #include "chrome/browser/sharing/sms/sms_fetch_request_handler.h"
...@@ -410,19 +411,35 @@ void SharingService::OnDeviceUnregistered( ...@@ -410,19 +411,35 @@ void SharingService::OnDeviceUnregistered(
} }
bool SharingService::IsSyncEnabled() const { bool SharingService::IsSyncEnabled() const {
return sync_service_ && if (!sync_service_)
sync_service_->GetTransportState() == return false;
syncer::SyncService::TransportState::ACTIVE &&
sync_service_->GetActiveDataTypes().HasAll(GetRequiredSyncDataTypes()); bool is_sync_enabled =
sync_service_->GetTransportState() ==
syncer::SyncService::TransportState::ACTIVE &&
sync_service_->GetActiveDataTypes().HasAll(GetRequiredSyncDataTypes());
// TODO(crbug.com/1012226): Remove local sync check when we have dedicated
// Sharing data type.
if (base::FeatureList::IsEnabled(kSharingDeriveVapidKey))
is_sync_enabled &= !sync_service_->IsLocalSyncEnabled();
return is_sync_enabled;
} }
bool SharingService::IsSyncDisabled() const { bool SharingService::IsSyncDisabled() const {
return sync_service_ && (sync_service_->GetTransportState() == if (!sync_service_)
syncer::SyncService::TransportState::DISABLED || return false;
(sync_service_->GetTransportState() ==
syncer::SyncService::TransportState::ACTIVE && bool is_sync_disabled =
!sync_service_->GetActiveDataTypes().HasAll( sync_service_->GetTransportState() ==
GetRequiredSyncDataTypes()))); syncer::SyncService::TransportState::DISABLED ||
(sync_service_->GetTransportState() ==
syncer::SyncService::TransportState::ACTIVE &&
!sync_service_->GetActiveDataTypes().HasAll(GetRequiredSyncDataTypes()));
// TODO(crbug.com/1012226): Remove local sync check when we have dedicated
// Sharing data type.
if (base::FeatureList::IsEnabled(kSharingDeriveVapidKey))
is_sync_disabled |= sync_service_->IsLocalSyncEnabled();
return is_sync_disabled;
} }
syncer::ModelTypeSet SharingService::GetRequiredSyncDataTypes() const { syncer::ModelTypeSet SharingService::GetRequiredSyncDataTypes() const {
......
...@@ -566,6 +566,24 @@ TEST_F(SharingServiceTest, DeviceUnregistrationSyncDisabled) { ...@@ -566,6 +566,24 @@ TEST_F(SharingServiceTest, DeviceUnregistrationSyncDisabled) {
GetSharingService()->GetStateForTesting()); GetSharingService()->GetStateForTesting());
} }
TEST_F(SharingServiceTest, DeviceUnregistrationLocalSyncEnabled) {
// Enable the registration feature and transport mode required features.
scoped_feature_list_.InitWithFeatures(
/*enabled_features=*/{kSharingDeviceRegistration, kSharingUseDeviceInfo,
kSharingDeriveVapidKey},
/*disabled_features=*/{});
test_sync_service_.SetTransportState(
syncer::SyncService::TransportState::ACTIVE);
test_sync_service_.SetActiveDataTypes({syncer::DEVICE_INFO});
test_sync_service_.SetLocalSyncEnabled(true);
// Create new SharingService instance with sync disabled at constructor.
GetSharingService();
EXPECT_EQ(1, sharing_device_registration_->unregistration_attempts());
EXPECT_EQ(SharingService::State::DISABLED,
GetSharingService()->GetStateForTesting());
}
TEST_F(SharingServiceTest, DeviceRegisterAndUnregister) { TEST_F(SharingServiceTest, DeviceRegisterAndUnregister) {
// Enable the feature. // Enable the feature.
scoped_feature_list_.InitAndEnableFeature(kSharingDeviceRegistration); scoped_feature_list_.InitAndEnableFeature(kSharingDeviceRegistration);
...@@ -659,6 +677,29 @@ TEST_F(SharingServiceTest, StartListeningToFCMAtConstructor) { ...@@ -659,6 +677,29 @@ TEST_F(SharingServiceTest, StartListeningToFCMAtConstructor) {
GetSharingService(); GetSharingService();
} }
TEST_F(SharingServiceTest, NoDevicesWhenLocalSyncEnabled) {
// Enable the registration feature and transport mode required features.
scoped_feature_list_.InitWithFeatures(
/*enabled_features=*/{kSharingDeviceRegistration, kSharingUseDeviceInfo,
kSharingDeriveVapidKey},
/*disabled_features=*/{});
test_sync_service_.SetTransportState(
syncer::SyncService::TransportState::ACTIVE);
test_sync_service_.SetActiveDataTypes({syncer::DEVICE_INFO});
test_sync_service_.SetLocalSyncEnabled(true);
std::string id = base::GenerateGUID();
std::unique_ptr<syncer::DeviceInfo> device_info =
CreateFakeDeviceInfo(id, kDeviceName);
fake_device_info_sync_service.GetDeviceInfoTracker()->Add(device_info.get());
std::vector<std::unique_ptr<syncer::DeviceInfo>> candidates =
GetSharingService()->GetDeviceCandidates(
sync_pb::SharingSpecificFields::CLICK_TO_CALL);
ASSERT_EQ(0u, candidates.size());
}
TEST_F(SharingServiceTest, NoDevicesWhenSyncDisabled) { TEST_F(SharingServiceTest, NoDevicesWhenSyncDisabled) {
scoped_feature_list_.InitAndEnableFeature(kSharingDeviceRegistration); scoped_feature_list_.InitAndEnableFeature(kSharingDeviceRegistration);
test_sync_service_.SetTransportState( test_sync_service_.SetTransportState(
......
...@@ -10,6 +10,8 @@ ...@@ -10,6 +10,8 @@
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/send_tab_to_self/send_tab_to_self_util.h" #include "chrome/browser/send_tab_to_self/send_tab_to_self_util.h"
#include "chrome/browser/sharing/sharing_service.h"
#include "chrome/browser/sharing/sharing_service_factory.h"
#include "chrome/browser/sync/profile_sync_service_factory.h" #include "chrome/browser/sync/profile_sync_service_factory.h"
#include "chrome/browser/sync/test/integration/single_client_status_change_checker.h" #include "chrome/browser/sync/test/integration/single_client_status_change_checker.h"
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
...@@ -76,6 +78,9 @@ IN_PROC_BROWSER_TEST_F(LocalSyncTest, ShouldStart) { ...@@ -76,6 +78,9 @@ IN_PROC_BROWSER_TEST_F(LocalSyncTest, ShouldStart) {
// Verify certain features are disabled. // Verify certain features are disabled.
EXPECT_FALSE(send_tab_to_self::IsUserSyncTypeActive(browser()->profile())); EXPECT_FALSE(send_tab_to_self::IsUserSyncTypeActive(browser()->profile()));
EXPECT_EQ(SharingService::State::DISABLED,
SharingServiceFactory::GetForBrowserContext(browser()->profile())
->GetStateForTesting());
} }
#endif // defined(OS_WIN) #endif // defined(OS_WIN)
......
...@@ -30,6 +30,9 @@ syncer::ModelTypeSet AllowedTypesInStandaloneTransportMode() { ...@@ -30,6 +30,9 @@ syncer::ModelTypeSet AllowedTypesInStandaloneTransportMode() {
syncer::SECURITY_EVENTS, syncer::SECURITY_EVENTS,
syncer::AUTOFILL_WALLET_DATA); syncer::AUTOFILL_WALLET_DATA);
allowed_types.PutAll(syncer::ControlTypes()); allowed_types.PutAll(syncer::ControlTypes());
if (base::FeatureList::IsEnabled(switches::kSyncDeviceInfoInTransportMode)) {
allowed_types.Put(syncer::DEVICE_INFO);
}
return allowed_types; return allowed_types;
} }
......
...@@ -27,6 +27,9 @@ syncer::ModelTypeSet AllowedTypesInStandaloneTransportMode() { ...@@ -27,6 +27,9 @@ syncer::ModelTypeSet AllowedTypesInStandaloneTransportMode() {
syncer::SECURITY_EVENTS, syncer::SECURITY_EVENTS,
syncer::AUTOFILL_WALLET_DATA); syncer::AUTOFILL_WALLET_DATA);
allowed_types.PutAll(syncer::ControlTypes()); allowed_types.PutAll(syncer::ControlTypes());
if (base::FeatureList::IsEnabled(switches::kSyncDeviceInfoInTransportMode)) {
allowed_types.Put(syncer::DEVICE_INFO);
}
return allowed_types; return allowed_types;
} }
......
...@@ -6716,6 +6716,27 @@ ...@@ -6716,6 +6716,27 @@
] ]
} }
], ],
"UnidoOnSignIn": [
{
"platforms": [
"android",
"chromeos",
"linux",
"mac",
"windows"
],
"experiments": [
{
"name": "Enabled",
"enable_features": [
"SharingDeriveVapidKey",
"SharingUseDeviceInfo",
"SyncDeviceInfoInTransportMode"
]
}
]
}
],
"UnifiedAutoplay": [ "UnifiedAutoplay": [
{ {
"platforms": [ "platforms": [
......
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