[PR] Don't save Payment Request data in PersonalDataManager in Incognito

Fixes the bug where changes to existing autofill profiles and credit cards
or adding new ones get reflected in PersonalDataManager when user is in
incognito mode.
Also refactors the details of adding/updating autofill profiles and credit
cards into the paymentrequest.mm instead of the coordinators.
Updates the unit tests accordingly.

Bug: 814130
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I2cc72032648d162304211fe3de6612bc23848350
Reviewed-on: https://chromium-review.googlesource.com/931625
Commit-Queue: Moe Ahmadi <mahmadi@chromium.org>
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539048}

components/autofill/core/browser/personal_data_manager.h

@@ -149,7 +149,7 @@ class PersonalDataManager : public KeyedService,
 
   // Updates the use stats and billing address id for the server |credit_card|.
   // Looks up the card by server_id.
-  void UpdateServerCardMetadata(const CreditCard& credit_card);
+  virtual void UpdateServerCardMetadata(const CreditCard& credit_card);
 
   // Resets the card for |guid| to the masked state.
   void ResetFullServerCard(const std::string& guid);

ios/chrome/browser/payments/BUILD.gn

@@ -32,6 +32,7 @@ source_set("payments") {
     ":constants",
     "//base",
     "//components/autofill/core/browser",
+    "//components/autofill/ios/browser",
     "//components/image_fetcher/ios",
     "//components/keyed_service/ios",
     "//components/payments/core",

ios/chrome/browser/payments/payment_request.h

@@ -184,6 +184,10 @@ class PaymentRequest : public PaymentOptionsProvider,
   virtual autofill::AutofillProfile* AddAutofillProfile(
       const autofill::AutofillProfile& profile);
 
+  // Updates the given |profile| in the PersonalDataManager if the user is
+  // not in incognito mode.
+  virtual void UpdateAutofillProfile(const autofill::AutofillProfile& profile);
+
   // Returns the available autofill profiles for this user to be used as
   // shipping profiles.
   const std::vector<autofill::AutofillProfile*>& shipping_profiles() const {
@@ -246,9 +250,14 @@ class PaymentRequest : public PaymentOptionsProvider,
     return supported_card_types_set_;
   }
 
-  // Creates and adds an AutofillPaymentInstrument, which makes a copy of
-  // |credit_card|.
-  virtual AutofillPaymentInstrument* AddAutofillPaymentInstrument(
+  // Creates and adds an AutofillPaymentInstrument to the list of payment
+  // instruments by making a copy of |credit_card|.
+  virtual AutofillPaymentInstrument* CreateAndAddAutofillPaymentInstrument(
+      const autofill::CreditCard& credit_card);
+
+  // Updates the given |credit_card| in the PersonalDataManager if the user is
+  // not in incognito mode.
+  virtual void UpdateAutofillPaymentInstrument(
       const autofill::CreditCard& credit_card);
 
   // Returns the available payment methods for this user that match a supported
@@ -344,6 +353,13 @@ class PaymentRequest : public PaymentOptionsProvider,
   // methods.
   void PopulateAvailablePaymentMethods();
 
+  // Creates and adds an AutofillPaymentInstrument to the list of payment
+  // instruments by making a copy of |credit_card|. Updates PersonalDataManager
+  // if not in incognito mode and |may_update_personal_data_manager| is true.
+  AutofillPaymentInstrument* CreateAndAddAutofillPaymentInstrument(
+      const autofill::CreditCard& credit_card,
+      bool may_update_personal_data_manager);
+
   // Sets the available shipping options as references to the shipping options
   // in |web_payment_request_|.
   void PopulateAvailableShippingOptions();

ios/chrome/browser/payments/payment_request.mm

@@ -16,6 +16,7 @@
 #include "components/autofill/core/browser/personal_data_manager.h"
 #include "components/autofill/core/browser/region_data_loader_impl.h"
 #include "components/autofill/core/browser/validation.h"
+#import "components/autofill/ios/browser/credit_card_util.h"
 #include "components/payments/core/autofill_payment_instrument.h"
 #include "components/payments/core/currency_formatter.h"
 #include "components/payments/core/features.h"
@@ -289,9 +290,21 @@ autofill::AutofillProfile* PaymentRequest::AddAutofillProfile(
   contact_profiles_.push_back(profile_cache_.back().get());
   shipping_profiles_.push_back(profile_cache_.back().get());
 
+  if (!IsIncognito())
+    personal_data_manager_->AddProfile(profile);
+
   return profile_cache_.back().get();
 }
 
+void PaymentRequest::UpdateAutofillProfile(
+    const autofill::AutofillProfile& profile) {
+  // Cached profile must be invalidated once the profile is modified.
+  profile_comparator()->Invalidate(profile);
+
+  if (!IsIncognito())
+    personal_data_manager_->UpdateProfile(profile);
+}
+
 const PaymentDetailsModifier* PaymentRequest::GetApplicableModifier(
     PaymentInstrument* selected_instrument) const {
   if (!selected_instrument ||
@@ -361,8 +374,17 @@ void PaymentRequest::PopulateAvailableProfiles() {
       profile_comparator_.FilterProfilesForShipping(raw_profiles_for_filtering);
 }
 
-AutofillPaymentInstrument* PaymentRequest::AddAutofillPaymentInstrument(
+AutofillPaymentInstrument*
+PaymentRequest::CreateAndAddAutofillPaymentInstrument(
     const autofill::CreditCard& credit_card) {
+  return CreateAndAddAutofillPaymentInstrument(
+      credit_card, /*may_update_personal_data_manager=*/true);
+}
+
+AutofillPaymentInstrument*
+PaymentRequest::CreateAndAddAutofillPaymentInstrument(
+    const autofill::CreditCard& credit_card,
+    bool may_update_personal_data_manager) {
   std::string basic_card_issuer_network =
       autofill::data_util::GetPaymentRequestData(credit_card.network())
           .basic_card_issuer_network;
@@ -398,10 +420,24 @@ AutofillPaymentInstrument* PaymentRequest::AddAutofillPaymentInstrument(
 
   payment_methods_.push_back(payment_method_cache_.back().get());
 
+  if (may_update_personal_data_manager && !IsIncognito())
+    personal_data_manager_->AddCreditCard(credit_card);
+
   return static_cast<AutofillPaymentInstrument*>(
       payment_method_cache_.back().get());
 }
 
+void PaymentRequest::UpdateAutofillPaymentInstrument(
+    const autofill::CreditCard& credit_card) {
+  if (IsIncognito())
+    return;
+
+  if (autofill::IsCreditCardLocal(credit_card))
+    personal_data_manager_->UpdateCreditCard(credit_card);
+  else
+    personal_data_manager_->UpdateServerCardMetadata(credit_card);
+}
+
 PaymentsProfileComparator* PaymentRequest::profile_comparator() {
   return &profile_comparator_;
 }
@@ -513,8 +549,12 @@ void PaymentRequest::PopulatePaymentMethodCache(
   for (auto& instrument : native_app_instruments)
     payment_method_cache_.push_back(std::move(instrument));
 
-  for (const auto* credit_card : credit_cards_to_suggest)
-    AddAutofillPaymentInstrument(*credit_card);
+  for (const auto* credit_card : credit_cards_to_suggest) {
+    // We only want to add the credit cards read from the PersonalDataManager to
+    // the list of payment instrument. Don't re-add them to PersonalDataManager.
+    CreateAndAddAutofillPaymentInstrument(
+        *credit_card, /*may_update_personal_data_manager=*/false);
+  }
 
   PopulateAvailablePaymentMethods();
 

ios/chrome/browser/payments/test_payment_request.h

@@ -41,26 +41,12 @@ class TestPaymentRequest : public PaymentRequest {
                      ios::ChromeBrowserState* browser_state,
                      web::WebState* web_state,
                      autofill::PersonalDataManager* personal_data_manager,
-                     id<PaymentRequestUIDelegate> payment_request_ui_delegate)
-      : PaymentRequest(web_payment_request,
-                       browser_state,
-                       web_state,
-                       personal_data_manager,
-                       payment_request_ui_delegate),
-        address_normalization_manager_(&address_normalizer_, "en-US"),
-        region_data_loader_(nullptr),
-        pref_service_(nullptr),
-        profile_comparator_(nullptr) {}
+                     id<PaymentRequestUIDelegate> payment_request_ui_delegate);
 
   TestPaymentRequest(const payments::WebPaymentRequest& web_payment_request,
                      ios::ChromeBrowserState* browser_state,
                      web::WebState* web_state,
-                     autofill::PersonalDataManager* personal_data_manager)
-      : TestPaymentRequest(web_payment_request,
-                           browser_state,
-                           web_state,
-                           personal_data_manager,
-                           nil) {}
+                     autofill::PersonalDataManager* personal_data_manager);
 
   ~TestPaymentRequest() override {}
 
@@ -101,6 +87,8 @@ class TestPaymentRequest : public PaymentRequest {
     selected_shipping_option_ = option;
   }
 
+  void set_is_incognito(bool is_incognito) { is_incognito_ = is_incognito; }
+
   // PaymentRequest
   autofill::AddressNormalizer* GetAddressNormalizer() override;
   autofill::AddressNormalizationManager* GetAddressNormalizationManager()
@@ -108,6 +96,7 @@ class TestPaymentRequest : public PaymentRequest {
   autofill::RegionDataLoader* GetRegionDataLoader() override;
   PrefService* GetPrefService() override;
   PaymentsProfileComparator* profile_comparator() override;
+  bool IsIncognito() const override;
 
  private:
   autofill::TestAddressNormalizer address_normalizer_;
@@ -122,6 +111,9 @@ class TestPaymentRequest : public PaymentRequest {
   // Not owned and must outlive this object.
   PaymentsProfileComparator* profile_comparator_;
 
+  // Whether the user is in incognito mode.
+  bool is_incognito_;
+
   DISALLOW_COPY_AND_ASSIGN(TestPaymentRequest);
 };
 

ios/chrome/browser/payments/test_payment_request.mm

@@ -17,6 +17,34 @@
 
 namespace payments {
 
+TestPaymentRequest::TestPaymentRequest(
+    const payments::WebPaymentRequest& web_payment_request,
+    ios::ChromeBrowserState* browser_state,
+    web::WebState* web_state,
+    autofill::PersonalDataManager* personal_data_manager,
+    id<PaymentRequestUIDelegate> payment_request_ui_delegate)
+    : PaymentRequest(web_payment_request,
+                     browser_state,
+                     web_state,
+                     personal_data_manager,
+                     payment_request_ui_delegate),
+      address_normalization_manager_(&address_normalizer_, "en-US"),
+      region_data_loader_(nullptr),
+      pref_service_(nullptr),
+      profile_comparator_(nullptr),
+      is_incognito_(false) {}
+
+TestPaymentRequest::TestPaymentRequest(
+    const payments::WebPaymentRequest& web_payment_request,
+    ios::ChromeBrowserState* browser_state,
+    web::WebState* web_state,
+    autofill::PersonalDataManager* personal_data_manager)
+    : TestPaymentRequest(web_payment_request,
+                         browser_state,
+                         web_state,
+                         personal_data_manager,
+                         /*payment_request_ui_delegate=*/nil) {}
+
 void TestPaymentRequest::ClearShippingProfiles() {
   shipping_profiles_.clear();
 }
@@ -64,4 +92,8 @@ PaymentsProfileComparator* TestPaymentRequest::profile_comparator() {
   return PaymentRequest::profile_comparator();
 }
 
+bool TestPaymentRequest::IsIncognito() const {
+  return is_incognito_;
+}
+
 }  // namespace payments

ios/chrome/browser/ui/payments/address_edit_coordinator.mm

@@ -12,7 +12,6 @@
 #include "components/autofill/core/browser/autofill_type.h"
 #include "components/autofill/core/browser/personal_data_manager.h"
 #include "components/autofill/core/common/autofill_constants.h"
-#include "components/payments/core/payments_profile_comparator.h"
 #include "ios/chrome/browser/payments/payment_request.h"
 #import "ios/chrome/browser/ui/autofill/autofill_ui_type_util.h"
 #import "ios/chrome/browser/ui/payments/address_edit_mediator.h"
@@ -108,9 +107,10 @@ using ::AutofillTypeFromAutofillUIType;
   // Create an empty autofill profile. If an address is being edited, copy over
   // the information.
   autofill::AutofillProfile address =
-      self.address ? *self.address
-                   : autofill::AutofillProfile(base::GenerateGUID(),
-                                               autofill::kSettingsOrigin);
+      self.address ? *self.address : autofill::AutofillProfile();
+
+  // Set the origin, or override it if the autofill profile is being edited.
+  address.set_origin(autofill::kSettingsOrigin);
 
   for (EditorField* field in fields) {
     address.SetInfo(autofill::AutofillType(
@@ -120,20 +120,12 @@ using ::AutofillTypeFromAutofillUIType;
   }
 
   if (!self.address) {
-    self.paymentRequest->GetPersonalDataManager()->AddProfile(address);
-
     // Add the profile to the list of profiles in |self.paymentRequest|.
     self.address = self.paymentRequest->AddAutofillProfile(address);
   } else {
-    // Override the origin.
-    address.set_origin(autofill::kSettingsOrigin);
-    self.paymentRequest->GetPersonalDataManager()->UpdateProfile(address);
-
-    // Cached profile must be invalidated once the profile is modified.
-    _paymentRequest->profile_comparator()->Invalidate(address);
-
     // Update the original profile instance that is being edited.
     *self.address = address;
+    self.paymentRequest->UpdateAutofillProfile(address);
   }
 
   [self.delegate addressEditCoordinator:self

ios/chrome/browser/ui/payments/address_edit_coordinator_unittest.mm

@@ -15,7 +15,6 @@
 #include "components/autofill/core/browser/country_names.h"
 #include "components/autofill/core/browser/test_personal_data_manager.h"
 #include "components/autofill/core/browser/test_region_data_loader.h"
-#include "components/payments/core/payments_profile_comparator.h"
 #include "ios/chrome/browser/payments/payment_request_test_util.h"
 #include "ios/chrome/browser/payments/payment_request_unittest_base.h"
 #include "ios/chrome/browser/payments/test_payment_request.h"
@@ -34,21 +33,8 @@
 #endif
 
 namespace {
-class MockTestPersonalDataManager : public autofill::TestPersonalDataManager {
- public:
-  MockTestPersonalDataManager() : TestPersonalDataManager() {}
-  MOCK_METHOD1(AddProfile, void(const autofill::AutofillProfile&));
-  MOCK_METHOD1(UpdateProfile, void(const autofill::AutofillProfile&));
-};
 
-class MockPaymentsProfileComparator
-    : public payments::PaymentsProfileComparator {
- public:
-  MockPaymentsProfileComparator(const std::string& app_locale,
-                                const payments::PaymentOptionsProvider& options)
-      : PaymentsProfileComparator(app_locale, options) {}
-  MOCK_METHOD1(Invalidate, void(const autofill::AutofillProfile&));
-};
+using ::testing::_;
 
 class MockTestPaymentRequest : public payments::TestPaymentRequest {
  public:
@@ -62,6 +48,7 @@ class MockTestPaymentRequest : public payments::TestPaymentRequest {
                                      personal_data_manager) {}
   MOCK_METHOD1(AddAutofillProfile,
                autofill::AutofillProfile*(const autofill::AutofillProfile&));
+  MOCK_METHOD1(UpdateAutofillProfile, void(const autofill::AutofillProfile&));
 };
 
 MATCHER_P4(ProfileMatches, name, country, state, phone_number, "") {
@@ -101,8 +88,6 @@ NSArray<EditorField*>* GetEditorFields() {
                       required:YES],
   ];
 }
-
-using ::testing::_;
 }  // namespace
 
 class PaymentRequestAddressEditCoordinatorTest
@@ -121,10 +106,6 @@ class PaymentRequestAddressEditCoordinatorTest
         payment_request_test_util::CreateTestWebPaymentRequest(),
         browser_state(), web_state(), &personal_data_manager_);
 
-    profile_comparator_ = std::make_unique<MockPaymentsProfileComparator>(
-        payment_request_->GetApplicationLocale(), *payment_request_.get());
-    payment_request_->SetProfileComparator(profile_comparator_.get());
-
     test_region_data_loader_.set_synchronous_callback(true);
     payment_request_->SetRegionDataLoader(&test_region_data_loader_);
   }
@@ -135,9 +116,8 @@ class PaymentRequestAddressEditCoordinatorTest
     PaymentRequestUnitTestBase::TearDown();
   }
 
-  MockTestPersonalDataManager personal_data_manager_;
+  autofill::TestPersonalDataManager personal_data_manager_;
   autofill::TestRegionDataLoader test_region_data_loader_;
-  std::unique_ptr<MockPaymentsProfileComparator> profile_comparator_;
   std::unique_ptr<MockTestPaymentRequest> payment_request_;
 };
 
@@ -209,15 +189,8 @@ TEST_F(PaymentRequestAddressEditCoordinatorTest, DidFinishCreating) {
               AddAutofillProfile(ProfileMatches("John Doe", "CA" /* Canada */,
                                                 "Quebec", "1 650-211-1111")))
       .Times(1);
-  // Expect an autofill profile to be added to the PersonalDataManager.
-  EXPECT_CALL(personal_data_manager_,
-              AddProfile(ProfileMatches("John Doe", "CA" /* Canada */, "Quebec",
-                                        "1 650-211-1111")))
-      .Times(1);
-  // No autofill profile should get updated in the PersonalDataManager.
-  EXPECT_CALL(personal_data_manager_, UpdateProfile(_)).Times(0);
-  // No autofill profile should get invalidated in PaymentsProfileComparator.
-  EXPECT_CALL(*profile_comparator_, Invalidate(_)).Times(0);
+  // No autofill profile should get updated in the PaymentRequest.
+  EXPECT_CALL(*payment_request_, UpdateAutofillProfile(_)).Times(0);
 
   // Call the controller delegate method.
   EXPECT_TRUE([base_view_controller.presentedViewController
@@ -270,17 +243,10 @@ TEST_F(PaymentRequestAddressEditCoordinatorTest, DidFinishEditing) {
 
   // No autofill profile should get added to the PaymentRequest.
   EXPECT_CALL(*payment_request_, AddAutofillProfile(_)).Times(0);
-  // No autofill profile should get added to the PersonalDataManager.
-  EXPECT_CALL(personal_data_manager_, AddProfile(_)).Times(0);
-  // Expect an autofill profile to be updated in the PersonalDataManager.
-  EXPECT_CALL(personal_data_manager_,
-              UpdateProfile(ProfileMatches("John Doe", "CA" /* Canada */,
-                                           "Quebec", "1 650-211-1111")))
-      .Times(1);
-  // Expect an autofill profile to be invalidated in PaymentsProfileComparator.
-  EXPECT_CALL(*profile_comparator_,
-              Invalidate(ProfileMatches("John Doe", "CA" /* Canada */, "Quebec",
-                                        "1 650-211-1111")))
+  // Expect an autofill profile to be updated in the PaymentRequest.
+  EXPECT_CALL(*payment_request_,
+              UpdateAutofillProfile(ProfileMatches(
+                  "John Doe", "CA" /* Canada */, "Quebec", "1 650-211-1111")))
       .Times(1);
 
   // Call the controller delegate method.

ios/chrome/browser/ui/payments/contact_info_edit_coordinator.mm

@@ -13,7 +13,6 @@
 #include "components/autofill/core/browser/personal_data_manager.h"
 #include "components/autofill/core/browser/validation.h"
 #include "components/autofill/core/common/autofill_constants.h"
-#include "components/payments/core/payments_profile_comparator.h"
 #include "ios/chrome/browser/payments/payment_request.h"
 #import "ios/chrome/browser/ui/autofill/autofill_ui_type_util.h"
 #import "ios/chrome/browser/ui/payments/contact_info_edit_mediator.h"
@@ -88,9 +87,9 @@ using ::AutofillTypeFromAutofillUIType;
   // Create an empty autofill profile. If a profile is being edited, copy over
   // the information.
   autofill::AutofillProfile profile =
-      self.profile ? *self.profile
-                   : autofill::AutofillProfile(base::GenerateGUID(),
-                                               autofill::kSettingsOrigin);
+      self.profile ? *self.profile : autofill::AutofillProfile();
+  // Set the origin, or override it if the autofill profile is being edited.
+  profile.set_origin(autofill::kSettingsOrigin);
 
   for (EditorField* field in fields) {
     profile.SetInfo(autofill::AutofillType(
@@ -100,20 +99,12 @@ using ::AutofillTypeFromAutofillUIType;
   }
 
   if (!self.profile) {
-    self.paymentRequest->GetPersonalDataManager()->AddProfile(profile);
-
     // Add the profile to the list of profiles in |self.paymentRequest|.
     self.profile = self.paymentRequest->AddAutofillProfile(profile);
   } else {
-    // Override the origin.
-    profile.set_origin(autofill::kSettingsOrigin);
-    self.paymentRequest->GetPersonalDataManager()->UpdateProfile(profile);
-
-    // Cached profile must be invalidated once the profile is modified.
-    _paymentRequest->profile_comparator()->Invalidate(profile);
-
     // Update the original profile instance that is being edited.
     *self.profile = profile;
+    self.paymentRequest->UpdateAutofillProfile(profile);
   }
 
   [self.delegate contactInfoEditCoordinator:self

ios/chrome/browser/ui/payments/contact_info_edit_coordinator_unittest.mm

@@ -14,7 +14,6 @@
 #include "components/autofill/core/browser/autofill_test_utils.h"
 #include "components/autofill/core/browser/test_personal_data_manager.h"
 #include "components/autofill/core/browser/test_region_data_loader.h"
-#include "components/payments/core/payments_profile_comparator.h"
 #include "ios/chrome/browser/payments/payment_request_test_util.h"
 #include "ios/chrome/browser/payments/payment_request_unittest_base.h"
 #include "ios/chrome/browser/payments/test_payment_request.h"
@@ -33,21 +32,8 @@
 #endif
 
 namespace {
-class MockTestPersonalDataManager : public autofill::TestPersonalDataManager {
- public:
-  MockTestPersonalDataManager() : TestPersonalDataManager() {}
-  MOCK_METHOD1(AddProfile, void(const autofill::AutofillProfile&));
-  MOCK_METHOD1(UpdateProfile, void(const autofill::AutofillProfile&));
-};
 
-class MockPaymentsProfileComparator
-    : public payments::PaymentsProfileComparator {
- public:
-  MockPaymentsProfileComparator(const std::string& app_locale,
-                                const payments::PaymentOptionsProvider& options)
-      : PaymentsProfileComparator(app_locale, options) {}
-  MOCK_METHOD1(Invalidate, void(const autofill::AutofillProfile&));
-};
+using ::testing::_;
 
 class MockTestPaymentRequest : public payments::TestPaymentRequest {
  public:
@@ -61,6 +47,7 @@ class MockTestPaymentRequest : public payments::TestPaymentRequest {
                                      personal_data_manager) {}
   MOCK_METHOD1(AddAutofillProfile,
                autofill::AutofillProfile*(const autofill::AutofillProfile&));
+  MOCK_METHOD1(UpdateAutofillProfile, void(const autofill::AutofillProfile&));
 };
 
 MATCHER_P3(ProfileMatches, name, email, phone_number, "") {
@@ -91,8 +78,6 @@ NSArray<EditorField*>* GetEditorFields() {
                       required:YES],
   ];
 }
-
-using ::testing::_;
 }  // namespace
 
 class PaymentRequestContactInfoEditCoordinatorTest
@@ -110,10 +95,6 @@ class PaymentRequestContactInfoEditCoordinatorTest
         payment_request_test_util::CreateTestWebPaymentRequest(),
         browser_state(), web_state(), &personal_data_manager_);
 
-    profile_comparator_ = std::make_unique<MockPaymentsProfileComparator>(
-        payment_request_->GetApplicationLocale(), *payment_request_.get());
-    payment_request_->SetProfileComparator(profile_comparator_.get());
-
     test_region_data_loader_.set_synchronous_callback(true);
     payment_request_->SetRegionDataLoader(&test_region_data_loader_);
   }
@@ -124,9 +105,8 @@ class PaymentRequestContactInfoEditCoordinatorTest
     PaymentRequestUnitTestBase::TearDown();
   }
 
-  MockTestPersonalDataManager personal_data_manager_;
+  autofill::TestPersonalDataManager personal_data_manager_;
   autofill::TestRegionDataLoader test_region_data_loader_;
-  std::unique_ptr<MockPaymentsProfileComparator> profile_comparator_;
   std::unique_ptr<MockTestPaymentRequest> payment_request_;
 };
 
@@ -198,15 +178,8 @@ TEST_F(PaymentRequestContactInfoEditCoordinatorTest, DidFinishCreating) {
               AddAutofillProfile(
                   ProfileMatches("John Doe", "john@doe.com", "1 650-211-1111")))
       .Times(1);
-  // Expect an autofill profile to be added to the PersonalDataManager.
-  EXPECT_CALL(
-      personal_data_manager_,
-      AddProfile(ProfileMatches("John Doe", "john@doe.com", "1 650-211-1111")))
-      .Times(1);
-  // No autofill profile should get updated in the PersonalDataManager.
-  EXPECT_CALL(personal_data_manager_, UpdateProfile(_)).Times(0);
-  // No autofill profile should get invalidated in PaymentsProfileComparator.
-  EXPECT_CALL(*profile_comparator_, Invalidate(_)).Times(0);
+  // No autofill profile should get updated in the PaymentRequest.
+  EXPECT_CALL(*payment_request_, UpdateAutofillProfile(_)).Times(0);
 
   // Call the controller delegate method.
   EXPECT_TRUE([base_view_controller.presentedViewController
@@ -259,18 +232,11 @@ TEST_F(PaymentRequestContactInfoEditCoordinatorTest, DidFinishEditing) {
 
   // No autofill profile should get added to the PaymentRequest.
   EXPECT_CALL(*payment_request_, AddAutofillProfile(_)).Times(0);
-  // No autofill profile should get added to the PersonalDataManager.
-  EXPECT_CALL(personal_data_manager_, AddProfile(_)).Times(0);
-  // Expect an autofill profile to be updated in the PersonalDataManager.
-  EXPECT_CALL(personal_data_manager_,
-              UpdateProfile(
+  // Expect an autofill profile to be updated in the PaymentRequest.
+  EXPECT_CALL(*payment_request_,
+              UpdateAutofillProfile(
                   ProfileMatches("John Doe", "john@doe.com", "1 650-211-1111")))
       .Times(1);
-  // Expect an autofill profile to be invalidated in PaymentsProfileComparator.
-  EXPECT_CALL(
-      *profile_comparator_,
-      Invalidate(ProfileMatches("John Doe", "john@doe.com", "1 650-211-1111")))
-      .Times(1);
 
   // Call the controller delegate method.
   EXPECT_TRUE([base_view_controller.presentedViewController

ios/chrome/browser/ui/payments/credit_card_edit_coordinator.mm

@@ -128,9 +128,10 @@
   // Create an empty credit card. If a credit card is being edited, copy over
   // the information.
   autofill::CreditCard creditCard =
-      _creditCard ? *_creditCard
-                  : autofill::CreditCard(base::GenerateGUID(),
-                                         autofill::kSettingsOrigin);
+      _creditCard ? *_creditCard : autofill::CreditCard();
+
+  // Set the origin, or override it if the card is being edited.
+  creditCard.set_origin(autofill::kSettingsOrigin);
 
   for (EditorField* field in fields) {
     if (field.autofillUIType == AutofillUITypeCreditCardExpDate) {
@@ -159,25 +160,15 @@
   }
 
   if (!_creditCard) {
-    if (saveCreditCard)
-      _paymentRequest->GetPersonalDataManager()->AddCreditCard(creditCard);
-
     // Add the credit card to the list of payment methods in |_paymentRequest|.
-    _paymentMethod = _paymentRequest->AddAutofillPaymentInstrument(creditCard);
+    if (saveCreditCard) {
+      _paymentMethod =
+          _paymentRequest->CreateAndAddAutofillPaymentInstrument(creditCard);
+    }
   } else {
-    // Override the origin.
-    creditCard.set_origin(autofill::kSettingsOrigin);
-
     // Update the original credit card instance that is being edited.
     *_creditCard = creditCard;
-
-    if (autofill::IsCreditCardLocal(creditCard)) {
-      _paymentRequest->GetPersonalDataManager()->UpdateCreditCard(creditCard);
-    } else {
-      // Update server credit card's billing address.
-      _paymentRequest->GetPersonalDataManager()->UpdateServerCardMetadata(
-          creditCard);
-    }
+    _paymentRequest->UpdateAutofillPaymentInstrument(creditCard);
   }
 
   [_delegate creditCardEditCoordinator:self

ios/chrome/browser/ui/payments/credit_card_edit_coordinator_unittest.mm

@@ -32,12 +32,8 @@
 #endif
 
 namespace {
-class MockTestPersonalDataManager : public autofill::TestPersonalDataManager {
- public:
-  MockTestPersonalDataManager() : TestPersonalDataManager() {}
-  MOCK_METHOD1(AddCreditCard, void(const autofill::CreditCard&));
-  MOCK_METHOD1(UpdateCreditCard, void(const autofill::CreditCard&));
-};
+
+using ::testing::_;
 
 class MockPaymentRequest : public payments::TestPaymentRequest {
  public:
@@ -50,8 +46,10 @@ class MockPaymentRequest : public payments::TestPaymentRequest {
                                      web_state,
                                      personal_data_manager) {}
   MOCK_METHOD1(
-      AddAutofillPaymentInstrument,
+      CreateAndAddAutofillPaymentInstrument,
       payments::AutofillPaymentInstrument*(const autofill::CreditCard&));
+  MOCK_METHOD1(UpdateAutofillPaymentInstrument,
+               void(const autofill::CreditCard&));
 };
 
 MATCHER_P5(CreditCardMatches,
@@ -109,8 +107,6 @@ NSArray<EditorField*>* GetEditorFields(bool save_card) {
                       required:YES],
   ];
 }
-
-using ::testing::_;
 }  // namespace
 
 class PaymentRequestCreditCardEditCoordinatorTest
@@ -129,7 +125,7 @@ class PaymentRequestCreditCardEditCoordinatorTest
 
   void TearDown() override { PaymentRequestUnitTestBase::TearDown(); }
 
-  MockTestPersonalDataManager personal_data_manager_;
+  autofill::TestPersonalDataManager personal_data_manager_;
   std::unique_ptr<MockPaymentRequest> payment_request_;
 };
 
@@ -199,16 +195,12 @@ TEST_F(PaymentRequestCreditCardEditCoordinatorTest, DidFinishCreatingWithSave) {
 
   // Expect a payment method to be added to the PaymentRequest.
   EXPECT_CALL(*payment_request_,
-              AddAutofillPaymentInstrument(CreditCardMatches(
+              CreateAndAddAutofillPaymentInstrument(CreditCardMatches(
                   "4111111111111111", "John Doe", "12", "2090", "12345")))
       .Times(1);
-  // Expect a payment method to be added to the PersonalDataManager.
-  EXPECT_CALL(personal_data_manager_,
-              AddCreditCard(CreditCardMatches("4111111111111111", "John Doe",
-                                              "12", "2090", "12345")))
-      .Times(1);
-  // No payment method should get updated in the PersonalDataManager.
-  EXPECT_CALL(personal_data_manager_, UpdateCreditCard(_)).Times(0);
+
+  // No payment method should get updated in the PaymentRequest.
+  EXPECT_CALL(*payment_request_, UpdateAutofillPaymentInstrument(_)).Times(0);
 
   // Call the controller delegate method.
   EXPECT_TRUE([base_view_controller.presentedViewController
@@ -256,15 +248,11 @@ TEST_F(PaymentRequestCreditCardEditCoordinatorTest, DidFinishCreatingNoSave) {
   base::test::ios::SpinRunLoopWithMaxDelay(base::TimeDelta::FromSecondsD(1.0));
   EXPECT_NE(nil, base_view_controller.presentedViewController);
 
-  // Expect a payment method to be added to the PaymentRequest.
-  EXPECT_CALL(*payment_request_,
-              AddAutofillPaymentInstrument(CreditCardMatches(
-                  "4111111111111111", "John Doe", "12", "2090", "12345")))
-      .Times(1);
-  // No payment method should get added to the PersonalDataManager.
-  EXPECT_CALL(personal_data_manager_, AddCreditCard(_)).Times(0);
-  // No payment method should get updated in the PersonalDataManager.
-  EXPECT_CALL(personal_data_manager_, UpdateCreditCard(_)).Times(0);
+  // No payment method should get added to the PaymentRequest.
+  EXPECT_CALL(*payment_request_, CreateAndAddAutofillPaymentInstrument(_))
+      .Times(0);
+  // No payment method should get updated in the PaymentRequest.
+  EXPECT_CALL(*payment_request_, UpdateAutofillPaymentInstrument(_)).Times(0);
 
   // Call the controller delegate method.
   EXPECT_TRUE([base_view_controller.presentedViewController
@@ -318,13 +306,12 @@ TEST_F(PaymentRequestCreditCardEditCoordinatorTest, DidFinishEditing) {
   EXPECT_NE(nil, base_view_controller.presentedViewController);
 
   // No payment method should get added to the PaymentRequest.
-  EXPECT_CALL(*payment_request_, AddAutofillPaymentInstrument(_)).Times(0);
-  // No payment method should get added to the PersonalDataManager.
-  EXPECT_CALL(personal_data_manager_, AddCreditCard(_)).Times(0);
-  // Expect a payment method to be updated in the PersonalDataManager.
-  EXPECT_CALL(personal_data_manager_,
-              UpdateCreditCard(CreditCardMatches("4111111111111111", "John Doe",
-                                                 "12", "2090", "12345")))
+  EXPECT_CALL(*payment_request_, CreateAndAddAutofillPaymentInstrument(_))
+      .Times(0);
+  // No payment method should get updated in the PaymentRequest.
+  EXPECT_CALL(*payment_request_,
+              UpdateAutofillPaymentInstrument(CreditCardMatches(
+                  "4111111111111111", "John Doe", "12", "2090", "12345")))
       .Times(1);
 
   // Call the controller delegate method.