[power bookmarks] Hook up sync to power_bookmarks

Bug: 1376612
Change-Id: I6acce22b80560208c8caa2fffd2c1edce23b0ffb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4175972
Reviewed-by: Rushan Suleymanov <rushans@google.com>
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Reviewed-by: Sky Malice <skym@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1097005}

chrome/browser/sync/chrome_sync_client.cc

@@ -22,6 +22,7 @@
 #include "chrome/browser/invalidation/profile_invalidation_provider_factory.h"
 #include "chrome/browser/password_manager/account_password_store_factory.h"
 #include "chrome/browser/password_manager/password_store_factory.h"
+#include "chrome/browser/power_bookmarks/power_bookmark_service_factory.h"
 #include "chrome/browser/prefs/pref_service_syncable_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/reading_list/reading_list_model_factory.h"
@@ -265,7 +266,8 @@ ChromeSyncClient::ChromeSyncClient(Profile* profile)
       web_data_service_thread_, profile_web_data_service_,
       account_web_data_service_, profile_password_store_,
       account_password_store_,
-      BookmarkSyncServiceFactory::GetForProfile(profile_));
+      BookmarkSyncServiceFactory::GetForProfile(profile_),
+      PowerBookmarkServiceFactory::GetForBrowserContext(profile_));
 
   signin::IdentityManager* identity_manager =
       IdentityManagerFactory::GetForProfile(profile_);

chrome/browser/sync/sync_service_factory.cc

@@ -22,6 +22,7 @@
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/password_manager/account_password_store_factory.h"
 #include "chrome/browser/password_manager/password_store_factory.h"
+#include "chrome/browser/power_bookmarks/power_bookmark_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
@@ -231,13 +232,14 @@ SyncServiceFactory::SyncServiceFactory()
   DependsOn(gcm::GCMProfileServiceFactory::GetInstance());
   DependsOn(HistoryServiceFactory::GetInstance());
   DependsOn(IdentityManagerFactory::GetInstance());
-  DependsOn(SyncInvalidationsServiceFactory::GetInstance());
   DependsOn(ModelTypeStoreServiceFactory::GetInstance());
   DependsOn(PasswordStoreFactory::GetInstance());
+  DependsOn(PowerBookmarkServiceFactory::GetInstance());
   DependsOn(SecurityEventRecorderFactory::GetInstance());
   DependsOn(SendTabToSelfSyncServiceFactory::GetInstance());
   DependsOn(SharingMessageBridgeFactory::GetInstance());
   DependsOn(SpellcheckServiceFactory::GetInstance());
+  DependsOn(SyncInvalidationsServiceFactory::GetInstance());
 #if BUILDFLAG(ENABLE_SUPERVISED_USERS)
   DependsOn(SupervisedUserServiceFactory::GetInstance());
   DependsOn(SupervisedUserSettingsServiceFactory::GetInstance());

chrome/browser/sync/test/integration/DEPS

@@ -1,4 +1,10 @@
 specific_include_rules = {
+  "ash_lacros_sync_test.cc": [
+    "+components/power_bookmarks",
+  ],
+  "local_sync_test.cc": [
+    "+components/power_bookmarks",
+  ],
   "two_client_web_apps_integration_test_base.h": [
     "+chrome/browser/ui/views",
   ],

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

@@ -5,6 +5,7 @@
 #include "ash/constants/ash_features.h"
 #include "chrome/browser/ash/crosapi/browser_util.h"
 #include "chrome/browser/sync/test/integration/sync_test.h"
+#include "components/power_bookmarks/core/power_bookmark_features.h"
 #include "components/sync/base/features.h"
 #include "components/sync/driver/sync_service.h"
 #include "components/sync/driver/sync_service_impl.h"
@@ -103,6 +104,10 @@ IN_PROC_BROWSER_TEST_F(LacrosPrimaryAshSyncTest, AshSyncsAllTypes) {
     expected_active_types.Put(syncer::TYPED_URLS);
   }
 
+  if (base::FeatureList::IsEnabled(power_bookmarks::kPowerBookmarkBackend)) {
+    expected_active_types.Put(syncer::POWER_BOOKMARK);
+  }
+
   // All of the model types, both browser and OS, should be active.
   EXPECT_EQ(sync_service->GetActiveDataTypes(), expected_active_types);
 }

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

@@ -18,6 +18,7 @@
 #include "chrome/browser/ui/ui_features.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "components/browser_sync/browser_sync_switches.h"
+#include "components/power_bookmarks/core/power_bookmark_features.h"
 #include "components/sync/base/command_line_switches.h"
 #include "components/sync/base/features.h"
 #include "components/sync/base/model_type.h"
@@ -115,6 +116,10 @@ IN_PROC_BROWSER_TEST_F(LocalSyncTest, ShouldStart) {
     expected_active_data_types.Put(syncer::SAVED_TAB_GROUP);
   }
 
