Move form_parsed_timestamp out of autofill_metrics and put in form_structure.
This will issue of using FormInteractionsUkmLogger in AutofillManager: AutofillManager.Reset() may be called before logging the metrics, which will make FormParsedTimeStamp null when logging the metrics.
This fix will keep FormParsedTimeStamp in form_structure and explicitly pass the FormParsedTimeStamp in the callback.
We have to make full_card_request take form_parsed_timestamp and pass it back to autofill_manager.OnFullCardRequestSucceeded. Since the time stamp may be destructed or overrided by another form interaction.
Bug: 736495
Change-Id: I0068db7bb8c3681937aac9736c1bad0d15b43928
TBR=jochen
Change-Id: I0068db7bb8c3681937aac9736c1bad0d15b43928
Reviewed-on: https://chromium-review.googlesource.com/549026
Commit-Queue: Shanfeng Zhang <[email protected]>
Reviewed-by: Mathieu Perreault <[email protected]>
Reviewed-by: mahmadi <[email protected]>
Reviewed-by: Sebastien Seguin-Gagnon <[email protected]>
Reviewed-by: Roger McFarlane <[email protected]>
Cr-Commit-Position: refs/heads/master@{#488558}diff --git a/components/autofill/core/browser/autofill_assistant.cc b/components/autofill/core/browser/autofill_assistant.cc
index 02b20bd..7453cc1 100644
--- a/components/autofill/core/browser/autofill_assistant.cc
+++ b/components/autofill/core/browser/autofill_assistant.cc
@@ -59,8 +59,10 @@
autofill_manager_->GetAsFullCardRequestUIDelegate());
}
-void AutofillAssistant::OnFullCardRequestSucceeded(const CreditCard& card,
- const base::string16& cvc) {
+void AutofillAssistant::OnFullCardRequestSucceeded(
+ const payments::FullCardRequest& /* full_card_request */,
+ const CreditCard& card,
+ const base::string16& cvc) {
autofill_manager_->FillCreditCardForm(kNoQueryId, *credit_card_form_data_,
credit_card_form_data_->fields[0], card,
cvc);
diff --git a/components/autofill/core/browser/autofill_assistant.h b/components/autofill/core/browser/autofill_assistant.h
index b383faa..79f097f 100644
--- a/components/autofill/core/browser/autofill_assistant.h
+++ b/components/autofill/core/browser/autofill_assistant.h
@@ -44,8 +44,10 @@
void OnUserDidAcceptCreditCardFill(const CreditCard& card);
// payments::FullCardRequest::ResultDelegate:
- void OnFullCardRequestSucceeded(const CreditCard& card,
- const base::string16& cvc) override;
+ void OnFullCardRequestSucceeded(
+ const payments::FullCardRequest& full_card_request,
+ const CreditCard& card,
+ const base::string16& cvc) override;
void OnFullCardRequestFailed() override;
// Holds the FormData to be filled with a credit card.
diff --git a/components/autofill/core/browser/autofill_manager.cc b/components/autofill/core/browser/autofill_manager.cc
index 2c49c88..a197be67 100644
--- a/components/autofill/core/browser/autofill_manager.cc
+++ b/components/autofill/core/browser/autofill_manager.cc
@@ -528,7 +528,8 @@
UpdatePendingForm(form);
if (!user_did_type_ || autofill_field->is_autofilled)
- form_interactions_ukm_logger_->LogTextFieldDidChange(*autofill_field);
+ form_interactions_ukm_logger_->LogTextFieldDidChange(
+ *autofill_field, form_structure->form_parsed_timestamp());
if (!user_did_type_) {
user_did_type_ = true;
@@ -736,6 +737,10 @@
const FormData& form,
const FormFieldData& field,
const CreditCard& credit_card) {
+ FormStructure* form_structure = NULL;
+ AutofillField* autofill_field = NULL;
+ if (!GetCachedFormAndField(form, field, &form_structure, &autofill_field))
+ return;
if (action == AutofillDriver::FORM_DATA_ACTION_FILL) {
if (credit_card.record_type() == CreditCard::MASKED_SERVER_CARD &&
WillFillCreditCardNumber(form, field)) {
@@ -743,18 +748,22 @@
unmasking_form_ = form;
unmasking_field_ = field;
masked_card_ = credit_card;
- GetOrCreateFullCardRequest()->GetFullCard(
+ payments::FullCardRequest* full_card_request =
+ CreateFullCardRequest(form_structure->form_parsed_timestamp());
+ full_card_request->GetFullCard(
masked_card_, AutofillClient::UNMASK_FOR_AUTOFILL,
weak_ptr_factory_.GetWeakPtr(), weak_ptr_factory_.GetWeakPtr());
- credit_card_form_event_logger_->OnDidSelectMaskedServerCardSuggestion();
+ credit_card_form_event_logger_->OnDidSelectMaskedServerCardSuggestion(
+ form_structure->form_parsed_timestamp());
return;
}
- credit_card_form_event_logger_->OnDidFillSuggestion(credit_card);
+ credit_card_form_event_logger_->OnDidFillSuggestion(
+ credit_card, form_structure->form_parsed_timestamp());
}
- FillOrPreviewDataModelForm(action, query_id, form, field, credit_card,
- true /* is_credit_card */,
- base::string16() /* cvc */);
+ FillOrPreviewDataModelForm(
+ action, query_id, form, field, credit_card, true /* is_credit_card */,
+ base::string16() /* cvc */, form_structure, autofill_field);
}
void AutofillManager::FillOrPreviewProfileForm(
@@ -763,12 +772,17 @@
const FormData& form,
const FormFieldData& field,
const AutofillProfile& profile) {
+ FormStructure* form_structure = NULL;
+ AutofillField* autofill_field = NULL;
+ if (!GetCachedFormAndField(form, field, &form_structure, &autofill_field))
+ return;
if (action == AutofillDriver::FORM_DATA_ACTION_FILL)
- address_form_event_logger_->OnDidFillSuggestion(profile);
+ address_form_event_logger_->OnDidFillSuggestion(
+ profile, form_structure->form_parsed_timestamp());
- FillOrPreviewDataModelForm(action, query_id, form, field, profile,
- false /* is_credit_card */,
- base::string16() /* cvc */);
+ FillOrPreviewDataModelForm(
+ action, query_id, form, field, profile, false /* is_credit_card */,
+ base::string16() /* cvc */, form_structure, autofill_field);
}
void AutofillManager::FillOrPreviewForm(
@@ -858,9 +872,11 @@
}
if (autofill_field->Type().group() == CREDIT_CARD) {
- credit_card_form_event_logger_->OnDidShowSuggestions(*autofill_field);
+ credit_card_form_event_logger_->OnDidShowSuggestions(
+ *autofill_field, form_structure->form_parsed_timestamp());
} else {
- address_form_event_logger_->OnDidShowSuggestions(*autofill_field);
+ address_form_event_logger_->OnDidShowSuggestions(
+ *autofill_field, form_structure->form_parsed_timestamp());
}
}
}
@@ -970,7 +986,13 @@
full_card_request_.reset(new payments::FullCardRequest(
client_, payments_client_.get(), personal_data_));
}
+ return full_card_request_.get();
+}
+payments::FullCardRequest* AutofillManager::CreateFullCardRequest(
+ const base::TimeTicks& form_parsed_timestamp) {
+ full_card_request_.reset(new payments::FullCardRequest(
+ client_, payments_client_.get(), personal_data_, form_parsed_timestamp));
return full_card_request_.get();
}
@@ -1106,9 +1128,12 @@
}
}
-void AutofillManager::OnFullCardRequestSucceeded(const CreditCard& card,
- const base::string16& cvc) {
- credit_card_form_event_logger_->OnDidFillSuggestion(masked_card_);
+void AutofillManager::OnFullCardRequestSucceeded(
+ const payments::FullCardRequest& full_card_request,
+ const CreditCard& card,
+ const base::string16& cvc) {
+ credit_card_form_event_logger_->OnDidFillSuggestion(
+ masked_card_, full_card_request.form_parsed_timestamp());
FillCreditCardForm(unmasking_query_id_, unmasking_form_, unmasking_field_,
card, cvc);
masked_card_ = CreditCard();
@@ -1715,7 +1740,21 @@
AutofillField* autofill_field = NULL;
if (!GetCachedFormAndField(form, field, &form_structure, &autofill_field))
return;
+ FillOrPreviewDataModelForm(action, query_id, form, field, data_model,
+ is_credit_card, cvc, form_structure,
+ autofill_field);
+}
+void AutofillManager::FillOrPreviewDataModelForm(
+ AutofillDriver::RendererFormDataAction action,
+ int query_id,
+ const FormData& form,
+ const FormFieldData& field,
+ const AutofillDataModel& data_model,
+ bool is_credit_card,
+ const base::string16& cvc,
+ FormStructure* form_structure,
+ AutofillField* autofill_field) {
DCHECK(form_structure);
DCHECK(autofill_field);
@@ -2096,6 +2135,7 @@
// Ownership is transferred to |form_structures_| which maintains it until
// the manager is Reset() or destroyed. It is safe to use references below
// as long as receivers don't take ownership.
+ form_structure->set_form_parsed_timestamp(TimeTicks::Now());
form_structures_.push_back(std::move(form_structure));
*parsed_form_structure = form_structures_.back().get();
(*parsed_form_structure)->DetermineHeuristicTypes(client_->GetUkmRecorder());
diff --git a/components/autofill/core/browser/autofill_manager.h b/components/autofill/core/browser/autofill_manager.h
index f24f885..2754928a 100644
--- a/components/autofill/core/browser/autofill_manager.h
+++ b/components/autofill/core/browser/autofill_manager.h
@@ -150,6 +150,9 @@
payments::FullCardRequest* GetOrCreateFullCardRequest();
+ payments::FullCardRequest* CreateFullCardRequest(
+ const base::TimeTicks& form_parsed_timestamp);
+
base::WeakPtr<payments::FullCardRequest::UIDelegate>
GetAsFullCardRequestUIDelegate() {
return weak_ptr_factory_.GetWeakPtr();
@@ -297,8 +300,10 @@
std::unique_ptr<base::DictionaryValue> legal_message) override;
// payments::FullCardRequest::ResultDelegate:
- void OnFullCardRequestSucceeded(const CreditCard& card,
- const base::string16& cvc) override;
+ void OnFullCardRequestSucceeded(
+ const payments::FullCardRequest& full_card_request,
+ const CreditCard& card,
+ const base::string16& cvc) override;
void OnFullCardRequestFailed() override;
// payments::FullCardRequest::UIDelegate:
@@ -350,6 +355,9 @@
const FormFieldData& field,
const AutofillProfile& profile);
+ // TODO(rogerm) here to see if these can be merged. FormData should be a
+ // subset of the data in FormStructure and FormFieldData a subset of that in
+ // AutofillField.
// Fills or previews |data_model| in the |form|.
void FillOrPreviewDataModelForm(AutofillDriver::RendererFormDataAction action,
int query_id,
@@ -358,6 +366,15 @@
const AutofillDataModel& data_model,
bool is_credit_card,
const base::string16& cvc);
+ void FillOrPreviewDataModelForm(AutofillDriver::RendererFormDataAction action,
+ int query_id,
+ const FormData& form,
+ const FormFieldData& field,
+ const AutofillDataModel& data_model,
+ bool is_credit_card,
+ const base::string16& cvc,
+ FormStructure* form_structure,
+ AutofillField* autofill_field);
// Creates a FormStructure using the FormData received from the renderer. Will
// return an empty scoped_ptr if the data should not be processed for upload
diff --git a/components/autofill/core/browser/autofill_manager_unittest.cc b/components/autofill/core/browser/autofill_manager_unittest.cc
index 5828b3d..baa1ebe 100644
--- a/components/autofill/core/browser/autofill_manager_unittest.cc
+++ b/components/autofill/core/browser/autofill_manager_unittest.cc
@@ -693,6 +693,7 @@
}
void AddSeenForm(std::unique_ptr<FormStructure> form) {
+ form->set_form_parsed_timestamp(base::TimeTicks::Now());
form_structures()->push_back(std::move(form));
}
@@ -7079,6 +7080,7 @@
std::unique_ptr<FormStructure> form_structure(new FormStructure(form));
form_structure->set_is_signin_upload(true);
+ form_structure->set_form_parsed_timestamp(base::TimeTicks::Now());
form_structure->field(1)->set_possible_types({autofill::PASSWORD});
std::string signature = form_structure->FormSignatureAsStr();
diff --git a/components/autofill/core/browser/autofill_metrics.cc b/components/autofill/core/browser/autofill_metrics.cc
index 4cee6df..c376565 100644
--- a/components/autofill/core/browser/autofill_metrics.cc
+++ b/components/autofill/core/browser/autofill_metrics.cc
@@ -409,8 +409,9 @@
(predicted_type << 16) | actual_type);
form_interactions_ukm_logger->LogFieldType(
- form.form_signature(), field.GetFieldSignature(), prediction_source,
- metric_type, predicted_type, actual_type);
+ form.form_parsed_timestamp(), form.form_signature(),
+ field.GetFieldSignature(), prediction_source, metric_type, predicted_type,
+ actual_type);
// NO_SERVER_DATA is the equivalent of predicting UNKNOWN.
if (predicted_type == NO_SERVER_DATA)
@@ -908,6 +909,7 @@
// static
void AutofillMetrics::LogAutofillFormSubmittedState(
AutofillFormSubmittedState state,
+ const base::TimeTicks& form_parsed_timestamp,
AutofillMetrics::FormInteractionsUkmLogger* form_interactions_ukm_logger) {
UMA_HISTOGRAM_ENUMERATION("Autofill.FormSubmittedState", state,
AUTOFILL_FORM_SUBMITTED_STATE_ENUM_SIZE);
@@ -942,7 +944,7 @@
NOTREACHED();
break;
}
- form_interactions_ukm_logger->LogFormSubmitted(state);
+ form_interactions_ukm_logger->LogFormSubmitted(state, form_parsed_timestamp);
}
// static
@@ -1090,8 +1092,10 @@
}
void AutofillMetrics::FormEventLogger::OnDidShowSuggestions(
- const AutofillField& field) {
- form_interactions_ukm_logger_->LogSuggestionsShown(field);
+ const AutofillField& field,
+ const base::TimeTicks& form_parsed_timestamp) {
+ form_interactions_ukm_logger_->LogSuggestionsShown(field,
+ form_parsed_timestamp);
Log(AutofillMetrics::FORM_EVENT_SUGGESTIONS_SHOWN);
if (!has_logged_suggestions_shown_) {
@@ -1112,9 +1116,11 @@
}
}
-void AutofillMetrics::FormEventLogger::OnDidSelectMaskedServerCardSuggestion() {
+void AutofillMetrics::FormEventLogger::OnDidSelectMaskedServerCardSuggestion(
+ const base::TimeTicks& form_parsed_timestamp) {
DCHECK(is_for_credit_card_);
- form_interactions_ukm_logger_->LogSelectedMaskedServerCard();
+ form_interactions_ukm_logger_->LogSelectedMaskedServerCard(
+ form_parsed_timestamp);
Log(AutofillMetrics::FORM_EVENT_MASKED_SERVER_CARD_SUGGESTION_SELECTED);
if (!has_logged_masked_server_card_suggestion_selected_) {
@@ -1125,10 +1131,11 @@
}
void AutofillMetrics::FormEventLogger::OnDidFillSuggestion(
- const CreditCard& credit_card) {
+ const CreditCard& credit_card,
+ const base::TimeTicks& form_parsed_timestamp) {
DCHECK(is_for_credit_card_);
form_interactions_ukm_logger_->LogDidFillSuggestion(
- static_cast<int>(credit_card.record_type()));
+ static_cast<int>(credit_card.record_type()), form_parsed_timestamp);
if (credit_card.record_type() == CreditCard::MASKED_SERVER_CARD)
Log(AutofillMetrics::FORM_EVENT_MASKED_SERVER_CARD_SUGGESTION_FILLED);
@@ -1163,10 +1170,11 @@
}
void AutofillMetrics::FormEventLogger::OnDidFillSuggestion(
- const AutofillProfile& profile) {
+ const AutofillProfile& profile,
+ const base::TimeTicks& form_parsed_timestamp) {
DCHECK(!is_for_credit_card_);
form_interactions_ukm_logger_->LogDidFillSuggestion(
- static_cast<int>(profile.record_type()));
+ static_cast<int>(profile.record_type()), form_parsed_timestamp);
if (profile.record_type() == AutofillProfile::SERVER_PROFILE)
Log(AutofillMetrics::FORM_EVENT_SERVER_SUGGESTION_FILLED);
@@ -1292,7 +1300,6 @@
return;
url_ = url;
- form_parsed_timestamp_ = base::TimeTicks::Now();
}
void AutofillMetrics::FormInteractionsUkmLogger::LogInteractedWithForm(
@@ -1317,7 +1324,8 @@
}
void AutofillMetrics::FormInteractionsUkmLogger::LogSuggestionsShown(
- const AutofillField& field) {
+ const AutofillField& field,
+ const base::TimeTicks& form_parsed_timestamp) {
if (!CanLog())
return;
@@ -1334,10 +1342,11 @@
builder->AddMetric(internal::kUKMServerTypeMetricName,
static_cast<int>(field.server_type()));
builder->AddMetric(internal::kUKMMillisecondsSinceFormParsedMetricName,
- MillisecondsSinceFormParsed());
+ MillisecondsSinceFormParsed(form_parsed_timestamp));
}
-void AutofillMetrics::FormInteractionsUkmLogger::LogSelectedMaskedServerCard() {
+void AutofillMetrics::FormInteractionsUkmLogger::LogSelectedMaskedServerCard(
+ const base::TimeTicks& form_parsed_timestamp) {
if (!CanLog())
return;
@@ -1348,11 +1357,12 @@
ukm_recorder_->GetEntryBuilder(
source_id_, internal::kUKMSelectedMaskedServerCardEntryName);
builder->AddMetric(internal::kUKMMillisecondsSinceFormParsedMetricName,
- MillisecondsSinceFormParsed());
+ MillisecondsSinceFormParsed(form_parsed_timestamp));
}
void AutofillMetrics::FormInteractionsUkmLogger::LogDidFillSuggestion(
- int record_type) {
+ int record_type,
+ const base::TimeTicks& form_parsed_timestamp) {
if (!CanLog())
return;
@@ -1364,11 +1374,12 @@
internal::kUKMSuggestionFilledEntryName);
builder->AddMetric(internal::kUKMRecordTypeMetricName, record_type);
builder->AddMetric(internal::kUKMMillisecondsSinceFormParsedMetricName,
- MillisecondsSinceFormParsed());
+ MillisecondsSinceFormParsed(form_parsed_timestamp));
}
void AutofillMetrics::FormInteractionsUkmLogger::LogTextFieldDidChange(
- const AutofillField& field) {
+ const AutofillField& field,
+ const base::TimeTicks& form_parsed_timestamp) {
if (!CanLog())
return;
@@ -1391,7 +1402,7 @@
builder->AddMetric(internal::kUKMIsAutofilledMetricName, field.is_autofilled);
builder->AddMetric(internal::kUKMIsEmptyMetricName, field.IsEmpty());
builder->AddMetric(internal::kUKMMillisecondsSinceFormParsedMetricName,
- MillisecondsSinceFormParsed());
+ MillisecondsSinceFormParsed(form_parsed_timestamp));
}
void AutofillMetrics::FormInteractionsUkmLogger::LogFieldFillStatus(
@@ -1408,7 +1419,7 @@
ukm_recorder_->GetEntryBuilder(source_id_,
internal::kUKMFieldFillStatusEntryName);
builder->AddMetric(internal::kUKMMillisecondsSinceFormParsedMetricName,
- MillisecondsSinceFormParsed());
+ MillisecondsSinceFormParsed(form.form_parsed_timestamp()));
builder->AddMetric(internal::kUKMFormSignatureMetricName,
static_cast<int64_t>(form.form_signature()));
builder->AddMetric(internal::kUKMFieldSignatureMetricName,
@@ -1421,7 +1432,10 @@
static_cast<int64_t>(field.previously_autofilled()));
}
+// TODO(szhangcs): Take FormStructure and AutofillField and extract
+// FormSignature and TimeTicks inside the function.
void AutofillMetrics::FormInteractionsUkmLogger::LogFieldType(
+ const base::TimeTicks& form_parsed_timestamp,
FormSignature form_signature,
FieldSignature field_signature,
QualityMetricPredictionSource prediction_source,
@@ -1438,7 +1452,7 @@
ukm_recorder_->GetEntryBuilder(source_id_,
internal::kUKMFieldTypeEntryName);
builder->AddMetric(internal::kUKMMillisecondsSinceFormParsedMetricName,
- MillisecondsSinceFormParsed());
+ MillisecondsSinceFormParsed(form_parsed_timestamp));
builder->AddMetric(internal::kUKMFormSignatureMetricName,
static_cast<int64_t>(form_signature));
builder->AddMetric(internal::kUKMFieldSignatureMetricName,
@@ -1454,7 +1468,8 @@
}
void AutofillMetrics::FormInteractionsUkmLogger::LogFormSubmitted(
- AutofillFormSubmittedState state) {
+ AutofillFormSubmittedState state,
+ const base::TimeTicks& form_parsed_timestamp) {
if (!CanLog())
return;
@@ -1466,13 +1481,13 @@
internal::kUKMFormSubmittedEntryName);
builder->AddMetric(internal::kUKMAutofillFormSubmittedStateMetricName,
static_cast<int>(state));
- if (form_parsed_timestamp_.is_null())
+ if (form_parsed_timestamp.is_null())
DCHECK(state == NON_FILLABLE_FORM_OR_NEW_DATA ||
state == FILLABLE_FORM_AUTOFILLED_NONE_DID_NOT_SHOW_SUGGESTIONS)
<< state;
else
builder->AddMetric(internal::kUKMMillisecondsSinceFormParsedMetricName,
- MillisecondsSinceFormParsed());
+ MillisecondsSinceFormParsed(form_parsed_timestamp));
}
void AutofillMetrics::FormInteractionsUkmLogger::UpdateSourceURL(
@@ -1486,14 +1501,13 @@
return ukm_recorder_ && url_.is_valid();
}
-int64_t
-AutofillMetrics::FormInteractionsUkmLogger::MillisecondsSinceFormParsed()
- const {
- DCHECK(!form_parsed_timestamp_.is_null());
+int64_t AutofillMetrics::FormInteractionsUkmLogger::MillisecondsSinceFormParsed(
+ const base::TimeTicks& form_parsed_timestamp) const {
+ DCHECK(!form_parsed_timestamp.is_null());
// Use the pinned timestamp as the current time if it's set.
base::TimeTicks now =
pinned_timestamp_.is_null() ? base::TimeTicks::Now() : pinned_timestamp_;
- return (now - form_parsed_timestamp_).InMilliseconds();
+ return (now - form_parsed_timestamp).InMilliseconds();
}
void AutofillMetrics::FormInteractionsUkmLogger::GetNewSourceID() {
diff --git a/components/autofill/core/browser/autofill_metrics.h b/components/autofill/core/browser/autofill_metrics.h
index 8ba9c49..56d133c 100644
--- a/components/autofill/core/browser/autofill_metrics.h
+++ b/components/autofill/core/browser/autofill_metrics.h
@@ -747,20 +747,26 @@
void LogInteractedWithForm(bool is_for_credit_card,
size_t local_record_type_count,
size_t server_record_type_count);
- void LogSuggestionsShown(const AutofillField& field);
- void LogSelectedMaskedServerCard();
- void LogDidFillSuggestion(int record_type);
- void LogTextFieldDidChange(const AutofillField& field);
+ void LogSuggestionsShown(const AutofillField& field,
+ const base::TimeTicks& form_parsed_timestamp);
+ void LogSelectedMaskedServerCard(
+ const base::TimeTicks& form_parsed_timestamp);
+ void LogDidFillSuggestion(int record_type,
+ const base::TimeTicks& form_parsed_timestamp);
+ void LogTextFieldDidChange(const AutofillField& field,
+ const base::TimeTicks& form_parsed_timestamp);
void LogFieldFillStatus(const FormStructure& form,
const AutofillField& field,
QualityMetricType metric_type);
- void LogFieldType(FormSignature form_signature,
+ void LogFieldType(const base::TimeTicks& form_parsed_timestamp,
+ FormSignature form_signature,
FieldSignature field_signature,
QualityMetricPredictionSource prediction_source,
QualityMetricType metric_type,
ServerFieldType predicted_type,
ServerFieldType actual_type);
- void LogFormSubmitted(AutofillFormSubmittedState state);
+ void LogFormSubmitted(AutofillFormSubmittedState state,
+ const base::TimeTicks& form_parsed_timestamp);
// We initialize |url_| with the form's URL when we log the first form
// interaction. Later, we may update |url_| with the |source_url()| for the
@@ -769,13 +775,13 @@
private:
bool CanLog() const;
- int64_t MillisecondsSinceFormParsed() const;
+ int64_t MillisecondsSinceFormParsed(
+ const base::TimeTicks& form_parsed_timestamp) const;
void GetNewSourceID();
ukm::UkmRecorder* ukm_recorder_; // Weak reference.
ukm::SourceId source_id_ = -1;
GURL url_;
- base::TimeTicks form_parsed_timestamp_;
base::TimeTicks pinned_timestamp_;
};
@@ -951,6 +957,7 @@
// state of the form.
static void LogAutofillFormSubmittedState(
AutofillFormSubmittedState state,
+ const base::TimeTicks& form_parsed_timestamp,
FormInteractionsUkmLogger* form_interactions_ukm_logger);
// This should be called when determining the heuristic types for a form's
@@ -1021,15 +1028,19 @@
void OnDidPollSuggestions(const FormFieldData& field);
- void OnDidShowSuggestions(const AutofillField& field);
+ void OnDidShowSuggestions(const AutofillField& field,
+ const base::TimeTicks& form_parsed_timestamp);
- void OnDidSelectMaskedServerCardSuggestion();
+ void OnDidSelectMaskedServerCardSuggestion(
+ const base::TimeTicks& form_parsed_timestamp);
// In case of masked cards, caller must make sure this gets called before
// the card is upgraded to a full card.
- void OnDidFillSuggestion(const CreditCard& credit_card);
+ void OnDidFillSuggestion(const CreditCard& credit_card,
+ const base::TimeTicks& form_parsed_timestamp);
- void OnDidFillSuggestion(const AutofillProfile& profile);
+ void OnDidFillSuggestion(const AutofillProfile& profile,
+ const base::TimeTicks& form_parsed_timestamp);
void OnWillSubmitForm();
diff --git a/components/autofill/core/browser/autofill_metrics_unittest.cc b/components/autofill/core/browser/autofill_metrics_unittest.cc
index 7fe99a2..d598b7a8 100644
--- a/components/autofill/core/browser/autofill_metrics_unittest.cc
+++ b/components/autofill/core/browser/autofill_metrics_unittest.cc
@@ -293,6 +293,7 @@
std::unique_ptr<TestFormStructure> form_structure =
base::MakeUnique<TestFormStructure>(empty_form);
form_structure->SetFieldTypes(heuristic_types, server_types);
+ form_structure->set_form_parsed_timestamp(TimeTicks::Now());
form_structures()->push_back(std::move(form_structure));
form_interactions_ukm_logger()->OnFormsParsed(form.origin);
@@ -3105,6 +3106,42 @@
autofill_manager_->AddSeenForm(form, field_types, field_types);
{
+ // Simulating submission with suggestion shown. Form is submmitted and
+ // autofill manager is reset before UploadFormDataAsyncCallback is
+ // triggered.
+ base::HistogramTester histogram_tester;
+ autofill_manager_->DidShowSuggestions(true /* is_new_popup */, form, field);
+ autofill_manager_->OnQueryFormFieldAutofill(0, form, field, gfx::RectF());
+ autofill_manager_->ResetRunLoop();
+ autofill_manager_->OnWillSubmitForm(form, TimeTicks::Now());
+ autofill_manager_->OnFormSubmitted(form);
+ autofill_manager_->Reset();
+ // Trigger UploadFormDataAsyncCallback.
+ autofill_manager_->RunRunLoop();
+ histogram_tester.ExpectBucketCount(
+ "Autofill.FormEvents.CreditCard",
+ AutofillMetrics::FORM_EVENT_SUGGESTION_SHOWN_SUBMITTED_ONCE, 1);
+ histogram_tester.ExpectBucketCount(
+ "Autofill.FormEvents.CreditCard",
+ AutofillMetrics::FORM_EVENT_SUGGESTION_SHOWN_WILL_SUBMIT_ONCE, 1);
+
+ VerifyFormInteractionUkm(
+ test_ukm_recorder_, form, internal::kUKMSuggestionsShownEntryName,
+ {{{internal::kUKMMillisecondsSinceFormParsedMetricName, 0},
+ {internal::kUKMHeuristicTypeMetricName, CREDIT_CARD_NUMBER},
+ {internal::kUKMHtmlFieldTypeMetricName, HTML_TYPE_UNSPECIFIED},
+ {internal::kUKMServerTypeMetricName, CREDIT_CARD_NUMBER}}});
+ VerifySubmitFormUkm(test_ukm_recorder_, form,
+ AutofillMetrics::NON_FILLABLE_FORM_OR_NEW_DATA);
+ }
+
+ // Reset the autofill manager state and purge UKM logs.
+ autofill_manager_->Reset();
+ test_ukm_recorder_.Purge();
+
+ autofill_manager_->AddSeenForm(form, field_types, field_types);
+
+ {
// Simulating submission with filled local data.
base::HistogramTester histogram_tester;
autofill_manager_->OnQueryFormFieldAutofill(0, form, field, gfx::RectF());
@@ -3809,6 +3846,35 @@
autofill_manager_->AddSeenForm(form, field_types, field_types);
{
+ // Simulating submission with no filled data. Form is submmitted and
+ // autofill manager is reset before UploadFormDataAsyncCallback is
+ // triggered.
+ base::HistogramTester histogram_tester;
+ autofill_manager_->OnQueryFormFieldAutofill(0, form, field, gfx::RectF());
+ autofill_manager_->ResetRunLoop();
+ autofill_manager_->OnWillSubmitForm(form, TimeTicks::Now());
+ autofill_manager_->OnFormSubmitted(form);
+ autofill_manager_->Reset();
+ // Trigger UploadFormDataAsyncCallback.
+ autofill_manager_->RunRunLoop();
+ histogram_tester.ExpectBucketCount(
+ "Autofill.FormEvents.Address",
+ AutofillMetrics::FORM_EVENT_NO_SUGGESTION_WILL_SUBMIT_ONCE, 1);
+ histogram_tester.ExpectBucketCount(
+ "Autofill.FormEvents.Address",
+ AutofillMetrics::FORM_EVENT_NO_SUGGESTION_SUBMITTED_ONCE, 1);
+
+ VerifySubmitFormUkm(test_ukm_recorder_, form,
+ AutofillMetrics::NON_FILLABLE_FORM_OR_NEW_DATA);
+ }
+
+ // Reset the autofill manager state and purge UKM logs.
+ autofill_manager_->Reset();
+ test_ukm_recorder_.Purge();
+
+ autofill_manager_->AddSeenForm(form, field_types, field_types);
+
+ {
// Simulating submission with suggestion shown.
base::HistogramTester histogram_tester;
autofill_manager_->DidShowSuggestions(true /* is_new_popup */, form, field);
diff --git a/components/autofill/core/browser/form_structure.cc b/components/autofill/core/browser/form_structure.cc
index a18ae1b..131247af 100644
--- a/components/autofill/core/browser/form_structure.cc
+++ b/components/autofill/core/browser/form_structure.cc
@@ -680,6 +680,9 @@
UpdateAutofillCount();
+ // Update form parsed timestamp
+ set_form_parsed_timestamp(cached_form.form_parsed_timestamp());
+
// The form signature should match between query and upload requests to the
// server. On many websites, form elements are dynamically added, removed, or
// rearranged via JavaScript between page load and form submission, so we
@@ -803,7 +806,7 @@
if (form_interactions_ukm_logger->url() != source_url())
form_interactions_ukm_logger->UpdateSourceURL(source_url());
AutofillMetrics::LogAutofillFormSubmittedState(
- state, form_interactions_ukm_logger);
+ state, form_parsed_timestamp_, form_interactions_ukm_logger);
}
}
diff --git a/components/autofill/core/browser/form_structure.h b/components/autofill/core/browser/form_structure.h
index c70a23a8..b8f592e2 100644
--- a/components/autofill/core/browser/form_structure.h
+++ b/components/autofill/core/browser/form_structure.h
@@ -231,6 +231,13 @@
}
UploadRequired upload_required() const { return upload_required_; }
+ void set_form_parsed_timestamp(const base::TimeTicks form_parsed_timestamp) {
+ form_parsed_timestamp_ = form_parsed_timestamp;
+ }
+ base::TimeTicks form_parsed_timestamp() const {
+ return form_parsed_timestamp_;
+ }
+
bool all_fields_are_passwords() const { return all_fields_are_passwords_; }
bool is_signin_upload() const { return is_signin_upload_; }
@@ -345,6 +352,9 @@
// the form name, and the form field names in a 64-bit hash.
FormSignature form_signature_;
+ // When a form is parsed on this page.
+ base::TimeTicks form_parsed_timestamp_;
+
DISALLOW_COPY_AND_ASSIGN(FormStructure);
};
diff --git a/components/autofill/core/browser/payments/full_card_request.cc b/components/autofill/core/browser/payments/full_card_request.cc
index 68177a70..9bc55f4 100644
--- a/components/autofill/core/browser/payments/full_card_request.cc
+++ b/components/autofill/core/browser/payments/full_card_request.cc
@@ -19,12 +19,22 @@
FullCardRequest::FullCardRequest(RiskDataLoader* risk_data_loader,
payments::PaymentsClient* payments_client,
PersonalDataManager* personal_data_manager)
+ : FullCardRequest(risk_data_loader,
+ payments_client,
+ personal_data_manager,
+ base::TimeTicks()) {}
+
+FullCardRequest::FullCardRequest(RiskDataLoader* risk_data_loader,
+ payments::PaymentsClient* payments_client,
+ PersonalDataManager* personal_data_manager,
+ base::TimeTicks form_parsed_timestamp)
: risk_data_loader_(risk_data_loader),
payments_client_(payments_client),
personal_data_manager_(personal_data_manager),
result_delegate_(nullptr),
ui_delegate_(nullptr),
should_unmask_card_(false),
+ form_parsed_timestamp_(form_parsed_timestamp),
weak_ptr_factory_(this) {
DCHECK(risk_data_loader_);
DCHECK(payments_client_);
@@ -87,7 +97,7 @@
if (!should_unmask_card_) {
if (result_delegate_)
- result_delegate_->OnFullCardRequestSucceeded(request_->card,
+ result_delegate_->OnFullCardRequestSucceeded(*this, request_->card,
response.cvc);
if (ui_delegate_)
ui_delegate_->OnUnmaskVerificationResult(AutofillClient::SUCCESS);
@@ -152,7 +162,7 @@
if (result_delegate_)
result_delegate_->OnFullCardRequestSucceeded(
- request_->card, request_->user_response.cvc);
+ *this, request_->card, request_->user_response.cvc);
Reset();
break;
}
diff --git a/components/autofill/core/browser/payments/full_card_request.h b/components/autofill/core/browser/payments/full_card_request.h
index 9db61fd..ce3f1776 100644
--- a/components/autofill/core/browser/payments/full_card_request.h
+++ b/components/autofill/core/browser/payments/full_card_request.h
@@ -30,8 +30,10 @@
class ResultDelegate {
public:
virtual ~ResultDelegate() = default;
- virtual void OnFullCardRequestSucceeded(const CreditCard& card,
- const base::string16& cvc) = 0;
+ virtual void OnFullCardRequestSucceeded(
+ const payments::FullCardRequest& full_card_request,
+ const CreditCard& card,
+ const base::string16& cvc) = 0;
virtual void OnFullCardRequestFailed() = 0;
};
@@ -51,6 +53,10 @@
FullCardRequest(RiskDataLoader* risk_data_loader,
payments::PaymentsClient* payments_client,
PersonalDataManager* personal_data_manager);
+ FullCardRequest(RiskDataLoader* risk_data_loader,
+ payments::PaymentsClient* payments_client,
+ PersonalDataManager* personal_data_manager,
+ base::TimeTicks form_parsed_timestamp);
~FullCardRequest();
// Retrieves the pan and cvc for |card| and invokes
@@ -72,6 +78,11 @@
// Called by the payments client when a card has been unmasked.
void OnDidGetRealPan(AutofillClient::PaymentsRpcResult result,
const std::string& real_pan);
+
+ base::TimeTicks form_parsed_timestamp() const {
+ return form_parsed_timestamp_;
+ }
+
private:
// CardUnmaskDelegate:
void OnUnmaskResponse(const UnmaskResponse& response) override;
@@ -108,6 +119,9 @@
// histograms.
base::Time real_pan_request_timestamp_;
+ // The timestamp when the form is parsed. For histograms.
+ base::TimeTicks form_parsed_timestamp_;
+
// Enables destroying FullCardRequest while CVC prompt is showing or a server
// communication is pending.
base::WeakPtrFactory<FullCardRequest> weak_ptr_factory_;
diff --git a/components/autofill/core/browser/payments/full_card_request_unittest.cc b/components/autofill/core/browser/payments/full_card_request_unittest.cc
index 5d01e9b..9498dc5 100644
--- a/components/autofill/core/browser/payments/full_card_request_unittest.cc
+++ b/components/autofill/core/browser/payments/full_card_request_unittest.cc
@@ -20,7 +20,6 @@
#include "net/url_request/url_request_test_util.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
-
namespace autofill {
namespace payments {
@@ -30,8 +29,10 @@
class MockResultDelegate : public FullCardRequest::ResultDelegate,
public base::SupportsWeakPtr<MockResultDelegate> {
public:
- MOCK_METHOD2(OnFullCardRequestSucceeded,
- void(const CreditCard&, const base::string16&));
+ MOCK_METHOD3(OnFullCardRequestSucceeded,
+ void(const payments::FullCardRequest&,
+ const CreditCard&,
+ const base::string16&));
MOCK_METHOD0(OnFullCardRequestFailed, void());
};
@@ -139,6 +140,7 @@
TEST_F(FullCardRequestTest, GetFullCardPanAndCvcForMaskedServerCard) {
EXPECT_CALL(*result_delegate(),
OnFullCardRequestSucceeded(
+ testing::Ref(*request()),
CardMatches(CreditCard::FULL_SERVER_CARD, "4111"),
base::ASCIIToUTF16("123")));
EXPECT_CALL(*ui_delegate(), ShowUnmaskPrompt(_, _, _));
@@ -160,7 +162,8 @@
TEST_F(FullCardRequestTest, GetFullCardPanAndCvcForLocalCard) {
EXPECT_CALL(
*result_delegate(),
- OnFullCardRequestSucceeded(CardMatches(CreditCard::LOCAL_CARD, "4111"),
+ OnFullCardRequestSucceeded(testing::Ref(*request()),
+ CardMatches(CreditCard::LOCAL_CARD, "4111"),
base::ASCIIToUTF16("123")));
EXPECT_CALL(*ui_delegate(), ShowUnmaskPrompt(_, _, _));
EXPECT_CALL(*ui_delegate(),
@@ -181,6 +184,7 @@
TEST_F(FullCardRequestTest, GetFullCardPanAndCvcForFullServerCard) {
EXPECT_CALL(*result_delegate(),
OnFullCardRequestSucceeded(
+ testing::Ref(*request()),
CardMatches(CreditCard::FULL_SERVER_CARD, "4111"),
base::ASCIIToUTF16("123")));
EXPECT_CALL(*ui_delegate(), ShowUnmaskPrompt(_, _, _));
@@ -204,6 +208,7 @@
TEST_F(FullCardRequestTest,
GetFullCardPanAndCvcForFullServerCardInExpiredStatus) {
EXPECT_CALL(*result_delegate(), OnFullCardRequestSucceeded(
+ testing::Ref(*request()),
CardMatches(CreditCard::FULL_SERVER_CARD,
"4111", "12", "2051"),
base::ASCIIToUTF16("123")));
@@ -232,6 +237,7 @@
// expiration date in the past.
TEST_F(FullCardRequestTest, GetFullCardPanAndCvcForExpiredFullServerCard) {
EXPECT_CALL(*result_delegate(), OnFullCardRequestSucceeded(
+ testing::Ref(*request()),
CardMatches(CreditCard::FULL_SERVER_CARD,
"4111", "12", "2051"),
base::ASCIIToUTF16("123")));
@@ -280,7 +286,8 @@
EXPECT_CALL(*result_delegate(), OnFullCardRequestFailed()).Times(0);
EXPECT_CALL(
*result_delegate(),
- OnFullCardRequestSucceeded(CardMatches(CreditCard::LOCAL_CARD, "4111"),
+ OnFullCardRequestSucceeded(testing::Ref(*request()),
+ CardMatches(CreditCard::LOCAL_CARD, "4111"),
base::ASCIIToUTF16("123")))
.Times(2);
EXPECT_CALL(*ui_delegate(), ShowUnmaskPrompt(_, _, _)).Times(2);
@@ -382,6 +389,7 @@
EXPECT_CALL(*result_delegate(), OnFullCardRequestFailed()).Times(0);
EXPECT_CALL(*result_delegate(),
OnFullCardRequestSucceeded(
+ testing::Ref(*request()),
CardMatches(CreditCard::FULL_SERVER_CARD, "4111"),
base::ASCIIToUTF16("123")));
EXPECT_CALL(*ui_delegate(), ShowUnmaskPrompt(_, _, _));
@@ -407,6 +415,7 @@
// Verify updating expiration date for a masked server card.
TEST_F(FullCardRequestTest, UpdateExpDateForMaskedServerCard) {
EXPECT_CALL(*result_delegate(), OnFullCardRequestSucceeded(
+ testing::Ref(*request()),
CardMatches(CreditCard::FULL_SERVER_CARD,
"4111", "12", "2050"),
base::ASCIIToUTF16("123")));
@@ -430,6 +439,7 @@
// Verify updating expiration date for an unmasked server card.
TEST_F(FullCardRequestTest, UpdateExpDateForFullServerCard) {
EXPECT_CALL(*result_delegate(), OnFullCardRequestSucceeded(
+ testing::Ref(*request()),
CardMatches(CreditCard::FULL_SERVER_CARD,
"4111", "12", "2050"),
base::ASCIIToUTF16("123")));
@@ -456,6 +466,7 @@
TEST_F(FullCardRequestTest, UpdateExpDateForLocalCard) {
EXPECT_CALL(*result_delegate(),
OnFullCardRequestSucceeded(
+ testing::Ref(*request()),
CardMatches(CreditCard::LOCAL_CARD, "4111", "12", "2051"),
base::ASCIIToUTF16("123")));
EXPECT_CALL(*ui_delegate(), ShowUnmaskPrompt(_, _, _));
@@ -485,6 +496,7 @@
// Verify saving full PAN on disk.
TEST_F(FullCardRequestTest, SaveRealPan) {
EXPECT_CALL(*result_delegate(), OnFullCardRequestSucceeded(
+ testing::Ref(*request()),
CardMatches(CreditCard::FULL_SERVER_CARD,
"4111", "12", "2050"),
base::ASCIIToUTF16("123")));
@@ -513,6 +525,7 @@
TEST_F(FullCardRequestTest, UnmaskForPaymentRequest) {
EXPECT_CALL(*result_delegate(),
OnFullCardRequestSucceeded(
+ testing::Ref(*request()),
CardMatches(CreditCard::FULL_SERVER_CARD, "4111"),
base::ASCIIToUTF16("123")));
EXPECT_CALL(*ui_delegate(), ShowUnmaskPrompt(_, _, _));
@@ -535,6 +548,7 @@
TEST_F(FullCardRequestTest, IsGettingFullCardForMaskedServerCard) {
EXPECT_CALL(*result_delegate(),
OnFullCardRequestSucceeded(
+ testing::Ref(*request()),
CardMatches(CreditCard::FULL_SERVER_CARD, "4111"),
base::ASCIIToUTF16("123")));
EXPECT_CALL(*ui_delegate(), ShowUnmaskPrompt(_, _, _));
@@ -570,7 +584,8 @@
TEST_F(FullCardRequestTest, IsGettingFullCardForLocalCard) {
EXPECT_CALL(
*result_delegate(),
- OnFullCardRequestSucceeded(CardMatches(CreditCard::LOCAL_CARD, "4111"),
+ OnFullCardRequestSucceeded(testing::Ref(*request()),
+ CardMatches(CreditCard::LOCAL_CARD, "4111"),
base::ASCIIToUTF16("123")));
EXPECT_CALL(*ui_delegate(), ShowUnmaskPrompt(_, _, _));
EXPECT_CALL(*ui_delegate(),
diff --git a/components/payments/content/payment_request_state_unittest.cc b/components/payments/content/payment_request_state_unittest.cc
index 0e0a014..02ca3f1 100644
--- a/components/payments/content/payment_request_state_unittest.cc
+++ b/components/payments/content/payment_request_state_unittest.cc
@@ -26,7 +26,7 @@
protected:
PaymentRequestStateTest()
: num_on_selected_information_changed_called_(0),
- test_payment_request_delegate_(/*personal_data_manager=*/nullptr),
+ test_payment_request_delegate_(&test_personal_data_manager_),
journey_logger_(test_payment_request_delegate_.IsIncognito(),
GURL("http://www.test.com"),
test_payment_request_delegate_.GetUkmRecorder()),
diff --git a/components/payments/core/BUILD.gn b/components/payments/core/BUILD.gn
index ed5343e6..f762260 100644
--- a/components/payments/core/BUILD.gn
+++ b/components/payments/core/BUILD.gn
@@ -78,6 +78,7 @@
"//components/autofill/core/browser:test_support",
"//components/pref_registry",
"//components/prefs",
+ "//net:test_support",
]
}
@@ -109,6 +110,7 @@
"//components/strings:components_strings_grit",
"//components/ukm",
"//components/ukm:test_support",
+ "//net:test_support",
"//testing/gmock",
"//testing/gtest",
"//third_party/libaddressinput:test_support",
diff --git a/components/payments/core/DEPS b/components/payments/core/DEPS
index 8325aae1..59baa08 100644
--- a/components/payments/core/DEPS
+++ b/components/payments/core/DEPS
@@ -7,6 +7,7 @@
"+components/prefs",
"+components/pref_registry",
"+components/strings",
+ "+net",
"+services/metrics/public",
"+third_party/libaddressinput",
"+third_party/libphonenumber",
diff --git a/components/payments/core/autofill_payment_instrument.cc b/components/payments/core/autofill_payment_instrument.cc
index 86d98f0277..8673e6ce 100644
--- a/components/payments/core/autofill_payment_instrument.cc
+++ b/components/payments/core/autofill_payment_instrument.cc
@@ -162,6 +162,7 @@
}
void AutofillPaymentInstrument::OnFullCardRequestSucceeded(
+ const autofill::payments::FullCardRequest& /* full_card_request */,
const autofill::CreditCard& card,
const base::string16& cvc) {
DCHECK(delegate_);
diff --git a/components/payments/core/autofill_payment_instrument.h b/components/payments/core/autofill_payment_instrument.h
index 3600918..726ac16 100644
--- a/components/payments/core/autofill_payment_instrument.h
+++ b/components/payments/core/autofill_payment_instrument.h
@@ -54,8 +54,10 @@
const std::vector<std::string>& supported_networks) const override;
// autofill::payments::FullCardRequest::ResultDelegate:
- void OnFullCardRequestSucceeded(const autofill::CreditCard& card,
- const base::string16& cvc) override;
+ void OnFullCardRequestSucceeded(
+ const autofill::payments::FullCardRequest& full_card_request,
+ const autofill::CreditCard& card,
+ const base::string16& cvc) override;
void OnFullCardRequestFailed() override;
// AddressNormalizer::Delegate:
diff --git a/components/payments/core/autofill_payment_instrument_unittest.cc b/components/payments/core/autofill_payment_instrument_unittest.cc
index 117d587..328163b 100644
--- a/components/payments/core/autofill_payment_instrument_unittest.cc
+++ b/components/payments/core/autofill_payment_instrument_unittest.cc
@@ -10,9 +10,15 @@
#include "components/autofill/core/browser/autofill_profile.h"
#include "components/autofill/core/browser/autofill_test_utils.h"
#include "components/autofill/core/browser/credit_card.h"
+#include "components/autofill/core/browser/payments/full_card_request.h"
+#include "components/autofill/core/browser/payments/payments_client.h"
+#include "components/autofill/core/browser/personal_data_manager.h"
+#include "components/autofill/core/browser/test_autofill_client.h"
+#include "components/autofill/core/browser/test_personal_data_manager.h"
#include "components/payments/core/address_normalizer.h"
#include "components/payments/core/test_payment_request_delegate.h"
#include "components/strings/grit/components_strings.h"
+#include "net/url_request/url_request_test_util.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/base/l10n/l10n_util.h"
@@ -78,10 +84,20 @@
AddressNormalizer::Delegate* requester_;
};
-class FakePaymentRequestDelegate : public PaymentRequestDelegate {
+class FakePaymentRequestDelegate
+ : public PaymentRequestDelegate,
+ public autofill::payments::PaymentsClientDelegate {
public:
FakePaymentRequestDelegate()
- : locale_("en-US"), last_committed_url_("https://shop.com") {}
+ : locale_("en-US"),
+ last_committed_url_("https://shop.com"),
+ personal_data_("en-US"),
+ request_context_(new net::TestURLRequestContextGetter(
+ base::ThreadTaskRunnerHandle::Get())),
+ payments_client_(request_context_.get(), this),
+ full_card_request_(&autofill_client_,
+ &payments_client_,
+ &personal_data_) {}
void ShowDialog(PaymentRequest* request) override {}
void CloseDialog() override {}
@@ -120,7 +136,7 @@
void CompleteFullCardRequest() {
full_card_result_delegate_->OnFullCardRequestSucceeded(
- full_card_request_card_, base::ASCIIToUTF16("123"));
+ full_card_request_, full_card_request_card_, base::ASCIIToUTF16("123"));
}
autofill::RegionDataLoader* GetRegionDataLoader() override { return nullptr; }
@@ -131,7 +147,11 @@
std::string locale_;
const GURL last_committed_url_;
FakeAddressNormalizer address_normalizer_;
-
+ autofill::PersonalDataManager personal_data_;
+ scoped_refptr<net::TestURLRequestContextGetter> request_context_;
+ autofill::TestAutofillClient autofill_client_;
+ autofill::payments::PaymentsClient payments_client_;
+ autofill::payments::FullCardRequest full_card_request_;
autofill::CreditCard full_card_request_card_;
base::WeakPtr<autofill::payments::FullCardRequest::ResultDelegate>
full_card_result_delegate_;
@@ -328,7 +348,9 @@
// the billing address has been normalized and the card has been unmasked.
TEST_F(AutofillPaymentInstrumentTest,
InvokePaymentApp_NormalizationBeforeUnmask) {
- TestPaymentRequestDelegate delegate(/*personal_data_manager=*/nullptr);
+ auto personal_data_manager =
+ base::MakeUnique<autofill::TestPersonalDataManager>();
+ TestPaymentRequestDelegate delegate(personal_data_manager.get());
delegate.DelayFullCardRequestCompletion();
delegate.test_address_normalizer()->DelayNormalization();
@@ -357,7 +379,9 @@
// the billing address has been normalized and the card has been unmasked.
TEST_F(AutofillPaymentInstrumentTest,
InvokePaymentApp_UnmaskBeforeNormalization) {
- TestPaymentRequestDelegate delegate(/*personal_data_manager=*/nullptr);
+ auto personal_data_manager =
+ base::MakeUnique<autofill::TestPersonalDataManager>();
+ TestPaymentRequestDelegate delegate(personal_data_manager.get());
delegate.DelayFullCardRequestCompletion();
delegate.test_address_normalizer()->DelayNormalization();
diff --git a/components/payments/core/test_payment_request_delegate.cc b/components/payments/core/test_payment_request_delegate.cc
index 6679419..d379997 100644
--- a/components/payments/core/test_payment_request_delegate.cc
+++ b/components/payments/core/test_payment_request_delegate.cc
@@ -12,7 +12,12 @@
autofill::PersonalDataManager* personal_data_manager)
: personal_data_manager_(personal_data_manager),
locale_("en-US"),
- last_committed_url_("https://shop.com") {}
+ last_committed_url_("https://shop.com"),
+ request_context_(new TestURLRequestContextGetter(loop_.task_runner())),
+ payments_client_(request_context_.get(), &payments_cleint_delegate_),
+ full_card_request_(&autofill_client_,
+ &payments_client_,
+ personal_data_manager) {}
TestPaymentRequestDelegate::~TestPaymentRequestDelegate() {}
@@ -42,7 +47,7 @@
base::WeakPtr<autofill::payments::FullCardRequest::ResultDelegate>
result_delegate) {
if (instantaneous_full_card_request_result_) {
- result_delegate->OnFullCardRequestSucceeded(credit_card,
+ result_delegate->OnFullCardRequestSucceeded(full_card_request_, credit_card,
base::ASCIIToUTF16("123"));
return;
}
@@ -74,7 +79,7 @@
void TestPaymentRequestDelegate::CompleteFullCardRequest() {
DCHECK(instantaneous_full_card_request_result_ == false);
full_card_result_delegate_->OnFullCardRequestSucceeded(
- full_card_request_card_, base::ASCIIToUTF16("123"));
+ full_card_request_, full_card_request_card_, base::ASCIIToUTF16("123"));
}
std::string TestPaymentRequestDelegate::GetAuthenticatedEmail() const {
@@ -85,4 +90,40 @@
return nullptr;
}
+TestPaymentsClientDelegate::TestPaymentsClientDelegate() {}
+
+TestPaymentsClientDelegate::~TestPaymentsClientDelegate() {}
+
+void TestPaymentsClientDelegate::OnDidGetRealPan(
+ autofill::AutofillClient::PaymentsRpcResult result,
+ const std::string& real_pan) {}
+
+IdentityProvider* TestPaymentsClientDelegate::GetIdentityProvider() {
+ return nullptr;
+}
+
+void TestPaymentsClientDelegate::OnDidGetUploadDetails(
+ autofill::AutofillClient::PaymentsRpcResult result,
+ const base::string16& context_token,
+ std::unique_ptr<base::DictionaryValue> legal_message) {}
+
+void TestPaymentsClientDelegate::OnDidUploadCard(
+ autofill::AutofillClient::PaymentsRpcResult result,
+ const std::string& server_id) {}
+
+TestURLRequestContextGetter::TestURLRequestContextGetter(
+ scoped_refptr<base::SingleThreadTaskRunner> task_runner)
+ : task_runner_(task_runner) {}
+
+TestURLRequestContextGetter::~TestURLRequestContextGetter() {}
+
+net::URLRequestContext* TestURLRequestContextGetter::GetURLRequestContext() {
+ return nullptr;
+}
+
+scoped_refptr<base::SingleThreadTaskRunner>
+TestURLRequestContextGetter::GetNetworkTaskRunner() const {
+ return task_runner_;
+}
+
} // namespace payments
diff --git a/components/payments/core/test_payment_request_delegate.h b/components/payments/core/test_payment_request_delegate.h
index e0697a7..7bca98f 100644
--- a/components/payments/core/test_payment_request_delegate.h
+++ b/components/payments/core/test_payment_request_delegate.h
@@ -7,11 +7,48 @@
#include <string>
+#include "base/message_loop/message_loop.h"
+#include "components/autofill/core/browser/payments/full_card_request.h"
+#include "components/autofill/core/browser/payments/payments_client.h"
+#include "components/autofill/core/browser/test_autofill_client.h"
#include "components/payments/core/payment_request_delegate.h"
#include "components/payments/core/test_address_normalizer.h"
+#include "net/url_request/url_request_test_util.h"
namespace payments {
+class TestPaymentsClientDelegate
+ : public autofill::payments::PaymentsClientDelegate {
+ public:
+ TestPaymentsClientDelegate();
+ ~TestPaymentsClientDelegate();
+
+ private:
+ void OnDidGetRealPan(autofill::AutofillClient::PaymentsRpcResult result,
+ const std::string& real_pan) override;
+ IdentityProvider* GetIdentityProvider() override;
+ void OnDidGetUploadDetails(
+ autofill::AutofillClient::PaymentsRpcResult result,
+ const base::string16& context_token,
+ std::unique_ptr<base::DictionaryValue> legal_message) override;
+ void OnDidUploadCard(autofill::AutofillClient::PaymentsRpcResult result,
+ const std::string& server_id) override;
+};
+
+class TestURLRequestContextGetter : public net::URLRequestContextGetter {
+ public:
+ explicit TestURLRequestContextGetter(
+ scoped_refptr<base::SingleThreadTaskRunner> task_runner);
+ net::URLRequestContext* GetURLRequestContext() override;
+ scoped_refptr<base::SingleThreadTaskRunner> GetNetworkTaskRunner()
+ const override;
+
+ private:
+ ~TestURLRequestContextGetter() override;
+
+ scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
+};
+
class TestPaymentRequestDelegate : public PaymentRequestDelegate {
public:
TestPaymentRequestDelegate(
@@ -42,10 +79,16 @@
void CompleteFullCardRequest();
private:
+ base::MessageLoop loop_;
+ TestPaymentsClientDelegate payments_cleint_delegate_;
autofill::PersonalDataManager* personal_data_manager_;
std::string locale_;
const GURL last_committed_url_;
TestAddressNormalizer address_normalizer_;
+ scoped_refptr<TestURLRequestContextGetter> request_context_;
+ autofill::TestAutofillClient autofill_client_;
+ autofill::payments::PaymentsClient payments_client_;
+ autofill::payments::FullCardRequest full_card_request_;
bool instantaneous_full_card_request_result_ = true;
autofill::CreditCard full_card_request_card_;
