[go: nahoru, domu]

[Code Health] Remove uses of base::SupportsWeakPtr. (sync)

This CL removes a use of base::SupportsWeakPtr from SyncableService
abstract base class by:

1. Adding a pure virtual function to return a base::WeakPtr.
2. Adding base::WeakPtrFactory class instances to the deriving
classes, if they don't already have one.
3. Implementing the function in the derived classes.

In most cases, leaf classes that have WeakPtrFactory instances are
marked as final for safety. However, there are some cases where test
classes extends some production classes, so they are not marked as
final. This can be fixed by expanding the class hierarchies, but that
adds more work and complexity, so I didn't pursue that.

Bug: 100114, 647430
Change-Id: Ia1e39071fd28130c8ea84e2282928350ddb43f9f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5165864
Commit-Queue: David Bertoni <dbertoni@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1243158}
diff --git a/chrome/browser/ash/app_list/app_list_syncable_service.cc b/chrome/browser/ash/app_list/app_list_syncable_service.cc
index 66f10518..e7ad31d 100644
--- a/chrome/browser/ash/app_list/app_list_syncable_service.cc
+++ b/chrome/browser/ash/app_list/app_list_syncable_service.cc
@@ -1436,6 +1436,10 @@
   return std::nullopt;
 }
 
+base::WeakPtr<syncer::SyncableService> AppListSyncableService::AsWeakPtr() {
+  return weak_ptr_factory_.GetWeakPtr();
+}
+
 void AppListSyncableService::Shutdown() {
   app_service_apps_builder_.reset();
   if (ash::features::ArePromiseIconsEnabled()) {
diff --git a/chrome/browser/ash/app_list/app_list_syncable_service.h b/chrome/browser/ash/app_list/app_list_syncable_service.h
index 84081fd..bb60563 100644
--- a/chrome/browser/ash/app_list/app_list_syncable_service.h
+++ b/chrome/browser/ash/app_list/app_list_syncable_service.h
@@ -305,6 +305,7 @@
   std::optional<syncer::ModelError> ProcessSyncChanges(
       const base::Location& from_here,
       const syncer::SyncChangeList& change_list) override;
+  base::WeakPtr<SyncableService> AsWeakPtr() override;
 
   // KeyedService
   void Shutdown() override;
diff --git a/chrome/browser/ash/app_list/arc/arc_package_syncable_service.cc b/chrome/browser/ash/app_list/arc/arc_package_syncable_service.cc
index 17e20d6..f058bf3 100644
--- a/chrome/browser/ash/app_list/arc/arc_package_syncable_service.cc
+++ b/chrome/browser/ash/app_list/arc/arc_package_syncable_service.cc
@@ -253,6 +253,10 @@
   return std::nullopt;
 }
 
+base::WeakPtr<syncer::SyncableService> ArcPackageSyncableService::AsWeakPtr() {
+  return weak_ptr_factory_.GetWeakPtr();
+}
+
 bool ArcPackageSyncableService::SyncStarted() {
   if (sync_processor_.get())
     return true;
diff --git a/chrome/browser/ash/app_list/arc/arc_package_syncable_service.h b/chrome/browser/ash/app_list/arc/arc_package_syncable_service.h
index 3ecfbb26..cc886c0 100644
--- a/chrome/browser/ash/app_list/arc/arc_package_syncable_service.h
+++ b/chrome/browser/ash/app_list/arc/arc_package_syncable_service.h
@@ -12,6 +12,7 @@
 #include <unordered_map>
 
 #include "base/memory/raw_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "chrome/browser/ash/app_list/arc/arc_app_list_prefs.h"
 #include "chrome/browser/ash/app_list/arc/arc_app_sync_metrics_helper.h"
 #include "chrome/browser/ash/arc/session/arc_session_manager_observer.h"
@@ -73,6 +74,7 @@
   std::optional<syncer::ModelError> ProcessSyncChanges(
       const base::Location& from_here,
       const syncer::SyncChangeList& change_list) override;
+  base::WeakPtr<SyncableService> AsWeakPtr() override;
 
   bool SyncStarted();
 
@@ -147,6 +149,8 @@
   const raw_ptr<ArcAppListPrefs, ExperimentalAsh> prefs_;
 
   ArcAppSyncMetricsHelper metrics_helper_;
+
+  base::WeakPtrFactory<ArcPackageSyncableService> weak_ptr_factory_{this};
 };
 
 }  // namespace arc
