Fix sync_integration_tests with SplitSettingsSync enabled

Enable OS sync in SyncTest, because the feature will be on by
default, and because several ModelTypes become OS-specific
(e.g. APPS, APP_LIST).

Adjust a few test expectations as well.

Bug: 1060289

Change-Id: Iee32868b4a2d1ac13b9e63787c2d5e140aa98139
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2103651
Commit-Queue: James Cook <jamescook@chromium.org>
Auto-Submit: James Cook <jamescook@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751424}

chrome/browser/sync/test/integration/os_sync_test.cc

@@ -4,11 +4,7 @@
 
 #include "chrome/browser/sync/test/integration/os_sync_test.h"
 
-#include "chrome/browser/profiles/profile.h"
 #include "chromeos/constants/chromeos_features.h"
-#include "components/prefs/pref_service.h"
-#include "components/sync/base/pref_names.h"
-#include "testing/gtest/include/gtest/gtest.h"
 
 OsSyncTest::OsSyncTest(TestType type) : SyncTest(type) {
   settings_feature_list_.InitAndEnableFeature(
@@ -16,15 +12,3 @@ OsSyncTest::OsSyncTest(TestType type) : SyncTest(type) {
 }
 
 OsSyncTest::~OsSyncTest() = default;
-
-bool OsSyncTest::SetupClients() {
-  if (!SyncTest::SetupClients())
-    return false;
-  // Enable the OS sync feature for all profiles.
-  for (Profile* profile : GetAllProfiles()) {
-    PrefService* prefs = profile->GetPrefs();
-    EXPECT_FALSE(prefs->GetBoolean(syncer::prefs::kOsSyncFeatureEnabled));
-    prefs->SetBoolean(syncer::prefs::kOsSyncFeatureEnabled, true);
-  }
-  return true;
-}

chrome/browser/sync/test/integration/os_sync_test.h

@@ -8,8 +8,9 @@
 #include "base/test/scoped_feature_list.h"
 #include "chrome/browser/sync/test/integration/sync_test.h"
 
-// Test suite for Chrome OS sync. Enables the SplitSettingsSync feature and the
-// OS sync enabled preference before running the test.
+// Test suite for Chrome OS sync. Enables the SplitSettingsSync feature.
+// TODO(jamescook): When SplitSettingsSync is on-by-default this class can be
+// deleted.
 class OsSyncTest : public SyncTest {
  public:
   explicit OsSyncTest(TestType type);
@@ -18,9 +19,6 @@ class OsSyncTest : public SyncTest {
   OsSyncTest(const OsSyncTest&) = delete;
   OsSyncTest& operator=(const OsSyncTest&) = delete;
 
-  // SyncTest:
-  bool SetupClients() override;
-
  private:
   // The names |scoped_feature_list_| and |feature_list_| are both used in
   // superclasses.

chrome/browser/sync/test/integration/single_client_app_list_sync_test.cc

@@ -18,6 +18,7 @@
 #include "chrome/browser/ui/app_list/app_list_syncable_service_factory.h"
 #include "chrome/browser/ui/app_list/internal_app/internal_app_metadata.h"
 #include "chrome/browser/ui/app_list/page_break_constants.h"
+#include "chromeos/constants/chromeos_features.h"
 #include "components/sync/base/user_selectable_type.h"
 #include "components/sync/driver/sync_service.h"
 #include "components/sync/driver/sync_user_settings.h"
@@ -195,9 +196,14 @@ IN_PROC_BROWSER_TEST_F(SingleClientAppListSyncTest, LocalStorage) {
 
   ASSERT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait());
 
-  // Disable app sync.
-  sync_service->GetUserSettings()->SetSelectedTypes(
-      false, syncer::UserSelectableTypeSet());
+  // Disable app sync by disabling all user-selectable types.
+  if (chromeos::features::IsSplitSettingsSyncEnabled()) {
+    sync_service->GetUserSettings()->SetSelectedOsTypes(
+        /*sync_all_os_types=*/false, syncer::UserSelectableOsTypeSet());
+  } else {
+    sync_service->GetUserSettings()->SetSelectedTypes(
+        /*sync_everything=*/false, syncer::UserSelectableTypeSet());
+  }
 
   // Change data when sync is off.
   for (const auto& app_id : app_ids) {
@@ -211,9 +217,14 @@ IN_PROC_BROWSER_TEST_F(SingleClientAppListSyncTest, LocalStorage) {
 
   EXPECT_FALSE(SyncItemsMatch(service, &compare_service));
 
-  // Restore sync and sync data should override local changes.
-  sync_service->GetUserSettings()->SetSelectedTypes(
-      true, syncer::UserSelectableTypeSet());
+  // Restore app sync and sync data should override local changes.
+  if (chromeos::features::IsSplitSettingsSyncEnabled()) {
+    sync_service->GetUserSettings()->SetSelectedOsTypes(
+        /*sync_all_os_types=*/true, syncer::UserSelectableOsTypeSet());
+  } else {
+    sync_service->GetUserSettings()->SetSelectedTypes(
+        /*sync_everything=*/true, syncer::UserSelectableTypeSet());
+  }
   EXPECT_TRUE(AppListSyncUpdateWaiter(service).Wait());
   EXPECT_TRUE(SyncItemsMatch(service, &compare_service));
 }

chrome/browser/sync/test/integration/single_client_standalone_transport_sync_test.cc

@@ -29,6 +29,8 @@
 namespace {
 
 syncer::ModelTypeSet AllowedTypesInStandaloneTransportMode() {
+  static_assert(41 == syncer::ModelType::NUM_ENTRIES,
+                "Add new types below if they run in transport mode");
   // Only some special whitelisted types (and control types) are allowed in
   // standalone transport mode.
   syncer::ModelTypeSet allowed_types(
@@ -38,6 +40,18 @@ syncer::ModelTypeSet AllowedTypesInStandaloneTransportMode() {
   if (base::FeatureList::IsEnabled(switches::kSyncDeviceInfoInTransportMode)) {
     allowed_types.Put(syncer::DEVICE_INFO);
   }
+#if defined(OS_CHROMEOS)
+  // OS sync types run in transport mode.
+  if (chromeos::features::IsSplitSettingsSyncEnabled()) {
+    allowed_types.PutAll({syncer::APPS, syncer::APP_SETTINGS, syncer::APP_LIST,
+                          syncer::APP_SETTINGS, syncer::ARC_PACKAGE,
+                          syncer::PRINTERS, syncer::OS_PREFERENCES,
+                          syncer::OS_PRIORITY_PREFERENCES});
+  }
+  if (base::FeatureList::IsEnabled(switches::kSyncWifiConfigurations)) {
+    allowed_types.Put(syncer::WIFI_CONFIGURATIONS);
+  }
+#endif  // defined(OS_CHROMEOS)
   return allowed_types;
 }
 

chrome/browser/sync/test/integration/sync_test.cc

@@ -85,6 +85,7 @@
 #include "chrome/browser/ui/app_list/test/fake_app_list_model_updater.h"
 #include "chromeos/components/account_manager/account_manager.h"
 #include "chromeos/components/account_manager/account_manager_factory.h"
+#include "chromeos/constants/chromeos_features.h"
 #include "chromeos/constants/chromeos_switches.h"
 #include "components/arc/arc_util.h"
 #endif  // defined(OS_CHROMEOS)
@@ -601,6 +602,14 @@ bool SyncTest::SetupClients() {
   }
 
 #if defined(OS_CHROMEOS)
+  // SplitSettingsSync makes several types (e.g. APPS, APP_LIST, PRINTERS) into
+  // OS sync types. OS sync is on-by-default, so enable it here.
+  if (chromeos::features::IsSplitSettingsSyncEnabled()) {
+    for (int i = 0; i < num_clients(); ++i) {
+      GetSyncService(i)->GetUserSettings()->SetOsSyncFeatureEnabled(true);
+    }
+  }
+
   if (ArcAppListPrefsFactory::IsFactorySetForSyncTest()) {
     // Init SyncArcPackageHelper to ensure that the arc services are initialized
     // for each Profile, only can be called after test profiles are created.

chrome/browser/sync/test/integration/two_client_app_list_sync_test.cc

@@ -21,6 +21,8 @@
 #include "chromeos/constants/chromeos_features.h"
 #include "components/prefs/pref_service.h"
 #include "components/sync/base/pref_names.h"
+#include "components/sync/base/user_selectable_type.h"
+#include "components/sync/driver/sync_user_settings.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/test/test_utils.h"
 #include "extensions/browser/extension_prefs.h"
@@ -37,6 +39,11 @@ using apps_helper::IsAppEnabled;
 using apps_helper::IsIncognitoEnabled;
 using apps_helper::UninstallApp;
 using apps_helper::WaitForAppService;
+using syncer::SyncUserSettings;
+using syncer::UserSelectableOsType;
+using syncer::UserSelectableOsTypeSet;
+using syncer::UserSelectableType;
+using syncer::UserSelectableTypeSet;
 
 namespace {
 
@@ -331,16 +338,31 @@ IN_PROC_BROWSER_TEST_F(TwoClientAppListSyncTest, DisableApps) {
   ASSERT_TRUE(SetupSync());
   ASSERT_TRUE(AllProfilesHaveSameAppList());
 
-  // Disable APP_LIST by disabling kApps since APP_LIST is in kApps groups.
-  ASSERT_TRUE(
-      GetClient(1)->DisableSyncForType(syncer::UserSelectableType::kApps));
+  // Disable APP_LIST by disabling apps sync.
+  SyncUserSettings* settings = GetClient(1)->service()->GetUserSettings();
+  if (chromeos::features::IsSplitSettingsSyncEnabled()) {
+    UserSelectableOsTypeSet types = settings->GetSelectedOsTypes();
+    types.Remove(UserSelectableOsType::kOsApps);
+    settings->SetSelectedOsTypes(/*sync_all_os_types=*/false, types);
+  } else {
+    UserSelectableTypeSet types = settings->GetSelectedTypes();
+    types.Remove(UserSelectableType::kApps);
+    settings->SetSelectedTypes(/*sync_everything=*/false, types);
+  }
   InstallHostedApp(GetProfile(0), 0);
   ASSERT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait());
   ASSERT_FALSE(AllProfilesHaveSameAppList());
 
-  // Enable APP_LIST by enabling kApps since APP_LIST is in kApps groups.
-  ASSERT_TRUE(
-      GetClient(1)->EnableSyncForType(syncer::UserSelectableType::kApps));
+  // Enable APP_LIST by enabling apps sync.
+  if (chromeos::features::IsSplitSettingsSyncEnabled()) {
+    UserSelectableOsTypeSet types = settings->GetSelectedOsTypes();
+    types.Put(UserSelectableOsType::kOsApps);
+    settings->SetSelectedOsTypes(/*sync_all_os_types=*/false, types);
+  } else {
+    UserSelectableTypeSet types = settings->GetSelectedTypes();
+    types.Put(UserSelectableType::kApps);
+    settings->SetSelectedTypes(/*sync_everything=*/false, types);
+  }
   AwaitQuiescenceAndInstallAppsPendingForSync();
 
   ASSERT_TRUE(AllProfilesHaveSameAppList());