[Payments] Normalize billing address before sending to the merchant.

chrome/browser/autofill/android/personal_data_manager_android.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 "chrome/browser/autofill/android/personal_data_manager_android.h"
 
 #include <stddef.h>
 #include <algorithm>
 #include <memory>
 #include <utility>
@@ skipping 249 matching lines @@
     delete this;
   }
 
   std::unique_ptr<CreditCard> card_;
   ScopedJavaGlobalRef<jobject> jdelegate_;
 
   DISALLOW_COPY_AND_ASSIGN(FullCardRequester);
 };
 
 // Self-deleting requester of address normalization.
 class AddressNormalizationRequester

[please use gerrit instead, 2016/10/18 17:30:32]

personal_data_manager_android controls the life-cycle of this object in
entirety, because only personal_data_manager_android is the one that calls
AddressNormalizationRequester::OnRulesSuccessfullyLoaded(). Therefore, let's not
make this object self-deleting. (Sorry for the "drive-by".) Instead, make
personal_data_manager_android own this object. The easiest way to accomplish
that is through two steps:

(1) Remove "delete this" on line 298.

(2) Change container data structure for these objects to use smart pointers
std::unique_ptr<Delegate> instead of raw pointers Delegate*. Smart pointers
self-delete when they go out of scope. They also self-delete when you remove
them from their container.

[sebsg, 2016/10/18 22:44:32]