diff --git a/chrome/browser/extensions/api/storage/sync_storage_backend.cc b/chrome/browser/extensions/api/storage/sync_storage_backend.cc
index 7f2c578a..cb260874 100644
--- a/chrome/browser/extensions/api/storage/sync_storage_backend.cc
+++ b/chrome/browser/extensions/api/storage/sync_storage_backend.cc
@@ -236,6 +236,10 @@
   return absl::nullopt;
 }
 
+base::WeakPtr<syncer::SyncableService> SyncStorageBackend::AsWeakPtr() {
+  return weak_ptr_factory_.GetWeakPtr();
+}
+
 void SyncStorageBackend::StopSyncing(syncer::ModelType type) {
   DCHECK(IsOnBackendSequence());
   DCHECK(type == syncer::EXTENSION_SETTINGS || type == syncer::APP_SETTINGS);
diff --git a/chrome/browser/extensions/api/storage/sync_storage_backend.h b/chrome/browser/extensions/api/storage/sync_storage_backend.h
index 29e64e5..94896ff 100644
--- a/chrome/browser/extensions/api/storage/sync_storage_backend.h
+++ b/chrome/browser/extensions/api/storage/sync_storage_backend.h
@@ -11,6 +11,7 @@
 #include <string>
 
 #include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
 #include "base/values.h"
 #include "components/sync/model/syncable_service.h"
 #include "components/value_store/value_store_factory.h"
@@ -29,7 +30,7 @@
 // Manages ValueStore objects for extensions, including routing
 // changes from sync to them.
 // Lives entirely on the FILE thread.
-class SyncStorageBackend : public syncer::SyncableService {
+class SyncStorageBackend final : public syncer::SyncableService {
  public:
   // |storage_factory| is use to create leveldb storage areas.
   // |observers| is the list of observers to settings changes.
@@ -59,6 +60,7 @@
       const base::Location& from_here,
       const syncer::SyncChangeList& change_list) override;
   void StopSyncing(syncer::ModelType type) override;
+  base::WeakPtr<SyncableService> AsWeakPtr() override;
 
  private:
   // Gets a weak reference to the storage area for a given extension,
@@ -93,6 +95,8 @@
   std::unique_ptr<syncer::SyncChangeProcessor> sync_processor_;
 
   syncer::SyncableService::StartSyncFlare flare_;
+
+  base::WeakPtrFactory<SyncStorageBackend> weak_ptr_factory_{this};
 };
 
 }  // namespace extensions
diff --git a/chrome/browser/extensions/extension_sync_service.cc b/chrome/browser/extensions/extension_sync_service.cc
index 164ac33..1c07ba2 100644
--- a/chrome/browser/extensions/extension_sync_service.cc
+++ b/chrome/browser/extensions/extension_sync_service.cc
@@ -231,6 +231,10 @@
   return absl::nullopt;
 }
 
