[SyncInvalidations] Send data types from ProfileSyncService to SyncInvalidationsService.

Bug: 1102312
Change-Id: If7bb0c7cbfc4338561ac83bc6d507680cab87b14
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2379735
Commit-Queue: Paula Vidas <paulavidas@google.com>
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Rushan Suleymanov <rushans@google.com>
Cr-Commit-Position: refs/heads/master@{#803442}

components/sync/driver/profile_sync_service.cc

@@ -21,6 +21,7 @@
 #include "base/task/post_task.h"
 #include "base/task/thread_pool.h"
 #include "base/time/time.h"
+#include "build/build_config.h"
 #include "components/invalidation/public/invalidation_service.h"
 #include "components/signin/public/base/signin_metrics.h"
 #include "components/signin/public/identity_manager/account_info.h"
@@ -45,6 +46,7 @@
 #include "components/sync/engine/net/http_post_provider_factory.h"
 #include "components/sync/engine/polling_constants.h"
 #include "components/sync/engine/sync_encryption_handler.h"
+#include "components/sync/invalidations/sync_invalidations_service.h"
 #include "components/sync/model/sync_error.h"
 #include "components/version_info/version_info_values.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
@@ -252,7 +254,12 @@ ProfileSyncService::ProfileSyncService(InitParams init_params)
       start_behavior_(init_params.start_behavior),
       passphrase_prompt_triggered_by_version_(false),
       is_stopping_and_clearing_(false),
-      should_record_trusted_vault_error_shown_on_startup_(true) {
+      should_record_trusted_vault_error_shown_on_startup_(true),
+#if defined(OS_ANDROID)
+      sessions_invalidations_enabled_(false) {
+#else
+      sessions_invalidations_enabled_(true) {
+#endif
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(sync_client_);
   DCHECK(IsLocalSyncEnabled() || identity_manager_ != nullptr);
@@ -1337,18 +1344,14 @@ void ProfileSyncService::ConfigureDataTypeManager(ConfigureReason reason) {
   DCHECK(!configure_context.cache_guid.empty());
   DCHECK_NE(configure_context.reason, CONFIGURE_REASON_UNKNOWN);
 
-  // Note: When local Sync is enabled, then we want full-sync mode (not just
-  // transport), even though Sync-the-feature is not considered enabled.
-  bool use_transport_only_mode =
-      !IsSyncFeatureEnabled() && !IsLocalSyncEnabled();
+  const bool use_transport_only_mode = UseTransportOnlyMode();
 
-  ModelTypeSet types = GetPreferredDataTypes();
-  // In transport-only mode, only a subset of data types is supported.
   if (use_transport_only_mode) {
-    types = Intersection(types, GetModelTypesForTransportOnlyMode());
     configure_context.sync_mode = SyncMode::kTransportOnly;
   }
-  data_type_manager_->Configure(types, configure_context);
+  data_type_manager_->Configure(GetDataTypesToConfigure(), configure_context);
+
+  UpdateDataTypesForInvalidations();
 
   // Record in UMA whether we're configuring the full Sync feature or only the
   // transport.
@@ -1378,6 +1381,12 @@ void ProfileSyncService::ConfigureDataTypeManager(ConfigureReason reason) {
   }
 }
 
+bool ProfileSyncService::UseTransportOnlyMode() const {
+  // Note: When local Sync is enabled, then we want full-sync mode (not just
+  // transport), even though Sync-the-feature is not considered enabled.
+  return !IsSyncFeatureEnabled() && !IsLocalSyncEnabled();
+}
+
 ModelTypeSet ProfileSyncService::GetModelTypesForTransportOnlyMode() const {
   ModelTypeSet allowed_types = {
       SECURITY_EVENTS,
@@ -1421,6 +1430,30 @@ ModelTypeSet ProfileSyncService::GetModelTypesForTransportOnlyMode() const {
   return allowed_types;
 }
 
+ModelTypeSet ProfileSyncService::GetDataTypesToConfigure() const {
+  ModelTypeSet types = GetPreferredDataTypes();
+  // In transport-only mode, only a subset of data types is supported.
+  if (UseTransportOnlyMode()) {
+    types = Intersection(types, GetModelTypesForTransportOnlyMode());
+  }
+  return types;
+}
+
+void ProfileSyncService::UpdateDataTypesForInvalidations() {
+  SyncInvalidationsService* invalidations_service =
+      sync_client_->GetSyncInvalidationsService();
+  if (!invalidations_service) {
+    return;
+  }
+
+  // No need to register invalidations for commit-only types.
+  ModelTypeSet types = Difference(GetDataTypesToConfigure(), CommitOnlyTypes());
+  if (!sessions_invalidations_enabled_) {
+    types.Remove(SESSIONS);
+  }
+  invalidations_service->SetSubscribedDataTypes(types);
+}
+
 SyncCycleSnapshot ProfileSyncService::GetLastCycleSnapshotForDebugging() const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   return last_snapshot_;
@@ -1781,6 +1814,9 @@ void ProfileSyncService::SetInvalidationsForSessionsEnabled(bool enabled) {
   if (engine_ && engine_->IsInitialized()) {
     engine_->SetInvalidationsForSessionsEnabled(enabled);
   }
+
+  sessions_invalidations_enabled_ = enabled;
+  UpdateDataTypesForInvalidations();
 }
 
 void ProfileSyncService::AddTrustedVaultDecryptionKeysFromWeb(

components/sync/driver/profile_sync_service.h

@@ -291,10 +291,19 @@ class ProfileSyncService : public SyncService,
   // Helper to install and configure a data type manager.
   void ConfigureDataTypeManager(ConfigureReason reason);
 
+  bool UseTransportOnlyMode() const;
+
   // Returns the ModelTypes allowed in transport-only mode (i.e. those that are
   // not tied to sync-the-feature).
   ModelTypeSet GetModelTypesForTransportOnlyMode() const;
 
+  // If in transport-only mode, returns only preferred data types which are
+  // allowed in transport-only mode. Otherwise, returns all preferred data
+  // types.
+  ModelTypeSet GetDataTypesToConfigure() const;
+
+  void UpdateDataTypesForInvalidations();
+
   // Shuts down the engine sync components.
   // |reason| dictates if syncing is being disabled or not.
   void ShutdownImpl(ShutdownReason reason);
@@ -485,6 +494,11 @@ class ProfileSyncService : public SyncService,
   // recorded or trusted vault passphrase type wasn't used on startup.
   bool should_record_trusted_vault_error_shown_on_startup_;
 
+  // Whether we want to receive invalidations for the SESSIONS data type. This
+  // is typically false on Android (to save network traffic), but true on all
+  // other platforms.
+  bool sessions_invalidations_enabled_;
+
   // This weak factory invalidates its issued pointers when Sync is disabled.
   base::WeakPtrFactory<ProfileSyncService> sync_enabled_weak_factory_{this};
 

components/sync/driver/profile_sync_service_bundle.cc

@@ -37,6 +37,8 @@ ProfileSyncServiceBundle::CreateSyncClientMock() {
   ON_CALL(*sync_client, GetPrefService()).WillByDefault(Return(&pref_service_));
   ON_CALL(*sync_client, GetSyncApiComponentFactory())
       .WillByDefault(Return(&component_factory_));
+  ON_CALL(*sync_client, GetSyncInvalidationsService())
+      .WillByDefault(Return(sync_invalidations_service()));
   // Used by control types.
   ON_CALL(*sync_client, CreateModelWorkerForGroup(GROUP_PASSIVE))
       .WillByDefault(Return(base::MakeRefCounted<PassiveModelWorker>()));

components/sync/driver/profile_sync_service_bundle.h

@@ -15,6 +15,8 @@
 #include "components/sync/driver/profile_sync_service.h"
 #include "components/sync/driver/sync_api_component_factory_mock.h"
 #include "components/sync/driver/sync_client_mock.h"
+#include "components/sync/invalidations/mock_sync_invalidations_service.h"
+#include "components/sync/invalidations/switches.h"
 #include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "services/network/test/test_url_loader_factory.h"
 
@@ -61,12 +63,22 @@ class ProfileSyncServiceBundle {
     return identity_provider_.get();
   }
 
+  MockSyncInvalidationsService* sync_invalidations_service() {
+    if (base::FeatureList::IsEnabled(
+            switches::kSubscribeForSyncInvalidations)) {
+      return &sync_invalidations_service_;
+    } else {
+      return nullptr;
+    }
+  }
+
  private:
   sync_preferences::TestingPrefServiceSyncable pref_service_;
   network::TestURLLoaderFactory test_url_loader_factory_;
   signin::IdentityTestEnvironment identity_test_env_;
   testing::NiceMock<SyncApiComponentFactoryMock> component_factory_;
   std::unique_ptr<invalidation::ProfileIdentityProvider> identity_provider_;
+  testing::NiceMock<MockSyncInvalidationsService> sync_invalidations_service_;
 
   DISALLOW_COPY_AND_ASSIGN(ProfileSyncServiceBundle);
 };

components/sync/driver/profile_sync_service_unittest.cc

@@ -35,6 +35,7 @@
 #include "components/sync/driver/sync_service_observer.h"
 #include "components/sync/driver/sync_token_status.h"
 #include "components/sync/engine/fake_sync_engine.h"
+#include "components/sync/invalidations/switches.h"
 #include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "components/version_info/version_info_values.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -43,6 +44,7 @@
 
 using testing::_;
 using testing::ByMove;
+using testing::Not;
 using testing::Return;
 
 namespace syncer {
@@ -177,14 +179,16 @@ class ProfileSyncServiceTest : public ::testing::Test {
 
   void SignIn() { identity_test_env()->MakePrimaryAccountAvailable(kTestUser); }
 
-  void CreateService(ProfileSyncService::StartBehavior behavior) {
+  void CreateService(ProfileSyncService::StartBehavior behavior,
+                     ModelTypeSet registered_types =
+                         ModelTypeSet(BOOKMARKS, SUPERVISED_USER_SETTINGS)) {
     DCHECK(!service_);
 
-    // Include a regular controller and a transport-mode controller.
+    // Default includes a regular controller and a transport-mode controller.
     DataTypeController::TypeVector controllers;
-    controllers.push_back(std::make_unique<FakeDataTypeController>(BOOKMARKS));
-    controllers.push_back(
-        std::make_unique<FakeDataTypeController>(SUPERVISED_USER_SETTINGS));
+    for (const ModelType type : registered_types) {
+      controllers.push_back(std::make_unique<FakeDataTypeController>(type));
+    }
 
     std::unique_ptr<SyncClientMock> sync_client =
         profile_sync_service_bundle_.CreateSyncClientMock();
@@ -308,6 +312,10 @@ class ProfileSyncServiceTest : public ::testing::Test {
     return profile_sync_service_bundle_.component_factory();
   }
 
+  MockSyncInvalidationsService* sync_invalidations_service() {
+    return profile_sync_service_bundle_.sync_invalidations_service();
+  }
+
   void SetDemographics(int birth_year,
                        metrics::UserDemographicsProto_Gender gender) {
     base::DictionaryValue dict;
@@ -352,6 +360,20 @@ class ProfileSyncServiceTestWithStopSyncInPausedState
   base::test::ScopedFeatureList override_features_;
 };
 
+class ProfileSyncServiceTestWithSubscribeForSyncInvalidations
+    : public ProfileSyncServiceTest {
+ public:
+  ProfileSyncServiceTestWithSubscribeForSyncInvalidations() {
+    override_features_.InitAndEnableFeature(
+        switches::kSubscribeForSyncInvalidations);
+  }
+
+  ~ProfileSyncServiceTestWithSubscribeForSyncInvalidations() override = default;
+
+ private:
+  base::test::ScopedFeatureList override_features_;
+};
+
 // Gets the now time used for testing user demographics.
 base::Time GetNowTime() {
   base::Time now;
@@ -1599,5 +1621,32 @@ TEST_F(ProfileSyncServiceTest,
   EXPECT_EQ(signin::GetTestGaiaIdForEmail(kTestUser), sync_prefs.GetGaiaId());
 }
 
+TEST_F(ProfileSyncServiceTestWithSubscribeForSyncInvalidations,
+       ShouldSendDataTypesToSyncInvalidationsService) {
+  CreateService(ProfileSyncService::AUTO_START);
+  SignIn();
+  EXPECT_CALL(*sync_invalidations_service(), SetSubscribedDataTypes(_));
+  InitializeForFirstSync();
+}
+
+MATCHER(ContainsSessions, "") {
+  return arg.Has(SESSIONS);
+}
+
+TEST_F(ProfileSyncServiceTestWithSubscribeForSyncInvalidations,
+       ShouldEnableAndDisableInvalidationsForSessions) {
+  CreateService(ProfileSyncService::AUTO_START,
+                ModelTypeSet(SESSIONS, TYPED_URLS));
+  SignIn();
+  InitializeForNthSync();
+
+  EXPECT_CALL(*sync_invalidations_service(),
+              SetSubscribedDataTypes(ContainsSessions()));
+  service()->SetInvalidationsForSessionsEnabled(true);
+  EXPECT_CALL(*sync_invalidations_service(),
+              SetSubscribedDataTypes(Not(ContainsSessions())));
+  service()->SetInvalidationsForSessionsEnabled(false);
+}
+
 }  // namespace
 }  // namespace syncer