[sync] Remove GetRegisteredDataTypes() from SyncService

The concept of registered datatypes is somewhat advanced and should not
be part of SyncService's public API.

Instead, this patch moves it to the subclass, ProfileSyncService, and
makes it private except for testing.

Change-Id: I0934a6a97009bf6589f1075ba37fdc1a52490643
Bug: 915154
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2463832Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Jérôme Lebel <jlebel@chromium.org>
Commit-Queue: Jérôme Lebel <jlebel@chromium.org>
Auto-Submit: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816511}

chrome/browser/sync/profile_sync_service_factory_unittest.cc

@@ -18,8 +18,8 @@
 #include "components/sync/base/model_type.h"
 #include "components/sync/base/sync_base_switches.h"
 #include "components/sync/driver/data_type_controller.h"
+#include "components/sync/driver/profile_sync_service.h"
 #include "components/sync/driver/sync_driver_switches.h"
-#include "components/sync/driver/sync_service.h"
 #include "content/public/test/browser_task_environment.h"
 #include "extensions/buildflags/buildflags.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -187,9 +187,9 @@ TEST_F(ProfileSyncServiceFactoryTest, DisableSyncFlag) {
 // Verify that a normal (no command line flags) PSS can be created and
 // properly initialized.
 TEST_F(ProfileSyncServiceFactoryTest, CreatePSSDefault) {
-  syncer::SyncService* pss =
-      ProfileSyncServiceFactory::GetForProfile(profile());
-  syncer::ModelTypeSet types = pss->GetRegisteredDataTypes();
+  syncer::ProfileSyncService* pss =
+      ProfileSyncServiceFactory::GetAsProfileSyncServiceForProfile(profile());
+  syncer::ModelTypeSet types = pss->GetRegisteredDataTypesForTest();
   EXPECT_EQ(DefaultDatatypesCount(), types.Size());
   CheckDefaultDatatypesInSetExcept(types, syncer::ModelTypeSet());
 
@@ -202,9 +202,9 @@ TEST_F(ProfileSyncServiceFactoryTest, CreatePSSDefault) {
 TEST_F(ProfileSyncServiceFactoryTest, CreatePSSDisableOne) {
   syncer::ModelTypeSet disabled_types(syncer::AUTOFILL);
   SetDisabledTypes(disabled_types);
-  syncer::SyncService* pss =
-      ProfileSyncServiceFactory::GetForProfile(profile());
-  syncer::ModelTypeSet types = pss->GetRegisteredDataTypes();
+  syncer::ProfileSyncService* pss =
+      ProfileSyncServiceFactory::GetAsProfileSyncServiceForProfile(profile());
+  syncer::ModelTypeSet types = pss->GetRegisteredDataTypesForTest();
   EXPECT_EQ(DefaultDatatypesCount() - disabled_types.Size(), types.Size());
   CheckDefaultDatatypesInSetExcept(types, disabled_types);
 
@@ -218,9 +218,9 @@ TEST_F(ProfileSyncServiceFactoryTest, CreatePSSDisableMultiple) {
   syncer::ModelTypeSet disabled_types(syncer::AUTOFILL_PROFILE,
                                       syncer::BOOKMARKS);
   SetDisabledTypes(disabled_types);
-  syncer::SyncService* pss =
-      ProfileSyncServiceFactory::GetForProfile(profile());
-  syncer::ModelTypeSet types = pss->GetRegisteredDataTypes();
+  syncer::ProfileSyncService* pss =
+      ProfileSyncServiceFactory::GetAsProfileSyncServiceForProfile(profile());
+  syncer::ModelTypeSet types = pss->GetRegisteredDataTypesForTest();
   EXPECT_EQ(DefaultDatatypesCount() - disabled_types.Size(), types.Size());
   CheckDefaultDatatypesInSetExcept(types, disabled_types);
 

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

@@ -127,7 +127,7 @@ class EnableDisableSingleClientTest : public SyncTest {
       ASSERT_TRUE(GetClient(0)->AwaitSyncSetupCompletion());
     }
 
-    registered_data_types_ = GetSyncService(0)->GetRegisteredDataTypes();
+    registered_data_types_ = GetSyncService(0)->GetRegisteredDataTypesForTest();
     multi_grouped_types_ = MultiGroupTypes(registered_data_types_);
     registered_selectable_types_ = GetRegisteredSelectableTypes(0);
   }

chrome/browser/ui/app_list/arc/arc_app_unittest.cc

@@ -78,7 +78,7 @@
 #include "components/services/app_service/public/cpp/app_update.h"
 #include "components/services/app_service/public/cpp/stub_icon_loader.h"
 #include "components/services/app_service/public/mojom/types.mojom.h"
-#include "components/sync/driver/sync_service.h"
+#include "components/sync/driver/profile_sync_service.h"
 #include "components/sync/model/fake_sync_change_processor.h"
 #include "components/sync/model/sync_data.h"
 #include "components/sync/model/sync_error_factory_mock.h"
@@ -3202,8 +3202,9 @@ TEST_P(ArcAppModelBuilderTest, DontRemoveRuntimeAppOnPackageChange) {
 }
 
 TEST_P(ArcAppModelBuilderTest, PackageSyncableServiceEnabled) {
-  EXPECT_TRUE(ProfileSyncServiceFactory::GetForProfile(profile_.get())
-                  ->GetRegisteredDataTypes()
+  EXPECT_TRUE(ProfileSyncServiceFactory::GetAsProfileSyncServiceForProfile(
+                  profile_.get())
+                  ->GetRegisteredDataTypesForTest()
                   .Has(syncer::ARC_PACKAGE));
 }
 
@@ -3211,8 +3212,9 @@ TEST_P(ArcAppModelBuilderTest, PackageSyncableServiceDisabled) {
   base::test::ScopedCommandLine command_line;
   command_line.GetProcessCommandLine()->AppendSwitch(
       chromeos::switches::kArcDisableAppSync);
-  EXPECT_FALSE(ProfileSyncServiceFactory::GetForProfile(profile_.get())
-                   ->GetRegisteredDataTypes()
+  EXPECT_FALSE(ProfileSyncServiceFactory::GetAsProfileSyncServiceForProfile(
+                   profile_.get())
+                   ->GetRegisteredDataTypesForTest()
                    .Has(syncer::ARC_PACKAGE));
 }
 

chrome/browser/ui/webui/settings/people_handler_unittest.cc

@@ -240,8 +240,6 @@ class PeopleHandlerTest : public ChromeRenderViewHostTestHarness {
     ON_CALL(*mock_sync_service_->GetMockUserSettings(),
             GetExplicitPassphraseTime())
         .WillByDefault(Return(base::Time()));
-    ON_CALL(*mock_sync_service_, GetRegisteredDataTypes())
-        .WillByDefault(Return(syncer::ModelTypeSet()));
     ON_CALL(*mock_sync_service_, GetSetupInProgressHandle())
         .WillByDefault(
             Return(ByMove(std::make_unique<syncer::SyncSetupInProgressHandle>(

components/sync/driver/fake_sync_service.cc

@@ -68,10 +68,6 @@ void FakeSyncService::StopAndClear() {}
 
 void FakeSyncService::OnDataTypeRequestsSyncStartup(ModelType type) {}
 
-ModelTypeSet FakeSyncService::GetRegisteredDataTypes() const {
-  return ModelTypeSet();
-}
-
 ModelTypeSet FakeSyncService::GetPreferredDataTypes() const {
   return ModelTypeSet();
 }

components/sync/driver/fake_sync_service.h

@@ -40,7 +40,6 @@ class FakeSyncService : public SyncService {
   bool HasObserver(const SyncServiceObserver* observer) const override;
   void OnDataTypeRequestsSyncStartup(ModelType type) override;
   void StopAndClear() override;
-  ModelTypeSet GetRegisteredDataTypes() const override;
   ModelTypeSet GetPreferredDataTypes() const override;
   std::unique_ptr<SyncSetupInProgressHandle> GetSetupInProgressHandle()
       override;

components/sync/driver/mock_sync_service.h

@@ -45,7 +45,6 @@ class MockSyncService : public SyncService {
                std::unique_ptr<SyncSetupInProgressHandle>());
   MOCK_CONST_METHOD0(IsSetupInProgress, bool());
 
-  MOCK_CONST_METHOD0(GetRegisteredDataTypes, ModelTypeSet());
   MOCK_CONST_METHOD0(GetPreferredDataTypes, ModelTypeSet());
   MOCK_CONST_METHOD0(GetActiveDataTypes, ModelTypeSet());
   MOCK_CONST_METHOD0(GetBackedOffDataTypes, ModelTypeSet());

components/sync/driver/profile_sync_service.cc

@@ -370,6 +370,10 @@ void ProfileSyncService::StartSyncingWithServer() {
   }
 }
 
+ModelTypeSet ProfileSyncService::GetRegisteredDataTypesForTest() const {
+  return GetRegisteredDataTypes();
+}
+
 bool ProfileSyncService::IsDataTypeControllerRunningForTest(
     ModelType type) const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -1249,18 +1253,6 @@ bool ProfileSyncService::HasObserver(
   return observers_.HasObserver(observer);
 }
 
-ModelTypeSet ProfileSyncService::GetRegisteredDataTypes() const {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  ModelTypeSet registered_types;
-  // The |data_type_controllers_| are determined by command-line flags;
-  // that's effectively what controls the values returned here.
-  for (const std::pair<const ModelType, std::unique_ptr<DataTypeController>>&
-           type_and_controller : data_type_controllers_) {
-    registered_types.Put(type_and_controller.first);
-  }
-  return registered_types;
-}
-
 ModelTypeSet ProfileSyncService::GetPreferredDataTypes() const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   return user_settings_->GetPreferredDataTypes();
@@ -1368,6 +1360,18 @@ bool ProfileSyncService::UseTransportOnlyMode() const {
   return !IsSyncFeatureEnabled() && !IsLocalSyncEnabled();
 }
 
+ModelTypeSet ProfileSyncService::GetRegisteredDataTypes() const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  ModelTypeSet registered_types;
+  // The |data_type_controllers_| are determined by command-line flags;
+  // that's effectively what controls the values returned here.
+  for (const std::pair<const ModelType, std::unique_ptr<DataTypeController>>&
+           type_and_controller : data_type_controllers_) {
+    registered_types.Put(type_and_controller.first);
+  }
+  return registered_types;
+}
+
 ModelTypeSet ProfileSyncService::GetModelTypesForTransportOnlyMode() const {
   ModelTypeSet allowed_types = {
       DEVICE_INFO,

components/sync/driver/profile_sync_service.h

@@ -122,7 +122,6 @@ class ProfileSyncService : public SyncService,
   std::unique_ptr<SyncSetupInProgressHandle> GetSetupInProgressHandle()
       override;
   bool IsSetupInProgress() const override;
-  ModelTypeSet GetRegisteredDataTypes() const override;
   ModelTypeSet GetPreferredDataTypes() const override;
   ModelTypeSet GetActiveDataTypes() const override;
   ModelTypeSet GetBackedOffDataTypes() const override;
@@ -238,6 +237,8 @@ class ProfileSyncService : public SyncService,
   void OverrideNetworkForTest(const CreateHttpPostProviderFactory&
                                   create_http_post_provider_factory_cb);
 
+  ModelTypeSet GetRegisteredDataTypesForTest() const;
+
   bool IsDataTypeControllerRunningForTest(ModelType type) const;
 
   // Some tests rely on injecting calls to the encryption observer.
@@ -294,6 +295,10 @@ class ProfileSyncService : public SyncService,
 
   bool UseTransportOnlyMode() const;
 
+  // Returns the set of data types that are supported in principle, possibly
+  // influenced by command-line options.
+  ModelTypeSet GetRegisteredDataTypes() const;
+
   // Returns the ModelTypes allowed in transport-only mode (i.e. those that are
   // not tied to sync-the-feature).
   ModelTypeSet GetModelTypesForTransportOnlyMode() const;

components/sync/driver/sync_service.h

@@ -300,10 +300,6 @@ class SyncService : public KeyedService {
   // DATA TYPE STATE
   //////////////////////////////////////////////////////////////////////////////
 
-  // Returns the set of data types that are supported in principle. These will
-  // typically only change via a command-line option.
-  virtual syncer::ModelTypeSet GetRegisteredDataTypes() const = 0;
-
   // Returns the set of types which are preferred for enabling. This is a
   // superset of the active types (see GetActiveDataTypes()). This also includes
   // any forced types.

components/sync/driver/test_sync_service.cc

@@ -198,10 +198,6 @@ bool TestSyncService::IsSetupInProgress() const {
   return setup_in_progress_;
 }
 
-ModelTypeSet TestSyncService::GetRegisteredDataTypes() const {
-  return ModelTypeSet::All();
-}
-
 ModelTypeSet TestSyncService::GetPreferredDataTypes() const {
   return preferred_data_types_;
 }

components/sync/driver/test_sync_service.h

@@ -72,7 +72,6 @@ class TestSyncService : public SyncService {
       override;
   bool IsSetupInProgress() const override;
 
-  ModelTypeSet GetRegisteredDataTypes() const override;
   ModelTypeSet GetPreferredDataTypes() const override;
   ModelTypeSet GetActiveDataTypes() const override;
   ModelTypeSet GetBackedOffDataTypes() const override;

ios/chrome/browser/sync/profile_sync_service_factory_unittest.cc

@@ -14,8 +14,8 @@
 #include "components/sync/base/model_type.h"
 #include "components/sync/base/pref_names.h"
 #include "components/sync/driver/data_type_controller.h"
+#include "components/sync/driver/profile_sync_service.h"
 #include "components/sync/driver/sync_driver_switches.h"
-#include "components/sync/driver/sync_service.h"
 #include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
 #include "ios/web/public/test/web_task_environment.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -114,9 +114,10 @@ TEST_F(ProfileSyncServiceFactoryTest, DisableSyncFlag) {
 // Verify that a normal (no command line flags) PSS can be created and
 // properly initialized.
 TEST_F(ProfileSyncServiceFactoryTest, CreatePSSDefault) {
-  syncer::SyncService* sync_service =
-      ProfileSyncServiceFactory::GetForBrowserState(chrome_browser_state());
-  syncer::ModelTypeSet types = sync_service->GetRegisteredDataTypes();
+  syncer::ProfileSyncService* sync_service =
+      ProfileSyncServiceFactory::GetAsProfileSyncServiceForBrowserState(
+          chrome_browser_state());
+  syncer::ModelTypeSet types = sync_service->GetRegisteredDataTypesForTest();
   EXPECT_EQ(DefaultDatatypesCount(), types.Size());
   CheckDefaultDatatypesInSetExcept(types, syncer::ModelTypeSet());
 }
@@ -126,9 +127,10 @@ TEST_F(ProfileSyncServiceFactoryTest, CreatePSSDefault) {
 TEST_F(ProfileSyncServiceFactoryTest, CreatePSSDisableOne) {
   syncer::ModelTypeSet disabled_types(syncer::AUTOFILL);
   SetDisabledTypes(disabled_types);
-  syncer::SyncService* sync_service =
-      ProfileSyncServiceFactory::GetForBrowserState(chrome_browser_state());
-  syncer::ModelTypeSet types = sync_service->GetRegisteredDataTypes();
+  syncer::ProfileSyncService* sync_service =
+      ProfileSyncServiceFactory::GetAsProfileSyncServiceForBrowserState(
+          chrome_browser_state());
+  syncer::ModelTypeSet types = sync_service->GetRegisteredDataTypesForTest();
   EXPECT_EQ(DefaultDatatypesCount() - disabled_types.Size(), types.Size());
   CheckDefaultDatatypesInSetExcept(types, disabled_types);
 }
@@ -139,9 +141,10 @@ TEST_F(ProfileSyncServiceFactoryTest, CreatePSSDisableMultiple) {
   syncer::ModelTypeSet disabled_types(syncer::AUTOFILL_PROFILE,
                                       syncer::BOOKMARKS);
   SetDisabledTypes(disabled_types);
-  syncer::SyncService* sync_service =
-      ProfileSyncServiceFactory::GetForBrowserState(chrome_browser_state());
-  syncer::ModelTypeSet types = sync_service->GetRegisteredDataTypes();
+  syncer::ProfileSyncService* sync_service =
+      ProfileSyncServiceFactory::GetAsProfileSyncServiceForBrowserState(
+          chrome_browser_state());
+  syncer::ModelTypeSet types = sync_service->GetRegisteredDataTypesForTest();
   EXPECT_EQ(DefaultDatatypesCount() - disabled_types.Size(), types.Size());
   CheckDefaultDatatypesInSetExcept(types, disabled_types);
 }

ios/chrome/browser/ui/settings/passphrase_table_view_controller_test.mm

@@ -89,8 +89,6 @@ void PassphraseTableViewControllerTest::SetUp() {
       ProfileSyncServiceFactory::GetInstance()->SetTestingFactoryAndUse(
           chrome_browser_state_.get(),
           base::BindRepeating(&CreateNiceMockSyncService)));
-  ON_CALL(*fake_sync_service_, GetRegisteredDataTypes())
-      .WillByDefault(Return(syncer::ModelTypeSet()));
 
   // Set up non-default return values for our sync service mock.
   ON_CALL(*fake_sync_service_->GetMockUserSettings(), IsPassphraseRequired())