+  if (base::FeatureList::IsEnabled(power_bookmarks::kPowerBookmarkBackend)) {
+    expected_active_data_types.Put(syncer::POWER_BOOKMARK);
+  }
+
   // The dictionary is currently only synced on Windows, Linux, and Lacros.
   // TODO(crbug.com/1052397): Reassess whether the following block needs to be
   // included in lacros-chrome once build flag switch of lacros-chrome is

chrome/test/BUILD.gn

@@ -10194,6 +10194,7 @@ source_set("sync_integration_test_support") {
     "//chrome/browser",
     "//chrome/browser/favicon",
     "//components/autofill/content/browser",
+    "//components/power_bookmarks/core:features",
   ]
   deps = [
     "//base",

components/browser_sync/BUILD.gn

@@ -28,6 +28,8 @@ static_library("browser_sync") {
     "//components/history/core/browser",
     "//components/history/core/common",
     "//components/password_manager/core/browser",
+    "//components/power_bookmarks/core",
+    "//components/power_bookmarks/core:features",
     "//components/prefs",
     "//components/reading_list/core",
     "//components/reading_list/features:flags",

components/browser_sync/DEPS

@@ -8,6 +8,7 @@ include_rules = [
   "+components/history/core/common",
   "+components/keyed_service/core",
   "+components/password_manager/core/browser",
+  "+components/power_bookmarks/core",
   "+components/prefs",
   "+components/reading_list/core",
   "+components/reading_list/features",

components/browser_sync/sync_api_component_factory_impl.cc

@@ -32,6 +32,8 @@
 #include "components/history/core/common/pref_names.h"
 #include "components/password_manager/core/browser/password_store_interface.h"
 #include "components/password_manager/core/browser/sync/password_model_type_controller.h"
+#include "components/power_bookmarks/core/power_bookmark_features.h"
+#include "components/power_bookmarks/core/power_bookmark_service.h"
 #include "components/prefs/pref_service.h"
 #include "components/reading_list/core/reading_list_model.h"
 #include "components/reading_list/features/reading_list_switches.h"
@@ -156,7 +158,8 @@ SyncApiComponentFactoryImpl::SyncApiComponentFactoryImpl(
         profile_password_store,
     const scoped_refptr<password_manager::PasswordStoreInterface>&
         account_password_store,
-    sync_bookmarks::BookmarkSyncService* bookmark_sync_service)
+    sync_bookmarks::BookmarkSyncService* bookmark_sync_service,
+    power_bookmarks::PowerBookmarkService* power_bookmark_service)
     : sync_client_(sync_client),
       channel_(channel),
       ui_thread_(ui_thread),
@@ -169,7 +172,8 @@ SyncApiComponentFactoryImpl::SyncApiComponentFactoryImpl(
       web_data_service_in_memory_(web_data_service_in_memory),
       profile_password_store_(profile_password_store),
       account_password_store_(account_password_store),
-      bookmark_sync_service_(bookmark_sync_service) {
+      bookmark_sync_service_(bookmark_sync_service),
+      power_bookmark_service_(power_bookmark_service) {
   DCHECK(sync_client_);
 }
 
@@ -299,6 +303,14 @@ SyncApiComponentFactoryImpl::CreateCommonDataTypeControllers(
                   ->GetBookmarkSyncControllerDelegate(favicon_service)
                   .get())));
     }
+
+    if (!disabled_types.Has(syncer::POWER_BOOKMARK) &&
+        power_bookmark_service_ &&
+        base::FeatureList::IsEnabled(power_bookmarks::kPowerBookmarkBackend)) {
+      controllers.push_back(std::make_unique<ModelTypeController>(
+          syncer::POWER_BOOKMARK,
+          power_bookmark_service_->CreateSyncControllerDelegate()));
+    }
   }
 
   // TypedUrl sync is enabled by default.  Register unless explicitly disabled.

components/browser_sync/sync_api_component_factory_impl.h

@@ -35,6 +35,10 @@ namespace sync_bookmarks {
 class BookmarkSyncService;
 }
 
+namespace power_bookmarks {
+class PowerBookmarkService;
+}
+
 namespace browser_sync {
 
 class BrowserSyncClient;
@@ -54,7 +58,8 @@ class SyncApiComponentFactoryImpl : public syncer::SyncApiComponentFactory {
           profile_password_store,
       const scoped_refptr<password_manager::PasswordStoreInterface>&
           account_password_store,
-      sync_bookmarks::BookmarkSyncService* bookmark_sync_service);
+      sync_bookmarks::BookmarkSyncService* bookmark_sync_service,
+      power_bookmarks::PowerBookmarkService* power_bookmark_service);
   SyncApiComponentFactoryImpl(const SyncApiComponentFactoryImpl&) = delete;
   SyncApiComponentFactoryImpl& operator=(const SyncApiComponentFactoryImpl&) =
       delete;
