[sync] Update SESSIONS -> PROXY_TAB prefs migration way

The old migration way requires changing of sync settings to complete
the migration (https://codereview.chromium.org/11961030/). That way
also blocks updates in SyncPrefs class required for crbug.com/906611
since it uses SyncPrefs internal logic. New way is implemented outside
of SyncPrefs and force the migration on browser start.

Bug: 906611
Change-Id: I0518798c5246e6436449ab79ddd648e68a52507c
Reviewed-on: https://chromium-review.googlesource.com/c/1434281Reviewed-by: Dominic Battré <battre@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Dominic Battré <battre@chromium.org>
Auto-Submit: Maksim Moskvitin <mmoskvitin@google.com>
Cr-Commit-Position: refs/heads/master@{#626112}

chrome/browser/prefs/browser_prefs.cc

@@ -903,4 +903,5 @@ void MigrateObsoleteProfilePrefs(Profile* profile) {
   // Added 1/2019.
   profile_prefs->ClearPref(kLastUpdateCheck);
   profile_prefs->ClearPref(kNextUpdateCheck);
+  syncer::MigrateSessionsToProxyTabsPrefs(profile_prefs);
 }

components/sync/base/sync_prefs.cc

@@ -477,14 +477,6 @@ bool SyncPrefs::GetDataTypePreferred(ModelType type) const {
   if (AlwaysPreferredUserTypes().Has(type))
     return true;
 
-  if (type == PROXY_TABS &&
-      pref_service_->GetUserPrefValue(pref_name) == nullptr &&
-      pref_service_->IsUserModifiablePreference(pref_name)) {
-    // If there is no tab sync preference yet (i.e. newly enabled type),
-    // default to the session sync preference value.
-    pref_name = GetPrefNameForDataType(SESSIONS);
-  }
-
   return pref_service_->GetBoolean(pref_name);
 }
 
@@ -652,4 +644,15 @@ bool SyncPrefs::IsLocalSyncEnabled() const {
   return local_sync_enabled_;
 }
 
+void MigrateSessionsToProxyTabsPrefs(PrefService* pref_service) {
+  if (pref_service->GetUserPrefValue(prefs::kSyncTabs) == nullptr &&
+      pref_service->GetUserPrefValue(prefs::kSyncSessions) != nullptr &&
+      pref_service->IsUserModifiablePreference(prefs::kSyncTabs)) {
+    // If there is no tab sync preference yet (i.e. newly enabled type),
+    // default to the session sync preference value.
+    bool sessions_pref_value = pref_service->GetBoolean(prefs::kSyncSessions);
+    pref_service->SetBoolean(prefs::kSyncTabs, sessions_pref_value);
+  }
+}
+
 }  // namespace syncer

components/sync/base/sync_prefs.h

@@ -262,6 +262,8 @@ class SyncPrefs : public CryptoSyncPrefs,
   DISALLOW_COPY_AND_ASSIGN(SyncPrefs);
 };
 
+void MigrateSessionsToProxyTabsPrefs(PrefService* pref_service);
+
 }  // namespace syncer
 
 #endif  // COMPONENTS_SYNC_BASE_SYNC_PREFS_H_

components/sync/base/sync_prefs_unittest.cc

@@ -186,8 +186,14 @@ TEST_F(SyncPrefsTest, DeleteDirectivesAndProxyTabsMigration) {
                                          /*registered_types=*/registered_types,
                                          /*preferred_types=*/registered_types);
 
-  // Manually enable typed urls (to simulate the old world).
+  // Manually enable typed urls (to simulate the old world) and perform the
+  // migration to check it doesn't affect the proxy tab preference value.
   pref_service_.SetBoolean(prefs::kSyncTypedUrls, true);
+  // TODO(crbug.com/906611): now we make an extra assumption that the migration
+  // can be called a second time and it will do the real migration if during the
+  // first call this migration wasn't needed. Maybe consider splitting this
+  // test?
+  MigrateSessionsToProxyTabsPrefs(&pref_service_);
 
   // Proxy tabs should not be enabled (since sessions wasn't), but history
   // delete directives should (since typed urls was).
@@ -196,11 +202,13 @@ TEST_F(SyncPrefsTest, DeleteDirectivesAndProxyTabsMigration) {
   EXPECT_FALSE(preferred_types.Has(PROXY_TABS));
   EXPECT_TRUE(preferred_types.Has(HISTORY_DELETE_DIRECTIVES));
 
-  // Now manually enable sessions, which should result in proxy tabs also being
-  // enabled. Also, manually disable typed urls, which should mean that history
-  // delete directives are not enabled.
+  // Now manually enable sessions and perform the migration, which should result
+  // in proxy tabs also being enabled. Also, manually disable typed urls, which
+  // should mean that history delete directives are not enabled.
   pref_service_.SetBoolean(prefs::kSyncTypedUrls, false);
   pref_service_.SetBoolean(prefs::kSyncSessions, true);
+  MigrateSessionsToProxyTabsPrefs(&pref_service_);
+
   preferred_types = sync_prefs_->GetPreferredDataTypes(UserTypes());
   EXPECT_TRUE(preferred_types.Has(PROXY_TABS));
   EXPECT_FALSE(preferred_types.Has(HISTORY_DELETE_DIRECTIVES));