Add integration test coverage for passwords USS migrator

Passwords have a few particularities when it comes to reading from the
sync Directory, as reflected in the TODO now removed. This patch adds
integration tests to exercise the migrator, which seems to work just
fine.

Bug: 949038
Change-Id: I058964587fd94873c0f86836d262adb18a2761d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1557021
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#648642}

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

@@ -14,6 +14,8 @@
 #include "components/sync/driver/profile_sync_service.h"
 #include "components/sync/driver/sync_driver_switches.h"
 
+namespace {
+
 using passwords_helper::AddLogin;
 using passwords_helper::CreateTestPasswordForm;
 using passwords_helper::GetPasswordCount;
@@ -24,6 +26,9 @@ using passwords_helper::ProfileContainsSamePasswordFormsAsVerifier;
 
 using autofill::PasswordForm;
 
+using testing::ElementsAre;
+using testing::IsEmpty;
+
 class SingleClientPasswordsSyncTest : public FeatureToggler, public SyncTest {
  public:
   SingleClientPasswordsSyncTest()
@@ -199,3 +204,58 @@ IN_PROC_BROWSER_TEST_P(SingleClientPasswordsSyncTest,
 INSTANTIATE_TEST_SUITE_P(USS,
                          SingleClientPasswordsSyncTest,
                          ::testing::Values(false, true));
+
+class SingleClientPasswordsSyncUssMigratorTest : public SyncTest {
+ public:
+  SingleClientPasswordsSyncUssMigratorTest() : SyncTest(SINGLE_CLIENT) {}
+  ~SingleClientPasswordsSyncUssMigratorTest() override {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(SingleClientPasswordsSyncUssMigratorTest);
+};
+
+// Creates and syncs two passwords before USS being enabled.
+IN_PROC_BROWSER_TEST_F(SingleClientPasswordsSyncUssMigratorTest,
+                       PRE_ExerciseUssMigrator) {
+  base::test::ScopedFeatureList override_features;
+  override_features.InitAndDisableFeature(switches::kSyncUSSPasswords);
+
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  AddLogin(GetPasswordStore(0), CreateTestPasswordForm(0));
+  AddLogin(GetPasswordStore(0), CreateTestPasswordForm(1));
+  ASSERT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait());
+  ASSERT_EQ(2, GetPasswordCount(0));
+}
+
+// Now that local passwords, the local sync directory and the sever are
+// populated with two passwords, USS is enabled for passwords.
+IN_PROC_BROWSER_TEST_F(SingleClientPasswordsSyncUssMigratorTest,
+                       ExerciseUssMigrator) {
+  base::test::ScopedFeatureList override_features;
+  override_features.InitAndEnableFeature(switches::kSyncUSSPasswords);
+
+  base::HistogramTester histogram_tester;
+  ASSERT_TRUE(SetupClients()) << "SetupClients() failed.";
+  ASSERT_EQ(2, GetPasswordCount(0));
+#if defined(CHROMEOS)
+  // identity::SetRefreshTokenForPrimaryAccount() is needed on ChromeOS in order
+  // to get a non-empty refresh token on startup.
+  GetClient(0)->SignInPrimaryAccount();
+#endif  // defined(CHROMEOS)
+  ASSERT_TRUE(GetClient(0)->AwaitSyncSetupCompletion());
+  ASSERT_EQ(2, GetPasswordCount(0));
+
+  EXPECT_EQ(1, histogram_tester.GetBucketCount(
+                   "Sync.USSMigrationSuccess",
+                   syncer::ModelTypeToHistogramInt(syncer::PASSWORDS)));
+  EXPECT_THAT(
+      histogram_tester.GetAllSamples("Sync.USSMigrationEntityCount.PASSWORD"),
+      ElementsAre(base::Bucket(/*min=*/2, /*count=*/1)));
+  EXPECT_THAT(histogram_tester.GetAllSamples("Sync.DataTypeStartFailures2"),
+              IsEmpty());
+  EXPECT_EQ(
+      0, histogram_tester.GetBucketCount("Sync.ModelTypeEntityChange3.PASSWORD",
+                                         /*REMOTE_INITIAL_UPDATE=*/5));
+}
+
+}  // namespace

components/sync/engine_impl/uss_migrator.cc

@@ -64,8 +64,6 @@ bool ExtractSyncEntity(ModelType type,
     DCHECK_NE(BOOKMARKS, type);
   }
 
-  // It looks like there are fancy other ways to get e.g. passwords specifics
-  // out of Entry. Do we need to special-case them when we ship those types?
   entity->mutable_specifics()->CopyFrom(entry.GetServerSpecifics());
   return true;
 }