Commit 030a7728 authored by sandromaggi's avatar sandromaggi Committed by Commit Bot

[Autofill Assistant] Move card logic to c++

This CL moves a part of the card initialization logic
from Java to c++. Certain parts are currently remaining
in Java.
Ideally, we would remove the necessity for
AutofillPaymentApp in Java and move this part of the
logic to c++ as well, which is currently not easily
possible without recreating some of the logic.

Bug: b/144681042
Change-Id: I64e7ad0337503da5ed3a08b465c760392e0045fb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1921972
Commit-Queue: Sandro Maggi <sandromaggi@google.com>
Reviewed-by: default avatarClemens Arbesser <arbesser@google.com>
Cr-Commit-Position: refs/heads/master@{#718091}
parent 95098dc2
......@@ -143,7 +143,6 @@ public class AssistantCollectUserDataCoordinator {
* Explicitly clean up.
*/
public void destroy() {
mViewHolder.destroy();
mViewHolder = null;
}
......
......@@ -18,13 +18,11 @@ import org.chromium.chrome.browser.payments.AutofillAddress;
import org.chromium.chrome.browser.payments.AutofillContact;
import org.chromium.chrome.browser.payments.AutofillPaymentInstrument;
import org.chromium.content_public.browser.WebContents;
import org.chromium.payments.mojom.PaymentMethodData;
import org.chromium.ui.modelutil.PropertyModel;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
/**
* State for the header of the Autofill Assistant.
......@@ -34,6 +32,31 @@ public class AssistantCollectUserDataModel extends PropertyModel {
// TODO(crbug.com/806868): add |setAvailableProfiles| and |setAvailablePaymentMethods| from
// native. Implement |setShippingAddress|, |setContactDetails| and |setPaymentMethod|.
/**
* This class holds a the credit card and billing address information required to create an
* AutofillPaymentInstrument
*/
public static class PaymentTuple {
private final PersonalDataManager.CreditCard mCreditCard;
@Nullable
private final PersonalDataManager.AutofillProfile mBillingAddress;
public PaymentTuple(PersonalDataManager.CreditCard creditCard,
@Nullable PersonalDataManager.AutofillProfile billingAddress) {
mCreditCard = creditCard;
mBillingAddress = billingAddress;
}
public PersonalDataManager.CreditCard getCreditCard() {
return mCreditCard;
}
@Nullable
public PersonalDataManager.AutofillProfile getBillingAddress() {
return mBillingAddress;
}
}
public static final WritableObjectPropertyKey<AssistantCollectUserDataDelegate> DELEGATE =
new WritableObjectPropertyKey<>();
......@@ -87,16 +110,12 @@ public class AssistantCollectUserDataModel extends PropertyModel {
public static final WritableObjectPropertyKey<List<PersonalDataManager.AutofillProfile>>
AVAILABLE_PROFILES = new WritableObjectPropertyKey<>();
public static final WritableObjectPropertyKey<List<AutofillPaymentInstrument>>
public static final WritableObjectPropertyKey<List<AssistantCollectUserDataModel.PaymentTuple>>
AVAILABLE_AUTOFILL_PAYMENT_METHODS = new WritableObjectPropertyKey<>();
public static final WritableObjectPropertyKey<List<String>> SUPPORTED_BASIC_CARD_NETWORKS =
new WritableObjectPropertyKey<>();
/** The available payment methods, e.g., |BASIC_CARD|. */
public static final WritableObjectPropertyKey<Map<String, PaymentMethodData>>
SUPPORTED_PAYMENT_METHODS = new WritableObjectPropertyKey<>();
/** The available login choices. */
public static final WritableObjectPropertyKey<List<AssistantLoginChoice>> AVAILABLE_LOGINS =
new WritableObjectPropertyKey<>();
......@@ -144,11 +163,11 @@ public class AssistantCollectUserDataModel extends PropertyModel {
REQUEST_EMAIL, REQUEST_PHONE, REQUEST_SHIPPING_ADDRESS, REQUEST_PAYMENT,
ACCEPT_TERMS_AND_CONDITIONS_TEXT, SHOW_TERMS_AS_CHECKBOX, REQUEST_LOGIN_CHOICE,
AVAILABLE_PROFILES, AVAILABLE_AUTOFILL_PAYMENT_METHODS,
SUPPORTED_BASIC_CARD_NETWORKS, SUPPORTED_PAYMENT_METHODS, AVAILABLE_LOGINS,
EXPANDED_SECTION, REQUIRE_BILLING_POSTAL_CODE, BILLING_POSTAL_CODE_MISSING_TEXT,
REQUEST_DATE_RANGE, DATE_RANGE_START, DATE_RANGE_START_LABEL, DATE_RANGE_END,
DATE_RANGE_END_LABEL, PREPENDED_SECTIONS, APPENDED_SECTIONS,
TERMS_REQUIRE_REVIEW_TEXT, THIRDPARTY_PRIVACY_NOTICE_TEXT);
SUPPORTED_BASIC_CARD_NETWORKS, AVAILABLE_LOGINS, EXPANDED_SECTION,
REQUIRE_BILLING_POSTAL_CODE, BILLING_POSTAL_CODE_MISSING_TEXT, REQUEST_DATE_RANGE,
DATE_RANGE_START, DATE_RANGE_START_LABEL, DATE_RANGE_END, DATE_RANGE_END_LABEL,
PREPENDED_SECTIONS, APPENDED_SECTIONS, TERMS_REQUIRE_REVIEW_TEXT,
THIRDPARTY_PRIVACY_NOTICE_TEXT);
/**
* Set initial state for basic type properties (others are implicitly null).
......@@ -387,4 +406,24 @@ public class AssistantCollectUserDataModel extends PropertyModel {
private void setAutofillProfiles(List<PersonalDataManager.AutofillProfile> profiles) {
set(AVAILABLE_PROFILES, profiles);
}
@CalledByNative
private static List<AssistantCollectUserDataModel.PaymentTuple>
createAutofillPaymentMethodList() {
return new ArrayList<>();
}
@CalledByNative
private static void addAutofillPaymentMethod(
List<AssistantCollectUserDataModel.PaymentTuple> paymentTuples,
PersonalDataManager.CreditCard card,
@Nullable PersonalDataManager.AutofillProfile billingAddress) {
paymentTuples.add(new PaymentTuple(card, billingAddress));
}
@CalledByNative
private void setAutofillPaymentMethods(
List<AssistantCollectUserDataModel.PaymentTuple> paymentTuples) {
set(AVAILABLE_AUTOFILL_PAYMENT_METHODS, paymentTuples);
}
}
......@@ -17,7 +17,6 @@ import org.chromium.base.test.util.CallbackHelper;
import org.chromium.chrome.browser.autofill.CardType;
import org.chromium.chrome.browser.autofill.PersonalDataManager;
import org.chromium.chrome.browser.autofill.PersonalDataManager.AutofillProfile;
import org.chromium.chrome.browser.autofill.PersonalDataManager.CreditCard;
import org.chromium.chrome.browser.autofill_assistant.user_data.AssistantChoiceList;
import org.chromium.chrome.browser.autofill_assistant.user_data.AssistantCollectUserDataCoordinator;
import org.chromium.chrome.browser.autofill_assistant.user_data.AssistantCollectUserDataDelegate;
......@@ -206,41 +205,28 @@ public class AutofillAssistantCollectUserDataTestHelper {
* @param fullName The full name for the profile to create.
* @param email The email for the profile to create.
* @param postcode The postcode of the billing address.
* @return the GUID of the created profile.
* @return the profile.
*/
public PersonalDataManager.AutofillProfile createDummyProfile(
String fullName, String email, String postcode) {
return new PersonalDataManager.AutofillProfile("" /* guid */,
"https://www.example.com" /* origin */, fullName, "Acme Inc.", "123 Main",
"California", "Los Angeles", "", postcode, "", "Uzbekistan", "555 123-4567", email,
"");
"California", "Los Angeles", "", postcode, "", "UZ", "555 123-4567", email, "");
}
public PersonalDataManager.AutofillProfile createDummyProfile(String fullName, String email) {
return createDummyProfile(fullName, email, "90210");
}
public CreditCard getCreditCard(final String guid) {
return TestThreadUtils.runOnUiThreadBlockingNoException(
() -> PersonalDataManager.getInstance().getCreditCard(guid));
}
public String getShippingAddressLabelWithoutCountryForPaymentRequest(AutofillProfile profile) {
return TestThreadUtils.runOnUiThreadBlockingNoException(
()
-> PersonalDataManager.getInstance()
.getShippingAddressLabelWithoutCountryForPaymentRequest(
profile));
}
public String getShippingAddressLabelWithCountryForPaymentRequest(AutofillProfile profile) {
return TestThreadUtils.runOnUiThreadBlockingNoException(
()
-> PersonalDataManager.getInstance()
.getShippingAddressLabelWithCountryForPaymentRequest(profile));
}
/**
* Adds a credit card with dummy data to the personal data manager.
*
* @param billingAddressId The billing address profile GUID.
* @return the GUID of the created credit card
*/
public String addDummyCreditCard(String billingAddressId) throws TimeoutException {
PersonalDataManager.CreditCard card = createDummyCreditCard(billingAddressId);
public String setCreditCard(final CreditCard card) throws TimeoutException {
int callCount = mOnPersonalDataChangedHelper.getCallCount();
String guid = TestThreadUtils.runOnUiThreadBlockingNoException(
() -> PersonalDataManager.getInstance().setCreditCard(card));
......@@ -249,34 +235,19 @@ public class AutofillAssistantCollectUserDataTestHelper {
}
/**
* Adds a credit card with dummy data to the personal data manager.
* Create a credit card with dummy data.
*
* @param billingAddressId The billing address profile GUID.
* @return the GUID of the created credit card
* @return the card.
*/
public String addDummyCreditCard(String billingAddressId) throws TimeoutException {
public PersonalDataManager.CreditCard createDummyCreditCard(String billingAddressId) {
String profileName = TestThreadUtils.runOnUiThreadBlockingNoException(
() -> PersonalDataManager.getInstance().getProfile(billingAddressId).getFullName());
PersonalDataManager.CreditCard creditCard = new PersonalDataManager.CreditCard("",
"https://example.com", true, true, profileName, "4111111111111111", "1111", "12",
"2050", "visa", org.chromium.chrome.autofill_assistant.R.drawable.visa_card,
CardType.UNKNOWN, billingAddressId, "" /* serverId */);
return setCreditCard(creditCard);
}
public void deleteProfile(final String guid) throws TimeoutException {
int callCount = mOnPersonalDataChangedHelper.getCallCount();
TestThreadUtils.runOnUiThreadBlocking(
() -> PersonalDataManager.getInstance().deleteProfile(guid));
mOnPersonalDataChangedHelper.waitForCallback(callCount);
}
public void deleteCreditCard(final String guid) throws TimeoutException {
int callCount = mOnPersonalDataChangedHelper.getCallCount();
TestThreadUtils.runOnUiThreadBlocking(
() -> PersonalDataManager.getInstance().deleteCreditCard(guid));
mOnPersonalDataChangedHelper.waitForCallback(callCount);
return new PersonalDataManager.CreditCard("", "https://example.com", true, true,
profileName, "4111111111111111", "1111", "12", "2050", "visa",
org.chromium.chrome.autofill_assistant.R.drawable.visa_card, CardType.UNKNOWN,
billingAddressId, "" /* serverId */);
}
private void registerDataObserver() throws TimeoutException {
......
......@@ -996,6 +996,25 @@ void UiControllerAndroid::OnUserDataChanged(
}
Java_AssistantCollectUserDataModel_setAutofillProfiles(env, jmodel, jlist);
}
if (field_change == UserData::FieldChange::ALL ||
field_change == UserData::FieldChange::AVAILABLE_PAYMENT_INSTRUMENTS) {
auto jlist =
Java_AssistantCollectUserDataModel_createAutofillPaymentMethodList(env);
for (const auto& instrument : state->available_payment_instruments) {
Java_AssistantCollectUserDataModel_addAutofillPaymentMethod(
env, jlist,
autofill::PersonalDataManagerAndroid::CreateJavaCreditCardFromNative(
env, *(instrument->card)),
instrument->billing_address == nullptr
? nullptr
: autofill::PersonalDataManagerAndroid::
CreateJavaProfileFromNative(
env, *(instrument->billing_address)));
}
Java_AssistantCollectUserDataModel_setAutofillPaymentMethods(env, jmodel,
jlist);
}
}
// FormProto related methods.
......
......@@ -454,8 +454,10 @@ void CollectUserDataAction::ShowToUser(
// Add available profiles and start listening.
delegate_->GetPersonalDataManager()->AddObserver(this);
UpdatePersonalDataManagerFields(collect_user_data_options.get(),
user_data.get());
UpdatePersonalDataManagerProfiles(collect_user_data_options.get(),
user_data.get());
UpdatePersonalDataManagerCards(collect_user_data_options.get(),
user_data.get());
// Gather info for UMA histograms.
if (!shown_to_user_) {
......@@ -817,7 +819,7 @@ bool CollectUserDataAction::IsUserDataComplete(
user_data.date_time_range_end, options);
}
void CollectUserDataAction::UpdatePersonalDataManagerFields(
void CollectUserDataAction::UpdatePersonalDataManagerProfiles(
const CollectUserDataOptions* collect_user_data_options,
UserData* user_data,
UserData::FieldChange* field_change) {
......@@ -826,7 +828,7 @@ void CollectUserDataAction::UpdatePersonalDataManagerFields(
}
user_data->available_profiles.clear();
for (auto* profile :
for (const auto* profile :
delegate_->GetPersonalDataManager()->GetProfilesToSuggest()) {
user_data->available_profiles.emplace_back(
std::make_unique<autofill::AutofillProfile>(*profile));
......@@ -862,11 +864,54 @@ void CollectUserDataAction::UpdatePersonalDataManagerFields(
}
}
void CollectUserDataAction::UpdatePersonalDataManagerCards(
const CollectUserDataOptions* collect_user_data_options,
UserData* user_data,
UserData::FieldChange* field_change) {
if (collect_user_data_options == nullptr || user_data == nullptr) {
return;
}
user_data->available_payment_instruments.clear();
for (const auto* card :
delegate_->GetPersonalDataManager()->GetCreditCardsToSuggest(true)) {
if (std::find(
collect_user_data_options->supported_basic_card_networks.begin(),
collect_user_data_options->supported_basic_card_networks.end(),
autofill::data_util::GetPaymentRequestData(card->network())
.basic_card_issuer_network) !=
collect_user_data_options->supported_basic_card_networks.end()) {
auto payment_instrument = std::make_unique<PaymentInstrument>();
payment_instrument->card = std::make_unique<autofill::CreditCard>(*card);
if (!card->billing_address_id().empty()) {
auto* billing_address =
delegate_->GetPersonalDataManager()->GetProfileByGUID(
card->billing_address_id());
if (billing_address != nullptr) {
payment_instrument->billing_address =
std::make_unique<autofill::AutofillProfile>(*billing_address);
}
}
user_data->available_payment_instruments.emplace_back(
std::move(payment_instrument));
}
}
if (field_change != nullptr) {
*field_change = UserData::FieldChange::AVAILABLE_PAYMENT_INSTRUMENTS;
}
}
void CollectUserDataAction::OnPersonalDataChanged() {
personal_data_changed_ = true;
delegate_->WriteUserData(
base::BindOnce(&CollectUserDataAction::UpdatePersonalDataManagerFields,
base::BindOnce(&CollectUserDataAction::UpdatePersonalDataManagerProfiles,
weak_ptr_factory_.GetWeakPtr()));
delegate_->WriteUserData(
base::BindOnce(&CollectUserDataAction::UpdatePersonalDataManagerCards,
weak_ptr_factory_.GetWeakPtr()));
}
......
......@@ -74,7 +74,11 @@ class CollectUserDataAction : public Action,
const CollectUserDataOptions& collect_user_data_options);
// Update user data with the new state from personal data manager.
void UpdatePersonalDataManagerFields(
void UpdatePersonalDataManagerProfiles(
const CollectUserDataOptions* collect_user_data_options,
UserData* user_data,
UserData::FieldChange* field_change = nullptr);
void UpdatePersonalDataManagerCards(
const CollectUserDataOptions* collect_user_data_options,
UserData* user_data,
UserData::FieldChange* field_change = nullptr);
......
......@@ -1055,5 +1055,105 @@ TEST_F(CollectUserDataActionTest, SortsProfilesByCompleteness) {
action.ProcessAction(callback_.Get());
}
TEST_F(CollectUserDataActionTest, AttachesCreditCardsWithAddress) {
ON_CALL(mock_personal_data_manager_, IsAutofillCreditCardEnabled)
.WillByDefault(Return(true));
ON_CALL(mock_personal_data_manager_, ShouldSuggestServerCards)
.WillByDefault(Return(true));
autofill::AutofillProfile billing_address;
autofill::test::SetProfileInfo(
&billing_address, "Berta", "", "West", "berta.west@gmail.com", "",
"Baker Street 221b", "", "London", "", "WC2N 5DU", "UK", "+44");
ON_CALL(mock_personal_data_manager_, GetProfileByGUID("GUID"))
.WillByDefault(Return(&billing_address));
autofill::CreditCard card_with_address;
autofill::test::SetCreditCardInfo(&card_with_address, "John Doe",
"4111111111111111", "1", "2050",
/* billing_address_id= */ "GUID");
ON_CALL(mock_personal_data_manager_, GetCreditCards())
.WillByDefault(
Return(std::vector<autofill::CreditCard*>({&card_with_address})));
ON_CALL(mock_action_delegate_, CollectUserData(_, _))
.WillByDefault(Invoke(
[=](std::unique_ptr<CollectUserDataOptions> collect_user_data_options,
std::unique_ptr<UserData> user_data) {
user_data->succeed = true;
EXPECT_THAT(user_data->available_payment_instruments, SizeIs(1));
EXPECT_EQ(
user_data->available_payment_instruments[0]->card->Compare(
card_with_address),
0);
EXPECT_EQ(user_data->available_payment_instruments[0]
->billing_address->Compare(billing_address),
0);
std::move(collect_user_data_options->confirm_callback)
.Run(std::move(user_data));
}));
ActionProto action_proto;
auto* user_data = action_proto.mutable_collect_user_data();
SetRequiredTermsFields(user_data);
user_data->add_supported_basic_card_networks("visa");
EXPECT_CALL(
callback_,
Run(Pointee(Property(&ProcessedActionProto::status, ACTION_APPLIED))));
CollectUserDataAction action(&mock_action_delegate_, action_proto);
action.ProcessAction(callback_.Get());
}
TEST_F(CollectUserDataActionTest, AttachesCreditCardsWithoutAddress) {
ON_CALL(mock_personal_data_manager_, IsAutofillCreditCardEnabled)
.WillByDefault(Return(true));
ON_CALL(mock_personal_data_manager_, ShouldSuggestServerCards)
.WillByDefault(Return(true));
autofill::CreditCard card_without_address;
autofill::test::SetCreditCardInfo(&card_without_address, "John Doe",
"4111111111111111", "1", "2050",
/* billing_address_id= */ "");
ON_CALL(mock_personal_data_manager_, GetCreditCards())
.WillByDefault(
Return(std::vector<autofill::CreditCard*>({&card_without_address})));
ON_CALL(mock_action_delegate_, CollectUserData(_, _))
.WillByDefault(Invoke([=](std::unique_ptr<CollectUserDataOptions>
collect_user_data_options,
std::unique_ptr<UserData> user_data) {
user_data->succeed = true;
EXPECT_THAT(user_data->available_payment_instruments, SizeIs(1));
EXPECT_EQ(user_data->available_payment_instruments[0]->card->Compare(
card_without_address),
0);
EXPECT_EQ(
user_data->available_payment_instruments[0]->billing_address.get(),
nullptr);
std::move(collect_user_data_options->confirm_callback)
.Run(std::move(user_data));
}));
ActionProto action_proto;
auto* user_data = action_proto.mutable_collect_user_data();
SetRequiredTermsFields(user_data);
user_data->add_supported_basic_card_networks("visa");
EXPECT_CALL(mock_personal_data_manager_, GetProfileByGUID(_)).Times(0);
EXPECT_CALL(
callback_,
Run(Pointee(Property(&ProcessedActionProto::status, ACTION_APPLIED))));
CollectUserDataAction action(&mock_action_delegate_, action_proto);
action.ProcessAction(callback_.Get());
}
} // namespace
} // namespace autofill_assistant
......@@ -26,6 +26,8 @@ class MockPersonalDataManager : public autofill::PersonalDataManager {
MOCK_CONST_METHOD0(GetProfiles, std::vector<autofill::AutofillProfile*>());
MOCK_CONST_METHOD0(GetCreditCards, std::vector<autofill::CreditCard*>());
MOCK_CONST_METHOD0(IsAutofillProfileEnabled, bool());
MOCK_CONST_METHOD0(IsAutofillCreditCardEnabled, bool());
MOCK_CONST_METHOD0(ShouldSuggestServerCards, bool());
};
} // namespace autofill_assistant
......
......@@ -25,6 +25,9 @@ LoginChoice::LoginChoice(const std::string& _identifier,
LoginChoice::LoginChoice(const LoginChoice& another) = default;
LoginChoice::~LoginChoice() = default;
PaymentInstrument::PaymentInstrument() = default;
PaymentInstrument::~PaymentInstrument() = default;
UserData::UserData() = default;
UserData::~UserData() = default;
......
......@@ -62,6 +62,15 @@ struct LoginChoice {
base::Optional<InfoPopupProto> info_popup;
};
// Tuple for holding credit card and billing address;
struct PaymentInstrument {
PaymentInstrument();
~PaymentInstrument();
std::unique_ptr<autofill::CreditCard> card;
std::unique_ptr<autofill::AutofillProfile> billing_address;
};
// Struct for holding the user data.
struct UserData {
UserData();
......@@ -80,6 +89,7 @@ struct UserData {
DATE_TIME_RANGE_END,
ADDITIONAL_VALUES,
AVAILABLE_PROFILES,
AVAILABLE_PAYMENT_INSTRUMENTS,
};
bool succeed = false;
......@@ -96,6 +106,7 @@ struct UserData {
std::map<std::string, std::string> additional_values_to_store;
std::vector<std::unique_ptr<autofill::AutofillProfile>> available_profiles;
std::vector<std::unique_ptr<PaymentInstrument>> available_payment_instruments;
};
// Struct for holding the payment request options.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment