[Payments] Code Refactoring for the Subkey Request

chrome/browser/autofill/android/personal_data_manager_android.cc

Index: chrome/browser/autofill/android/personal_data_manager_android.cc
diff --git a/chrome/browser/autofill/android/personal_data_manager_android.cc b/chrome/browser/autofill/android/personal_data_manager_android.cc
index 34b2b9c3bbba87a64a87bb8d3ec31ba6023225e9..53bdd3678586e29d368befd963c2fce8921aadb2 100644
--- a/chrome/browser/autofill/android/personal_data_manager_android.cc
+++ b/chrome/browser/autofill/android/personal_data_manager_android.cc
@@ -308,35 +308,26 @@ class 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;
   }
 
-  ~AndroidSubKeyRequesterDelegate() override {}
+  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:
-  // 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_));
-    }
-  }
+  ~AndroidSubKeyRequesterDelegate() override {}
 
   ScopedJavaGlobalRef<jobject> jdelegate_;
-  std::string region_code_;
-  base::WeakPtr<PersonalDataManagerAndroid> personal_data_manager_android_;
 
   DISALLOW_COPY_AND_ASSIGN(AndroidSubKeyRequesterDelegate);
 };
@@ -353,12 +344,11 @@ PersonalDataManagerAndroid::PersonalDataManagerAndroid(JNIEnv* env, jobject obj)
                   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);
 }
 
@@ -752,7 +742,8 @@ 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(
@@ -788,51 +779,24 @@ jboolean PersonalDataManagerAndroid::HasCreditCards(
   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(
@@ -855,11 +819,6 @@ ScopedJavaLocalRef<jobjectArray> PersonalDataManagerAndroid::GetCreditCardGUIDs(
   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,