Skip to content

Commit

Permalink
[Fast Checkout] Fix success metrics
Browse files Browse the repository at this point in the history
Adds AutofillTriggerSource to all autofilling methods to determine which
source triggered Autofill. Uses the trigger source to not count Autofill
invokes from Fast Checkout. This is extremely important because
otherwise there is no way to figure out if the feature is successful or
not.

Bug: 1334642,1427611
Change-Id: Ia64e2294700e98720d5d7fb812eb4c7e4866b848
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4370088
Reviewed-by: Matthias Körber <koerber@google.com>
Reviewed-by: Dominic Battré <battre@chromium.org>
Commit-Queue: Wolfgang Billenstein <bwolfgang@google.com>
Cr-Commit-Position: refs/heads/main@{#1122313}
  • Loading branch information
Wolfgang Billenstein authored and Chromium LUCI CQ committed Mar 27, 2023
1 parent 7c00387 commit ee621c4
Show file tree
Hide file tree
Showing 28 changed files with 360 additions and 177 deletions.
Expand Up @@ -125,7 +125,8 @@ void FillCard(content::RenderFrameHost* rfh,
test::SetCreditCardInfo(&card, kNameFull, kNumber, kExpMonth, kExpYear, "");
auto* manager = TestAutofillManager::GetForRenderFrameHost(rfh);
manager->FillCreditCardFormImpl(form, triggered_field, card,
base::ASCIIToUTF16(base::StringPiece(kCvc)));
base::ASCIIToUTF16(base::StringPiece(kCvc)),
AutofillTriggerSource::kPopup);
}

// Clicks the first input, textarea, or select in `rfh`.
Expand Down
10 changes: 6 additions & 4 deletions chrome/browser/fast_checkout/fast_checkout_client_impl.cc
Expand Up @@ -368,8 +368,9 @@ void FastCheckoutClientImpl::TryToFillForms() {
static_cast<autofill::BrowserAutofillManager*>(autofill_manager_.get())
->SetFastCheckoutRunId(autofill::FieldTypeGroup::kAddressHome,
run_id_);
autofill_manager_->FillProfileForm(*autofill_profile,
form->ToFormData(), *field);
autofill_manager_->FillProfileForm(
*autofill_profile, form->ToFormData(), *field,
autofill::AutofillTriggerSource::kFastCheckout);
}
}

Expand Down Expand Up @@ -406,8 +407,9 @@ void FastCheckoutClientImpl::FillCreditCardForm(
FillingState::kFilling;
static_cast<autofill::BrowserAutofillManager*>(autofill_manager_.get())
->SetFastCheckoutRunId(autofill::FieldTypeGroup::kCreditCard, run_id_);
autofill_manager_->FillCreditCardForm(form.ToFormData(), field, credit_card,
cvc);
autofill_manager_->FillCreditCardForm(
form.ToFormData(), field, credit_card, cvc,
autofill::AutofillTriggerSource::kFastCheckout);
}

