[Autofill local card migration] Send migration request only once risk data has been loaded

In this CL, we finish the basic two round pop-up window workflow.

We add LoadRiskData at the same time the first prompt pops up when the legal docs are successfully fetched. The callback of the first prompt(ShowLocalCardMigrationPrompt) is OnUserAcceptedIntermediateMigrationDialog() which will trigger the second prompt.

We will send the Migration request only when both the risk data is prepared and user accepted on the main prompt. Send Migration Request will included in the following CL.

Use the intermediate bubble as the main prompt for now. Need to wait for Siyu's Main Prompt skeleton check-in to replace the main prompt call.

TBR=rogerm@chromium.org

Bug: 852904
Change-Id: Ic881a64b7da93316ff8a8a0e84bc0c20a39a030d
Reviewed-on: https://chromium-review.googlesource.com/1149151
Commit-Queue: Sujie Zhu <sujiezhu@google.com>
Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
Reviewed-by: Jared Saul <jsaul@google.com>
Cr-Commit-Position: refs/heads/master@{#585579}

components/autofill/core/browser/form_data_importer.cc

@@ -138,7 +138,8 @@ void FormDataImporter::ImportFormData(const FormStructure& submitted_form,
   if (local_card_migration_manager_ &&
       local_card_migration_manager_->ShouldOfferLocalCardMigration(
           imported_credit_card_record_type_)) {
-    local_card_migration_manager_->AttemptToOfferLocalCardMigration();
+    local_card_migration_manager_->AttemptToOfferLocalCardMigration(
+        /*is_from_settings_page=*/false);
     return;
   }
 

components/autofill/core/browser/local_card_migration_manager.cc

@@ -68,7 +68,8 @@ bool LocalCardMigrationManager::ShouldOfferLocalCardMigration(
           !migratable_credit_cards_.empty());
 }
 
-void LocalCardMigrationManager::AttemptToOfferLocalCardMigration() {
+void LocalCardMigrationManager::AttemptToOfferLocalCardMigration(
+    bool is_from_settings_page) {
   // Abort the migration if |payments_client_| is nullptr.
   if (!payments_client_)
     return;
@@ -88,21 +89,26 @@ void LocalCardMigrationManager::AttemptToOfferLocalCardMigration() {
       /*pan_first_six=*/std::string(), upload_request_.active_experiments,
       app_locale_,
       base::BindOnce(&LocalCardMigrationManager::OnDidGetUploadDetails,
-                     weak_ptr_factory_.GetWeakPtr()),
+                     weak_ptr_factory_.GetWeakPtr(), is_from_settings_page),
       payments::kMigrateCardBillableServiceNumber);
 }
 
-// TODO(crbug.com/852904): Pops up a larger, modal dialog showing the local
-// cards to be uploaded. Pass the reference of vector<MigratableCreditCard> and
-// the callback function is OnConfirmLocalCardsMigration().
+// Callback function when user agrees to migration on the intermediate dialog.
+// Call ShowMainMigrationDialog() to pop up a larger, modal dialog showing the
+// local cards to be uploaded.
 void LocalCardMigrationManager::OnUserAcceptedIntermediateMigrationDialog() {
-  user_accepted_main_migration_dialog_ = false;
-  // Pops up a larger, modal dialog showing the local cards to be uploaded.
-  client_->ConfirmMigrateLocalCardToCloud(
-      migratable_credit_cards_,
-      base::BindOnce(
-          &LocalCardMigrationManager::OnUserAcceptedMainMigrationDialog,
-          weak_ptr_factory_.GetWeakPtr()));
+  ShowMainMigrationDialog();
+}
+
+// Send the migration request once risk data is available.
+void LocalCardMigrationManager::OnUserAcceptedMainMigrationDialog() {
+  user_accepted_main_migration_dialog_ = true;
+  // Populating risk data and offering migration two-round pop-ups occur
+  // asynchronously. If |migration_risk_data_| has already been loaded, send the
+  // migrate local cards request. Otherwise, continue to wait and let
+  // OnDidGetUploadRiskData handle it.
+  if (!migration_risk_data_.empty())
+    SendMigrateLocalCardsRequest();
 }
 
 bool LocalCardMigrationManager::IsCreditCardMigrationEnabled() {
@@ -122,24 +128,61 @@ bool LocalCardMigrationManager::IsCreditCardMigrationEnabled() {
 }
 
 void LocalCardMigrationManager::OnDidGetUploadDetails(
+    bool is_from_settings_page,
     AutofillClient::PaymentsRpcResult result,
     const base::string16& context_token,
     std::unique_ptr<base::DictionaryValue> legal_message) {
   if (result == AutofillClient::SUCCESS) {
     upload_request_.context_token = context_token;
     legal_message_ = std::move(legal_message);
+    migration_risk_data_.clear();
     // If we successfully received the legal docs, trigger the offer-to-migrate
-    // dialog.
-    client_->ShowLocalCardMigrationDialog(base::BindOnce(
-        &LocalCardMigrationManager::OnUserAcceptedIntermediateMigrationDialog,
+    // dialog. If triggered from settings page, we pop-up the main prompt
+    // directly. If not, we pop up the intermediate bubble.
+    if (is_from_settings_page) {
+      // Pops up a larger, modal dialog showing the local cards to be uploaded.
+      ShowMainMigrationDialog();
+    } else {
+      client_->ShowLocalCardMigrationDialog(base::BindOnce(
+          &LocalCardMigrationManager::OnUserAcceptedIntermediateMigrationDialog,
+          weak_ptr_factory_.GetWeakPtr()));
+    }
+    // TODO(crbug.com/876895): Clean up the LoadRiskData Bind/BindRepeating
+    // usages
+    client_->LoadRiskData(base::BindRepeating(
+        &LocalCardMigrationManager::OnDidGetMigrationRiskData,
         weak_ptr_factory_.GetWeakPtr()));
-    // TODO(crbug.com/852904): Call the client LoadRiskData()
   }
 }
 
-// TODO(crbug.com/852904): Send the upload request once risk data is available.
-void LocalCardMigrationManager::OnUserAcceptedMainMigrationDialog() {
-  user_accepted_main_migration_dialog_ = true;
+void LocalCardMigrationManager::OnDidGetMigrationRiskData(
+    const std::string& risk_data) {
+  migration_risk_data_ = risk_data;
+  // Populating risk data and offering migration two-round pop-ups occur
+  // asynchronously. If the main migration dialog has already been accepted,
+  // send the migrate local cards request. Otherwise, continue to wait for the
+  // user to accept the two round dialog.
+  if (user_accepted_main_migration_dialog_)
+    SendMigrateLocalCardsRequest();
+}
+
+// TODO(crbug.com/852904): Send the migration request. Will call payments_client
+// to create a new PaymentsRequest. Also create a new callback function
+// OnDidMigrateLocalCards.
+void LocalCardMigrationManager::SendMigrateLocalCardsRequest() {}
+
+// Pops up a larger, modal dialog showing the local cards to be uploaded. Pass
+// the reference of vector<MigratableCreditCard> and the callback function is
+// OnUserAcceptedMainMigrationDialog(). Can be called when user agrees to
+// migration on the intermediate dialog or directly from settings page.
+void LocalCardMigrationManager::ShowMainMigrationDialog() {
+  user_accepted_main_migration_dialog_ = false;
+  // Pops up a larger, modal dialog showing the local cards to be uploaded.
+  client_->ConfirmMigrateLocalCardToCloud(
+      migratable_credit_cards_,
+      base::BindOnce(
+          &LocalCardMigrationManager::OnUserAcceptedMainMigrationDialog,
+          weak_ptr_factory_.GetWeakPtr()));
 }
 
 int LocalCardMigrationManager::GetDetectedValues() const {

components/autofill/core/browser/local_card_migration_manager.h

@@ -59,13 +59,22 @@ class LocalCardMigrationManager {
   // migration are satisfied. Initializes the local card list for upload.
   bool ShouldOfferLocalCardMigration(int imported_credit_card_record_type);
 
-  // Called from FormDataImporter when all migration requirements are met.
-  // Fetches legal documents and triggers the OnDidGetUploadDetails callback.
-  void AttemptToOfferLocalCardMigration();
+  // Called from FormDataImporter or settings page when all migration
+  // requirements are met. Fetches legal documents and triggers the
+  // OnDidGetUploadDetails callback. |is_from_settings_page| to denote the user
+  // triggers the migration from settings page. It will trigger the main prompt
+  // directly if the get upload details call returns success.
+  void AttemptToOfferLocalCardMigration(bool is_from_settings_page);
 
   // Callback function when user agrees to migration on the intermediate dialog.
   // Pops up a larger, modal dialog showing the local cards to be uploaded.
-  void OnUserAcceptedIntermediateMigrationDialog();
+  // Exposed for testing.
+  virtual void OnUserAcceptedIntermediateMigrationDialog();
+
+  // Callback function when user confirms migration on the main migration
+  // dialog. Sets |user_accepted_main_migration_dialog_| and sends the migration
+  // request once risk data is available. Exposed for testing.
+  virtual void OnUserAcceptedMainMigrationDialog();
 
   // Check that the user is signed in, syncing, and the proper experiment
   // flags are enabled. Override in the test class.
@@ -75,18 +84,20 @@ class LocalCardMigrationManager {
   // name if it exists on all cards, and existence of Payments customer).
   int GetDetectedValues() const;
 
+  // Fetch all migratable credit cards and store in |migratable_credit_cards_|.
+  void GetMigratableCreditCards();
+
  protected:
   // Callback after successfully getting the legal documents. On success,
   // displays the offer-to-migrate dialog, which the user can accept or not.
-  // Exposed for testing.
+  // When |is_from_settings_page| is true, it will trigger the main prompt
+  // directly. If not, trigger the intermediate prompt. Exposed for testing.
   virtual void OnDidGetUploadDetails(
+      bool is_from_settings_page,
       AutofillClient::PaymentsRpcResult result,
       const base::string16& context_token,
       std::unique_ptr<base::DictionaryValue> legal_message);
 
-  // Fetch all migratable credit cards and store in |migratable_credit_cards_|.
-  void GetMigratableCreditCards();
-
   AutofillClient* const client_;
 
   // Handles Payments service requests.
@@ -94,10 +105,16 @@ class LocalCardMigrationManager {
   payments::PaymentsClient* payments_client_;
 
  private:
-  // Callback function when user confirms migration on the main migration
-  // dialog. Sets |user_accepted_main_migration_dialog_| and sends the upload
-  // request.
-  void OnUserAcceptedMainMigrationDialog();
+  // Pops up a larger, modal dialog showing the local cards to be uploaded.
+  void ShowMainMigrationDialog();
+
+  // Callback function when migration risk data is ready. Saves risk data in
+  // |migration_risk_data_| and calls SendMigrateLocalCardsRequest if the user
+  // has accepted the main migration dialog.
+  void OnDidGetMigrationRiskData(const std::string& risk_data);
+
+  // Finalizes the migration request and calls PaymentsClient.
+  void SendMigrateLocalCardsRequest();
 
   std::unique_ptr<base::DictionaryValue> legal_message_;
 
@@ -119,6 +136,8 @@ class LocalCardMigrationManager {
   // on the main dialog.
   bool user_accepted_main_migration_dialog_ = false;
 
+  std::string migration_risk_data_;
+
   base::WeakPtrFactory<LocalCardMigrationManager> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(LocalCardMigrationManager);

components/autofill/core/browser/local_card_migration_manager_unittest.cc

@@ -595,4 +595,62 @@ TEST_F(LocalCardMigrationManagerTest,
       CreditCardSaveManager::DetectedValue::HAS_GOOGLE_PAYMENTS_ACCOUNT);
 }
 
+// Verify that when triggering from settings page, intermediate prompt will not
+// be triggered.
+TEST_F(LocalCardMigrationManagerTest,
+       MigrateCreditCard_TriggerFromSettingsPage) {
+  EnableAutofillCreditCardLocalCardMigrationExperiment();
+  personal_data_.ClearCreditCards();
+  personal_data_.ClearProfiles();
+  credit_card_save_manager_->SetCreditCardUploadEnabled(true);
+  // Set the billing_customer_number Priority Preference to designate
+  // existence of a Payments account.
+  autofill_client_.GetPrefs()->SetDouble(prefs::kAutofillBillingCustomerNumber,
+                                         12345);
+  // Add a local credit card. One migratable credit card will still trigger
+  // migration on settings page.
+  AddLocalCrediCard(personal_data_, "Flo Master", "4111111111111111", "11",
+                    test::NextYear().c_str(), "1");
+
+  // Do the same operation as we bridge back from the settings page.
+  local_card_migration_manager_->GetMigratableCreditCards();
+  local_card_migration_manager_->AttemptToOfferLocalCardMigration(true);
+
+  EXPECT_FALSE(local_card_migration_manager_->IntermediatePromptWasShown());
+  EXPECT_TRUE(local_card_migration_manager_->MainPromptWasShown());
+}
+
+// Verify that when triggering from submitted form, intermediate prompt and main
+// prompt are both triggered.
+TEST_F(LocalCardMigrationManagerTest,
+       MigrateCreditCard_TriggerFromSubmittedForm) {
+  EnableAutofillCreditCardLocalCardMigrationExperiment();
+  personal_data_.ClearCreditCards();
+  personal_data_.ClearProfiles();
+  credit_card_save_manager_->SetCreditCardUploadEnabled(true);
+  // Set the billing_customer_number Priority Preference to designate
+  // existence of a Payments account.
+  autofill_client_.GetPrefs()->SetDouble(prefs::kAutofillBillingCustomerNumber,
+                                         12345);
+  // Add a local credit card whose |TypeAndLastFourDigits| matches what we will
+  // enter below.
+  AddLocalCrediCard(personal_data_, "Flo Master", "4111111111111111", "11",
+                    test::NextYear().c_str(), "1");
+  // Add another local credit card, so it will trigger migration.
+  AddLocalCrediCard(personal_data_, "Flo Master", "5555555555554444", "11",
+                    test::NextYear().c_str(), "1");
+
+  // Set up our credit card form data.
+  FormData credit_card_form;
+  test::CreateTestCreditCardFormData(&credit_card_form, true, false);
+  FormsSeen(std::vector<FormData>(1, credit_card_form));
+
+  // Edit the data, and submit.
+  EditCreditCardFrom(credit_card_form, "Flo Master", "4111111111111111", "11",
+                     test::NextYear().c_str(), "123");
+  FormSubmitted(credit_card_form);
+  EXPECT_TRUE(local_card_migration_manager_->IntermediatePromptWasShown());
+  EXPECT_TRUE(local_card_migration_manager_->MainPromptWasShown());
+}
+
 }  // namespace autofill

components/autofill/core/browser/test_local_card_migration_manager.cc

@@ -38,14 +38,34 @@ bool TestLocalCardMigrationManager::LocalCardMigrationWasTriggered() {
   return local_card_migration_was_triggered_;
 }
 
+bool TestLocalCardMigrationManager::IntermediatePromptWasShown() {
+  return intermediate_prompt_was_shown_;
+}
+
+bool TestLocalCardMigrationManager::MainPromptWasShown() {
+  return main_prompt_was_shown_;
+}
+
+void TestLocalCardMigrationManager::
+    OnUserAcceptedIntermediateMigrationDialog() {
+  intermediate_prompt_was_shown_ = true;
+  LocalCardMigrationManager::OnUserAcceptedIntermediateMigrationDialog();
+}
+
+void TestLocalCardMigrationManager::OnUserAcceptedMainMigrationDialog() {
+  main_prompt_was_shown_ = true;
+  LocalCardMigrationManager::OnUserAcceptedMainMigrationDialog();
+}
+
 void TestLocalCardMigrationManager::OnDidGetUploadDetails(
+    bool is_from_settings_page,
     AutofillClient::PaymentsRpcResult result,
     const base::string16& context_token,
     std::unique_ptr<base::DictionaryValue> legal_message) {
   if (result == AutofillClient::SUCCESS) {
     local_card_migration_was_triggered_ = true;
-    LocalCardMigrationManager::OnDidGetUploadDetails(result, context_token,
-                                                     std::move(legal_message));
+    LocalCardMigrationManager::OnDidGetUploadDetails(
+        is_from_settings_page, result, context_token, std::move(legal_message));
   }
 }
 

components/autofill/core/browser/test_local_card_migration_manager.h

@@ -32,17 +32,36 @@ class TestLocalCardMigrationManager : public LocalCardMigrationManager {
   // user is signed in/syncing.
   bool IsCreditCardMigrationEnabled() override;
 
-  // Returns whether the first round migration pop-up window was triggered.
+  // Returns whether the local card migration was triggered.
   bool LocalCardMigrationWasTriggered();
 
+  // Returns whether the first round intermediate pop-up window was shown.
+  bool IntermediatePromptWasShown();
+
+  // Returns whether the main prompt window was shown.
+  bool MainPromptWasShown();
+
+  // Override the base function. When called, represents the intermediate prompt
+  // is shown. Set the |intermediate_prompt_was_shown_|.
+  void OnUserAcceptedIntermediateMigrationDialog() override;
+
+  // Override the base function. When called, represent the main prompt is
+  // shown. Set the |main_prompt_was_shown_|.
+  void OnUserAcceptedMainMigrationDialog() override;
+
  private:
   void OnDidGetUploadDetails(
+      bool is_from_settings_page,
       AutofillClient::PaymentsRpcResult result,
       const base::string16& context_token,
       std::unique_ptr<base::DictionaryValue> legal_message) override;
 
   bool local_card_migration_was_triggered_ = false;
 
+  bool intermediate_prompt_was_shown_ = false;
+
+  bool main_prompt_was_shown_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(TestLocalCardMigrationManager);
 };