Commit ccedfa2a authored by Maria Kazinova's avatar Maria Kazinova Committed by Commit Bot

[IOS] Stop propagating autofill suggestions that are not longer relevant

to AutofillAgent.

This fixes a bug when no longer required suggestions are returned
asynchronously and are propagated to AutofillAgent latest instead of
actually requested suggestions.

Bug: 1097015
Change-Id: I263bcf619b1a4898a68837890abe3e5a7a483f8d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2421460Reviewed-by: default avatarJohn Wu <jzw@chromium.org>
Reviewed-by: default avatarOlivier Robin <olivierrobin@chromium.org>
Reviewed-by: default avatarVadym Doroshenko  <dvadym@chromium.org>
Commit-Queue: Maria Kazinova <kazinova@google.com>
Cr-Commit-Position: refs/heads/master@{#810811}
parent 9c3f4e0c
...@@ -500,6 +500,11 @@ class AutofillClient : public RiskDataLoader { ...@@ -500,6 +500,11 @@ class AutofillClient : public RiskDataLoader {
// Returns a LogManager instance. May be null for platforms that don't support // Returns a LogManager instance. May be null for platforms that don't support
// this. // this.
virtual LogManager* GetLogManager() const; virtual LogManager* GetLogManager() const;
#if defined(OS_IOS)
// Checks whether the qurrent query is the most recent one.
virtual bool IsQueryIDRelevant(int query_id) = 0;
#endif
}; };
} // namespace autofill } // namespace autofill
......
...@@ -87,6 +87,10 @@ void AutofillExternalDelegate::OnSuggestionsReturned( ...@@ -87,6 +87,10 @@ void AutofillExternalDelegate::OnSuggestionsReturned(
bool is_all_server_suggestions) { bool is_all_server_suggestions) {
if (query_id != query_id_) if (query_id != query_id_)
return; return;
#if defined(OS_IOS)
if (!manager_->client()->IsQueryIDRelevant(query_id))
return;
#endif
std::vector<Suggestion> suggestions(input_suggestions); std::vector<Suggestion> suggestions(input_suggestions);
......
...@@ -46,7 +46,7 @@ namespace autofill { ...@@ -46,7 +46,7 @@ namespace autofill {
namespace { namespace {
// A constant value to use as the Autofill query ID. // A constant value to use as the Autofill query ID.
const int kQueryId = 5; const int kRecentQueryId = 1;
// A constant value to use as an Autofill profile ID. // A constant value to use as an Autofill profile ID.
const int kAutofillProfileId = 1; const int kAutofillProfileId = 1;
...@@ -94,6 +94,9 @@ class MockAutofillClient : public TestAutofillClient { ...@@ -94,6 +94,9 @@ class MockAutofillClient : public TestAutofillClient {
MOCK_METHOD(void, HideAutofillPopup, (PopupHidingReason), (override)); MOCK_METHOD(void, HideAutofillPopup, (PopupHidingReason), (override));
MOCK_METHOD(void, ExecuteCommand, (int), (override)); MOCK_METHOD(void, ExecuteCommand, (int), (override));
// Mock the client query ID check.
bool IsQueryIDRelevant(int query_id) { return query_id == kRecentQueryId; }
private: private:
DISALLOW_COPY_AND_ASSIGN(MockAutofillClient); DISALLOW_COPY_AND_ASSIGN(MockAutofillClient);
}; };
...@@ -187,12 +190,12 @@ class AutofillExternalDelegateUnitTest : public testing::Test { ...@@ -187,12 +190,12 @@ class AutofillExternalDelegateUnitTest : public testing::Test {
external_delegate_->OnQuery(query_id, form, field, gfx::RectF()); external_delegate_->OnQuery(query_id, form, field, gfx::RectF());
} }
void IssueOnSuggestionsReturned() { void IssueOnSuggestionsReturned(int query_id) {
std::vector<Suggestion> suggestions; std::vector<Suggestion> suggestions;
suggestions.push_back(Suggestion()); suggestions.push_back(Suggestion());
suggestions[0].frontend_id = kAutofillProfileId; suggestions[0].frontend_id = kAutofillProfileId;
external_delegate_->OnSuggestionsReturned( external_delegate_->OnSuggestionsReturned(
kQueryId, suggestions, /*autoselect_first_suggestion=*/false); query_id, suggestions, /*autoselect_first_suggestion=*/false);
} }
base::test::SingleThreadTaskEnvironment task_environment_; base::test::SingleThreadTaskEnvironment task_environment_;
...@@ -216,7 +219,7 @@ class AutofillExternalDelegateCardsFromAccountTest ...@@ -216,7 +219,7 @@ class AutofillExternalDelegateCardsFromAccountTest
// Test that our external delegate called the virtual methods at the right time. // Test that our external delegate called the virtual methods at the right time.
TEST_F(AutofillExternalDelegateUnitTest, TestExternalDelegateVirtualCalls) { TEST_F(AutofillExternalDelegateUnitTest, TestExternalDelegateVirtualCalls) {
IssueOnQuery(kQueryId); IssueOnQuery(kRecentQueryId);
// The enums must be cast to ints to prevent compile errors on linux_rel. // The enums must be cast to ints to prevent compile errors on linux_rel.
auto element_ids = testing::ElementsAre( auto element_ids = testing::ElementsAre(
...@@ -230,7 +233,7 @@ TEST_F(AutofillExternalDelegateUnitTest, TestExternalDelegateVirtualCalls) { ...@@ -230,7 +233,7 @@ TEST_F(AutofillExternalDelegateUnitTest, TestExternalDelegateVirtualCalls) {
autofill_item.push_back(Suggestion()); autofill_item.push_back(Suggestion());
autofill_item[0].frontend_id = kAutofillProfileId; autofill_item[0].frontend_id = kAutofillProfileId;
external_delegate_->OnSuggestionsReturned( external_delegate_->OnSuggestionsReturned(
kQueryId, autofill_item, /*autoselect_first_suggestion=*/false); kRecentQueryId, autofill_item, /*autoselect_first_suggestion=*/false);
EXPECT_THAT(open_args.suggestions, SuggestionVectorIdsAre(element_ids)); EXPECT_THAT(open_args.suggestions, SuggestionVectorIdsAre(element_ids));
EXPECT_FALSE(open_args.autoselect_first_suggestion); EXPECT_FALSE(open_args.autoselect_first_suggestion);
EXPECT_EQ(open_args.popup_type, PopupType::kPersonalInformation); EXPECT_EQ(open_args.popup_type, PopupType::kPersonalInformation);
...@@ -254,7 +257,7 @@ TEST_F(AutofillExternalDelegateUnitTest, ...@@ -254,7 +257,7 @@ TEST_F(AutofillExternalDelegateUnitTest,
EXPECT_CALL(*autofill_manager_, ShouldShowCreditCardSigninPromo(_, _)) EXPECT_CALL(*autofill_manager_, ShouldShowCreditCardSigninPromo(_, _))
.WillOnce(testing::Return(true)); .WillOnce(testing::Return(true));
IssueOnQuery(kQueryId); IssueOnQuery(kRecentQueryId);
// The enums must be cast to ints to prevent compile errors on linux_rel. // The enums must be cast to ints to prevent compile errors on linux_rel.
auto element_ids = testing::ElementsAre( auto element_ids = testing::ElementsAre(
...@@ -270,7 +273,7 @@ TEST_F(AutofillExternalDelegateUnitTest, ...@@ -270,7 +273,7 @@ TEST_F(AutofillExternalDelegateUnitTest,
autofill_item.push_back(Suggestion()); autofill_item.push_back(Suggestion());
autofill_item[0].frontend_id = kAutofillProfileId; autofill_item[0].frontend_id = kAutofillProfileId;
external_delegate_->OnSuggestionsReturned( external_delegate_->OnSuggestionsReturned(
kQueryId, autofill_item, /*autoselect_first_suggestion=*/false); kRecentQueryId, autofill_item, /*autoselect_first_suggestion=*/false);
EXPECT_EQ(0, user_action_tester.GetActionCount( EXPECT_EQ(0, user_action_tester.GetActionCount(
"Signin_Impression_FromAutofillDropdown")); "Signin_Impression_FromAutofillDropdown"));
EXPECT_THAT(open_args.suggestions, SuggestionVectorIdsAre(element_ids)); EXPECT_THAT(open_args.suggestions, SuggestionVectorIdsAre(element_ids));
...@@ -296,7 +299,7 @@ TEST_F(AutofillExternalDelegateUnitTest, ...@@ -296,7 +299,7 @@ TEST_F(AutofillExternalDelegateUnitTest,
EXPECT_CALL(*autofill_manager_, ShouldShowCreditCardSigninPromo(_, _)) EXPECT_CALL(*autofill_manager_, ShouldShowCreditCardSigninPromo(_, _))
.WillOnce(testing::Return(true)); .WillOnce(testing::Return(true));
IssueOnQuery(kQueryId); IssueOnQuery(kRecentQueryId);
// The enums must be cast to ints to prevent compile errors on linux_rel. // The enums must be cast to ints to prevent compile errors on linux_rel.
auto element_ids = testing::ElementsAre( auto element_ids = testing::ElementsAre(
...@@ -311,7 +314,7 @@ TEST_F(AutofillExternalDelegateUnitTest, ...@@ -311,7 +314,7 @@ TEST_F(AutofillExternalDelegateUnitTest,
// This should call ShowAutofillPopup. // This should call ShowAutofillPopup.
std::vector<Suggestion> items; std::vector<Suggestion> items;
external_delegate_->OnSuggestionsReturned( external_delegate_->OnSuggestionsReturned(
kQueryId, items, /*autoselect_first_suggestion=*/false); kRecentQueryId, items, /*autoselect_first_suggestion=*/false);
EXPECT_EQ(1, user_action_tester.GetActionCount( EXPECT_EQ(1, user_action_tester.GetActionCount(
"Signin_Impression_FromAutofillDropdown")); "Signin_Impression_FromAutofillDropdown"));
EXPECT_THAT(open_args.suggestions, SuggestionVectorIdsAre(element_ids)); EXPECT_THAT(open_args.suggestions, SuggestionVectorIdsAre(element_ids));
...@@ -331,7 +334,7 @@ TEST_F(AutofillExternalDelegateUnitTest, ...@@ -331,7 +334,7 @@ TEST_F(AutofillExternalDelegateUnitTest,
// Test that data list elements for a node will appear in the Autofill popup. // Test that data list elements for a node will appear in the Autofill popup.
TEST_F(AutofillExternalDelegateUnitTest, ExternalDelegateDataList) { TEST_F(AutofillExternalDelegateUnitTest, ExternalDelegateDataList) {
IssueOnQuery(kQueryId); IssueOnQuery(kRecentQueryId);
std::vector<base::string16> data_list_items; std::vector<base::string16> data_list_items;
data_list_items.push_back(base::string16()); data_list_items.push_back(base::string16());
...@@ -358,7 +361,7 @@ TEST_F(AutofillExternalDelegateUnitTest, ExternalDelegateDataList) { ...@@ -358,7 +361,7 @@ TEST_F(AutofillExternalDelegateUnitTest, ExternalDelegateDataList) {
autofill_item.push_back(Suggestion()); autofill_item.push_back(Suggestion());
autofill_item[0].frontend_id = kAutofillProfileId; autofill_item[0].frontend_id = kAutofillProfileId;
external_delegate_->OnSuggestionsReturned( external_delegate_->OnSuggestionsReturned(
kQueryId, autofill_item, /*autoselect_first_suggestion=*/false); kRecentQueryId, autofill_item, /*autoselect_first_suggestion=*/false);
EXPECT_THAT(open_args.suggestions, SuggestionVectorIdsAre(element_ids)); EXPECT_THAT(open_args.suggestions, SuggestionVectorIdsAre(element_ids));
EXPECT_FALSE(open_args.autoselect_first_suggestion); EXPECT_FALSE(open_args.autoselect_first_suggestion);
EXPECT_EQ(open_args.popup_type, PopupType::kPersonalInformation); EXPECT_EQ(open_args.popup_type, PopupType::kPersonalInformation);
...@@ -372,7 +375,7 @@ TEST_F(AutofillExternalDelegateUnitTest, ExternalDelegateDataList) { ...@@ -372,7 +375,7 @@ TEST_F(AutofillExternalDelegateUnitTest, ExternalDelegateDataList) {
autofill_item.clear(); autofill_item.clear();
external_delegate_->OnSuggestionsReturned( external_delegate_->OnSuggestionsReturned(
kQueryId, autofill_item, /*autoselect_first_suggestion=*/false); kRecentQueryId, autofill_item, /*autoselect_first_suggestion=*/false);
EXPECT_THAT(open_args.suggestions, EXPECT_THAT(open_args.suggestions,
SuggestionVectorIdsAre(testing::ElementsAre( SuggestionVectorIdsAre(testing::ElementsAre(
static_cast<int>(POPUP_ITEM_ID_DATALIST_ENTRY)))); static_cast<int>(POPUP_ITEM_ID_DATALIST_ENTRY))));
...@@ -382,7 +385,7 @@ TEST_F(AutofillExternalDelegateUnitTest, ExternalDelegateDataList) { ...@@ -382,7 +385,7 @@ TEST_F(AutofillExternalDelegateUnitTest, ExternalDelegateDataList) {
// Test that datalist values can get updated while a popup is showing. // Test that datalist values can get updated while a popup is showing.
TEST_F(AutofillExternalDelegateUnitTest, UpdateDataListWhileShowingPopup) { TEST_F(AutofillExternalDelegateUnitTest, UpdateDataListWhileShowingPopup) {
IssueOnQuery(kQueryId); IssueOnQuery(kRecentQueryId);
EXPECT_CALL(autofill_client_, ShowAutofillPopup).Times(0); EXPECT_CALL(autofill_client_, ShowAutofillPopup).Times(0);
...@@ -413,7 +416,7 @@ TEST_F(AutofillExternalDelegateUnitTest, UpdateDataListWhileShowingPopup) { ...@@ -413,7 +416,7 @@ TEST_F(AutofillExternalDelegateUnitTest, UpdateDataListWhileShowingPopup) {
autofill_item.push_back(Suggestion()); autofill_item.push_back(Suggestion());
autofill_item[0].frontend_id = kAutofillProfileId; autofill_item[0].frontend_id = kAutofillProfileId;
external_delegate_->OnSuggestionsReturned( external_delegate_->OnSuggestionsReturned(
kQueryId, autofill_item, /*autoselect_first_suggestion=*/false); kRecentQueryId, autofill_item, /*autoselect_first_suggestion=*/false);
EXPECT_THAT(open_args.suggestions, SuggestionVectorIdsAre(element_ids)); EXPECT_THAT(open_args.suggestions, SuggestionVectorIdsAre(element_ids));
EXPECT_FALSE(open_args.autoselect_first_suggestion); EXPECT_FALSE(open_args.autoselect_first_suggestion);
EXPECT_EQ(open_args.popup_type, PopupType::kPersonalInformation); EXPECT_EQ(open_args.popup_type, PopupType::kPersonalInformation);
...@@ -436,7 +439,7 @@ TEST_F(AutofillExternalDelegateUnitTest, UpdateDataListWhileShowingPopup) { ...@@ -436,7 +439,7 @@ TEST_F(AutofillExternalDelegateUnitTest, UpdateDataListWhileShowingPopup) {
// Test that we _don't_ de-dupe autofill values against datalist values. We // Test that we _don't_ de-dupe autofill values against datalist values. We
// keep both with a separator. // keep both with a separator.
TEST_F(AutofillExternalDelegateUnitTest, DuplicateAutofillDatalistValues) { TEST_F(AutofillExternalDelegateUnitTest, DuplicateAutofillDatalistValues) {
IssueOnQuery(kQueryId); IssueOnQuery(kRecentQueryId);
std::vector<base::string16> data_list_values{base::ASCIIToUTF16("Rick"), std::vector<base::string16> data_list_values{base::ASCIIToUTF16("Rick"),
base::ASCIIToUTF16("Beyonce")}; base::ASCIIToUTF16("Beyonce")};
...@@ -468,7 +471,7 @@ TEST_F(AutofillExternalDelegateUnitTest, DuplicateAutofillDatalistValues) { ...@@ -468,7 +471,7 @@ TEST_F(AutofillExternalDelegateUnitTest, DuplicateAutofillDatalistValues) {
autofill_item[0].label = ASCIIToUTF16("Deckard"); autofill_item[0].label = ASCIIToUTF16("Deckard");
autofill_item[0].frontend_id = kAutofillProfileId; autofill_item[0].frontend_id = kAutofillProfileId;
external_delegate_->OnSuggestionsReturned( external_delegate_->OnSuggestionsReturned(
kQueryId, autofill_item, /*autoselect_first_suggestion=*/false); kRecentQueryId, autofill_item, /*autoselect_first_suggestion=*/false);
EXPECT_THAT(open_args.suggestions, SuggestionVectorIdsAre(element_ids)); EXPECT_THAT(open_args.suggestions, SuggestionVectorIdsAre(element_ids));
EXPECT_FALSE(open_args.autoselect_first_suggestion); EXPECT_FALSE(open_args.autoselect_first_suggestion);
EXPECT_EQ(open_args.popup_type, PopupType::kPersonalInformation); EXPECT_EQ(open_args.popup_type, PopupType::kPersonalInformation);
...@@ -477,7 +480,7 @@ TEST_F(AutofillExternalDelegateUnitTest, DuplicateAutofillDatalistValues) { ...@@ -477,7 +480,7 @@ TEST_F(AutofillExternalDelegateUnitTest, DuplicateAutofillDatalistValues) {
// Test that we de-dupe autocomplete values against datalist values, keeping the // Test that we de-dupe autocomplete values against datalist values, keeping the
// latter in case of a match. // latter in case of a match.
TEST_F(AutofillExternalDelegateUnitTest, DuplicateAutocompleteDatalistValues) { TEST_F(AutofillExternalDelegateUnitTest, DuplicateAutocompleteDatalistValues) {
IssueOnQuery(kQueryId); IssueOnQuery(kRecentQueryId);
std::vector<base::string16> data_list_values{base::ASCIIToUTF16("Rick"), std::vector<base::string16> data_list_values{base::ASCIIToUTF16("Rick"),
base::ASCIIToUTF16("Beyonce")}; base::ASCIIToUTF16("Beyonce")};
...@@ -513,7 +516,8 @@ TEST_F(AutofillExternalDelegateUnitTest, DuplicateAutocompleteDatalistValues) { ...@@ -513,7 +516,8 @@ TEST_F(AutofillExternalDelegateUnitTest, DuplicateAutocompleteDatalistValues) {
autocomplete_items[1].value = ASCIIToUTF16("Cain"); autocomplete_items[1].value = ASCIIToUTF16("Cain");
autocomplete_items[1].frontend_id = POPUP_ITEM_ID_AUTOCOMPLETE_ENTRY; autocomplete_items[1].frontend_id = POPUP_ITEM_ID_AUTOCOMPLETE_ENTRY;
external_delegate_->OnSuggestionsReturned( external_delegate_->OnSuggestionsReturned(
kQueryId, autocomplete_items, /*autoselect_first_suggestion=*/false); kRecentQueryId, autocomplete_items,
/*autoselect_first_suggestion=*/false);
EXPECT_THAT(open_args.suggestions, SuggestionVectorIdsAre(element_ids)); EXPECT_THAT(open_args.suggestions, SuggestionVectorIdsAre(element_ids));
EXPECT_FALSE(open_args.autoselect_first_suggestion); EXPECT_FALSE(open_args.autoselect_first_suggestion);
EXPECT_EQ(open_args.popup_type, PopupType::kPersonalInformation); EXPECT_EQ(open_args.popup_type, PopupType::kPersonalInformation);
...@@ -523,7 +527,7 @@ TEST_F(AutofillExternalDelegateUnitTest, DuplicateAutocompleteDatalistValues) { ...@@ -523,7 +527,7 @@ TEST_F(AutofillExternalDelegateUnitTest, DuplicateAutocompleteDatalistValues) {
// Autofill is disabled for a website. // Autofill is disabled for a website.
// Regression test for http://crbug.com/247880 // Regression test for http://crbug.com/247880
TEST_F(AutofillExternalDelegateUnitTest, AutofillWarnings) { TEST_F(AutofillExternalDelegateUnitTest, AutofillWarnings) {
IssueOnQuery(kQueryId); IssueOnQuery(kRecentQueryId);
AutofillClient::PopupOpenArgs open_args; AutofillClient::PopupOpenArgs open_args;
EXPECT_CALL(autofill_client_, ShowAutofillPopup) EXPECT_CALL(autofill_client_, ShowAutofillPopup)
...@@ -535,7 +539,7 @@ TEST_F(AutofillExternalDelegateUnitTest, AutofillWarnings) { ...@@ -535,7 +539,7 @@ TEST_F(AutofillExternalDelegateUnitTest, AutofillWarnings) {
autofill_item[0].frontend_id = autofill_item[0].frontend_id =
POPUP_ITEM_ID_INSECURE_CONTEXT_PAYMENT_DISABLED_MESSAGE; POPUP_ITEM_ID_INSECURE_CONTEXT_PAYMENT_DISABLED_MESSAGE;
external_delegate_->OnSuggestionsReturned( external_delegate_->OnSuggestionsReturned(
kQueryId, autofill_item, /*autoselect_first_suggestion=*/false); kRecentQueryId, autofill_item, /*autoselect_first_suggestion=*/false);
// The enums must be cast to ints to prevent compile errors on linux_rel. // The enums must be cast to ints to prevent compile errors on linux_rel.
EXPECT_THAT(open_args.suggestions, EXPECT_THAT(open_args.suggestions,
...@@ -551,7 +555,7 @@ TEST_F(AutofillExternalDelegateUnitTest, AutofillWarnings) { ...@@ -551,7 +555,7 @@ TEST_F(AutofillExternalDelegateUnitTest, AutofillWarnings) {
// entries in the vector. // entries in the vector.
TEST_F(AutofillExternalDelegateUnitTest, TEST_F(AutofillExternalDelegateUnitTest,
AutofillWarningsNotShown_WithSuggestions) { AutofillWarningsNotShown_WithSuggestions) {
IssueOnQuery(kQueryId); IssueOnQuery(kRecentQueryId);
AutofillClient::PopupOpenArgs open_args; AutofillClient::PopupOpenArgs open_args;
EXPECT_CALL(autofill_client_, ShowAutofillPopup) EXPECT_CALL(autofill_client_, ShowAutofillPopup)
...@@ -566,7 +570,7 @@ TEST_F(AutofillExternalDelegateUnitTest, ...@@ -566,7 +570,7 @@ TEST_F(AutofillExternalDelegateUnitTest,
suggestions[1].value = ASCIIToUTF16("Rick"); suggestions[1].value = ASCIIToUTF16("Rick");
suggestions[1].frontend_id = POPUP_ITEM_ID_AUTOCOMPLETE_ENTRY; suggestions[1].frontend_id = POPUP_ITEM_ID_AUTOCOMPLETE_ENTRY;
external_delegate_->OnSuggestionsReturned( external_delegate_->OnSuggestionsReturned(
kQueryId, suggestions, /*autoselect_first_suggestion=*/false); kRecentQueryId, suggestions, /*autoselect_first_suggestion=*/false);
// The enums must be cast to ints to prevent compile errors on linux_rel. // The enums must be cast to ints to prevent compile errors on linux_rel.
EXPECT_THAT(open_args.suggestions, EXPECT_THAT(open_args.suggestions,
...@@ -689,8 +693,8 @@ TEST_F(AutofillExternalDelegateUnitTest, ScanCreditCardPromptMetricsTest) { ...@@ -689,8 +693,8 @@ TEST_F(AutofillExternalDelegateUnitTest, ScanCreditCardPromptMetricsTest) {
EXPECT_CALL(*autofill_manager_, ShouldShowScanCreditCard(_, _)) EXPECT_CALL(*autofill_manager_, ShouldShowScanCreditCard(_, _))
.WillOnce(testing::Return(true)); .WillOnce(testing::Return(true));
base::HistogramTester histogram; base::HistogramTester histogram;
IssueOnQuery(kQueryId); IssueOnQuery(kRecentQueryId);
IssueOnSuggestionsReturned(); IssueOnSuggestionsReturned(kRecentQueryId);
external_delegate_->OnPopupShown(); external_delegate_->OnPopupShown();
histogram.ExpectUniqueSample("Autofill.ScanCreditCardPrompt", histogram.ExpectUniqueSample("Autofill.ScanCreditCardPrompt",
AutofillMetrics::SCAN_CARD_ITEM_SHOWN, 1); AutofillMetrics::SCAN_CARD_ITEM_SHOWN, 1);
...@@ -700,8 +704,8 @@ TEST_F(AutofillExternalDelegateUnitTest, ScanCreditCardPromptMetricsTest) { ...@@ -700,8 +704,8 @@ TEST_F(AutofillExternalDelegateUnitTest, ScanCreditCardPromptMetricsTest) {
EXPECT_CALL(*autofill_manager_, ShouldShowScanCreditCard(_, _)) EXPECT_CALL(*autofill_manager_, ShouldShowScanCreditCard(_, _))
.WillOnce(testing::Return(true)); .WillOnce(testing::Return(true));
base::HistogramTester histogram; base::HistogramTester histogram;
IssueOnQuery(kQueryId); IssueOnQuery(kRecentQueryId);
IssueOnSuggestionsReturned(); IssueOnSuggestionsReturned(kRecentQueryId);
external_delegate_->OnPopupShown(); external_delegate_->OnPopupShown();
external_delegate_->DidAcceptSuggestion(base::string16(), external_delegate_->DidAcceptSuggestion(base::string16(),
POPUP_ITEM_ID_SCAN_CREDIT_CARD, 0); POPUP_ITEM_ID_SCAN_CREDIT_CARD, 0);
...@@ -718,8 +722,8 @@ TEST_F(AutofillExternalDelegateUnitTest, ScanCreditCardPromptMetricsTest) { ...@@ -718,8 +722,8 @@ TEST_F(AutofillExternalDelegateUnitTest, ScanCreditCardPromptMetricsTest) {
EXPECT_CALL(*autofill_manager_, ShouldShowScanCreditCard(_, _)) EXPECT_CALL(*autofill_manager_, ShouldShowScanCreditCard(_, _))
.WillOnce(testing::Return(true)); .WillOnce(testing::Return(true));
base::HistogramTester histogram; base::HistogramTester histogram;
IssueOnQuery(kQueryId); IssueOnQuery(kRecentQueryId);
IssueOnSuggestionsReturned(); IssueOnSuggestionsReturned(kRecentQueryId);
external_delegate_->OnPopupShown(); external_delegate_->OnPopupShown();
external_delegate_->DidAcceptSuggestion(base::string16(), external_delegate_->DidAcceptSuggestion(base::string16(),
POPUP_ITEM_ID_CLEAR_FORM, 0); POPUP_ITEM_ID_CLEAR_FORM, 0);
...@@ -736,8 +740,8 @@ TEST_F(AutofillExternalDelegateUnitTest, ScanCreditCardPromptMetricsTest) { ...@@ -736,8 +740,8 @@ TEST_F(AutofillExternalDelegateUnitTest, ScanCreditCardPromptMetricsTest) {
EXPECT_CALL(*autofill_manager_, ShouldShowScanCreditCard(_, _)) EXPECT_CALL(*autofill_manager_, ShouldShowScanCreditCard(_, _))
.WillOnce(testing::Return(false)); .WillOnce(testing::Return(false));
base::HistogramTester histogram; base::HistogramTester histogram;
IssueOnQuery(kQueryId); IssueOnQuery(kRecentQueryId);
IssueOnSuggestionsReturned(); IssueOnSuggestionsReturned(kRecentQueryId);
external_delegate_->OnPopupShown(); external_delegate_->OnPopupShown();
histogram.ExpectTotalCount("Autofill.ScanCreditCardPrompt", 0); histogram.ExpectTotalCount("Autofill.ScanCreditCardPrompt", 0);
} }
...@@ -775,7 +779,7 @@ TEST_F(AutofillExternalDelegateUnitTest, IgnoreAutocompleteOffForAutofill) { ...@@ -775,7 +779,7 @@ TEST_F(AutofillExternalDelegateUnitTest, IgnoreAutocompleteOffForAutofill) {
field.is_focusable = true; field.is_focusable = true;
field.should_autocomplete = false; field.should_autocomplete = false;
external_delegate_->OnQuery(kQueryId, form, field, gfx::RectF()); external_delegate_->OnQuery(kRecentQueryId, form, field, gfx::RectF());
std::vector<Suggestion> autofill_items; std::vector<Suggestion> autofill_items;
autofill_items.push_back(Suggestion()); autofill_items.push_back(Suggestion());
...@@ -786,7 +790,7 @@ TEST_F(AutofillExternalDelegateUnitTest, IgnoreAutocompleteOffForAutofill) { ...@@ -786,7 +790,7 @@ TEST_F(AutofillExternalDelegateUnitTest, IgnoreAutocompleteOffForAutofill) {
EXPECT_CALL(autofill_client_, HideAutofillPopup(_)).Times(0); EXPECT_CALL(autofill_client_, HideAutofillPopup(_)).Times(0);
external_delegate_->OnSuggestionsReturned( external_delegate_->OnSuggestionsReturned(
kQueryId, autofill_items, /*autoselect_first_suggestion=*/false); kRecentQueryId, autofill_items, /*autoselect_first_suggestion=*/false);
} }
TEST_F(AutofillExternalDelegateUnitTest, ExternalDelegateFillFieldWithValue) { TEST_F(AutofillExternalDelegateUnitTest, ExternalDelegateFillFieldWithValue) {
...@@ -806,7 +810,7 @@ TEST_F(AutofillExternalDelegateUnitTest, ExternalDelegateFillFieldWithValue) { ...@@ -806,7 +810,7 @@ TEST_F(AutofillExternalDelegateUnitTest, ExternalDelegateFillFieldWithValue) {
} }
TEST_F(AutofillExternalDelegateUnitTest, ShouldShowGooglePayIcon) { TEST_F(AutofillExternalDelegateUnitTest, ShouldShowGooglePayIcon) {
IssueOnQuery(kQueryId); IssueOnQuery(kRecentQueryId);
auto element_icons = auto element_icons =
testing::ElementsAre(std::string(), testing::StartsWith("googlePay")); testing::ElementsAre(std::string(), testing::StartsWith("googlePay"));
...@@ -820,7 +824,8 @@ TEST_F(AutofillExternalDelegateUnitTest, ShouldShowGooglePayIcon) { ...@@ -820,7 +824,8 @@ TEST_F(AutofillExternalDelegateUnitTest, ShouldShowGooglePayIcon) {
// This should call ShowAutofillPopup. // This should call ShowAutofillPopup.
external_delegate_->OnSuggestionsReturned( external_delegate_->OnSuggestionsReturned(
kQueryId, autofill_item, /*autoselect_first_suggestion=*/false, true); kRecentQueryId, autofill_item, /*autoselect_first_suggestion=*/false,
true);
EXPECT_THAT(open_args.suggestions, SuggestionVectorIconsAre(element_icons)); EXPECT_THAT(open_args.suggestions, SuggestionVectorIconsAre(element_icons));
EXPECT_FALSE(open_args.autoselect_first_suggestion); EXPECT_FALSE(open_args.autoselect_first_suggestion);
EXPECT_EQ(open_args.popup_type, PopupType::kPersonalInformation); EXPECT_EQ(open_args.popup_type, PopupType::kPersonalInformation);
...@@ -828,7 +833,7 @@ TEST_F(AutofillExternalDelegateUnitTest, ShouldShowGooglePayIcon) { ...@@ -828,7 +833,7 @@ TEST_F(AutofillExternalDelegateUnitTest, ShouldShowGooglePayIcon) {
TEST_F(AutofillExternalDelegateUnitTest, TEST_F(AutofillExternalDelegateUnitTest,
ShouldNotShowGooglePayIconIfSuggestionsContainLocalCards) { ShouldNotShowGooglePayIconIfSuggestionsContainLocalCards) {
IssueOnQuery(kQueryId); IssueOnQuery(kRecentQueryId);
auto element_icons = testing::ElementsAre( auto element_icons = testing::ElementsAre(
std::string(), std::string(),
...@@ -843,14 +848,15 @@ TEST_F(AutofillExternalDelegateUnitTest, ...@@ -843,14 +848,15 @@ TEST_F(AutofillExternalDelegateUnitTest,
// This should call ShowAutofillPopup. // This should call ShowAutofillPopup.
external_delegate_->OnSuggestionsReturned( external_delegate_->OnSuggestionsReturned(
kQueryId, autofill_item, /*autoselect_first_suggestion=*/false, false); kRecentQueryId, autofill_item, /*autoselect_first_suggestion=*/false,
false);
EXPECT_THAT(open_args.suggestions, SuggestionVectorIconsAre(element_icons)); EXPECT_THAT(open_args.suggestions, SuggestionVectorIconsAre(element_icons));
EXPECT_FALSE(open_args.autoselect_first_suggestion); EXPECT_FALSE(open_args.autoselect_first_suggestion);
EXPECT_EQ(open_args.popup_type, PopupType::kPersonalInformation); EXPECT_EQ(open_args.popup_type, PopupType::kPersonalInformation);
} }
TEST_F(AutofillExternalDelegateUnitTest, ShouldUseNewSettingName) { TEST_F(AutofillExternalDelegateUnitTest, ShouldUseNewSettingName) {
IssueOnQuery(kQueryId); IssueOnQuery(kRecentQueryId);
auto element_values = testing::ElementsAre( auto element_values = testing::ElementsAre(
base::string16(), l10n_util::GetStringUTF16(IDS_AUTOFILL_MANAGE)); base::string16(), l10n_util::GetStringUTF16(IDS_AUTOFILL_MANAGE));
...@@ -864,7 +870,7 @@ TEST_F(AutofillExternalDelegateUnitTest, ShouldUseNewSettingName) { ...@@ -864,7 +870,7 @@ TEST_F(AutofillExternalDelegateUnitTest, ShouldUseNewSettingName) {
// This should call ShowAutofillPopup. // This should call ShowAutofillPopup.
external_delegate_->OnSuggestionsReturned( external_delegate_->OnSuggestionsReturned(
kQueryId, autofill_item, /*autoselect_first_suggestion=*/false); kRecentQueryId, autofill_item, /*autoselect_first_suggestion=*/false);
EXPECT_THAT(open_args.suggestions, SuggestionVectorValuesAre(element_values)); EXPECT_THAT(open_args.suggestions, SuggestionVectorValuesAre(element_values));
EXPECT_FALSE(open_args.autoselect_first_suggestion); EXPECT_FALSE(open_args.autoselect_first_suggestion);
EXPECT_EQ(open_args.popup_type, PopupType::kPersonalInformation); EXPECT_EQ(open_args.popup_type, PopupType::kPersonalInformation);
...@@ -875,7 +881,7 @@ TEST_F(AutofillExternalDelegateUnitTest, ShouldUseNewSettingName) { ...@@ -875,7 +881,7 @@ TEST_F(AutofillExternalDelegateUnitTest, ShouldUseNewSettingName) {
// row in the footer. // row in the footer.
TEST_F(AutofillExternalDelegateCardsFromAccountTest, TEST_F(AutofillExternalDelegateCardsFromAccountTest,
ShouldShowCardsFromAccountOptionWithCards) { ShouldShowCardsFromAccountOptionWithCards) {
IssueOnQuery(kQueryId); IssueOnQuery(kRecentQueryId);
auto element_values = testing::ElementsAre( auto element_values = testing::ElementsAre(
base::string16(), base::string16(),
...@@ -890,7 +896,7 @@ TEST_F(AutofillExternalDelegateCardsFromAccountTest, ...@@ -890,7 +896,7 @@ TEST_F(AutofillExternalDelegateCardsFromAccountTest,
autofill_item[0].frontend_id = kAutofillProfileId; autofill_item[0].frontend_id = kAutofillProfileId;
external_delegate_->OnSuggestionsReturned( external_delegate_->OnSuggestionsReturned(
kQueryId, autofill_item, /*autoselect_first_suggestion=*/false); kRecentQueryId, autofill_item, /*autoselect_first_suggestion=*/false);
EXPECT_THAT(open_args.suggestions, SuggestionVectorValuesAre(element_values)); EXPECT_THAT(open_args.suggestions, SuggestionVectorValuesAre(element_values));
EXPECT_FALSE(open_args.autoselect_first_suggestion); EXPECT_FALSE(open_args.autoselect_first_suggestion);
EXPECT_EQ(open_args.popup_type, PopupType::kPersonalInformation); EXPECT_EQ(open_args.popup_type, PopupType::kPersonalInformation);
...@@ -901,7 +907,7 @@ TEST_F(AutofillExternalDelegateCardsFromAccountTest, ...@@ -901,7 +907,7 @@ TEST_F(AutofillExternalDelegateCardsFromAccountTest,
// *not* show up in this case. // *not* show up in this case.
TEST_F(AutofillExternalDelegateCardsFromAccountTest, TEST_F(AutofillExternalDelegateCardsFromAccountTest,
ShouldShowCardsFromAccountOptionWithoutCards) { ShouldShowCardsFromAccountOptionWithoutCards) {
IssueOnQuery(kQueryId); IssueOnQuery(kRecentQueryId);
auto element_values = testing::ElementsAre( auto element_values = testing::ElementsAre(
l10n_util::GetStringUTF16(IDS_AUTOFILL_SHOW_ACCOUNT_CARDS)); l10n_util::GetStringUTF16(IDS_AUTOFILL_SHOW_ACCOUNT_CARDS));
...@@ -910,11 +916,25 @@ TEST_F(AutofillExternalDelegateCardsFromAccountTest, ...@@ -910,11 +916,25 @@ TEST_F(AutofillExternalDelegateCardsFromAccountTest,
.WillOnce(testing::SaveArg<0>(&open_args)); .WillOnce(testing::SaveArg<0>(&open_args));
external_delegate_->OnSuggestionsReturned( external_delegate_->OnSuggestionsReturned(
kQueryId, std::vector<Suggestion>(), kRecentQueryId, std::vector<Suggestion>(),
/*autoselect_first_suggestion=*/false); /*autoselect_first_suggestion=*/false);
EXPECT_THAT(open_args.suggestions, SuggestionVectorValuesAre(element_values)); EXPECT_THAT(open_args.suggestions, SuggestionVectorValuesAre(element_values));
EXPECT_FALSE(open_args.autoselect_first_suggestion); EXPECT_FALSE(open_args.autoselect_first_suggestion);
EXPECT_EQ(open_args.popup_type, PopupType::kPersonalInformation); EXPECT_EQ(open_args.popup_type, PopupType::kPersonalInformation);
} }
#if defined(OS_IOS)
// Tests that outdated returned suggestions are discarded.
TEST_F(AutofillExternalDelegateCardsFromAccountTest,
ShouldDiscardOutdatedSuggestions) {
int older_query_id = kRecentQueryId - 1;
IssueOnQuery(older_query_id);
EXPECT_CALL(autofill_client_, ShowAutofillPopup).Times(0);
external_delegate_->OnSuggestionsReturned(
older_query_id, std::vector<Suggestion>(),
/*autoselect_first_suggestion=*/false);
}
#endif
} // namespace autofill } // namespace autofill
...@@ -263,6 +263,12 @@ void TestAutofillClient::LoadRiskData( ...@@ -263,6 +263,12 @@ void TestAutofillClient::LoadRiskData(
std::move(callback).Run("some risk data"); std::move(callback).Run("some risk data");
} }
#if defined(OS_IOS)
bool TestAutofillClient::IsQueryIDRelevant(int query_id) {
return true;
}
#endif
void TestAutofillClient::InitializeUKMSources() { void TestAutofillClient::InitializeUKMSources() {
test_ukm_recorder_.UpdateSourceURL(source_id_, form_origin_); test_ukm_recorder_.UpdateSourceURL(source_id_, form_origin_);
} }
......
...@@ -148,6 +148,10 @@ class TestAutofillClient : public AutofillClient { ...@@ -148,6 +148,10 @@ class TestAutofillClient : public AutofillClient {
void LoadRiskData( void LoadRiskData(
base::OnceCallback<void(const std::string&)> callback) override; base::OnceCallback<void(const std::string&)> callback) override;
#if defined(OS_IOS)
bool IsQueryIDRelevant(int query_id) override;
#endif
// Initializes UKM source from form_origin_. This needs to be called // Initializes UKM source from form_origin_. This needs to be called
// in unittests after calling Purge for ukm recorder to re-initialize // in unittests after calling Purge for ukm recorder to re-initialize
// sources. // sources.
......
...@@ -195,6 +195,9 @@ void UpdateFieldManagerForClearedIDs( ...@@ -195,6 +195,9 @@ void UpdateFieldManagerForClearedIDs(
_last_submitted_autofill_driver; _last_submitted_autofill_driver;
scoped_refptr<FieldDataManager> _fieldDataManager; scoped_refptr<FieldDataManager> _fieldDataManager;
// ID of the last Autofill query made. Used to discard outdated suggestions.
int _lastQueryID;
} }
@end @end
...@@ -226,6 +229,7 @@ void UpdateFieldManagerForClearedIDs( ...@@ -226,6 +229,7 @@ void UpdateFieldManagerForClearedIDs(
UniqueIDDataTabHelper* uniqueIDDataTabHelper = UniqueIDDataTabHelper* uniqueIDDataTabHelper =
UniqueIDDataTabHelper::FromWebState(_webState); UniqueIDDataTabHelper::FromWebState(_webState);
_fieldDataManager = uniqueIDDataTabHelper->GetFieldDataManager(); _fieldDataManager = uniqueIDDataTabHelper->GetFieldDataManager();
_lastQueryID = 0;
} }
return self; return self;
} }
...@@ -336,9 +340,6 @@ autofillManagerFromWebState:(web::WebState*)webState ...@@ -336,9 +340,6 @@ autofillManagerFromWebState:(web::WebState*)webState
if (!autofillManager) if (!autofillManager)
return; return;
// Passed to delegates; we don't use it so it's set to zero.
int queryId = 0;
// Find the right field. // Find the right field.
autofill::FormFieldData field; autofill::FormFieldData field;
GetFormField(&field, form, SysNSStringToUTF16(fieldIdentifier)); GetFormField(&field, form, SysNSStringToUTF16(fieldIdentifier));
...@@ -350,7 +351,7 @@ autofillManagerFromWebState:(web::WebState*)webState ...@@ -350,7 +351,7 @@ autofillManagerFromWebState:(web::WebState*)webState
// Query the AutofillManager for suggestions. Results will arrive in // Query the AutofillManager for suggestions. Results will arrive in
// -showAutofillPopup:popupDelegate:. // -showAutofillPopup:popupDelegate:.
autofillManager->OnQueryFormFieldAutofill( autofillManager->OnQueryFormFieldAutofill(
queryId, form, field, gfx::RectF(), ++_lastQueryID, form, field, gfx::RectF(),
/*autoselect_first_suggestion=*/false); /*autoselect_first_suggestion=*/false);
} }
...@@ -610,6 +611,10 @@ autofillManagerFromWebState:(web::WebState*)webState ...@@ -610,6 +611,10 @@ autofillManagerFromWebState:(web::WebState*)webState
popupDelegate:base::WeakPtr<autofill::AutofillPopupDelegate>()]; popupDelegate:base::WeakPtr<autofill::AutofillPopupDelegate>()];
} }
- (bool)isQueryIDRelevant:(int)queryID {
return queryID == _lastQueryID;
}
#pragma mark - CRWWebStateObserver #pragma mark - CRWWebStateObserver
- (void)webStateWasShown:(web::WebState*)webState { - (void)webStateWasShown:(web::WebState*)webState {
......
...@@ -23,6 +23,9 @@ struct Suggestion; ...@@ -23,6 +23,9 @@ struct Suggestion;
- (void)hideAutofillPopup; - (void)hideAutofillPopup;
// Checks whether the qurrent query is the most recent one.
- (bool)isQueryIDRelevant:(int)queryID;
@end @end
#endif // COMPONENTS_AUTOFILL_IOS_BROWSER_AUTOFILL_CLIENT_IOS_BRIDGE_H_ #endif // COMPONENTS_AUTOFILL_IOS_BROWSER_AUTOFILL_CLIENT_IOS_BRIDGE_H_
...@@ -121,6 +121,8 @@ class ChromeAutofillClientIOS : public AutofillClient { ...@@ -121,6 +121,8 @@ class ChromeAutofillClientIOS : public AutofillClient {
LogManager* GetLogManager() const override; LogManager* GetLogManager() const override;
bool IsQueryIDRelevant(int query_id) override;
private: private:
PrefService* pref_service_; PrefService* pref_service_;
syncer::SyncService* sync_service_; syncer::SyncService* sync_service_;
......
...@@ -396,4 +396,8 @@ LogManager* ChromeAutofillClientIOS::GetLogManager() const { ...@@ -396,4 +396,8 @@ LogManager* ChromeAutofillClientIOS::GetLogManager() const {
return log_manager_.get(); return log_manager_.get();
} }
bool ChromeAutofillClientIOS::IsQueryIDRelevant(int query_id) {
return [bridge_ isQueryIDRelevant:query_id];
}
} // namespace autofill } // namespace autofill
...@@ -339,6 +339,10 @@ using autofill::FieldRendererId; ...@@ -339,6 +339,10 @@ using autofill::FieldRendererId;
[_autofillAgent hideAutofillPopup]; [_autofillAgent hideAutofillPopup];
} }
- (bool)isQueryIDRelevant:(int)queryID {
return [_autofillAgent isQueryIDRelevant:queryID];
}
- (void) - (void)
confirmCreditCardAccountName:(const base::string16&)name confirmCreditCardAccountName:(const base::string16&)name
callback: callback:
......
...@@ -116,6 +116,8 @@ class WebViewAutofillClientIOS : public AutofillClient { ...@@ -116,6 +116,8 @@ class WebViewAutofillClientIOS : public AutofillClient {
void set_bridge(id<CWVAutofillClientIOSBridge> bridge) { bridge_ = bridge; } void set_bridge(id<CWVAutofillClientIOSBridge> bridge) { bridge_ = bridge; }
bool IsQueryIDRelevant(int query_id) override;
private: private:
PrefService* pref_service_; PrefService* pref_service_;
PersonalDataManager* personal_data_manager_; PersonalDataManager* personal_data_manager_;
......
...@@ -304,4 +304,8 @@ LogManager* WebViewAutofillClientIOS::GetLogManager() const { ...@@ -304,4 +304,8 @@ LogManager* WebViewAutofillClientIOS::GetLogManager() const {
return log_manager_.get(); return log_manager_.get();
} }
bool WebViewAutofillClientIOS::IsQueryIDRelevant(int query_id) {
return [bridge_ isQueryIDRelevant:query_id];
}
} // namespace autofill } // namespace autofill
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