[Payments] Update server card billing if the address has already converted

components/autofill/core/browser/personal_data_manager.cc

Index: components/autofill/core/browser/personal_data_manager.cc
diff --git a/components/autofill/core/browser/personal_data_manager.cc b/components/autofill/core/browser/personal_data_manager.cc
index 590f814d6073c07ed0ea15fb6515ecb2bb0dbd9d..0c9735ecfe6e756fd199780f5dfee8176a6e4ac2 100644
--- a/components/autofill/core/browser/personal_data_manager.cc
+++ b/components/autofill/core/browser/personal_data_manager.cc
@@ -55,6 +55,8 @@ using ::i18n::addressinput::AddressField;
 using ::i18n::addressinput::GetStreetAddressLinesAsSingleLine;
 using ::i18n::addressinput::STREET_ADDRESS;
 
+const int LOCAL_GUID_LENGTH = 36;

[Mathieu, 2017/03/06 20:40:34]

comment?

[sebsg, 2017/03/06 21:26:40]

Done.

+
 template<typename T>
 class FormGroupMatchesByGUIDFunctor {
  public:
@@ -1249,7 +1251,7 @@ void PersonalDataManager::NotifyPersonalDataChanged() {
   // If new data was synced, try to convert new server profiles.

[Mathieu, 2017/03/06 20:40:34]

update comment

[sebsg, 2017/03/06 21:26:39]

Done.

   if (has_synced_new_data_) {
     has_synced_new_data_ = false;
-    ConvertWalletAddressesToLocalProfiles();
+    ConvertWalletAddressesAndUpdateWalletCards();
   }
 }
 
@@ -1850,7 +1852,7 @@ void PersonalDataManager::UpdateCardsBillingAddressReference(
   }
 }
 
