Refactor password sync integration tests

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

Index: chrome/browser/sync/test/integration/multiple_client_passwords_sync_test.cc
diff --git a/chrome/browser/sync/test/integration/multiple_client_passwords_sync_test.cc b/chrome/browser/sync/test/integration/multiple_client_passwords_sync_test.cc
index c1c5f6f87f9e4c28580c441693a806e6432416a9..b7244159074af4d5b412c02ca7fdd347bf4a2ff5 100644
--- a/chrome/browser/sync/test/integration/multiple_client_passwords_sync_test.cc
+++ b/chrome/browser/sync/test/integration/multiple_client_passwords_sync_test.cc
@@ -5,17 +5,31 @@
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/sync/test/integration/passwords_helper.h"
+#include "chrome/browser/sync/test/integration/profile_sync_service_harness.h"
+#include "chrome/browser/sync/test/integration/sync_integration_test_util.h"
 #include "chrome/browser/sync/test/integration/sync_test.h"
 #include "components/password_manager/core/browser/password_form_data.h"
 
 using passwords_helper::AddLogin;
+using passwords_helper::AllProfilesContainSamePasswordFormsAsVerifier;
 using passwords_helper::AwaitAllProfilesContainSamePasswordForms;
+using passwords_helper::AwaitProfileContainsSamePasswordFormsAsVerifier;
 using passwords_helper::CreateTestPasswordForm;
 using passwords_helper::GetPasswordCount;
 using passwords_helper::GetPasswordStore;
+using passwords_helper::GetVerifierPasswordCount;
+using passwords_helper::GetVerifierPasswordStore;
+using passwords_helper::ProfileContainsSamePasswordFormsAsVerifier;
+using passwords_helper::SetDecryptionPassphrase;
+using passwords_helper::SetEncryptionPassphrase;
+using sync_integration_test_util::AwaitPassphraseAccepted;
+using sync_integration_test_util::AwaitPassphraseRequired;
 
 using autofill::PasswordForm;
 
