Avoid touching TrackedPreferencesMigrator's ref count before it's fully constructed

The first reference to a TrackedPreferencesMigrator instance used to be
made in its constructor implicitly. Though this case is safe as the
reference keeps alive until the other reference is made, it's error
prone to make the first reference in the ctor, and an upcoming change
to base::Bind rejects to make the first reference.

This CL moves the implicit ref count manipulation through base::Bind()
out of the constructor, so that we can avoid making the implicit first
reference creation.

Bug: 866456
Change-Id: I3563296c0f972d57ad8cc0c2f6e32658b9d6293d
Reviewed-on: https://chromium-review.googlesource.com/1156706Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579550}

services/preferences/tracked/tracked_preferences_migration.cc

@@ -23,6 +23,8 @@ namespace {
 class TrackedPreferencesMigrator
     : public base::RefCounted<TrackedPreferencesMigrator> {
  public:
+  enum PrefFilterID { UNPROTECTED_PREF_FILTER, PROTECTED_PREF_FILTER };
+
   TrackedPreferencesMigrator(
       const std::set<std::string>& unprotected_pref_names,
       const std::set<std::string>& protected_pref_names,
@@ -39,13 +41,6 @@ class TrackedPreferencesMigrator
       InterceptablePrefFilter* unprotected_pref_filter,
       InterceptablePrefFilter* protected_pref_filter);
 
- private:
-  friend class base::RefCounted<TrackedPreferencesMigrator>;
-
-  enum PrefFilterID { UNPROTECTED_PREF_FILTER, PROTECTED_PREF_FILTER };
-
-  ~TrackedPreferencesMigrator();
-
   // Stores the data coming in from the filter identified by |id| into this
   // class and then calls MigrateIfReady();
   void InterceptFilterOnLoad(
@@ -54,6 +49,11 @@ class TrackedPreferencesMigrator
           finalize_filter_on_load,
       std::unique_ptr<base::DictionaryValue> prefs);
 
+ private:
+  friend class base::RefCounted<TrackedPreferencesMigrator>;
+
+  ~TrackedPreferencesMigrator();
+
   // Proceeds with migration if both |unprotected_prefs_| and |protected_prefs_|
   // have been set.
   void MigrateIfReady();
@@ -215,14 +215,6 @@ TrackedPreferencesMigrator::TrackedPreferencesMigrator(
           register_on_successful_protected_store_write_callback),
       unprotected_pref_hash_store_(std::move(unprotected_pref_hash_store)),
       protected_pref_hash_store_(std::move(protected_pref_hash_store)) {
-  // The callbacks bound below will own this TrackedPreferencesMigrator by
-  // reference.
-  unprotected_pref_filter->InterceptNextFilterOnLoad(
-      base::Bind(&TrackedPreferencesMigrator::InterceptFilterOnLoad, this,
-                 UNPROTECTED_PREF_FILTER));
-  protected_pref_filter->InterceptNextFilterOnLoad(
-      base::Bind(&TrackedPreferencesMigrator::InterceptFilterOnLoad, this,
-                 PROTECTED_PREF_FILTER));
 }
 
 TrackedPreferencesMigrator::~TrackedPreferencesMigrator() {}
@@ -322,13 +314,21 @@ void SetupTrackedPreferencesMigration(
     std::unique_ptr<PrefHashStore> protected_pref_hash_store,
     InterceptablePrefFilter* unprotected_pref_filter,
     InterceptablePrefFilter* protected_pref_filter) {
-  scoped_refptr<TrackedPreferencesMigrator> prefs_migrator(
-      new TrackedPreferencesMigrator(
-          unprotected_pref_names, protected_pref_names,
-          unprotected_store_cleaner, protected_store_cleaner,
-          register_on_successful_unprotected_store_write_callback,
-          register_on_successful_protected_store_write_callback,
-          std::move(unprotected_pref_hash_store),
-          std::move(protected_pref_hash_store), unprotected_pref_filter,
-          protected_pref_filter));
+  auto prefs_migrator = base::MakeRefCounted<TrackedPreferencesMigrator>(
+      unprotected_pref_names, protected_pref_names, unprotected_store_cleaner,
+      protected_store_cleaner,
+      register_on_successful_unprotected_store_write_callback,
+      register_on_successful_protected_store_write_callback,
+      std::move(unprotected_pref_hash_store),
+      std::move(protected_pref_hash_store), unprotected_pref_filter,
+      protected_pref_filter);
+
+  // The callbacks bound below will own this TrackedPreferencesMigrator by
+  // reference.
+  unprotected_pref_filter->InterceptNextFilterOnLoad(base::Bind(
+      &TrackedPreferencesMigrator::InterceptFilterOnLoad, prefs_migrator,
+      TrackedPreferencesMigrator::UNPROTECTED_PREF_FILTER));
+  protected_pref_filter->InterceptNextFilterOnLoad(base::Bind(
+      &TrackedPreferencesMigrator::InterceptFilterOnLoad, prefs_migrator,
+      TrackedPreferencesMigrator::PROTECTED_PREF_FILTER));
 }