@@ -120,6 +125,7 @@ class SyncApiComponentFactoryImpl : public syncer::SyncApiComponentFactory {
   const scoped_refptr<password_manager::PasswordStoreInterface>
       account_password_store_;
   const raw_ptr<sync_bookmarks::BookmarkSyncService> bookmark_sync_service_;
+  const raw_ptr<power_bookmarks::PowerBookmarkService> power_bookmark_service_;
 };
 
 }  // namespace browser_sync

components/power_bookmarks/core/BUILD.gn

@@ -32,6 +32,7 @@ static_library("core") {
     "//components/keyed_service/core:core",
     "//components/power_bookmarks/common",
     "//components/power_bookmarks/storage",
+    "//components/sync/model:model",
     "//ui/base",
     "//url",
   ]

components/power_bookmarks/core/DEPS

@@ -2,5 +2,6 @@ include_rules = [
   "+components/bookmarks",
   "+components/keyed_service",
   "+components/sync/protocol",
+  "+components/sync/model",
   "+ui/base",
 ]

components/power_bookmarks/core/power_bookmark_service.cc

@@ -16,6 +16,7 @@
 #include "components/power_bookmarks/core/power_bookmark_utils.h"
 #include "components/power_bookmarks/core/proto/power_bookmark_meta.pb.h"
 #include "components/power_bookmarks/storage/power_bookmark_backend.h"
+#include "components/sync/model/proxy_model_type_controller_delegate.h"
 #include "components/sync/protocol/power_bookmark_specifics.pb.h"
 
 using bookmarks::BookmarkModel;
@@ -32,54 +33,76 @@ PowerBookmarkService::PowerBookmarkService(
   if (model_)
     model_->AddObserver(this);
 
-  backend_ = base::SequenceBound<PowerBookmarkBackend>(
-      backend_task_runner_, database_dir, frontend_task_runner, this);
-  backend_.AsyncCall(&PowerBookmarkBackend::Init)
-      .WithArgs(base::FeatureList::IsEnabled(kPowerBookmarkBackend));
+  scoped_refptr<PowerBookmarkBackend> backend =
+      base::MakeRefCounted<PowerBookmarkBackend>(database_dir,
+                                                 frontend_task_runner, this);
+  backend_.swap(backend);
+  backend_task_runner_->PostTask(
+      FROM_HERE,
+      base::BindOnce(&PowerBookmarkBackend::Init, backend_,
+                     base::FeatureList::IsEnabled(kPowerBookmarkBackend)));
 }
 
 PowerBookmarkService::~PowerBookmarkService() {
   if (model_)
     model_->RemoveObserver(this);
 
-  backend_.AsyncCall(&PowerBookmarkBackend::Shutdown);
+  backend_task_runner_->PostTask(
+      FROM_HERE,
+      base::BindOnce(&PowerBookmarkBackend::Shutdown, std::move(backend_)));
   backend_task_runner_ = nullptr;
 }
 