+base::WeakPtr<syncer::SyncableService> ExtensionSyncService::AsWeakPtr() {
+  return weak_ptr_factory_.GetWeakPtr();
+}
+
 ExtensionSyncData ExtensionSyncService::CreateSyncData(
     const Extension& extension) const {
   const std::string& id = extension.id();
diff --git a/chrome/browser/extensions/extension_sync_service.h b/chrome/browser/extensions/extension_sync_service.h
index a200637..fc1a80a 100644
--- a/chrome/browser/extensions/extension_sync_service.h
+++ b/chrome/browser/extensions/extension_sync_service.h
@@ -12,6 +12,7 @@
 
 #include "base/gtest_prod_util.h"
 #include "base/memory/raw_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/scoped_observation.h"
 #include "base/version.h"
 #include "chrome/browser/extensions/sync_bundle.h"
@@ -35,10 +36,10 @@
 
 // SyncableService implementation responsible for the APPS and EXTENSIONS data
 // types, i.e. "proper" apps/extensions (not themes).
-class ExtensionSyncService : public syncer::SyncableService,
-                             public KeyedService,
-                             public extensions::ExtensionRegistryObserver,
-                             public extensions::ExtensionPrefsObserver {
+class ExtensionSyncService final : public syncer::SyncableService,
+                                   public KeyedService,
+                                   public extensions::ExtensionRegistryObserver,
+                                   public extensions::ExtensionPrefsObserver {
  public:
   explicit ExtensionSyncService(Profile* profile);
 
@@ -66,6 +67,7 @@
   absl::optional<syncer::ModelError> ProcessSyncChanges(
       const base::Location& from_here,
       const syncer::SyncChangeList& change_list) override;
+  base::WeakPtr<SyncableService> AsWeakPtr() override;
 
   void SetSyncStartFlareForTesting(
       const syncer::SyncableService::StartSyncFlare& flare);
@@ -155,7 +157,9 @@
   // Run()ning tells sync to try and start soon, because syncable changes
   // have started happening. It will cause sync to call us back
   // asynchronously via MergeDataAndStartSyncing as soon as possible.
-  syncer::SyncableService::StartSyncFlare flare_;
+  SyncableService::StartSyncFlare flare_;
+
+  base::WeakPtrFactory<ExtensionSyncService> weak_ptr_factory_{this};
 };
 
 #endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_SYNC_SERVICE_H_
diff --git a/chrome/browser/spellchecker/spellcheck_custom_dictionary.cc b/chrome/browser/spellchecker/spellcheck_custom_dictionary.cc
index 2ded623..c993db6 100644
--- a/chrome/browser/spellchecker/spellcheck_custom_dictionary.cc
+++ b/chrome/browser/spellchecker/spellcheck_custom_dictionary.cc
@@ -411,6 +411,10 @@
   return absl::nullopt;
 }
 
+base::WeakPtr<syncer::SyncableService> SpellcheckCustomDictionary::AsWeakPtr() {
+  return weak_ptr_factory_.GetWeakPtr();
+}
+
 SpellcheckCustomDictionary::LoadFileResult::LoadFileResult()
     : is_valid_file(false) {}
 
diff --git a/chrome/browser/spellchecker/spellcheck_custom_dictionary.h b/chrome/browser/spellchecker/spellcheck_custom_dictionary.h
index b3846b9..7b5feda 100644
--- a/chrome/browser/spellchecker/spellcheck_custom_dictionary.h
+++ b/chrome/browser/spellchecker/spellcheck_custom_dictionary.h
@@ -38,8 +38,8 @@
 //   foo
 //   checksum_v1 = ec3df4034567e59e119fcf87f2d9bad4
 //
-class SpellcheckCustomDictionary : public SpellcheckDictionary,
-                                   public syncer::SyncableService {
+class SpellcheckCustomDictionary final : public SpellcheckDictionary,
+                                         public syncer::SyncableService {
  public:
   // A change to the dictionary.
   class Change {
@@ -178,6 +178,7 @@
   absl::optional<syncer::ModelError> ProcessSyncChanges(
       const base::Location& from_here,
       const syncer::SyncChangeList& change_list) override;
+  base::WeakPtr<SyncableService> AsWeakPtr() override;
 
  private:
   friend class DictionarySyncIntegrationTestHelper;
diff --git a/chrome/browser/themes/theme_syncable_service.cc b/chrome/browser/themes/theme_syncable_service.cc
index 7675a883..1c65b8b 100644
--- a/chrome/browser/themes/theme_syncable_service.cc
+++ b/chrome/browser/themes/theme_syncable_service.cc
@@ -209,6 +209,10 @@
   return syncer::ModelError(FROM_HERE, "Didn't find valid theme specifics");
 }
 
+base::WeakPtr<syncer::SyncableService> ThemeSyncableService::AsWeakPtr() {
+  return weak_ptr_factory_.GetWeakPtr();
+}
+
 ThemeSyncableService::ThemeSyncState ThemeSyncableService::MaybeSetTheme(
     const sync_pb::ThemeSpecifics& current_specs,
     const syncer::SyncData& sync_data) {
diff --git a/chrome/browser/themes/theme_syncable_service.h b/chrome/browser/themes/theme_syncable_service.h
index 7468b4c..c5c21a9 100644
--- a/chrome/browser/themes/theme_syncable_service.h
+++ b/chrome/browser/themes/theme_syncable_service.h
@@ -9,6 +9,7 @@
 
 #include "base/gtest_prod_util.h"
 #include "base/memory/raw_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "base/observer_list_types.h"
 #include "base/threading/thread_checker.h"
@@ -25,8 +26,8 @@
 class ThemeSpecifics;
 }
 
-class ThemeSyncableService : public syncer::SyncableService,
-                             public ThemeServiceObserver {
+class ThemeSyncableService final : public syncer::SyncableService,
+                                   public ThemeServiceObserver {
  public:
   // State of local theme after applying sync changes.
   enum class ThemeSyncState {
@@ -81,6 +82,7 @@
   absl::optional<syncer::ModelError> ProcessSyncChanges(
       const base::Location& from_here,
       const syncer::SyncChangeList& change_list) override;
+  base::WeakPtr<SyncableService> AsWeakPtr() override;
 
   // Client tag and title of the single theme sync_pb::SyncEntity of an account.
   static const char kSyncEntityClientTag[];
@@ -130,6 +132,8 @@
 
   base::ThreadChecker thread_checker_;
 
+  base::WeakPtrFactory<ThemeSyncableService> weak_ptr_factory_{this};
+
   FRIEND_TEST_ALL_PREFIXES(ThemeSyncableServiceTest, AreThemeSpecificsEqual);
 };
 
diff --git a/components/history/core/browser/sync/delete_directive_handler.cc b/components/history/core/browser/sync/delete_directive_handler.cc
index aed3aec1..217ee18 100644
--- a/components/history/core/browser/sync/delete_directive_handler.cc
+++ b/components/history/core/browser/sync/delete_directive_handler.cc
@@ -496,6 +496,10 @@
   return absl::nullopt;
 }
 
+base::WeakPtr<syncer::SyncableService> DeleteDirectiveHandler::AsWeakPtr() {
+  return weak_ptr_factory_.GetWeakPtr();
+}
+
 void DeleteDirectiveHandler::FinishProcessing(
     PostProcessingAction post_processing_action,
     const syncer::SyncDataList& delete_directives) {
diff --git a/components/history/core/browser/sync/delete_directive_handler.h b/components/history/core/browser/sync/delete_directive_handler.h
index 8fab3dd4..4ccc137 100644
--- a/components/history/core/browser/sync/delete_directive_handler.h
+++ b/components/history/core/browser/sync/delete_directive_handler.h
@@ -32,7 +32,7 @@
 // DeleteDirectiveHandler sends delete directives created locally to sync
 // engine to propagate to other clients. It also expires local history entries
 // according to given delete directives from server.
-class DeleteDirectiveHandler : public syncer::SyncableService {
+class DeleteDirectiveHandler final : public syncer::SyncableService {
  public:
   // This allows injecting HistoryService::ScheduleDBTask().
   using BackendTaskScheduler =
@@ -77,6 +77,7 @@
   absl::optional<syncer::ModelError> ProcessSyncChanges(
       const base::Location& from_here,
       const syncer::SyncChangeList& change_list) override;
+  base::WeakPtr<SyncableService> AsWeakPtr() override;
 
  private:
   class DeleteDirectiveTask;
diff --git a/components/search_engines/template_url_service.cc b/components/search_engines/template_url_service.cc
index dbc159c2..a30bc04 100644
--- a/components/search_engines/template_url_service.cc
+++ b/components/search_engines/template_url_service.cc
@@ -1475,6 +1475,10 @@
   return sync_processor_->ProcessSyncChanges(from_here, new_changes);
 }
 
+base::WeakPtr<syncer::SyncableService> TemplateURLService::AsWeakPtr() {
+  return weak_ptr_factory_.GetWeakPtr();
+}
+
 absl::optional<syncer::ModelError> TemplateURLService::MergeDataAndStartSyncing(
     syncer::ModelType type,
     const syncer::SyncDataList& initial_sync_data,
diff --git a/components/search_engines/template_url_service.h b/components/search_engines/template_url_service.h
index b029b7f..140f4d42 100644
--- a/components/search_engines/template_url_service.h
+++ b/components/search_engines/template_url_service.h
@@ -20,6 +20,7 @@
 #include "base/gtest_prod_util.h"
 #include "base/memory/raw_ptr.h"
 #include "base/memory/raw_ptr_exclusion.h"
+#include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "base/time/default_clock.h"
 #include "base/time/time.h"
@@ -77,9 +78,9 @@
 // is a KeywordWebDataService, deletion is handled by KeywordWebDataService,
 // otherwise TemplateURLService handles deletion.
 
-class TemplateURLService : public WebDataServiceConsumer,
-                           public KeyedService,
-                           public syncer::SyncableService {
+class TemplateURLService final : public WebDataServiceConsumer,
+                                 public KeyedService,
+                                 public syncer::SyncableService {
  public:
   using QueryTerms = std::map<std::string, std::string>;
   using TemplateURLVector = TemplateURL::TemplateURLVector;
@@ -498,6 +499,7 @@
       const syncer::SyncDataList& initial_sync_data,
       std::unique_ptr<syncer::SyncChangeProcessor> sync_processor) override;
   void StopSyncing(syncer::ModelType type) override;
+  base::WeakPtr<SyncableService> AsWeakPtr() override;
 
   // Processes a local TemplateURL change for Sync. |turl| is the TemplateURL
   // that has been modified, and |type| is the Sync ChangeType that took place.
@@ -993,6 +995,8 @@
   // android.
   std::unique_ptr<TemplateUrlServiceAndroid> template_url_service_android_;
 #endif
+
+  base::WeakPtrFactory<TemplateURLService> weak_ptr_factory_{this};
 };
 
 #endif  // COMPONENTS_SEARCH_ENGINES_TEMPLATE_URL_SERVICE_H_
diff --git a/components/supervised_user/core/browser/supervised_user_settings_service.cc b/components/supervised_user/core/browser/supervised_user_settings_service.cc
index c4313b5..880d62a 100644
--- a/components/supervised_user/core/browser/supervised_user_settings_service.cc
+++ b/components/supervised_user/core/browser/supervised_user_settings_service.cc
@@ -469,6 +469,11 @@
   return absl::nullopt;
 }
 
+base::WeakPtr<syncer::SyncableService>
+SupervisedUserSettingsService::AsWeakPtr() {
+  return weak_ptr_factory_.GetWeakPtr();
+}
+
 void SupervisedUserSettingsService::OnPrefValueChanged(const std::string& key) {
 }
 
diff --git a/components/supervised_user/core/browser/supervised_user_settings_service.h b/components/supervised_user/core/browser/supervised_user_settings_service.h
index b837756..d3886383 100644
--- a/components/supervised_user/core/browser/supervised_user_settings_service.h
+++ b/components/supervised_user/core/browser/supervised_user_settings_service.h
@@ -12,6 +12,7 @@
 #include "base/callback_list.h"
 #include "base/functional/callback.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
 #include "base/strings/string_piece.h"
 #include "base/values.h"
 #include "components/keyed_service/core/keyed_service.h"
@@ -170,6 +171,7 @@
   absl::optional<syncer::ModelError> ProcessSyncChanges(
       const base::Location& from_here,
       const syncer::SyncChangeList& change_list) override;
+  base::WeakPtr<SyncableService> AsWeakPtr() override;
 
   // PrefStore::Observer implementation:
   void OnPrefValueChanged(const std::string& key) override;
@@ -220,6 +222,8 @@
   ShutdownCallbackList shutdown_callback_list_;
 
   std::unique_ptr<syncer::SyncChangeProcessor> sync_processor_;
+
+  base::WeakPtrFactory<SupervisedUserSettingsService> weak_ptr_factory_{this};
 };
 
 }  // namespace supervised_user
diff --git a/components/sync/base/weak_handle_unittest.cc b/components/sync/base/weak_handle_unittest.cc
index fba7914..28a15e94 100644
--- a/components/sync/base/weak_handle_unittest.cc
+++ b/components/sync/base/weak_handle_unittest.cc
@@ -37,7 +37,13 @@
   base::WeakPtrFactory<Base> weak_ptr_factory_{this};
 };
 
-class Derived : public Base, public base::SupportsWeakPtr<Derived> {};
+class Derived : public Base {
+ public:
+  base::WeakPtr<Derived> AsWeakPtr() { return weak_ptr_factory_.GetWeakPtr(); }
+
+ private:
+  base::WeakPtrFactory<Derived> weak_ptr_factory_{this};
+};
 
 class WeakHandleTest : public ::testing::Test {
  protected:
diff --git a/components/sync/model/syncable_service.h b/components/sync/model/syncable_service.h
index 9543c938..9c42b909 100644
--- a/components/sync/model/syncable_service.h
+++ b/components/sync/model/syncable_service.h
@@ -19,10 +19,7 @@
 
 class SyncChangeProcessor;
 
-// TODO(zea): remove SupportsWeakPtr in favor of having all SyncableService
-// implementers provide a way of getting a weak pointer to themselves.
-// See crbug.com/100114.
-class SyncableService : public base::SupportsWeakPtr<SyncableService> {
+class SyncableService {
  public:
   SyncableService() = default;
 
@@ -78,6 +75,9 @@
   virtual absl::optional<ModelError> ProcessSyncChanges(
       const base::Location& from_here,
       const SyncChangeList& change_list) = 0;
+
+  // Get a WeakPtr to the instance.
+  virtual base::WeakPtr<SyncableService> AsWeakPtr() = 0;
 };
 
 }  // namespace syncer
diff --git a/components/sync/model/syncable_service_based_bridge_unittest.cc b/components/sync/model/syncable_service_based_bridge_unittest.cc
index fb79faf..50b22b4 100644
--- a/components/sync/model/syncable_service_based_bridge_unittest.cc
+++ b/components/sync/model/syncable_service_based_bridge_unittest.cc
@@ -8,6 +8,7 @@
 
 #include "base/functional/bind.h"
 #include "base/functional/callback_helpers.h"
+#include "base/memory/weak_ptr.h"
 #include "base/run_loop.h"
 #include "base/test/bind.h"
 #include "base/test/metrics/histogram_tester.h"
@@ -81,6 +82,13 @@
                const SyncChangeList& change_list),
               (override));
   MOCK_METHOD(SyncDataList, GetAllSyncData, (ModelType type), (const override));
+
+  base::WeakPtr<SyncableService> AsWeakPtr() override {
+    return weak_ptr_factory_.GetWeakPtr();
+  }
+
+ private:
+  base::WeakPtrFactory<MockSyncableService> weak_ptr_factory_{this};
 };
 
 class SyncableServiceBasedBridgeTest : public ::testing::Test {
diff --git a/components/sync/test/fake_sync_api_component_factory.cc b/components/sync/test/fake_sync_api_component_factory.cc
index b5227f2..5904097 100644
--- a/components/sync/test/fake_sync_api_component_factory.cc
+++ b/components/sync/test/fake_sync_api_component_factory.cc
@@ -13,11 +13,16 @@
 namespace {
 
 // Subclass of DataTypeManagerImpl to support weak pointers.
-class TestDataTypeManagerImpl
-    : public DataTypeManagerImpl,
-      public base::SupportsWeakPtr<TestDataTypeManagerImpl> {
+class TestDataTypeManagerImpl final : public DataTypeManagerImpl {
  public:
   using DataTypeManagerImpl::DataTypeManagerImpl;
+
+  base::WeakPtr<TestDataTypeManagerImpl> AsWeakPtr() {
+    return weak_ptr_factory_.GetWeakPtr();
+  }
+
+ private:
+  base::WeakPtrFactory<TestDataTypeManagerImpl> weak_ptr_factory_{this};
 };
 
 }  // namespace
diff --git a/components/sync/test/fake_sync_engine.h b/components/sync/test/fake_sync_engine.h
index b611871..5918e03 100644
--- a/components/sync/test/fake_sync_engine.h
+++ b/components/sync/test/fake_sync_engine.h
@@ -25,8 +25,7 @@
 // get through initialization. It often returns null pointers or nonsense
 // values; it is not intended to be used in tests that depend on SyncEngine
 // behavior.
-class FakeSyncEngine : public SyncEngine,
-                       public base::SupportsWeakPtr<FakeSyncEngine> {
+class FakeSyncEngine final : public SyncEngine {
  public:
   static constexpr char kTestBirthday[] = "1";
 
@@ -113,6 +112,10 @@
   void GetNigoriNodeForDebugging(AllNodesCallback callback) override;
   void RecordNigoriMemoryUsageAndCountsHistograms() override;
 
+  base::WeakPtr<FakeSyncEngine> AsWeakPtr() {
+    return weak_ptr_factory_.GetWeakPtr();
+  }
+
  private:
   const bool allow_init_completion_;
   const bool is_first_time_sync_configure_;
@@ -125,6 +128,7 @@
   CoreAccountId authenticated_account_id_;
   bool started_handling_invalidations_ = false;
   bool is_next_poll_time_in_the_past_ = false;
+  base::WeakPtrFactory<FakeSyncEngine> weak_ptr_factory_{this};
 };
 
 }  // namespace syncer
diff --git a/components/sync_preferences/pref_model_associator.cc b/components/sync_preferences/pref_model_associator.cc
index 1fd6115..9dfc27e 100644
--- a/components/sync_preferences/pref_model_associator.cc
+++ b/components/sync_preferences/pref_model_associator.cc
@@ -365,6 +365,10 @@
   return absl::nullopt;
 }
 
+base::WeakPtr<syncer::SyncableService> PrefModelAssociator::AsWeakPtr() {
+  return weak_ptr_factory_.GetWeakPtr();
+}
+
 void PrefModelAssociator::AddSyncedPrefObserver(const std::string& name,
                                                 SyncedPrefObserver* observer) {
   auto& observers = synced_pref_observers_[name];
diff --git a/components/sync_preferences/pref_model_associator.h b/components/sync_preferences/pref_model_associator.h
index b05af138..d08655a 100644
--- a/components/sync_preferences/pref_model_associator.h
+++ b/components/sync_preferences/pref_model_associator.h
@@ -12,6 +12,7 @@
 
 #include "base/memory/raw_ptr.h"
 #include "base/memory/scoped_refptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "base/sequence_checker.h"
 #include "base/values.h"
@@ -45,8 +46,8 @@
 };
 
 // Contains all preference sync related logic.
-class PrefModelAssociator : public syncer::SyncableService,
-                            public PrefStore::Observer {
+class PrefModelAssociator final : public syncer::SyncableService,
+                                  public PrefStore::Observer {
  public:
   // The |client| is not owned and must outlive this object.
   // |user_prefs| is the PrefStore to be hooked up to Sync.
@@ -91,6 +92,7 @@
   absl::optional<syncer::ModelError> ProcessSyncChanges(
       const base::Location& from_here,
       const syncer::SyncChangeList& change_list) override;
+  base::WeakPtr<SyncableService> AsWeakPtr() override;
 
   // PrefStore::Observer implementation.
   void OnPrefValueChanged(const std::string& key) override;
@@ -206,6 +208,8 @@
       synced_pref_observers_;
 
   SEQUENCE_CHECKER(sequence_checker_);
+
+  base::WeakPtrFactory<PrefModelAssociator> weak_ptr_factory_{this};
 };
 
 }  // namespace sync_preferences