[Payments] Code Refactoring for the Subkey Request

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 290 matching lines @@
 
  private:
   ~AndroidAddressNormalizerDelegate() override {}
 
   ScopedJavaGlobalRef<jobject> jdelegate_;
 
   DISALLOW_COPY_AND_ASSIGN(AndroidAddressNormalizerDelegate);
 };
 
 class AndroidSubKeyRequesterDelegate
-    : public PersonalDataManagerAndroid::SubKeyRequestDelegate,
+    : public ::payments::SubKeyRequester::Delegate,
       public base::SupportsWeakPtr<AndroidSubKeyRequesterDelegate> {
  public:
   AndroidSubKeyRequesterDelegate(
       JNIEnv* env,
-      const base::android::JavaParamRef<jobject>& jdelegate,
-      const std::string& region_code,
-      base::WeakPtr<PersonalDataManagerAndroid> personal_data_manager_android) {
+      const base::android::JavaParamRef<jobject>& jdelegate) {
     jdelegate_.Reset(env, jdelegate);
-    region_code_ = region_code;
-    personal_data_manager_android_ = personal_data_manager_android;
   }
 
+  void OnSubKeysReceived(std::vector<std::string> sub_keys) override {
+    JNIEnv* env = base::android::AttachCurrentThread();
+    Java_GetSubKeysRequestDelegate_onSubKeysReceived(
+        env, jdelegate_, base::android::ToJavaArrayOfStrings(env, sub_keys));
+    delete this;
+  }
+
+ private:
   ~AndroidSubKeyRequesterDelegate() override {}
 
- private:
-  // PersonalDataManagerAndroid::SubKeyRequestDelegate:
-  void OnRulesSuccessfullyLoaded() override {
-    if (personal_data_manager_android_) {
-      JNIEnv* env = base::android::AttachCurrentThread();
-      Java_GetSubKeysRequestDelegate_onSubKeysReceived(
-          env, jdelegate_,
-          personal_data_manager_android_->GetSubKeys(env, region_code_));
-    }
-  }
-
   ScopedJavaGlobalRef<jobject> jdelegate_;
-  std::string region_code_;
-  base::WeakPtr<PersonalDataManagerAndroid> personal_data_manager_android_;
 
   DISALLOW_COPY_AND_ASSIGN(AndroidSubKeyRequesterDelegate);
 };
 
 }  // namespace
 
 PersonalDataManagerAndroid::PersonalDataManagerAndroid(JNIEnv* env, jobject obj)
     : weak_java_obj_(env, obj),
       personal_data_manager_(PersonalDataManagerFactory::GetForProfile(
           ProfileManager::GetActiveUserProfile())),
       address_normalizer_(
           std::unique_ptr<::i18n::addressinput::Source>(
               new autofill::ChromeMetadataSource(
                   I18N_ADDRESS_VALIDATION_DATA_URL,
                   personal_data_manager_->GetURLRequestContextGetter())),
           ValidationRulesStorageFactory::CreateStorage()),