+std::unique_ptr<syncer::ModelTypeControllerDelegate>
+PowerBookmarkService::CreateSyncControllerDelegate() {
+  return std::make_unique<syncer::ProxyModelTypeControllerDelegate>(
+      backend_task_runner_,
+      base::BindRepeating(&PowerBookmarkBackend::GetSyncControllerDelegate,
+                          backend_.get()));
+}
+
 void PowerBookmarkService::GetPowersForURL(
     const GURL& url,
     const sync_pb::PowerBookmarkSpecifics::PowerType& power_type,
     PowersCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  backend_.AsyncCall(&PowerBookmarkBackend::GetPowersForURL)
-      .WithArgs(url, power_type)
-      .Then(std::move(callback));
+  backend_task_runner_->PostTaskAndReplyWithResult(
+      FROM_HERE,
+      base::BindOnce(&PowerBookmarkBackend::GetPowersForURL, backend_, url,
+                     power_type),
+      std::move(callback));
 }
 
 void PowerBookmarkService::GetPowerOverviewsForType(
     const sync_pb::PowerBookmarkSpecifics::PowerType& power_type,
     PowerOverviewsCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  backend_.AsyncCall(&PowerBookmarkBackend::GetPowerOverviewsForType)
-      .WithArgs(power_type)
-      .Then(std::move(callback));
+  backend_task_runner_->PostTaskAndReplyWithResult(
+      FROM_HERE,
+      base::BindOnce(&PowerBookmarkBackend::GetPowerOverviewsForType, backend_,
+                     power_type),
+      std::move(callback));
 }
 
 void PowerBookmarkService::SearchPowers(const SearchParams& search_params,
                                         PowersCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  backend_.AsyncCall(&PowerBookmarkBackend::SearchPowers)
-      .WithArgs(search_params)
-      .Then(std::move(callback));
+  backend_task_runner_->PostTaskAndReplyWithResult(
+      FROM_HERE,
+      base::BindOnce(&PowerBookmarkBackend::SearchPowers, backend_,
+                     search_params),
+      std::move(callback));
 }
 
 void PowerBookmarkService::SearchPowerOverviews(
     const SearchParams& search_params,
     PowerOverviewsCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  backend_.AsyncCall(&PowerBookmarkBackend::SearchPowerOverviews)
-      .WithArgs(search_params)
-      .Then(std::move(callback));
+  backend_task_runner_->PostTaskAndReplyWithResult(
+      FROM_HERE,
+      base::BindOnce(&PowerBookmarkBackend::SearchPowerOverviews, backend_,
+                     search_params),
+      std::move(callback));
 }
 
 void PowerBookmarkService::CreatePower(std::unique_ptr<Power> power,
@@ -93,36 +116,43 @@ void PowerBookmarkService::CreatePower(std::unique_ptr<Power> power,
     power->set_time_added(now);
   if (power->time_modified().is_null())
     power->set_time_modified(now);
-  backend_.AsyncCall(&PowerBookmarkBackend::CreatePower)
-      .WithArgs(std::move(power))
-      .Then(std::move(callback));
+  backend_task_runner_->PostTaskAndReplyWithResult(
+      FROM_HERE,
+      base::BindOnce(&PowerBookmarkBackend::CreatePower, backend_,
+                     std::move(power)),
+      std::move(callback));
 }
 
 void PowerBookmarkService::UpdatePower(std::unique_ptr<Power> power,
                                        SuccessCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   power->set_time_modified(base::Time::Now());
-  backend_.AsyncCall(&PowerBookmarkBackend::UpdatePower)
-      .WithArgs(std::move(power))
-      .Then(std::move(callback));
+  backend_task_runner_->PostTaskAndReplyWithResult(
+      FROM_HERE,
+      base::BindOnce(&PowerBookmarkBackend::UpdatePower, backend_,
+                     std::move(power)),
+      std::move(callback));
 }
 
 void PowerBookmarkService::DeletePower(const base::GUID& guid,
                                        SuccessCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  backend_.AsyncCall(&PowerBookmarkBackend::DeletePower)
-      .WithArgs(guid)
-      .Then(std::move(callback));
+  backend_task_runner_->PostTaskAndReplyWithResult(
+      FROM_HERE,
+      base::BindOnce(&PowerBookmarkBackend::DeletePower, backend_, guid),
+      std::move(callback));
 }
 
 void PowerBookmarkService::DeletePowersForURL(
     const GURL& url,
     const sync_pb::PowerBookmarkSpecifics::PowerType& power_type,
     SuccessCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  backend_.AsyncCall(&PowerBookmarkBackend::DeletePowersForURL)
-      .WithArgs(url, power_type)
-      .Then(std::move(callback));
+  backend_task_runner_->PostTaskAndReplyWithResult(
+      FROM_HERE,
+      base::BindOnce(&PowerBookmarkBackend::DeletePowersForURL, backend_, url,
+                     power_type),
+      std::move(callback));
 }
 
 void PowerBookmarkService::AddObserver(PowerBookmarkObserver* observer) {

components/power_bookmarks/core/power_bookmark_service.h

@@ -24,6 +24,10 @@ namespace bookmarks {
 class BookmarkModel;
 }  // namespace bookmarks
 
+namespace syncer {
+class ModelTypeControllerDelegate;
+}  // namespace syncer
+
 namespace power_bookmarks {
 
 class Power;
@@ -63,6 +67,11 @@ class PowerBookmarkService : public KeyedService,
 
   ~PowerBookmarkService() override;
 
+  // For sync codebase only: instantiates a controller delegate to interact with
+  // PowerBookmarkSyncBridge. Must be called from the UI thread.
+  std::unique_ptr<syncer::ModelTypeControllerDelegate>
+  CreateSyncControllerDelegate();
+
   // Returns a vector of Powers for the given `url` through the given
   // `callback`. Use `power_type` to restrict which type is returned or use
   // POWER_TYPE_UNSPECIFIED to return everything.
@@ -139,7 +148,7 @@ class PowerBookmarkService : public KeyedService,
 
  private:
   raw_ptr<bookmarks::BookmarkModel> model_;
-  base::SequenceBound<PowerBookmarkBackend> backend_;
+  scoped_refptr<PowerBookmarkBackend> backend_;
   scoped_refptr<base::SequencedTaskRunner> backend_task_runner_;
 
   base::ObserverList<PowerBookmarkObserver>::Unchecked observers_;