diff options
6 files changed, 106 insertions, 192 deletions
diff --git a/chrome/renderer/autofill/password_autofill_agent_browsertest.cc b/chrome/renderer/autofill/password_autofill_agent_browsertest.cc index 4e60b37..451b1fc 100644 --- a/chrome/renderer/autofill/password_autofill_agent_browsertest.cc +++ b/chrome/renderer/autofill/password_autofill_agent_browsertest.cc @@ -417,22 +417,47 @@ class PasswordAutofillAgentTest : public ChromeRenderViewTest { EXPECT_EQ(end, username_element_.selectionEnd()); } - // Checks the message sent to PasswordAutofillManager to build the suggestion - // list. |username| is the expected username field value, and |show_all| is - // the expected flag for the PasswordAutofillManager, whether to show all - // suggestions, or only those starting with |username|. - void CheckSuggestions(const std::string& username, bool show_all) { + void ExpectOneCredential(const base::string16& username) { const IPC::Message* message = render_thread_->sink().GetFirstMessageMatching( AutofillHostMsg_ShowPasswordSuggestions::ID); - EXPECT_TRUE(message); - Tuple4<autofill::FormFieldData, base::string16, bool, gfx::RectF> args; + ASSERT_TRUE(message); + Tuple4<autofill::FormFieldData, + gfx::RectF, + std::vector<base::string16>, + std::vector<base::string16> > args; AutofillHostMsg_ShowPasswordSuggestions::Read(message, &args); - EXPECT_EQ(2u, fill_data_.basic_data.fields.size()); - EXPECT_EQ(fill_data_.basic_data.fields[0].name, args.a.name); - EXPECT_EQ(ASCIIToUTF16(username), args.a.value); - EXPECT_EQ(ASCIIToUTF16(username), args.b); - EXPECT_EQ(show_all, args.c); + ASSERT_EQ(1u, args.c.size()); + EXPECT_TRUE(args.c[0] == username); + } + + void ExpectAllCredentials() { + std::set<base::string16> usernames; + usernames.insert(username1_); + usernames.insert(username2_); + usernames.insert(username3_); + usernames.insert(alternate_username3_); + + const IPC::Message* message = + render_thread_->sink().GetFirstMessageMatching( + AutofillHostMsg_ShowPasswordSuggestions::ID); + ASSERT_TRUE(message); + Tuple4<autofill::FormFieldData, + gfx::RectF, + std::vector<base::string16>, + std::vector<base::string16> > args; + AutofillHostMsg_ShowPasswordSuggestions::Read(message, &args); + ASSERT_EQ(4u, args.c.size()); + std::set<base::string16>::iterator it; + + for (int i = 0; i < 4; i++) { + it = usernames.find(args.c[i]); + EXPECT_TRUE(it != usernames.end()); + if (it != usernames.end()) + usernames.erase(it); + } + + EXPECT_TRUE(usernames.empty()); render_thread_->sink().ClearMessages(); } @@ -1430,7 +1455,7 @@ TEST_F(PasswordAutofillAgentTest, ClickAndSelect) { SimulateOnFillPasswordForm(fill_data_); SimulateElementClick(kUsernameName); SimulateSuggestionChoice(username_element_); - CheckSuggestions(kAliceUsername, true); + ExpectAllCredentials(); CheckTextFieldsDOMState(kAliceUsername, true, kAlicePassword, true); } @@ -1456,7 +1481,7 @@ TEST_F(PasswordAutofillAgentTest, CredentialsOnClick) { render_thread_->sink().ClearMessages(); static_cast<PageClickListener*>(autofill_agent_) ->FormControlElementClicked(username_element_, false); - CheckSuggestions(std::string(), false); + ExpectAllCredentials(); // Now simulate a user typing in an unrecognized username and then // clicking on the username element. This should also produce a message with @@ -1465,7 +1490,7 @@ TEST_F(PasswordAutofillAgentTest, CredentialsOnClick) { render_thread_->sink().ClearMessages(); static_cast<PageClickListener*>(autofill_agent_) ->FormControlElementClicked(username_element_, true); - CheckSuggestions("baz", true); + ExpectAllCredentials(); // Now simulate a user typing in the first letter of the username and then // clicking on the username element. While the typing of the first letter will @@ -1475,7 +1500,7 @@ TEST_F(PasswordAutofillAgentTest, CredentialsOnClick) { render_thread_->sink().ClearMessages(); static_cast<PageClickListener*>(autofill_agent_) ->FormControlElementClicked(username_element_, true); - CheckSuggestions(kAliceUsername, true); + ExpectAllCredentials(); } // The user types in a password, but then just before sending the form off, a diff --git a/components/autofill/content/common/autofill_messages.h b/components/autofill/content/common/autofill_messages.h index e339a64..25d8277 100644 --- a/components/autofill/content/common/autofill_messages.h +++ b/components/autofill/content/common/autofill_messages.h @@ -285,12 +285,13 @@ IPC_MESSAGE_ROUTED2(AutofillHostMsg_AddPasswordFormMapping, autofill::FormFieldData, /* the user name field */ autofill::PasswordFormFillData /* password pairings */) -// Instruct the browser to show a popup with suggestions for the form field. +// Instruct the browser to show a popup with the following suggestions from the +// password manager. IPC_MESSAGE_ROUTED4(AutofillHostMsg_ShowPasswordSuggestions, autofill::FormFieldData /* the form field */, - base::string16 /* username typed by user */, - bool /* show all suggestions */, - gfx::RectF /* input field bounds, window-relative */) + gfx::RectF /* input field bounds, window-relative */, + std::vector<base::string16> /* suggestions */, + std::vector<base::string16> /* realms */) // Inform browser of data list values for the curent field. IPC_MESSAGE_ROUTED2(AutofillHostMsg_SetDataList, diff --git a/components/autofill/content/renderer/password_autofill_agent.cc b/components/autofill/content/renderer/password_autofill_agent.cc index f2882c9..c598692 100644 --- a/components/autofill/content/renderer/password_autofill_agent.cc +++ b/components/autofill/content/renderer/password_autofill_agent.cc @@ -240,41 +240,45 @@ bool FillDataContainsUsername(const PasswordFormFillData& fill_data) { return !fill_data.basic_data.fields[0].name.empty(); } -// Sets |suggestions_present| to true if there are any suggestions to be derived -// from |fill_data|. Unless |show_all| is true, only considers suggestions with -// usernames having |current_username| as a prefix. Returns true if a username -// from the |fill_data.other_possible_usernames| would be included in the -// suggestions. -bool GetSuggestionsStats(const PasswordFormFillData& fill_data, - const base::string16& current_username, - bool show_all, - bool* suggestions_present) { - *suggestions_present = false; - - for (const auto& usernames : fill_data.other_possible_usernames) { - for (size_t i = 0; i < usernames.second.size(); ++i) { - if (show_all || - StartsWith(usernames.second[i], current_username, false)) { - *suggestions_present = true; - return true; - } - } +// This function attempts to fill |suggestions| and |realms| form |fill_data| +// based on |current_username|. Returns true when |suggestions| gets filled +// from |fill_data.other_possible_usernames|, else returns false. +bool GetSuggestions(const PasswordFormFillData& fill_data, + const base::string16& current_username, + std::vector<base::string16>* suggestions, + std::vector<base::string16>* realms, + bool show_all) { + bool other_possible_username_shown = false; + if (show_all || + StartsWith( + fill_data.basic_data.fields[0].value, current_username, false)) { + suggestions->push_back(fill_data.basic_data.fields[0].value); + realms->push_back(base::UTF8ToUTF16(fill_data.preferred_realm)); } - if (show_all || StartsWith(fill_data.basic_data.fields[0].value, - current_username, false)) { - *suggestions_present = true; - return false; + for (PasswordFormFillData::LoginCollection::const_iterator iter = + fill_data.additional_logins.begin(); + iter != fill_data.additional_logins.end(); + ++iter) { + if (show_all || StartsWith(iter->first, current_username, false)) { + suggestions->push_back(iter->first); + realms->push_back(base::UTF8ToUTF16(iter->second.realm)); + } } - for (const auto& login : fill_data.additional_logins) { - if (show_all || StartsWith(login.first, current_username, false)) { - *suggestions_present = true; - return false; + for (PasswordFormFillData::UsernamesCollection::const_iterator iter = + fill_data.other_possible_usernames.begin(); + iter != fill_data.other_possible_usernames.end(); + ++iter) { + for (size_t i = 0; i < iter->second.size(); ++i) { + if (show_all || StartsWith(iter->second[i], current_username, false)) { + other_possible_username_shown = true; + suggestions->push_back(iter->second[i]); + realms->push_back(base::UTF8ToUTF16(iter->first.realm)); + } } } - - return false; + return other_possible_username_shown; } // This function attempts to fill |username_element| and |password_element| @@ -1104,6 +1108,15 @@ bool PasswordAutofillAgent::ShowSuggestionPopup( if (!webview) return false; + std::vector<base::string16> suggestions; + std::vector<base::string16> realms; + if (GetSuggestions( + fill_data, user_input.value(), &suggestions, &realms, show_all)) { + usernames_usage_ = OTHER_POSSIBLE_USERNAME_SHOWN; + } + + DCHECK_EQ(suggestions.size(), realms.size()); + FormData form; FormFieldData field; FindFormAndFieldForFormControlElement( @@ -1118,14 +1131,8 @@ bool PasswordAutofillAgent::ShowSuggestionPopup( bounding_box.width() * scale, bounding_box.height() * scale); Send(new AutofillHostMsg_ShowPasswordSuggestions( - routing_id(), field, user_input.value(), show_all, bounding_box_scaled)); - - bool suggestions_present = false; - if (GetSuggestionsStats(fill_data, user_input.value(), show_all, - &suggestions_present)) { - usernames_usage_ = OTHER_POSSIBLE_USERNAME_SHOWN; - } - return suggestions_present; + routing_id(), field, bounding_box_scaled, suggestions, realms)); + return !suggestions.empty(); } void PasswordAutofillAgent::PerformInlineAutocomplete( diff --git a/components/password_manager/core/browser/password_autofill_manager.cc b/components/password_manager/core/browser/password_autofill_manager.cc index ab03c02..01bf791 100644 --- a/components/password_manager/core/browser/password_autofill_manager.cc +++ b/components/password_manager/core/browser/password_autofill_manager.cc @@ -2,59 +2,16 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "components/password_manager/core/browser/password_autofill_manager.h" - -#include <vector> - #include "base/logging.h" -#include "base/strings/string16.h" -#include "base/strings/string_util.h" -#include "base/strings/utf_string_conversions.h" #include "components/autofill/core/browser/autofill_driver.h" #include "components/autofill/core/browser/popup_item_ids.h" #include "components/autofill/core/common/autofill_data_validation.h" +#include "components/password_manager/core/browser/password_autofill_manager.h" #include "components/password_manager/core/browser/password_manager_client.h" #include "components/password_manager/core/browser/password_manager_driver.h" namespace password_manager { -namespace { - -// This function attempts to fill |suggestions| and |realms| form |fill_data| -// based on |current_username|. Unless |show_all| is true, it only picks -// suggestions where the username has |current_username| as a prefix. -void GetSuggestions(const autofill::PasswordFormFillData& fill_data, - const base::string16& current_username, - std::vector<base::string16>* suggestions, - std::vector<base::string16>* realms, - bool show_all) { - if (show_all || - StartsWith( - fill_data.basic_data.fields[0].value, current_username, false)) { - suggestions->push_back(fill_data.basic_data.fields[0].value); - realms->push_back(base::UTF8ToUTF16(fill_data.preferred_realm)); - } - - for (const auto& login : fill_data.additional_logins) { - if (show_all || StartsWith(login.first, current_username, false)) { - suggestions->push_back(login.first); - realms->push_back(base::UTF8ToUTF16(login.second.realm)); - } - } - - for (const auto& usernames : fill_data.other_possible_usernames) { - for (size_t i = 0; i < usernames.second.size(); ++i) { - if (show_all || - StartsWith(usernames.second[i], current_username, false)) { - suggestions->push_back(usernames.second[i]); - realms->push_back(base::UTF8ToUTF16(usernames.first.realm)); - } - } - } -} - -} // namespace - //////////////////////////////////////////////////////////////////////////////// // PasswordAutofillManager, public: @@ -109,18 +66,9 @@ void PasswordAutofillManager::OnAddPasswordFormMapping( void PasswordAutofillManager::OnShowPasswordSuggestions( const autofill::FormFieldData& field, - const base::string16& typed_username, - bool show_all, - const gfx::RectF& bounds) { - std::vector<base::string16> suggestions; - std::vector<base::string16> realms; - LoginToPasswordInfoMap::const_iterator fill_data_it = - login_to_password_info_.find(field); - DCHECK(fill_data_it != login_to_password_info_.end()); - GetSuggestions(fill_data_it->second, typed_username, &suggestions, &realms, - show_all); - DCHECK_EQ(suggestions.size(), realms.size()); - + const gfx::RectF& bounds, + const std::vector<base::string16>& suggestions, + const std::vector<base::string16>& realms) { if (!autofill::IsValidString16Vector(suggestions) || !autofill::IsValidString16Vector(realms) || suggestions.size() != realms.size()) diff --git a/components/password_manager/core/browser/password_autofill_manager.h b/components/password_manager/core/browser/password_autofill_manager.h index f3d4f5d..88420c9 100644 --- a/components/password_manager/core/browser/password_autofill_manager.h +++ b/components/password_manager/core/browser/password_autofill_manager.h @@ -44,10 +44,11 @@ class PasswordAutofillManager : public autofill::AutofillPopupDelegate { // Handles a request from the renderer to show a popup with the given // |suggestions| from the password manager. - void OnShowPasswordSuggestions(const autofill::FormFieldData& field, - const base::string16& typed_username, - bool show_all, - const gfx::RectF& bounds); + void OnShowPasswordSuggestions( + const autofill::FormFieldData& field, + const gfx::RectF& bounds, + const std::vector<base::string16>& suggestions, + const std::vector<base::string16>& realms); // Invoked to clear any page specific cached values. void Reset(); diff --git a/components/password_manager/core/browser/password_autofill_manager_unittest.cc b/components/password_manager/core/browser/password_autofill_manager_unittest.cc index ab09812..c2056c1 100644 --- a/components/password_manager/core/browser/password_autofill_manager_unittest.cc +++ b/components/password_manager/core/browser/password_autofill_manager_unittest.cc @@ -2,16 +2,13 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "components/password_manager/core/browser/password_autofill_manager.h" - #include "base/compiler_specific.h" #include "base/message_loop/message_loop.h" #include "base/strings/utf_string_conversions.h" #include "components/autofill/core/browser/popup_item_ids.h" #include "components/autofill/core/browser/test_autofill_client.h" #include "components/autofill/core/browser/test_autofill_driver.h" -#include "components/autofill/core/common/form_field_data.h" -#include "components/autofill/core/common/password_form_fill_data.h" +#include "components/password_manager/core/browser/password_autofill_manager.h" #include "components/password_manager/core/browser/stub_password_manager_client.h" #include "components/password_manager/core/browser/stub_password_manager_driver.h" #include "testing/gmock/include/gmock/gmock.h" @@ -170,16 +167,10 @@ TEST_F(PasswordAutofillManagerTest, ExternalDelegatePasswordSuggestions) { InitializePasswordAutofillManager(client.get(), autofill_client.get()); gfx::RectF element_bounds; - autofill::PasswordFormFillData data; - data.basic_data.fields.resize(2); - data.basic_data.fields[0].value = test_username_; - data.basic_data.fields[1].value = test_password_; - data.preferred_realm = "http://foo.com/"; - autofill::FormFieldData dummy_key; - password_autofill_manager_->OnAddPasswordFormMapping(dummy_key, data); - - EXPECT_CALL(*client->mock_driver(), - FillSuggestion(test_username_, test_password_)); + std::vector<base::string16> suggestions; + suggestions.push_back(test_username_); + std::vector<base::string16> realms; + realms.push_back(base::ASCIIToUTF16("http://foo.com/")); // The enums must be cast to ints to prevent compile errors on linux_rel. EXPECT_CALL(*autofill_client, @@ -192,71 +183,12 @@ TEST_F(PasswordAutofillManagerTest, ExternalDelegatePasswordSuggestions) { testing::ElementsAre(autofill::POPUP_ITEM_ID_PASSWORD_ENTRY), _)); password_autofill_manager_->OnShowPasswordSuggestions( - dummy_key, base::string16(), false, element_bounds); + username_field_, element_bounds, suggestions, realms); // Accepting a suggestion should trigger a call to hide the popup. EXPECT_CALL(*autofill_client, HideAutofillPopup()); password_autofill_manager_->DidAcceptSuggestion( - test_username_, autofill::POPUP_ITEM_ID_PASSWORD_ENTRY); -} - -// Test that OnShowPasswordSuggestions correctly matches the given FormFieldData -// to the known PasswordFormFillData, and extracts the right suggestions. -TEST_F(PasswordAutofillManagerTest, ExtractSuggestions) { - scoped_ptr<TestPasswordManagerClient> client(new TestPasswordManagerClient); - scoped_ptr<MockAutofillClient> autofill_client(new MockAutofillClient); - InitializePasswordAutofillManager(client.get(), autofill_client.get()); - - gfx::RectF element_bounds; - autofill::PasswordFormFillData data; - data.basic_data.fields.resize(2); - data.basic_data.fields[0].value = test_username_; - data.basic_data.fields[1].value = test_password_; - data.preferred_realm = "http://foo.com/"; - - autofill::PasswordAndRealm additional; - additional.realm = "https://foobarrealm.org"; - base::string16 additional_username(base::ASCIIToUTF16("John Foo")); - data.additional_logins[additional_username] = additional; - - autofill::UsernamesCollectionKey usernames_key; - usernames_key.realm = "http://yetanother.net"; - std::vector<base::string16> other_names; - base::string16 other_username(base::ASCIIToUTF16("John Different")); - other_names.push_back(other_username); - data.other_possible_usernames[usernames_key] = other_names; - - autofill::FormFieldData dummy_key; - password_autofill_manager_->OnAddPasswordFormMapping(dummy_key, data); - - // First, simulate displaying suggestions matching an empty prefix. - EXPECT_CALL(*autofill_client, - ShowAutofillPopup( - element_bounds, _, - testing::UnorderedElementsAre( - test_username_, additional_username, other_username), - _, _, _, _)); - password_autofill_manager_->OnShowPasswordSuggestions( - dummy_key, base::string16(), false, element_bounds); - - // Now simulate displaying suggestions matching "John". - EXPECT_CALL(*autofill_client, - ShowAutofillPopup(element_bounds, _, - testing::UnorderedElementsAre( - additional_username, other_username), - _, _, _, _)); - password_autofill_manager_->OnShowPasswordSuggestions( - dummy_key, base::ASCIIToUTF16("John"), false, element_bounds); - - // Finally, simulate displaying all suggestions, without any prefix matching. - EXPECT_CALL(*autofill_client, - ShowAutofillPopup( - element_bounds, _, - testing::UnorderedElementsAre( - test_username_, additional_username, other_username), - _, _, _, _)); - password_autofill_manager_->OnShowPasswordSuggestions( - dummy_key, base::ASCIIToUTF16("xyz"), true, element_bounds); + suggestions[0], autofill::POPUP_ITEM_ID_PASSWORD_ENTRY); } } // namespace password_manager |