Allow deleting autofill password suggestions on Shift+Delete

components/autofill/core/browser/autofill_external_delegate.cc

Index: components/autofill/core/browser/autofill_external_delegate.cc
diff --git a/components/autofill/core/browser/autofill_external_delegate.cc b/components/autofill/core/browser/autofill_external_delegate.cc
index 7bbdeec287bd1a3ac4b0a06fa94f8cb1ea5895f1..1247d94b8e5a2bb9d349be62688eab183f5e2dd3 100644
--- a/components/autofill/core/browser/autofill_external_delegate.cc
+++ b/components/autofill/core/browser/autofill_external_delegate.cc
@@ -35,10 +35,12 @@ AutofillExternalDelegate::~AutofillExternalDelegate() {}
 void AutofillExternalDelegate::OnQuery(int query_id,
                                        const FormData& form,
                                        const FormFieldData& field,
+                                       const PasswordForm& password_form,
                                        const gfx::RectF& element_bounds,
                                        bool display_warning_if_disabled) {
   autofill_query_form_ = form;
   autofill_query_field_ = field;
+  //autofill_query_password_form_ = password_form;

[vabr (Chromium), 2014/01/29 16:09:36]

If you don't need this line, then please remove it. Also remove the
password_form argument, if it's not used.

[riadh.chtara, 2014/02/04 16:22:03]

Done.

   display_warning_if_disabled_ = display_warning_if_disabled;
   autofill_query_id_ = query_id;
   element_bounds_ = element_bounds;
@@ -117,8 +119,10 @@ void AutofillExternalDelegate::OnShowPasswordSuggestions(
     const std::vector<base::string16>& suggestions,
     const std::vector<base::string16>& realms,
     const FormFieldData& field,
+    const PasswordForm& password_form,
     const gfx::RectF& element_bounds) {
   autofill_query_field_ = field;
+  autofill_query_password_form_ = password_form;
   element_bounds_ = element_bounds;
 
   if (suggestions.empty()) {
@@ -200,6 +204,11 @@ void AutofillExternalDelegate::RemoveSuggestion(const base::string16& value,
                                                 int identifier) {
   if (identifier > 0) {
     autofill_manager_->RemoveAutofillProfileOrCreditCard(identifier);
+  } else if (identifier == WebAutofillClient::MenuItemIDPasswordEntry) {
+    autofill_query_password_form_.username_value = value;

[vabr (Chromium), 2014/01/29 16:09:36]

It looks like you mix the |value| with the rest of
|autofill_query_password_form_|. Are you sure that's right?

Can it happen, that |autofill_query_password_form_| will contain the credentials
pair ("Joe User", "secret password"), and then this line replaces "Joe User"
with "Ann User", but keeps the "secret password"?

[riadh.chtara, 2014/02/04 16:22:03]

This is the line that solved the issue which was:
the |username_value|  of  |autofill_query_password_form_| is the actual value in
the form. So if you have two users : Joe and JoeUser. And you put in the user
input field Jo, |username_value| of |autofill_query_password_form_| is going to
be Jo.
The popup is going to be shown with two values. And if you select JoeUser form
the popup without validating your choice (by pressing enter), |value| is going
to be Joe.
Before, this line was added, we try to remove the saved login that have Jo as
its username and this why I got login not found issue.
Now if we add this line the problem is solved.
For the value of the password I don't have a problem with it.
Even if we have many credentials with different passwords, and we have also a
different password in the password field, every thing works fine.
I'm not sure how removing credential happens but I guess when deleting a
credential:
* it checks whether or not a completely match exists: with the same password and
user name, otherwise it checks if there is a partial match (only username is
matching).
or
* it just checks for the username and not the password, because it's not
possible to have two credentials with same user name and different passwords. 
But it works fine.

[vabr (Chromium), 2014/02/05 20:34:20]

Thanks for explaining the problem (with Jo insetad of Joe being in the form
submitted for deletion)!

But instead of modifying autofill_query_password_form_, could you please create
a local copy of autofill_query_password_form_ in this code block, change the
value for that (please add a comment explaining you are replacing the user-typed
prefix with the full name from the suggestion), and pass the local copy to
RemovePasswordSuggestion?

My concern is that if you modify autofill_query_password_form_, and the user
hits Submit without selecting anything, they could possibly store the password
for Joe under the username Jo.

On 2014/02/04 16:22:03, riadh.chtara wrote:

+    bool success = password_autofill_manager_.RemovePasswordSuggestion(
+        autofill_query_field_, autofill_query_password_form_);
+    DCHECK(success);
   } else {
     autofill_manager_->RemoveAutocompleteEntry(autofill_query_field_.name,
                                                value);