Done.

     : public PersonalDataManagerAndroid::Delegate,
       public base::SupportsWeakPtr<AddressNormalizationRequester> {
  public:
   AddressNormalizationRequester(
       JNIEnv* env,
       const base::android::JavaParamRef<jobject>& jdelegate,
       const std::string& region_code,
       const std::string& guid,
       base::WeakPtr<PersonalDataManagerAndroid> personal_data_manager_android) {
     jdelegate_.Reset(env, jdelegate);
     region_code_ = region_code;
     guid_ = guid;
     personal_data_manager_android_ = personal_data_manager_android;
     env_ = env;
   }
 
  private:
-  virtual ~AddressNormalizationRequester() {}
+  ~AddressNormalizationRequester() override {}
 
   void OnRulesSuccessfullyLoaded() override {
     if (personal_data_manager_android_) {
       JNIEnv* env = base::android::AttachCurrentThread();
       Java_NormalizedAddressRequestDelegate_onAddressNormalized(
           env, jdelegate_, personal_data_manager_android_->NormalizeAddress(
                                guid_, region_code_, env));
     }
 
     delete this;
@@ skipping 274 matching lines @@
     const JavaParamRef<jobject>& unused_obj,
     const JavaParamRef<jstring>& jguid,
     jint count,
     jint date) {
   DCHECK(count >= 0 && date >= 0);
 
   AutofillProfile* profile = personal_data_manager_->GetProfileByGUID(
       ConvertJavaStringToUTF8(env, jguid));
   profile->set_use_count(static_cast<size_t>(count));
   profile->set_use_date(base::Time::FromTimeT(date));
+
+  personal_data_manager_->UpdateProfileForTest(*profile);

[please use gerrit instead, 2016/10/18 17:30:32]

No tests here, so let's not add this call until we know for sure that we need
it.

[sebsg, 2016/10/18 22:44:32]

Done.

   personal_data_manager_->NotifyPersonalDataChangedForTest();
 }
 
 jint PersonalDataManagerAndroid::GetProfileUseCountForTesting(
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& unused_obj,
     const base::android::JavaParamRef<jstring>& jguid) {
   AutofillProfile* profile = personal_data_manager_->GetProfileByGUID(
       ConvertJavaStringToUTF8(env, jguid));
   return profile->use_count();
@@ skipping 23 matching lines @@
     const JavaParamRef<jobject>& unused_obj,
     const JavaParamRef<jstring>& jguid,
     jint count,
     jint date) {
   DCHECK(count >= 0 && date >= 0);
 
   CreditCard* card = personal_data_manager_->GetCreditCardByGUID(
       ConvertJavaStringToUTF8(env, jguid));
   card->set_use_count(static_cast<size_t>(count));
   card->set_use_date(base::Time::FromTimeT(date));
+
+  personal_data_manager_->UpdateCreditCardForTest(*card);

[please use gerrit instead, 2016/10/18 17:30:32]

Ditto

[sebsg, 2016/10/18 22:44:32]

Done.

   personal_data_manager_->NotifyPersonalDataChangedForTest();
 }
 
 jint PersonalDataManagerAndroid::GetCreditCardUseCountForTesting(
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& unused_obj,
     const base::android::JavaParamRef<jstring>& jguid) {
   CreditCard* card = personal_data_manager_->GetCreditCardByGUID(
       ConvertJavaStringToUTF8(env, jguid));
   return card->use_count();
@@ skipping 19 matching lines @@
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& unused_obj,
     const base::android::JavaParamRef<jstring>& jregion_code) {
   address_validator_.LoadRules(ConvertJavaStringToUTF8(env, jregion_code));
 }
 
 void PersonalDataManagerAndroid::OnAddressValidationRulesLoaded(
     const std::string& region_code,
     bool success) {
   // Check if an address normalization is pending.
-  std::map<std::string, Delegate*>::iterator it =
+  std::map<std::string, std::vector<Delegate*>>::iterator it =

[please use gerrit instead, 2016/10/18 17:30:32]

s/std::map<std::string, std::vector<Delegate*>>::iterator/auto/

[sebsg, 2016/10/18 22:44:32]

Done.

       pending_normalization_.find(region_code);
   if (it != pending_normalization_.end()) {
-    // The Delegate will self delete after normalizing.
-    it->second->OnRulesSuccessfullyLoaded();
+    // Each Delegate will self delete after normalizing.

[please use gerrit instead, 2016/10/18 17:30:32]

Remove this comment, please.

[sebsg, 2016/10/18 22:44:32]

Done.

+    std::vector<Delegate*> delegates = it->second;

[please use gerrit instead, 2016/10/18 17:30:32]

std::vector<std::unique_ptr<Delegate>>

[sebsg, 2016/10/18 22:44:32]

Done.

+    for (size_t i = 0; i < it->second.size(); ++i)

[please use gerrit instead, 2016/10/18 17:30:32]

s/it->second/delegates/

[sebsg, 2016/10/18 22:44:32]

Done.

+      it->second[i]->OnRulesSuccessfullyLoaded();

[please use gerrit instead, 2016/10/18 17:30:32]

s/it->second/delegates/

[sebsg, 2016/10/18 22:44:32]

Done.

     pending_normalization_.erase(it);

[please use gerrit instead, 2016/10/18 17:30:32]

This is the line that deletes the Delegate objects. Or, at least, it should. Put
a LOG(ERROR)<<"DELETE"; in the destructor of AddressNormalizationRequester
temporarily to verify.

[sebsg, 2016/10/18 22:44:32]

Done.

   }
 }
 
 jboolean PersonalDataManagerAndroid::StartAddressNormalization(
     JNIEnv* env,
     const JavaParamRef<jobject>& unused_obj,
     const JavaParamRef<jstring>& jguid,
     const JavaParamRef<jstring>& jregion_code,
     const JavaParamRef<jobject>& jdelegate) {
   const std::string region_code = ConvertJavaStringToUTF8(env, jregion_code);
   const std::string guid = ConvertJavaStringToUTF8(env, jguid);
 
   Delegate* requester = new AddressNormalizationRequester(

[please use gerrit instead, 2016/10/18 17:30:32]

std::unique_ptr<Delegate> requester = ...

[sebsg, 2016/10/18 22:44:32]

Done.

       env, jdelegate, region_code, guid, AsWeakPtr());
 
   // Check if the rules are already loaded.
   if (AreRulesLoadedForRegion(region_code)) {
     requester->OnRulesSuccessfullyLoaded();
     return false;
   } else {
     // Setup the variables so the profile gets normalized when the rules have
     // finished loading.
-    pending_normalization_.insert(
-        std::pair<std::string, Delegate*>(region_code, requester));
+    if (pending_normalization_.find(region_code) ==
+        pending_normalization_.end()) {
+      pending_normalization_.insert(
+          std::pair<std::string, std::vector<Delegate*>>(

[please use gerrit instead, 2016/10/18 17:30:32]

pending_normalization_.insert(std::make_pair(region_code,
{std::move(requester)}));

{requester} is "uniform initialization syntax". It's a C++11 feature that's
allowed in Chromium: https://chromium-cpp.appspot.com/. If it does not work for
some reason, then do this instead:

pending_normalization_.insert(std::make_pair(region_code,
std::vector<Delegate*>(1, std::move(requester))));

In either case, you don't need the extra .find() and .push_back() calls on lines
706-708.

[sebsg, 2016/10/18 22:44:32]

It seems you cannot use initializer lists with move semantics (it takes
everything as a const ref) But I got rid of the second find.

+              region_code, std::vector<Delegate*>()));
+    }
+
+    std::map<std::string, std::vector<Delegate*>>::iterator it =
+        pending_normalization_.find(region_code);
+    it->second.push_back(requester);
+
     return true;
   }
 }
 
 ScopedJavaLocalRef<jobject> PersonalDataManagerAndroid::NormalizeAddress(
     const std::string& guid,
     const std::string& region_code,
     JNIEnv* env) {
   AutofillProfile* profile = personal_data_manager_->GetProfileByGUID(guid);
 
@@ skipping 11 matching lines @@
                         base::UTF8ToUTF16(address_data.administrative_area));
     profile->SetRawInfo(ADDRESS_HOME_CITY,
                         base::UTF8ToUTF16(address_data.locality));
     profile->SetRawInfo(ADDRESS_HOME_DEPENDENT_LOCALITY,
                         base::UTF8ToUTF16(address_data.dependent_locality));
   }
 
   return CreateJavaProfileFromNative(env, *profile);
 }
 