+static const char* kValidPassphrase = "passphrase!";
+static const char* kAnotherValidPassphrase = "Mot de passe!";
+
 class MultipleClientPasswordsSyncTest : public SyncTest {
  public:
   MultipleClientPasswordsSyncTest() : SyncTest(MULTIPLE_CLIENT) {}
@@ -40,3 +54,142 @@ IN_PROC_BROWSER_TEST_F(MultipleClientPasswordsSyncTest, Sanity) {
   ASSERT_TRUE(AwaitAllProfilesContainSamePasswordForms());
   ASSERT_EQ(num_clients(), GetPasswordCount(0));
 }
+
+// TCM ID - 4577932.
+IN_PROC_BROWSER_TEST_F(MultipleClientPasswordsSyncTest, DisablePasswords) {
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  ASSERT_TRUE(AllProfilesContainSamePasswordFormsAsVerifier());
+
+  ASSERT_TRUE(GetClient(2)->DisableSyncForDatatype(syncer::PASSWORDS));
+  PasswordForm form = CreateTestPasswordForm(0);
+  AddLogin(GetVerifierPasswordStore(), form);
+  ASSERT_EQ(1, GetVerifierPasswordCount());
+  AddLogin(GetPasswordStore(0), form);
+  ASSERT_EQ(1, GetPasswordCount(0));
+
+  // Wait for the change to be picked up by client 1.
+  ASSERT_TRUE(AwaitProfileContainsSamePasswordFormsAsVerifier(1));
+
+  // Client 0 was always in sync, but client 2 is disabled and out of sync.
+  ASSERT_TRUE(ProfileContainsSamePasswordFormsAsVerifier(0));
+  ASSERT_FALSE(ProfileContainsSamePasswordFormsAsVerifier(2));
+
+  // Enable client 2 and wait for it to match.
+  ASSERT_TRUE(GetClient(2)->EnableSyncForDatatype(syncer::PASSWORDS));
+  ASSERT_TRUE(AwaitProfileContainsSamePasswordFormsAsVerifier(2));

[Nicolas Zea, 2014/05/06 00:16:37]

Hmm, given that we're calling this, why do we care about waiting for client 1 to
pick up changes? Either way we'll be waiting here for those changes.

Unlike the passphrase cases, do we care about getting the data at model
association time vs change processor time?

[rlarocque, 2014/05/06 00:33:36]

The expression ASSERT_EQ(1, GetPasswordCount(1)) is probably a left-over from
the old test.

It might still have some value, though.  It's not necessarily about asserting
that client 1 has exactly one password.  In combination with line 81, it asserts
that all stores have exactly one password.

If we assume that the verifier profile is managed correctly then the check is
not useful.

If you feel strongly about it I could remove this or have it assert against a
different profile.

[Nicolas Zea, 2014/05/06 18:42:15]

I'd rather just rely on the verifier to be correct, and test against that. It
seems to me that client 1 isn't necessary for this test (and the one below).

[rlarocque, 2014/05/06 23:40:47]

Done.

+  ASSERT_EQ(1, GetPasswordCount(1));
+  ASSERT_TRUE(AllProfilesContainSamePasswordFormsAsVerifier());
+}
+
+// TCM ID - 4649281.
+IN_PROC_BROWSER_TEST_F(MultipleClientPasswordsSyncTest, DisableSync) {
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  ASSERT_TRUE(AllProfilesContainSamePasswordFormsAsVerifier());
+
+  ASSERT_TRUE(GetClient(2)->DisableSyncForAllDatatypes());
+  PasswordForm form = CreateTestPasswordForm(0);
+  AddLogin(GetVerifierPasswordStore(), form);
+  ASSERT_EQ(1, GetVerifierPasswordCount());
+  AddLogin(GetPasswordStore(0), form);
+  ASSERT_EQ(1, GetPasswordCount(0));
+
+  // Wait for the change to be picked up by client 1.
+  ASSERT_TRUE(AwaitProfileContainsSamePasswordFormsAsVerifier(1));
+
+  // Client 0 was always in sync, but client 2 is disabled and out of sync.
+  ASSERT_TRUE(ProfileContainsSamePasswordFormsAsVerifier(0));
+  ASSERT_FALSE(ProfileContainsSamePasswordFormsAsVerifier(2));
+
+  // Enable client 2 and wait for it to match.
+  ASSERT_TRUE(GetClient(2)->EnableSyncForAllDatatypes());
+  ASSERT_TRUE(AwaitProfileContainsSamePasswordFormsAsVerifier(2));

[Nicolas Zea, 2014/05/06 00:16:37]

Same with this test

[rlarocque, 2014/05/06 23:40:47]

Done.

+  ASSERT_EQ(1, GetPasswordCount(1));
+  ASSERT_TRUE(AllProfilesContainSamePasswordFormsAsVerifier());
+}
+
+IN_PROC_BROWSER_TEST_F(MultipleClientPasswordsSyncTest,
+                       SetPassphraseAndThenSetupSync) {
+  ASSERT_TRUE(SetupClients());
+
+  ASSERT_TRUE(GetClient(0)->SetupSync());
+  SetEncryptionPassphrase(0, kValidPassphrase, ProfileSyncService::EXPLICIT);
+  ASSERT_TRUE(AwaitPassphraseAccepted(GetSyncService(0)));
+
+  // When client 1 hits a passphrase required state, we can infer that
+  // client 0's passphrase has been committed. to the server.
+  GetClient(1)->SetupSync();
+  ASSERT_TRUE(AwaitPassphraseRequired(GetSyncService(1)));
+
+  // Setup client 2 *after* the passphrase has been committed.
+  ASSERT_FALSE(GetClient(2)->SetupSync());
+  ASSERT_TRUE(AwaitPassphraseRequired(GetSyncService(2)));
+
+  // Get clients 1 and 2 out of the passphrase required state.
+  ASSERT_TRUE(SetDecryptionPassphrase(1, kValidPassphrase));
+  ASSERT_TRUE(AwaitPassphraseAccepted(GetSyncService(1)));
+  ASSERT_TRUE(SetDecryptionPassphrase(2, kValidPassphrase));
+  ASSERT_TRUE(AwaitPassphraseAccepted(GetSyncService(2)));
+
+  // For some reason, the tests won't pass unless these flags are set.
+  GetSyncService(1)->SetSyncSetupCompleted();

[Nicolas Zea, 2014/05/06 00:16:37]

SetupSync should be setting these bits appropriately. Is that not the case?

[rlarocque, 2014/05/06 00:33:36]

These lines are a left-over from the old integration test.  If I remove them,
the tests fail.

I didn't bother to investigate too much.  My goals for this CL did not involve
cleaning up this particular mess.

+  GetSyncService(1)->SetSetupInProgress(false);
+  GetSyncService(2)->SetSyncSetupCompleted();
+  GetSyncService(2)->SetSetupInProgress(false);
+
+  // Move around some passwords to make sure it's all working.
+  PasswordForm form0 = CreateTestPasswordForm(0);
+  AddLogin(GetPasswordStore(0), form0);
+
+  ASSERT_TRUE(AwaitAllProfilesContainSamePasswordForms());
+}
+
+IN_PROC_BROWSER_TEST_F(MultipleClientPasswordsSyncTest,
+                       SetDifferentPassphraseAndThenSetupSync) {
+  ASSERT_TRUE(SetupClients()) << "SetupClients() failed.";
+
+  ASSERT_TRUE(GetClient(0)->SetupSync());
+  SetEncryptionPassphrase(0, kValidPassphrase, ProfileSyncService::EXPLICIT);
+  ASSERT_TRUE(AwaitPassphraseAccepted(GetSyncService((0))));
+
+  // When client 1 hits a passphrase required state, we can infer that
+  // client 0's passphrase has been committed. to the server.
+  GetClient(1)->SetupSync();
+  ASSERT_TRUE(AwaitPassphraseRequired(GetSyncService(1)));
+
+  // Give client 1 the correct passphrase.
+  SetDecryptionPassphrase(1, kValidPassphrase);
+  ASSERT_TRUE(AwaitPassphraseAccepted(GetSyncService((1))));
+
+  // For some reason, the tests won't pass unless these flags are set.
+  GetSyncService(1)->SetSetupInProgress(false);
+  GetSyncService(1)->SetSyncSetupCompleted();
+
+  // Give client 2 a different passphrase so it fails to sync.
+  ASSERT_FALSE(GetClient(2)->SetupSync());
+  ASSERT_TRUE(AwaitPassphraseRequired(GetSyncService((2))));
+  SetDecryptionPassphrase(2, kAnotherValidPassphrase);
+  ASSERT_TRUE(AwaitPassphraseRequired(GetSyncService((2))));
+
+  // Add a password on 0 while client 2 has different passphrases.
+  PasswordForm form0 = CreateTestPasswordForm(0);
+  AddLogin(GetVerifierPasswordStore(), form0);
+  AddLogin(GetPasswordStore(0), form0);
+
+  // It should sync to client 1.
+  ASSERT_TRUE(AwaitProfileContainsSamePasswordFormsAsVerifier(1));
+
+  // But it won't get synced to 2.
+  ASSERT_FALSE(ProfileContainsSamePasswordFormsAsVerifier(2));
+
+  // Update 2 with the correct passphrase, the password should now sync over.
+  ASSERT_TRUE(AwaitPassphraseRequired(GetSyncService(2)));
+  ASSERT_TRUE(SetDecryptionPassphrase(2, kValidPassphrase));
+  ASSERT_TRUE(AwaitPassphraseAccepted(GetSyncService(2)));
+
+  // For some reason, the tests won't pass unless these flags are set.
+  GetSyncService(2)->SetSetupInProgress(false);
+  GetSyncService(2)->SetSyncSetupCompleted();
+
+  ASSERT_TRUE(AwaitProfileContainsSamePasswordFormsAsVerifier(2));
+  ASSERT_TRUE(AllProfilesContainSamePasswordFormsAsVerifier());
+}