[AF] Integration tests for wallet metadata involving offline mode

This CL introduces several further integration tests for autofill wallet
metadata. All these tests involve "being offline" which is simulated by
the fake server returning errors.

This CL also changes the behavior in a minor way: it changes merging
two entities that have never been used. The change is non-controversial;
the main motivation for it is to allow writing crisper integration
tests.

Bug: 894001
Change-Id: I6e29c5ff8f96f6b40bb77f77e881ce1bb3ffbe25
Reviewed-on: https://chromium-review.googlesource.com/c/1335931
Commit-Queue: Jan Krcal <jkrcal@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613053}

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

@@ -10,6 +10,8 @@
 #include "components/autofill/core/browser/personal_data_manager.h"
 #include "components/autofill/core/common/autofill_util.h"
 #include "components/browser_sync/profile_sync_service.h"
+#include "components/sync/test/fake_server/fake_server_http_post_provider.h"
+#include "net/base/network_change_notifier.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
@@ -32,9 +34,14 @@ using wallet_helper::UpdateServerCardMetadata;
 const char kDifferentBillingAddressId[] = "another address entity ID";
 constexpr char kLocalBillingAddressId[] =
     "local billing address ID has size 36";
+constexpr char kLocalBillingAddressId2[] =
+    "another local billing address id wow";
 static_assert(sizeof(kLocalBillingAddressId) == autofill::kLocalGuidSize + 1,
               "|kLocalBillingAddressId| has to have the right length to be "
               "considered a local guid");