-void PersonalDataManager::ConvertWalletAddressesToLocalProfiles() {
+void PersonalDataManager::ConvertWalletAddressesAndUpdateWalletCards() {
   // Copy the local profiles into a vector<AutofillProfile>. Theses are the
   // existing profiles. Get them sorted in decreasing order of frecency, so the
   // "best" profiles are checked first. Put the verified profiles last so the
@@ -1861,22 +1863,54 @@ void PersonalDataManager::ConvertWalletAddressesToLocalProfiles() {
     local_profiles.push_back(*existing_profile);
   }
 
+  // Since we are already iterating on all the server profiles to convert Wallet
+  // address and we will need to access them by guid later to update the Wallet

[Mathieu, 2017/03/06 20:40:34]

*addresses

[sebsg, 2017/03/06 21:26:39]

Done.

+  // cards, create a map here.
+  std::unordered_map<std::string, AutofillProfile*> server_profiles_map;

[Mathieu, 2017/03/06 20:40:34]

|server_id_profile_map| is clearer?

[sebsg, 2017/03/06 21:26:39]

Done.

+
   // Create the map used to update credit card's billing addresses after the
   // convertion/merge.
   std::unordered_map<std::string, std::string> guids_merge_map;
 
+  bool has_converted_addresses = ConvertWalletAddressesToLocalProfiles(
+      &local_profiles, &server_profiles_map, &guids_merge_map);
+  bool should_update_cards = UpdateWalletCardsAlreadyConvertedBillingAddresses(
+      &local_profiles, &server_profiles_map, &guids_merge_map);
+
+  if (has_converted_addresses) {
+    // Save the local profiles to the DB.
+    SetProfiles(&local_profiles);
+  }
+
+  if (should_update_cards || has_converted_addresses) {
+    // Update the credit cards billing address relationship.
+    UpdateCardsBillingAddressReference(guids_merge_map);
+
+    // Force a reload of the profiles and cards.
+    Refresh();
+  }
+}
+
+bool PersonalDataManager::ConvertWalletAddressesToLocalProfiles(
+    std::vector<AutofillProfile>* local_profiles,
+    std::unordered_map<std::string, AutofillProfile*>* server_profiles_map,
+    std::unordered_map<std::string, std::string>* guids_merge_map) {
   bool has_converted_addresses = false;
   for (std::unique_ptr<AutofillProfile>& wallet_address : server_profiles_) {
+    // Add the profile to the map.
+    server_profiles_map->insert(std::pair<std::string, AutofillProfile*>(

[Mathieu, 2017/03/06 20:40:34]

std::make_pair?

[sebsg, 2017/03/06 21:26:40]

Done.

+        wallet_address->server_id(), wallet_address.get()));
+
     // If the address has not been converted yet, convert it.
     if (!wallet_address->has_converted()) {
       // Try to merge the server address into a similar local profile, or create
       // a new local profile if no similar profile is found.
       std::string address_guid =
-          MergeServerAddressesIntoProfiles(*wallet_address, &local_profiles);
+          MergeServerAddressesIntoProfiles(*wallet_address, local_profiles);
 
       // Update the map to transfer the billing address relationship from the
       // server address to the converted/merged local profile.
-      guids_merge_map.insert(std::pair<std::string, std::string>(
+      guids_merge_map->insert(std::pair<std::string, std::string>(
           wallet_address->server_id(), address_guid));
 
       // Update the wallet addresses metadata to record the conversion.
@@ -1887,16 +1921,54 @@ void PersonalDataManager::ConvertWalletAddressesToLocalProfiles() {
     }
   }
 
-  if (has_converted_addresses) {
-    // Save the local profiles to the DB.
-    SetProfiles(&local_profiles);
-
-    // Update the credit cards billing address relationship.
-    UpdateCardsBillingAddressReference(guids_merge_map);
+  return has_converted_addresses;
+}
 
-    // Force a reload of the profiles and cards.
-    Refresh();
+bool PersonalDataManager::UpdateWalletCardsAlreadyConvertedBillingAddresses(
+    std::vector<AutofillProfile>* local_profiles,
+    std::unordered_map<std::string, AutofillProfile*>* server_profiles_map,
+    std::unordered_map<std::string, std::string>* guids_merge_map) {
+  // Look for server cards that still refer to server addresses but for which
+  // there is no mapping. This can happen if it's a new card for which the
+  // billing address has already been converted. This should be a no-op for most
+  // situations. Otherwise, it should affect only one Wallet card, sinces users
+  // do not add a lot of credit cards.
+  AutofillProfileComparator comparator(app_locale_);
+  bool should_update_cards = false;
+  for (std::unique_ptr<CreditCard>& wallet_card : server_credit_cards_) {
+    std::string billing_address_id = wallet_card->billing_address_id();
+
+    // If billing address refers to a server id and that id is not a key in the
+    // guids_merge_map, it means that the card is new but the address was
+    // already converted. Look for the matching converted profile.
+    if (!billing_address_id.empty() &&
+        billing_address_id.length() != LOCAL_GUID_LENGTH &&
+        guids_merge_map->find(billing_address_id) == guids_merge_map->end()) {
+      // Get the profile.
+      auto it = server_profiles_map->find(billing_address_id);
+      DCHECK(it != server_profiles_map->end());
+      if (it != server_profiles_map->end()) {
+        AutofillProfile* billing_address = it->second;
+
+        // Look for a matching local profile (DO NOT MERGE).
+        bool matching_profile_found = false;
+        for (auto& local_profile : *local_profiles) {
+          if (!matching_profile_found &&
+              comparator.AreMergeable(*billing_address, local_profile)) {
+            matching_profile_found = true;
+
+            // The Wallet address matches this local profile. Add this to the
+            // merge mapping.
+            guids_merge_map->insert(std::pair<std::string, std::string>(
+                billing_address_id, local_profile.guid()));
+            should_update_cards = true;
+          }
+        }
+      }
+    }
   }
+
+  return should_update_cards;
 }
 
 // TODO(crbug.com/687975): Reuse MergeProfiles in this function.