autofill::AutofillProfile*
Expand Down
31 changes: 19 additions & 12 deletions chrome/browser/fast_checkout/fast_checkout_client_impl_unittest.cc
Expand Up @@ -133,14 +133,16 @@ class MockBrowserAutofillManager : public autofill::TestBrowserAutofillManager {
FillProfileFormImpl,
(const autofill::FormData&,
const autofill::FormFieldData&,
const autofill::AutofillProfile&),
const autofill::AutofillProfile&,
const autofill::AutofillTriggerSource),
(override));
MOCK_METHOD(void,
FillCreditCardFormImpl,
(const autofill::FormData&,
const autofill::FormFieldData&,
const autofill::CreditCard&,
const std::u16string&),
const std::u16string&,
const autofill::AutofillTriggerSource),
(override));
MOCK_METHOD(void,
SetFastCheckoutRunId,
Expand Down Expand Up @@ -703,10 +705,12 @@ TEST_F(FastCheckoutClientImplTest, OnAfterLoadedServerPredictions_FillsForms) {
filling_state = FastCheckoutClientImpl::FillingState::kNotFilled;
}

EXPECT_CALL(*autofill_manager(),
FillProfileFormImpl(FormDataEqualTo(address_form_data),
FormFieldDataEqualTo(address_form_field_data),
Eq(*autofill_profile)));
EXPECT_CALL(
*autofill_manager(),
FillProfileFormImpl(FormDataEqualTo(address_form_data),
FormFieldDataEqualTo(address_form_field_data),
Eq(*autofill_profile),
Eq(autofill::AutofillTriggerSource::kFastCheckout)));
EXPECT_CALL(*autofill_manager(),
SetFastCheckoutRunId(autofill::FieldTypeGroup::kAddressHome,
fast_checkout_client()->run_id_));
Expand Down Expand Up @@ -744,7 +748,8 @@ TEST_F(FastCheckoutClientImplTest,
EXPECT_CALL(*autofill_manager(),
FillCreditCardFormImpl(
FormDataEqualTo(credit_card_form->ToFormData()),
FormFieldDataEqualTo(field), Eq(*credit_card), Eq(cvc)));
FormFieldDataEqualTo(field), Eq(*credit_card), Eq(cvc),
Eq(autofill::AutofillTriggerSource::kFastCheckout)));
EXPECT_CALL(*autofill_manager(),
SetFastCheckoutRunId(autofill::FieldTypeGroup::kCreditCard,
fast_checkout_client()->run_id_));
Expand Down Expand Up @@ -906,7 +911,8 @@ TEST_F(FastCheckoutClientImplTest,
EXPECT_CALL(*autofill_manager(),
FillCreditCardFormImpl(
FormDataEqualTo(credit_card_form->ToFormData()),
FormFieldDataEqualTo(field), Eq(*credit_card), Eq(cvc)));
FormFieldDataEqualTo(field), Eq(*credit_card), Eq(cvc),
Eq(autofill::AutofillTriggerSource::kFastCheckout)));
EXPECT_CALL(*autofill_manager(),
SetFastCheckoutRunId(autofill::FieldTypeGroup::kCreditCard,
fast_checkout_client()->run_id_));
Expand Down Expand Up @@ -1032,10 +1038,11 @@ TEST_F(FastCheckoutClientImplTest,
EXPECT_CALL(
*autofill_manager(),
SetFastCheckoutRunId(autofill::FieldTypeGroup::kCreditCard, Ne(0)));
EXPECT_CALL(
*autofill_manager(),
FillCreditCardFormImpl(FormDataEqualTo(credit_card_form->ToFormData()),
FormFieldDataEqualTo(field), _, Eq(u"")));
EXPECT_CALL(*autofill_manager(),
FillCreditCardFormImpl(
FormDataEqualTo(credit_card_form->ToFormData()),
FormFieldDataEqualTo(field), _, Eq(u""),
Eq(autofill::AutofillTriggerSource::kFastCheckout)));
StartRunAndSelectOptions({credit_card_form->form_signature()},
/*local_card=*/true);
EXPECT_FALSE(fast_checkout_client()->IsNotShownYet());
Expand Down
Expand Up @@ -201,8 +201,9 @@ void TouchToFillDelegateAndroidImpl::ScanCreditCard() {
void TouchToFillDelegateAndroidImpl::OnCreditCardScanned(
const CreditCard& card) {
HideTouchToFill();
manager_->FillCreditCardFormImpl(query_form_, query_field_, card,
std::u16string());
manager_->FillCreditCardFormImpl(
query_form_, query_field_, card, std::u16string(),
AutofillTriggerSource::kTouchToFillCreditCard);
}

void TouchToFillDelegateAndroidImpl::ShowCreditCardSettings() {
Expand All @@ -216,13 +217,14 @@ void TouchToFillDelegateAndroidImpl::SuggestionSelected(std::string unique_id,
if (is_virtual) {
manager_->FillOrPreviewVirtualCardInformation(
mojom::RendererFormDataAction::kFill, unique_id, query_form_,
query_field_);
query_field_, AutofillTriggerSource::kTouchToFillCreditCard);
} else {
PersonalDataManager* pdm = manager_->client()->GetPersonalDataManager();
DCHECK(pdm);
CreditCard* card = pdm->GetCreditCardByGUID(unique_id);
manager_->FillOrPreviewCreditCardForm(mojom::RendererFormDataAction::kFill,
query_form_, query_field_, card);
manager_->FillOrPreviewCreditCardForm(
mojom::RendererFormDataAction::kFill, query_form_, query_field_, card,
AutofillTriggerSource::kTouchToFillCreditCard);
}
}

Expand Down
Expand Up @@ -112,20 +112,23 @@ class MockBrowserAutofillManager : public TestBrowserAutofillManager {
(const FormData& form,
const FormFieldData& field,
const CreditCard& credit_card,
const std::u16string& cvc),
const std::u16string& cvc,
const AutofillTriggerSource trigger_source),
(override));
MOCK_METHOD(void,
FillOrPreviewCreditCardForm,
(mojom::RendererFormDataAction action,
const FormData& form,
const FormFieldData& field,
const CreditCard* credit_card));
const CreditCard* credit_card,
const AutofillTriggerSource trigger_source));
MOCK_METHOD(void,
FillOrPreviewVirtualCardInformation,
(mojom::RendererFormDataAction action,
const std::string& guid,
const FormData& form,
const FormFieldData& field));
const FormFieldData& field,
const AutofillTriggerSource trigger_source));
MOCK_METHOD(void,
DidShowSuggestions,
(bool has_autofill_suggestions,
Expand Down
Expand Up @@ -50,14 +50,16 @@ void AndroidAutofillManager::FillCreditCardFormImpl(
const FormData& form,
const FormFieldData& field,
const CreditCard& credit_card,
const std::u16string& cvc) {
const std::u16string& cvc,
AutofillTriggerSource trigger_source) {
NOTREACHED();
}

void AndroidAutofillManager::FillProfileFormImpl(
const FormData& form,
const FormFieldData& field,
const autofill::AutofillProfile& profile) {
const autofill::AutofillProfile& profile,
AutofillTriggerSource trigger_source) {
NOTREACHED();
}

Expand Down
Expand Up @@ -47,10 +47,12 @@ class AndroidAutofillManager : public AutofillManager {
void FillCreditCardFormImpl(const FormData& form,
const FormFieldData& field,
const CreditCard& credit_card,
const std::u16string& cvc) override;
const std::u16string& cvc,
AutofillTriggerSource trigger_source) override;
void FillProfileFormImpl(const FormData& form,
const FormFieldData& field,
const autofill::AutofillProfile& profile) override;
const autofill::AutofillProfile& profile,
AutofillTriggerSource trigger_source) override;

void OnFocusNoLongerOnFormImpl(bool had_interacted_form) override;

Expand Down
1 change: 1 addition & 0 deletions components/autofill/core/browser/BUILD.gn
Expand Up @@ -109,6 +109,7 @@ static_library("browser") {
"autofill_subject.h",
"autofill_suggestion_generator.cc",
"autofill_suggestion_generator.h",
"autofill_trigger_source.h",
"autofill_type.cc",
"autofill_type.h",
"browser_autofill_manager.cc",
Expand Down
30 changes: 19 additions & 11 deletions components/autofill/core/browser/autofill_external_delegate.cc
Expand Up @@ -21,6 +21,7 @@
#include "build/build_config.h"
#include "components/autofill/core/browser/autocomplete_history_manager.h"
#include "components/autofill/core/browser/autofill_driver.h"
#include "components/autofill/core/browser/autofill_trigger_source.h"
#include "components/autofill/core/browser/browser_autofill_manager.h"
#include "components/autofill/core/browser/metrics/autofill_metrics.h"
#include "components/autofill/core/browser/metrics/suggestions_list_metrics.h"
Expand Down Expand Up @@ -214,7 +215,8 @@ void AutofillExternalDelegate::DidSelectSuggestion(

// Only preview the data if it is a profile or a virtual card.
if (frontend_id > 0) {
FillAutofillFormData(frontend_id, true);
FillAutofillFormData(frontend_id, true,
AutofillTriggerSource::kKeyboardAccessory);
} else if (frontend_id == POPUP_ITEM_ID_AUTOCOMPLETE_ENTRY ||
frontend_id == POPUP_ITEM_ID_IBAN_ENTRY ||
frontend_id == POPUP_ITEM_ID_MERCHANT_PROMO_CODE_ENTRY) {
Expand All @@ -223,7 +225,7 @@ void AutofillExternalDelegate::DidSelectSuggestion(
} else if (frontend_id == POPUP_ITEM_ID_VIRTUAL_CREDIT_CARD_ENTRY) {
manager_->FillOrPreviewVirtualCardInformation(
mojom::RendererFormDataAction::kPreview, backend_id.value(),
query_form_, query_field_);
query_form_, query_field_, AutofillTriggerSource::kKeyboardAccessory);
}
}

Expand Down Expand Up @@ -273,8 +275,9 @@ void AutofillExternalDelegate::DidAcceptSuggestion(const Suggestion& suggestion,
query_form_, query_field_);
break;
case POPUP_ITEM_ID_SCAN_CREDIT_CARD:
manager_->client()->ScanCreditCard(base::BindOnce(
&AutofillExternalDelegate::OnCreditCardScanned, GetWeakPtr()));
manager_->client()->ScanCreditCard(
base::BindOnce(&AutofillExternalDelegate::OnCreditCardScanned,
GetWeakPtr(), AutofillTriggerSource::kPopup));
break;
case POPUP_ITEM_ID_CREDIT_CARD_SIGNIN_PROMO:
manager_->client()->ExecuteCommand(suggestion.frontend_id);
Expand All @@ -297,7 +300,7 @@ void AutofillExternalDelegate::DidAcceptSuggestion(const Suggestion& suggestion,
manager_->FillOrPreviewVirtualCardInformation(
mojom::RendererFormDataAction::kFill,
suggestion.GetPayload<Suggestion::BackendId>().value(), query_form_,
query_field_);
query_field_, AutofillTriggerSource::kPopup);
break;
case POPUP_ITEM_ID_SEE_PROMO_CODE_DETAILS:
manager_->OnSeePromoCodeOfferDetailsSelected(
Expand All @@ -309,7 +312,8 @@ void AutofillExternalDelegate::DidAcceptSuggestion(const Suggestion& suggestion,
autofill_metrics::LogAutofillSuggestionAcceptedIndex(
position, popup_type_, manager_->client()->IsOffTheRecord());
}
FillAutofillFormData(suggestion.frontend_id, false);
FillAutofillFormData(suggestion.frontend_id, false,
AutofillTriggerSource::kPopup);
break;
}

Expand Down Expand Up @@ -386,13 +390,17 @@ base::WeakPtr<AutofillExternalDelegate> AutofillExternalDelegate::GetWeakPtr() {
return weak_ptr_factory_.GetWeakPtr();
}

void AutofillExternalDelegate::OnCreditCardScanned(const CreditCard& card) {
void AutofillExternalDelegate::OnCreditCardScanned(
const AutofillTriggerSource trigger_source,
const CreditCard& card) {
manager_->FillCreditCardFormImpl(query_form_, query_field_, card,
std::u16string());
std::u16string(), trigger_source);
}

void AutofillExternalDelegate::FillAutofillFormData(int unique_id,
bool is_preview) {
void AutofillExternalDelegate::FillAutofillFormData(
int unique_id,
bool is_preview,
const AutofillTriggerSource trigger_source) {
// If the selected element is a warning we don't want to do anything.
if (IsAutofillWarningEntry(unique_id))
return;
Expand All @@ -404,7 +412,7 @@ void AutofillExternalDelegate::FillAutofillFormData(int unique_id,
DCHECK(driver_->RendererIsAvailable());
// Fill the values for the whole form.
manager_->FillOrPreviewForm(renderer_action, query_form_, query_field_,
unique_id);
unique_id, trigger_source);
}

void AutofillExternalDelegate::PossiblyRemoveAutofillWarnings(
Expand Down
8 changes: 6 additions & 2 deletions components/autofill/core/browser/autofill_external_delegate.h
Expand Up @@ -13,6 +13,7 @@
#include "base/gtest_prod_util.h"
#include "base/memory/raw_ptr.h"
#include "base/memory/weak_ptr.h"
#include "components/autofill/core/browser/autofill_trigger_source.h"
#include "components/autofill/core/browser/ui/autofill_popup_delegate.h"
#include "components/autofill/core/browser/ui/suggestion.h"
#include "components/autofill/core/common/aliases.h"
Expand Down Expand Up @@ -120,13 +121,16 @@ class AutofillExternalDelegate : public AutofillPopupDelegate {
FillCreditCardFormImpl);

// Called when a credit card is scanned using device camera.
void OnCreditCardScanned(const CreditCard& card);
void OnCreditCardScanned(const AutofillTriggerSource trigger_source,
const CreditCard& card);

// Fills the form with the Autofill data corresponding to |unique_id|.
// If |is_preview| is true then this is just a preview to show the user what
// would be selected and if |is_preview| is false then the user has selected
// this data.
void FillAutofillFormData(int unique_id, bool is_preview);
void FillAutofillFormData(int unique_id,
bool is_preview,
const AutofillTriggerSource trigger_source);

// Will remove Autofill warnings from |suggestions| if there are also
// autocomplete entries in the vector. Note: at this point, it is assumed that
Expand Down

0 comments on commit ee621c4

Please sign in to comment.