+static_assert(sizeof(kLocalBillingAddressId) == sizeof(kLocalBillingAddressId2),
+              "|kLocalBillingAddressId2| has to have the right length to be "
+              "considered a local guid");
 
 class TwoClientWalletSyncTest : public UssWalletSwitchToggler, public SyncTest {
  public:
@@ -98,6 +105,106 @@ IN_PROC_BROWSER_TEST_P(TwoClientWalletSyncTest, UpdateCreditCardMetadata) {
   EXPECT_EQ(new_use_date, credit_cards[0]->use_date());
 }
 
+IN_PROC_BROWSER_TEST_P(TwoClientWalletSyncTest,
+                       UpdateCreditCardMetadataWhileNotSyncing) {
+  InitWithDefaultFeatures();
+
+  GetFakeServer()->SetWalletData(
+      {CreateSyncWalletCard(/*name=*/"card-1", /*last_four=*/"0001",
+                            kDefaultBillingAddressID),
+       CreateDefaultSyncPaymentsCustomerData()});
+  ASSERT_TRUE(SetupSync());
+
+  // Sumulate going offline on both clients.
+  fake_server::FakeServerHttpPostProvider::DisableNetwork();
+
+  // Grab the current card on the first client.
+  std::vector<CreditCard*> credit_cards = GetServerCreditCards(0);
+  ASSERT_EQ(1u, credit_cards.size());
+  CreditCard card = *credit_cards[0];
+
+  // Simulate using it -- increase both its use count and use date.
+  ASSERT_EQ(1u, card.use_count());
+  card.set_use_count(2);
+  base::Time new_use_date = base::Time::Now();
+  ASSERT_NE(new_use_date, card.use_date());
+  card.set_use_date(new_use_date);
+  UpdateServerCardMetadata(0, card);
+
+  // Simulate going online again.
+  fake_server::FakeServerHttpPostProvider::EnableNetwork();
+  net::NetworkChangeNotifier::NotifyObserversOfNetworkChangeForTests(
+      net::NetworkChangeNotifier::CONNECTION_ETHERNET);
+
+  // Wait for the change to propagate.
+  EXPECT_TRUE(AutofillWalletChecker(0, 1).Wait());
+
+  credit_cards = GetServerCreditCards(0);
+  EXPECT_EQ(1U, credit_cards.size());
+  EXPECT_EQ(2u, credit_cards[0]->use_count());
+  EXPECT_EQ(new_use_date, credit_cards[0]->use_date());
+
+  credit_cards = GetServerCreditCards(1);
+  EXPECT_EQ(1U, credit_cards.size());
+  EXPECT_EQ(2u, credit_cards[0]->use_count());
+  EXPECT_EQ(new_use_date, credit_cards[0]->use_date());
+}
+
+IN_PROC_BROWSER_TEST_P(TwoClientWalletSyncTest,
+                       UpdateCreditCardMetadataConflictsWhileNotSyncing) {
+  InitWithDefaultFeatures();
+
+  GetFakeServer()->SetWalletData(
+      {CreateSyncWalletCard(/*name=*/"card-1", /*last_four=*/"0001",
+                            kDefaultBillingAddressID),
+       CreateDefaultSyncPaymentsCustomerData()});
+  ASSERT_TRUE(SetupSync());
+
+  // Sumulate going offline on both clients.
+  fake_server::FakeServerHttpPostProvider::DisableNetwork();
+
+  // Increase use stats on both clients, make use count higher on the first
+  // client and use date higher on the second client.
+  std::vector<CreditCard*> credit_cards = GetServerCreditCards(0);
+  ASSERT_EQ(1u, credit_cards.size());
+  CreditCard card = *credit_cards[0];
+  ASSERT_EQ(1u, card.use_count());
+  card.set_use_count(3);
+  base::Time lower_new_use_date = base::Time::Now();
+  ASSERT_NE(lower_new_use_date, card.use_date());
+  card.set_use_date(lower_new_use_date);
+  UpdateServerCardMetadata(0, card);
+
+  credit_cards = GetServerCreditCards(1);
+  ASSERT_EQ(1u, credit_cards.size());
+  card = *credit_cards[0];
+  ASSERT_EQ(1u, card.use_count());
+  card.set_use_count(2);
+  base::Time higher_new_use_date = base::Time::Now();
+  ASSERT_NE(higher_new_use_date, card.use_date());
+  ASSERT_LT(lower_new_use_date, higher_new_use_date);
+  card.set_use_date(higher_new_use_date);
+  UpdateServerCardMetadata(1, card);
+
+  // Simulate going online again.
+  fake_server::FakeServerHttpPostProvider::EnableNetwork();
+  net::NetworkChangeNotifier::NotifyObserversOfNetworkChangeForTests(
+      net::NetworkChangeNotifier::CONNECTION_ETHERNET);
+
+  // Wait for the clients to coverge and both resolve the conflicts by taking
+  // maxima in both components.
+  EXPECT_TRUE(AutofillWalletChecker(0, 1).Wait());
+
+  credit_cards = GetServerCreditCards(0);
+  EXPECT_EQ(1U, credit_cards.size());
+  EXPECT_EQ(3u, credit_cards[0]->use_count());
+  EXPECT_EQ(higher_new_use_date, credit_cards[0]->use_date());
+  credit_cards = GetServerCreditCards(1);
+  EXPECT_EQ(1U, credit_cards.size());
+  EXPECT_EQ(3u, credit_cards[0]->use_count());
+  EXPECT_EQ(higher_new_use_date, credit_cards[0]->use_date());
+}
+
 IN_PROC_BROWSER_TEST_P(TwoClientWalletSyncTest, UpdateServerAddressMetadata) {
   InitWithDefaultFeatures();
 
@@ -135,6 +242,104 @@ IN_PROC_BROWSER_TEST_P(TwoClientWalletSyncTest, UpdateServerAddressMetadata) {
   EXPECT_EQ(new_use_date, server_addresses[0]->use_date());
 }
 
+IN_PROC_BROWSER_TEST_P(TwoClientWalletSyncTest,
+                       UpdateServerAddressMetadataWhileNotSyncing) {
+  InitWithDefaultFeatures();
+
+  GetFakeServer()->SetWalletData(
+      {CreateSyncWalletAddress(/*name=*/"address-1", /*company=*/"Company-1"),
+       CreateDefaultSyncPaymentsCustomerData()});
+  ASSERT_TRUE(SetupSync());
+
+  // Sumulate going offline on both clients.
+  fake_server::FakeServerHttpPostProvider::DisableNetwork();
+
+  // Grab the current address on the first client.
+  std::vector<AutofillProfile*> server_addresses = GetServerProfiles(0);
+  ASSERT_EQ(1u, server_addresses.size());
+  AutofillProfile address = *server_addresses[0];
+
+  // Simulate using it -- increase both its use count and use date.
+  ASSERT_EQ(1u, address.use_count());
+  address.set_use_count(2);
+  base::Time new_use_date = base::Time::Now();
+  ASSERT_NE(new_use_date, address.use_date());
+  address.set_use_date(new_use_date);
+  UpdateServerAddressMetadata(0, address);
+
+  // Simulate going online again.
+  fake_server::FakeServerHttpPostProvider::EnableNetwork();
+  net::NetworkChangeNotifier::NotifyObserversOfNetworkChangeForTests(
+      net::NetworkChangeNotifier::CONNECTION_ETHERNET);
+
+  // Wait for the change to propagate.
+  EXPECT_TRUE(AutofillWalletChecker(0, 1).Wait());
+
+  server_addresses = GetServerProfiles(1);
+  EXPECT_EQ(1U, server_addresses.size());
+  EXPECT_EQ(2u, server_addresses[0]->use_count());
+  EXPECT_EQ(new_use_date, server_addresses[0]->use_date());
+
+  server_addresses = GetServerProfiles(0);
+  EXPECT_EQ(1U, server_addresses.size());
+  EXPECT_EQ(2u, server_addresses[0]->use_count());
+  EXPECT_EQ(new_use_date, server_addresses[0]->use_date());
+}
+
+IN_PROC_BROWSER_TEST_P(TwoClientWalletSyncTest,
+                       UpdateServerAddressMetadataConflictsWhileNotSyncing) {
+  InitWithDefaultFeatures();
+
+  GetFakeServer()->SetWalletData(
+      {CreateSyncWalletAddress(/*name=*/"address-1", /*company=*/"Company-1"),
+       CreateDefaultSyncPaymentsCustomerData()});
+  ASSERT_TRUE(SetupSync());
+
+  // Sumulate going offline on both clients.
+  fake_server::FakeServerHttpPostProvider::DisableNetwork();
+
+  // Increase use stats on both clients, make use count higher on the first
+  // client and use date higher on the second client.
+  std::vector<AutofillProfile*> server_addresses = GetServerProfiles(0);
+  ASSERT_EQ(1u, server_addresses.size());
+  AutofillProfile address = *server_addresses[0];
+  ASSERT_EQ(1u, address.use_count());
+  address.set_use_count(3);
+  base::Time lower_new_use_date = base::Time::Now();
+  ASSERT_NE(lower_new_use_date, address.use_date());
+  address.set_use_date(lower_new_use_date);
+  UpdateServerAddressMetadata(0, address);
+
+  server_addresses = GetServerProfiles(1);
+  ASSERT_EQ(1u, server_addresses.size());
+  address = *server_addresses[0];
+  ASSERT_EQ(1u, address.use_count());
+  address.set_use_count(2);
+  base::Time higher_new_use_date = base::Time::Now();
+  ASSERT_NE(higher_new_use_date, address.use_date());
+  ASSERT_LT(lower_new_use_date, higher_new_use_date);
+  address.set_use_date(higher_new_use_date);
+  UpdateServerAddressMetadata(1, address);
+
+  // Simulate going online again.
+  fake_server::FakeServerHttpPostProvider::EnableNetwork();
+  net::NetworkChangeNotifier::NotifyObserversOfNetworkChangeForTests(
+      net::NetworkChangeNotifier::CONNECTION_ETHERNET);
+
+  // Wait for the clients to coverge and both resolve the conflicts by taking
+  // maxima in both components.
+  EXPECT_TRUE(AutofillWalletChecker(0, 1).Wait());
+
+  server_addresses = GetServerProfiles(0);
+  EXPECT_EQ(1U, server_addresses.size());
+  EXPECT_EQ(3u, server_addresses[0]->use_count());
+  EXPECT_EQ(higher_new_use_date, server_addresses[0]->use_date());
+  server_addresses = GetServerProfiles(1);
+  EXPECT_EQ(1U, server_addresses.size());
+  EXPECT_EQ(3u, server_addresses[0]->use_count());
+  EXPECT_EQ(higher_new_use_date, server_addresses[0]->use_date());
+}
+
 IN_PROC_BROWSER_TEST_P(TwoClientWalletSyncTest,
                        UpdateCreditCardMetadataWithNewBillingAddressId) {
   InitWithDefaultFeatures();
@@ -263,6 +468,48 @@ IN_PROC_BROWSER_TEST_P(
   EXPECT_EQ(kLocalBillingAddressId, credit_cards[0]->billing_address_id());
 }
 
+IN_PROC_BROWSER_TEST_P(
+    TwoClientWalletSyncTest,
+    UpdateCreditCardMetadataWithChangedBillingAddressId_LocalToRemoteOffline) {
+  InitWithDefaultFeatures();
+
+  GetFakeServer()->SetWalletData(
+      {CreateSyncWalletCard(/*name=*/"card-1", /*last_four=*/"0001",
+                            kLocalBillingAddressId),
+       CreateDefaultSyncPaymentsCustomerData()});
+  ASSERT_TRUE(SetupSync());
+
+  // Sumulate going offline on both clients.
+  fake_server::FakeServerHttpPostProvider::DisableNetwork();
+
+  // Grab the current card on the first client.
+  std::vector<CreditCard*> credit_cards = GetServerCreditCards(0);
+  ASSERT_EQ(1U, credit_cards.size());
+  CreditCard card = *credit_cards[0];
+
+  // Update the billing address (replace a local profile by a remote profile).
+  ASSERT_EQ(kLocalBillingAddressId, card.billing_address_id());
+  card.set_billing_address_id(kDifferentBillingAddressId);
+  UpdateServerCardMetadata(0, card);
+
+  // Simulate going online again.
+  fake_server::FakeServerHttpPostProvider::EnableNetwork();
+  net::NetworkChangeNotifier::NotifyObserversOfNetworkChangeForTests(
+      net::NetworkChangeNotifier::CONNECTION_ETHERNET);
+
+  EXPECT_TRUE(AutofillWalletChecker(0, 1).Wait());
+
+  // Make sure both clients have the original billing_address_id (local profile
+  // wins).
+  credit_cards = GetServerCreditCards(1);
+  EXPECT_EQ(1U, credit_cards.size());
+  EXPECT_EQ(kLocalBillingAddressId, credit_cards[0]->billing_address_id());
+
+  credit_cards = GetServerCreditCards(0);
+  EXPECT_EQ(1U, credit_cards.size());
+  EXPECT_EQ(kLocalBillingAddressId, credit_cards[0]->billing_address_id());
+}
+
 IN_PROC_BROWSER_TEST_P(TwoClientWalletSyncTest,
                        ServerAddressConvertsToSameLocalAddress) {
   InitWithDefaultFeatures();

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

@@ -83,7 +83,8 @@ bool ListsMatch(int profile_a,
     }
     Item* expected_item = &list_a_map[item->server_id()];
     if (expected_item->Compare(*item) != 0 ||
-        expected_item->use_count() != item->use_count()) {
+        expected_item->use_count() != item->use_count() ||
+        expected_item->use_date() != item->use_date()) {
       DVLOG(1) << "Mismatch in profile with server_id " << item->server_id()
                << ".";
       return false;

components/autofill/core/browser/credit_card.cc

@@ -1027,8 +1027,10 @@ std::ostream& operator<<(std::ostream& os, const CreditCard& credit_card) {
             << base::UTF16ToUTF8(credit_card.GetRawInfo(CREDIT_CARD_NUMBER))
             << " "
             << base::UTF16ToUTF8(credit_card.GetRawInfo(CREDIT_CARD_EXP_MONTH))
-            << " " << base::UTF16ToUTF8(
-                          credit_card.GetRawInfo(CREDIT_CARD_EXP_4_DIGIT_YEAR));
+            << " "
+            << base::UTF16ToUTF8(
+                   credit_card.GetRawInfo(CREDIT_CARD_EXP_4_DIGIT_YEAR))
+            << " " << credit_card.use_count() << " " << credit_card.use_date();
 }
 
 void CreditCard::SetNameOnCardFromSeparateParts() {