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

components/autofill/core/browser/personal_data_manager.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/autofill/core/browser/personal_data_manager.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <list>
@@ skipping 37 matching lines @@
 #include "third_party/libaddressinput/src/cpp/include/libaddressinput/address_data.h"
 #include "third_party/libaddressinput/src/cpp/include/libaddressinput/address_formatter.h"
 
 namespace autofill {
 namespace {
 
 using ::i18n::addressinput::AddressField;
 using ::i18n::addressinput::GetStreetAddressLinesAsSingleLine;
 using ::i18n::addressinput::STREET_ADDRESS;
 
+// The length of a local profile GUID.
+const int LOCAL_GUID_LENGTH = 36;
+
 template<typename T>
 class FormGroupMatchesByGUIDFunctor {
  public:
   explicit FormGroupMatchesByGUIDFunctor(const std::string& guid)
       : guid_(guid) {
   }
 
   bool operator()(const T& form_group) {
     return form_group.guid() == guid_;
   }
@@ skipping 1171 matching lines @@
   std::string guid =
       MergeProfile(imported_profile, &web_profiles_, app_locale_, &profiles);
   SetProfiles(&profiles);
   return guid;
 }
 
 void PersonalDataManager::NotifyPersonalDataChanged() {
   for (PersonalDataManagerObserver& observer : observers_)
     observer.OnPersonalDataChanged();
 
-  // If new data was synced, try to convert new server profiles.
+  // If new data was synced, try to convert new server profiles and update
+  // server cards.
   if (has_synced_new_data_) {
     has_synced_new_data_ = false;
-    ConvertWalletAddressesToLocalProfiles();
+    ConvertWalletAddressesAndUpdateWalletCards();
   }
 }
 
 std::string PersonalDataManager::SaveImportedCreditCard(
     const CreditCard& imported_card) {
   DCHECK(!imported_card.number().empty());
   if (is_off_the_record_)
     return std::string();
 
   // Set to true if |imported_card| is merged into the credit card list.
@@ skipping 580 matching lines @@
     // If the card was modified, apply the changes to the database.
     if (was_modified) {
       if (credit_card->record_type() == CreditCard::LOCAL_CARD)
         database_->UpdateCreditCard(*credit_card);
       else
         database_->UpdateServerCardMetadata(*credit_card);
     }
   }
 }
 
-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
   // server addresses have a chance to merge into the non-verified local
   // profiles.
   std::vector<AutofillProfile> local_profiles;
   for (AutofillProfile* existing_profile : GetProfilesToSuggest()) {
     local_profiles.push_back(*existing_profile);
   }
 
+  // Since we are already iterating on all the server profiles to convert Wallet
+  // addresses and we will need to access them by guid later to update the
+  // Wallet cards, create a map here.
+  std::unordered_map<std::string, AutofillProfile*> server_id_profiles_map;
+
   // 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_id_profiles_map, &guids_merge_map);
+  bool should_update_cards = UpdateWalletCardsAlreadyConvertedBillingAddresses(
+      &local_profiles, &server_id_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_id_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_id_profiles_map->insert(
+        std::make_pair(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.
       wallet_address->set_has_converted(true);
       database_->UpdateServerAddressMetadata(*wallet_address);
 
       has_converted_addresses = true;
     }
   }
 
-  if (has_converted_addresses) {
-    // Save the local profiles to the DB.
-    SetProfiles(&local_profiles);
+  return has_converted_addresses;
+}
 
-    // Update the credit cards billing address relationship.
-    UpdateCardsBillingAddressReference(guids_merge_map);
+bool PersonalDataManager::UpdateWalletCardsAlreadyConvertedBillingAddresses(
+    std::vector<AutofillProfile>* local_profiles,
+    std::unordered_map<std::string, AutofillProfile*>* server_id_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();
 
-    // Force a reload of the profiles and cards.
-    Refresh();
+    // 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_id_profiles_map->find(billing_address_id);
+      DCHECK(it != server_id_profiles_map->end());

[Theresa, 2017/03/07 21:58:17]

I synced this morning and now I'm getting crashes on this DCHECK:

[FATAL:personal_data_manager.cc(1955)] Check failed: it !=
server_id_profiles_map->end().

             
/work/clankium/src/components/autofill/core/browser/personal_data_manager.cc:1955
  00951be5  ConvertWalletAddressesAndUpdateWalletCards                          
                                                                                
                                                                                
                                                                                
                                                                                
                                                                                
                                                          
/work/clankium/src/components/autofill/core/browser/personal_data_manager.cc:1883
  0022897b  NotifyPersonalDataChanged                                           
                                                                                
                                                                                
                                                                                
                                                                                
                                                                                
                                                          
/work/clankium/src/components/autofill/core/browser/personal_data_manager.cc:1260
  0022840d  OnWebDataServiceRequestDone                                         
                                                                                
                                                                                
                                                                                
                                                                                
                                                                                
                                                          
/work/clankium/src/components/autofill/core/browser/personal_data_manager.cc:410
  00004659  RequestCompletedOnThread                                            
                                                                                
                                                                                
                                                                                
                                                                                
                                                                                
                                                          
/work/clankium/src/components/webdata/common/web_data_request_manager.cc:177
  00004eff  Invoke<const scoped_refptr<WebDataRequestManager> &,
std::__ndk1::unique_ptr<WebDataRequest,
std::__ndk1::default_delete<WebDataRequest> >,
std::__ndk1::unique_ptr<WDTypedResult,
std::__ndk1::default_delete<WDTypedResult> > >                                  
                                                                                
                                                                                
                                                                                
                              /work/clankium/src/base/bind_internal.h:214
  v------>  MakeItSo<void (WebDataRequestManager::*const
&)(std::__ndk1::unique_ptr<WebDataRequest,
std::__ndk1::default_delete<WebDataRequest> >,
std::__ndk1::unique_ptr<WDTypedResult,
std::__ndk1::default_delete<WDTypedResult> >), const
scoped_refptr<WebDataRequestManager> &, std::__ndk1::unique_ptr<WebDataRequest,
std::__ndk1::default_delete<WebDataRequest> >,
std::__ndk1::unique_ptr<WDTypedResult,
std::__ndk1::default_delete<WDTypedResult> > >                                  
                                                          
/work/clankium/src/base/bind_internal.h:285
  v------>  RunImpl<void (WebDataRequestManager::*const
&)(std::__ndk1::unique_ptr<WebDataRequest,
std::__ndk1::default_delete<WebDataRequest> >,
std::__ndk1::unique_ptr<WDTypedResult,
std::__ndk1::default_delete<WDTypedResult> >), const
std::__ndk1::tuple<scoped_refptr<WebDataRequestManager>,
base::internal::PassedWrapper<std::__ndk1::unique_ptr<WebDataRequest,
std::__ndk1::default_delete<WebDataRequest> > >,
base::internal::PassedWrapper<std::__ndk1::unique_ptr<WDTypedResult,
std::__ndk1::default_delete<WDTypedResult> > > > &, 0, 1, 2> 
/work/clankium/src/base/bind_internal.h:361
  00004e9f  Run                                                                 
                                                                                
                                                                                
                                                                                
                                                                                
                                                                                
                                                          
/work/clankium/src/base/bind_internal.h:339
  0008d523  Run                                                                 
                                                                                
                                                                                
                                                                                
                                                                                
                                                                                
                                                          
/work/clankium/src/base/callback.h:68
  0008d409  RunTask                                                             
                                                                                
                                                                                
                                                                                
                                                                                
                                                                                
                                                          
/work/clankium/src/base/debug/task_annotator.cc:59
  000a7f61  RunTask                                                             
                                                                                
                                                                                
                                                                                
                                                                                
                                                                                
                                                          
/work/clankium/src/base/message_loop/message_loop.cc:423
  000a81bf  DeferOrRunPendingTask                                               
                                                                                
                                                                                
                                                                                
                                                                                
                                                                                
                                                          
/work/clankium/src/base/message_loop/message_loop.cc:434
  000a8389  DoWork                                                              
                                                                                
                                                                                
                                                                                
                                                                                
                                                                                
                                                          
/work/clankium/src/base/message_loop/message_loop.cc:527
  v------>  DoRunLoopOnce                                                       
                                                                                
                                                                                
                                                                                
                                                                                
                                                                                
                                                          
/work/clankium/src/base/message_loop/message_pump_android.cc:44
  000a94d1  Java_org_chromium_base_SystemMessageHandler_nativeDoRunLoopOnce     
                                                                                
                                                                                
                                                                                
                                                                                
                                                                                
                                                          
/work/clankium/src/out-gn/Debug/gen/base/base_jni_headers/base/jni/SystemMessageHandler_jni.h:44
  000161bd  offset 0x36000                                                      
                                                                                
                                                                                
                                                                                
                                                                                
                                                                                
                                                          
/data/data/com.google.android.apps.chrome/incremental-install-files/optimized-dexes/base.base_java.dex.dex

[sebsg, 2017/03/07 22:20:11]

I am very curious on how it could trigger. Can we move the discussion to
crbug.com/698364 ? I would like to try to repro

+      if (it != server_id_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.
 std::string PersonalDataManager::MergeServerAddressesIntoProfiles(
     const AutofillProfile& server_address,
     std::vector<AutofillProfile>* existing_profiles) {
   // Set to true if |existing_profiles| already contains an equivalent profile.
   bool matching_profile_found = false;
   std::string guid = server_address.guid();
 
@@ skipping 29 matching lines @@
       existing_profiles->back().SetRawInfo(EMAIL_ADDRESS, email);
 
     AutofillMetrics::LogWalletAddressConversionType(
         AutofillMetrics::CONVERTED_ADDRESS_ADDED);
   }
 
   return guid;
 }
 
 }  // namespace autofill