-void PersonalDataManagerAndroid::CancelPendingAddressNormalization(
+void PersonalDataManagerAndroid::CancelPendingAddressNormalizations(
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& unused_obj) {
+  // Delete all the pending normalization delegates.
+  for (auto it = pending_normalization_.begin();

[please use gerrit instead, 2016/10/18 17:30:32]

Since PersonalDataManagerAndroid now owns all of the Delegate objects, simply
clearning the pending_normalization_ container will delete all of the Delegate
objects inside of it.

[sebsg, 2016/10/18 22:44:32]

Done.

+       it != pending_normalization_.end(); it++) {
+    for (size_t i = 0; i < it->second.size(); ++i) {
+      delete it->second[i];
+    }
+  }
+
   pending_normalization_.clear();
 }
 
 ScopedJavaLocalRef<jobjectArray> PersonalDataManagerAndroid::GetProfileGUIDs(
     JNIEnv* env,
     const std::vector<AutofillProfile*>& profiles) {
   std::vector<base::string16> guids;
   for (AutofillProfile* profile : profiles)
     guids.push_back(base::UTF8ToUTF16(profile->guid()));
 
@@ skipping 92 matching lines @@
                base::android::ConvertJavaStringToUTF16(env, jcountry_name)));
 }
 
 static jlong Init(JNIEnv* env, const JavaParamRef<jobject>& obj) {
   PersonalDataManagerAndroid* personal_data_manager_android =
       new PersonalDataManagerAndroid(env, obj);
   return reinterpret_cast<intptr_t>(personal_data_manager_android);
 }
 
 }  // namespace autofill