-      address_validator_(
+      subkey_requester_(
           base::MakeUnique<autofill::ChromeMetadataSource>(
               I18N_ADDRESS_VALIDATION_DATA_URL,
               personal_data_manager_->GetURLRequestContextGetter()),
-          ValidationRulesStorageFactory::CreateStorage(),
-          this) {
+          ValidationRulesStorageFactory::CreateStorage()) {
   personal_data_manager_->AddObserver(this);
 }
 
 PersonalDataManagerAndroid::~PersonalDataManagerAndroid() {
   personal_data_manager_->RemoveObserver(this);
 }
 
 jboolean PersonalDataManagerAndroid::IsDataLoaded(
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& unused_obj) const {
@@ skipping 373 matching lines @@
     const base::android::JavaParamRef<jobject>& unused_obj,
     const base::android::JavaParamRef<jstring>& jregion_code) {
   address_normalizer_.LoadRulesForRegion(
       ConvertJavaStringToUTF8(env, jregion_code));
 }
 
 void PersonalDataManagerAndroid::LoadRulesForSubKeys(
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& unused_obj,
     const base::android::JavaParamRef<jstring>& jregion_code) {
-  address_validator_.LoadRules(ConvertJavaStringToUTF8(env, jregion_code));
+  subkey_requester_.LoadRulesForRegion(
+      ConvertJavaStringToUTF8(env, jregion_code));
 }
 
 void PersonalDataManagerAndroid::StartAddressNormalization(
     JNIEnv* env,
     const JavaParamRef<jobject>& unused_obj,
     const JavaParamRef<jobject>& jprofile,
     const JavaParamRef<jstring>& jregion_code,
     jint jtimeout_seconds,
     const JavaParamRef<jobject>& jdelegate) {
   const std::string region_code = ConvertJavaStringToUTF8(env, jregion_code);
@@ skipping 15 matching lines @@
     const base::android::JavaParamRef<jobject>& unused_obj) {
   return !personal_data_manager_->GetProfiles().empty();
 }
 
 jboolean PersonalDataManagerAndroid::HasCreditCards(
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& unused_obj) {
   return !personal_data_manager_->GetCreditCards().empty();
 }
 
-base::android::ScopedJavaLocalRef<jobjectArray>
-PersonalDataManagerAndroid::GetSubKeys(JNIEnv* env,
-                                       const std::string& region_code) {
-  std::vector<std::string> sub_keys =
-      address_validator_.GetRegionSubKeys(region_code);
-  return base::android::ToJavaArrayOfStrings(env, sub_keys);
-}
-
-void PersonalDataManagerAndroid::OnAddressValidationRulesLoaded(
-    const std::string& region_code,
-    bool success) {
-  // if |success| == false, AddressValidator::GetRegionSubKeys will return an
-  // empty list of sub-keys. => No need to check for |success|.
-  // Check if there is any sub-key request for that region code.
-  if (!pending_subkey_region_code_.compare(region_code))
-    pending_subkey_request_->OnRulesSuccessfullyLoaded();
-  pending_subkey_region_code_.clear();
-  pending_subkey_request_.reset();
-}
-
 void PersonalDataManagerAndroid::StartRegionSubKeysRequest(
     JNIEnv* env,
     const JavaParamRef<jobject>& unused_obj,
     const JavaParamRef<jstring>& jregion_code,
+    jint jtimeout_seconds,
     const JavaParamRef<jobject>& jdelegate) {
   const std::string region_code = ConvertJavaStringToUTF8(env, jregion_code);
-  std::unique_ptr<SubKeyRequestDelegate> requester =
-      base::MakeUnique<AndroidSubKeyRequesterDelegate>(
-          env, jdelegate, region_code, AsWeakPtr());
-
-  if (AreRulesLoadedForRegion(region_code)) {
-    requester->OnRulesSuccessfullyLoaded();
-  } else {
-    // Setup the variables so that the sub-keys request is sent, when the rules
-    // are loaded.
-    pending_subkey_region_code_ = region_code;
-    pending_subkey_request_ = std::move(requester);
-  }
+  // Self-deleting object.
+  AndroidSubKeyRequesterDelegate* requester =
+      new AndroidSubKeyRequesterDelegate(env, jdelegate);

[David Trainor- moved to gerrit, 2017/05/19 17:44:35]

This feels scary (with the "delete this" in the callback method).  If you need a
way to call a single function with some parameters just use a base::Callback or
base::OnceCallback into these classes instead.  Then you can bind the jobject as
a ScopedJavaGlobalRef to the bind.

Then you can remove the delegate concept

[Parastoo, 2017/05/23 18:09:52]

Done.

+  subkey_requester_.StartRegionSubKeysRequest(region_code, jtimeout_seconds,
+                                              requester);
 }
 
 void PersonalDataManagerAndroid::CancelPendingGetSubKeys(
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& unused_obj) {
-  pending_subkey_region_code_.clear();
-  pending_subkey_request_.reset();
+  subkey_requester_.CancelPendingGetSubKeys();
 }
 
 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()));
 
   return base::android::ToJavaArrayOfStrings(env, guids);
 }
 
 ScopedJavaLocalRef<jobjectArray> PersonalDataManagerAndroid::GetCreditCardGUIDs(
     JNIEnv* env,
     const std::vector<CreditCard*>& credit_cards) {
   std::vector<base::string16> guids;
   for (CreditCard* credit_card : credit_cards)
     guids.push_back(base::UTF8ToUTF16(credit_card->guid()));
 
   return base::android::ToJavaArrayOfStrings(env, guids);
 }
 
-bool PersonalDataManagerAndroid::AreRulesLoadedForRegion(
-    const std::string& region_code) {
-  return address_validator_.AreRulesLoadedForRegion(region_code);
-}
-
 ScopedJavaLocalRef<jobjectArray> PersonalDataManagerAndroid::GetProfileLabels(
     JNIEnv* env,
     bool address_only,
     bool include_name_in_label,
     bool include_organization_in_label,
     bool include_country_in_label,
     std::vector<AutofillProfile*> profiles) {
   std::unique_ptr<std::vector<ServerFieldType>> suggested_fields;
   size_t minimal_fields_shown = 2;
   if (address_only) {
@@ skipping 97 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