Commit f8a84d53 authored by sandromaggi's avatar sandromaggi Committed by Chromium LUCI CQ

[Autofill Assistant] Report credit card status details

This changes the |SelfDeleteFullCardRequester| to respond
with adding details to the failure status according to the
the result it got from the |FullCardRequest|.

The |UseCreditCardAction| does not treat a failed status,
but passes it on as the response to the action.

Bug: b/174304769
Change-Id: Idcad69cf74eab14a9a59d5b6dc1ea166b230db67
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2577218
Commit-Queue: Sandro Maggi <sandromaggi@google.com>
Reviewed-by: default avatarClemens Arbesser <arbesser@google.com>
Cr-Commit-Position: refs/heads/master@{#834643}
parent 398596d6
...@@ -170,7 +170,8 @@ class ActionDelegate { ...@@ -170,7 +170,8 @@ class ActionDelegate {
write_callback) = 0; write_callback) = 0;
using GetFullCardCallback = using GetFullCardCallback =
base::OnceCallback<void(std::unique_ptr<autofill::CreditCard> card, base::OnceCallback<void(const ClientStatus& status,
std::unique_ptr<autofill::CreditCard> card,
const base::string16& cvc)>; const base::string16& cvc)>;
// Asks for the full card information for |credit_card|. Might require the // Asks for the full card information for |credit_card|. Might require the
......
...@@ -181,16 +181,9 @@ class MockActionDelegate : public ActionDelegate { ...@@ -181,16 +181,9 @@ class MockActionDelegate : public ActionDelegate {
WriteUserData, WriteUserData,
void(base::OnceCallback<void(UserData*, UserData::FieldChange*)>)); void(base::OnceCallback<void(UserData*, UserData::FieldChange*)>));
void GetFullCard(const autofill::CreditCard* credit_card, MOCK_METHOD2(GetFullCard,
ActionDelegate::GetFullCardCallback callback) override { void(const autofill::CreditCard* credit_card,
OnGetFullCard(credit_card, callback); ActionDelegate::GetFullCardCallback callback));
}
MOCK_METHOD2(
OnGetFullCard,
void(const autofill::CreditCard* credit_card,
base::OnceCallback<void(std::unique_ptr<autofill::CreditCard> card,
const base::string16& cvc)>& callback));
MOCK_METHOD2(GetFieldValue, MOCK_METHOD2(GetFieldValue,
void(const ElementFinder::Result& element, void(const ElementFinder::Result& element,
......
...@@ -139,13 +139,15 @@ void UseCreditCardAction::OnWaitForElement(const ClientStatus& element_status) { ...@@ -139,13 +139,15 @@ void UseCreditCardAction::OnWaitForElement(const ClientStatus& element_status) {
} }
void UseCreditCardAction::OnGetFullCard( void UseCreditCardAction::OnGetFullCard(
const ClientStatus& status,
std::unique_ptr<autofill::CreditCard> card, std::unique_ptr<autofill::CreditCard> card,
const base::string16& cvc) { const base::string16& cvc) {
action_stopwatch_.StartActiveTime(); action_stopwatch_.StartActiveTime();
if (!card) { if (!status.ok()) {
EndAction(ClientStatus(GET_FULL_CARD_FAILED)); EndAction(status);
return; return;
} }
DCHECK(card);
std::vector<RequiredField> required_fields; std::vector<RequiredField> required_fields;
for (const auto& required_field_proto : proto_.use_card().required_fields()) { for (const auto& required_field_proto : proto_.use_card().required_fields()) {
......
...@@ -44,7 +44,8 @@ class UseCreditCardAction : public Action { ...@@ -44,7 +44,8 @@ class UseCreditCardAction : public Action {
void OnWaitForElement(const ClientStatus& element_status); void OnWaitForElement(const ClientStatus& element_status);
// Called after getting full credit card with its cvc. // Called after getting full credit card with its cvc.
void OnGetFullCard(std::unique_ptr<autofill::CreditCard> card, void OnGetFullCard(const ClientStatus& status,
std::unique_ptr<autofill::CreditCard> card,
const base::string16& cvc); const base::string16& cvc);
// Called when the form credit card has been filled. // Called when the form credit card has been filled.
......
...@@ -37,7 +37,6 @@ using ::testing::_; ...@@ -37,7 +37,6 @@ using ::testing::_;
using ::testing::Eq; using ::testing::Eq;
using ::testing::Expectation; using ::testing::Expectation;
using ::testing::InSequence; using ::testing::InSequence;
using ::testing::Invoke;
using ::testing::NotNull; using ::testing::NotNull;
using ::testing::Pointee; using ::testing::Pointee;
using ::testing::Return; using ::testing::Return;
...@@ -71,22 +70,26 @@ class UseCreditCardActionTest : public testing::Test { ...@@ -71,22 +70,26 @@ class UseCreditCardActionTest : public testing::Test {
ON_CALL(mock_action_delegate_, GetPersonalDataManager) ON_CALL(mock_action_delegate_, GetPersonalDataManager)
.WillByDefault(Return(&mock_personal_data_manager_)); .WillByDefault(Return(&mock_personal_data_manager_));
ON_CALL(mock_action_delegate_, RunElementChecks) ON_CALL(mock_action_delegate_, RunElementChecks)
.WillByDefault(Invoke([this](BatchElementChecker* checker) { .WillByDefault([this](BatchElementChecker* checker) {
checker->Run(&mock_web_controller_); checker->Run(&mock_web_controller_);
})); });
ON_CALL(mock_action_delegate_, OnShortWaitForElement(_, _)) ON_CALL(mock_action_delegate_, OnShortWaitForElement(_, _))
.WillByDefault(RunOnceCallback<1>(OkClientStatus(), .WillByDefault(RunOnceCallback<1>(OkClientStatus(),
base::TimeDelta::FromSeconds(0))); base::TimeDelta::FromSeconds(0)));
ON_CALL(mock_action_delegate_, OnGetFullCard) ON_CALL(mock_action_delegate_, GetFullCard)
.WillByDefault(Invoke([](const autofill::CreditCard* credit_card, .WillByDefault(
base::OnceCallback<void( [](const autofill::CreditCard* credit_card,
std::unique_ptr<autofill::CreditCard> card, base::OnceCallback<void(const ClientStatus&,
const base::string16& cvc)>& callback) { std::unique_ptr<autofill::CreditCard>,
std::move(callback).Run( const base::string16&)> callback) {
credit_card ? std::make_unique<autofill::CreditCard>(*credit_card) std::move(callback).Run(
: nullptr, credit_card ? OkClientStatus()
base::UTF8ToUTF16(kFakeCvc)); : ClientStatus(GET_FULL_CARD_FAILED),
})); credit_card
? std::make_unique<autofill::CreditCard>(*credit_card)
: nullptr,
base::UTF8ToUTF16(kFakeCvc));
});
test_util::MockFindAnyElement(mock_web_controller_); test_util::MockFindAnyElement(mock_web_controller_);
} }
......
...@@ -422,10 +422,8 @@ void ScriptExecutor::OnTermsAndConditionsLinkClicked( ...@@ -422,10 +422,8 @@ void ScriptExecutor::OnTermsAndConditionsLinkClicked(
std::move(callback).Run(link, user_data, user_model); std::move(callback).Run(link, user_data, user_model);
} }
void ScriptExecutor::GetFullCard( void ScriptExecutor::GetFullCard(const autofill::CreditCard* credit_card,
const autofill::CreditCard* credit_card, GetFullCardCallback callback) {
base::OnceCallback<void(std::unique_ptr<autofill::CreditCard> card,
const base::string16& cvc)> callback) {
DCHECK(credit_card); DCHECK(credit_card);
// User might be asked to provide the cvc. // User might be asked to provide the cvc.
...@@ -441,10 +439,11 @@ void ScriptExecutor::GetFullCard( ...@@ -441,10 +439,11 @@ void ScriptExecutor::GetFullCard(
} }
void ScriptExecutor::OnGetFullCard(GetFullCardCallback callback, void ScriptExecutor::OnGetFullCard(GetFullCardCallback callback,
const ClientStatus& status,
std::unique_ptr<autofill::CreditCard> card, std::unique_ptr<autofill::CreditCard> card,
const base::string16& cvc) { const base::string16& cvc) {
delegate_->EnterState(AutofillAssistantState::RUNNING); delegate_->EnterState(AutofillAssistantState::RUNNING);
std::move(callback).Run(std::move(card), cvc); std::move(callback).Run(status, std::move(card), cvc);
} }
void ScriptExecutor::Prompt( void ScriptExecutor::Prompt(
......
...@@ -430,6 +430,7 @@ class ScriptExecutor : public ActionDelegate, ...@@ -430,6 +430,7 @@ class ScriptExecutor : public ActionDelegate,
UserData* user_data, UserData* user_data,
const UserModel* user_model); const UserModel* user_model);
void OnGetFullCard(GetFullCardCallback callback, void OnGetFullCard(GetFullCardCallback callback,
const ClientStatus& status,
std::unique_ptr<autofill::CreditCard> card, std::unique_ptr<autofill::CreditCard> card,
const base::string16& cvc); const base::string16& cvc);
void OnChosen(UserAction::Callback callback, void OnChosen(UserAction::Callback callback,
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "components/autofill/core/browser/field_types.h" #include "components/autofill/core/browser/field_types.h"
#include "components/autofill/core/browser/payments/full_card_request.h" #include "components/autofill/core/browser/payments/full_card_request.h"
#include "components/autofill/core/browser/personal_data_manager.h" #include "components/autofill/core/browser/personal_data_manager.h"
#include "components/autofill_assistant/browser/client_status.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
namespace autofill_assistant { namespace autofill_assistant {
...@@ -55,18 +56,36 @@ void SelfDeleteFullCardRequester::OnFullCardRequestSucceeded( ...@@ -55,18 +56,36 @@ void SelfDeleteFullCardRequester::OnFullCardRequestSucceeded(
const FullCardRequest& /* full_card_request */, const FullCardRequest& /* full_card_request */,
const autofill::CreditCard& card, const autofill::CreditCard& card,
const base::string16& cvc) { const base::string16& cvc) {
std::move(callback_).Run(std::make_unique<autofill::CreditCard>(card), cvc); std::move(callback_).Run(OkClientStatus(),
std::make_unique<autofill::CreditCard>(card), cvc);
delete this; delete this;
} }
void SelfDeleteFullCardRequester::OnFullCardRequestFailed( void SelfDeleteFullCardRequester::OnFullCardRequestFailed(
FullCardRequest::FailureType failure_type) { FullCardRequest::FailureType failure_type) {
// Failed might because of cancel, so return nullptr to notice caller. ClientStatus status(GET_FULL_CARD_FAILED);
// AutofillErrorInfoProto::GetFullCardFailureType error_type =
// TODO(crbug.com/806868): Split the fail notification so that "cancel" and AutofillErrorInfoProto::UNKNOWN_FAILURE_TYPE;
// "wrong cvc" states can be handled differently. One should prompt a retry, switch (failure_type) {
// the other a graceful shutdown - the current behavior. case FullCardRequest::FailureType::PROMPT_CLOSED:
std::move(callback_).Run(nullptr, base::string16()); error_type = AutofillErrorInfoProto::PROMPT_CLOSED;
break;
case FullCardRequest::FailureType::VERIFICATION_DECLINED:
error_type = AutofillErrorInfoProto::VERIFICATION_DECLINED;
break;
case FullCardRequest::FailureType::GENERIC_FAILURE:
error_type = AutofillErrorInfoProto::GENERIC_FAILURE;
break;
default:
// Adding the default case such that additions to
// FullCardRequest::FailureType do not need to add new cases to Autofill
// Assistant code.
break;
}
status.mutable_details()
->mutable_autofill_error_info()
->set_get_full_card_failure_type(error_type);
std::move(callback_).Run(status, nullptr, base::string16());
delete this; delete this;
} }
} // namespace autofill_assistant } // namespace autofill_assistant
...@@ -877,6 +877,28 @@ message AutofillErrorInfoProto { ...@@ -877,6 +877,28 @@ message AutofillErrorInfoProto {
// Errors that occurred during fallback filling of autofill fields. // Errors that occurred during fallback filling of autofill fields.
repeated AutofillFieldError autofill_field_error = 5; repeated AutofillFieldError autofill_field_error = 5;
// These values should match the values in FullCardRequest::FailureType.
enum GetFullCardFailureType {
UNKNOWN_FAILURE_TYPE = 0;
// The user closed the prompt. The following scenarios are possible:
// 1) The user declined to enter their CVC and closed the prompt.
// 2) The user provided their CVC, got auth declined and then closed the
// prompt without attempting a second time.
// 3) The user provided their CVC and closed the prompt before waiting for
// the result.
PROMPT_CLOSED = 1;
// The card could not be looked up due to card auth declined or failed.
VERIFICATION_DECLINED = 2;
// The request failed for technical reasons, such as a closing page or lack
// of network connection.
GENERIC_FAILURE = 3;
}
// Failure type from the |FullCardRequest|.
optional GetFullCardFailureType get_full_card_failure_type = 6;
} }
// Message to report |SetFormFieldValueProto| related errors for debugging // Message to report |SetFormFieldValueProto| related errors for debugging
......
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